httputil: improve ctlLocalOnly errors

internal/httputil/client.go

@@ -7,6 +7,7 @@ import (
  "net"
  "net/http"
  "net/http/cookiejar"
+ "net/netip"
  "os"
  "path/filepath"
  "strings"
@@ -26,7 +27,7 @@ func NewClient(ctx context.Context, localOnly bool) (*http.Client, error) {
  dialer := &net.Dialer{}
  // Set a control function if we're restricting subnets.
  if localOnly {
- dialer.Control = ctlLocalOnly
+ dialer.ControlContext = ctlLocalOnly
  }
  tr.DialContext = dialer.DialContext
 
@@ -42,36 +43,47 @@ func NewClient(ctx context.Context, localOnly bool) (*http.Client, error) {
  }, nil
 }
 
-func ctlLocalOnly(network, address string, _ syscall.RawConn) error {
- // Future-proof for QUIC by allowing UDP here.
- if !strings.HasPrefix(network, "tcp") && !strings.HasPrefix(network, "udp") {
+func ctlLocalOnly(_ context.Context, network, address string, _ syscall.RawConn) error {
+ // Now that this has a Context'd version, we could jam a policy engine in
+ // here if someone really feeling froggy.
+ switch {
+ case strings.HasPrefix(network, "tcp"): // OK
+ case strings.HasPrefix(network, "udp"): // OK
+ case strings.HasPrefix(network, "unix"):
+ // Local by definition.
+ return nil
+ default:
  return &net.AddrError{
  Addr: network + "!" + address,
- Err: "disallowed by policy",
+ Err: fmt.Sprintf("disallowed by policy: network %q", network),
  }
  }
- host, _, err := net.SplitHostPort(address)
+
+ ap, err := netip.ParseAddrPort(address)
  if err != nil {
  return &net.AddrError{
- Addr: network + "!" + address,
- Err: "martian address",
+ Addr: address,
+ Err: fmt.Sprintf("unable to parse address: %v", err),
  }
  }
- addr := net.ParseIP(host)
- if addr == nil {
+ switch addr := ap.Addr(); {
+ case addr.IsMulticast():
+ // Assert this is a unicast address.
+ // There was a draft RFC for handling HTTP/3 over multicast QUIC, but it's expired so this seems OK to do.
  return &net.AddrError{
- Addr: network + "!" + address,
- Err: "martian address",
+ Addr: ap.String(),
+ Err: "disallowed by policy: address is multicast",
  }
- }
- if !addr.IsPrivate() &&
-  !addr.IsLoopback() &&
-  !addr.IsLinkLocalUnicast() {
+ case addr.IsLoopback(): // OK
+ case addr.IsLinkLocalUnicast(): // OK
+ case addr.IsPrivate(): // OK
+ default:
  return &net.AddrError{
- Addr: network + "!" + address,
- Err: "disallowed by policy",
+ Addr: ap.String(),
+ Err: "disallowed by policy: not loopback, link-local, or private",
  }
  }
+
  return nil
 }
 

internal/httputil/client_test.go

@@ -1,9 +1,12 @@
 package httputil
 
 import (
+ "context"
  "errors"
  "net"
  "testing"
+
+ "github.com/google/go-cmp/cmp"
 )
 
 func TestLocalOnly(t *testing.T) {
@@ -12,46 +15,66 @@ func TestLocalOnly(t *testing.T) {
  Addr string
  Err *net.AddrError
  }{
+ {Network: "tcp4", Addr: "192.168.0.1:443"},
+ {Network: "tcp4", Addr: "10.0.0.1:80"},
+ {Network: "tcp4", Addr: "127.0.0.1:443"},
+ {Network: "tcp6", Addr: "[fe80::]:443"},
+ {Network: "unix", Addr: "/run/sock"},
  {
- Network: "tcp4",
- Addr: "192.168.0.1:443",
- Err: nil,
+ Network: "ip6",
+ Addr: "::1",
+ Err: &net.AddrError{
+ Addr: "ip6!::1",
+ Err: `disallowed by policy: network "ip6"`,
+ },
  },
  {
  Network: "tcp4",
- Addr: "127.0.0.1:443",
- Err: nil,
+ Addr: "127.256:443",
+ Err: &net.AddrError{
+ Addr: "127.256:443",
+ Err: `unable to parse address: ParseAddr("127.256"): IPv4 field has value >255`,
+ },
  },
  {
- Network: "tcp6",
- Addr: "[fe80::]:443",
- Err: nil,
+ Network: "tcp4",
+ Addr: "224.0.0.1:443",
+ Err: &net.AddrError{
+ Addr: "224.0.0.1:443",
+ Err: "disallowed by policy: address is multicast",
+ },
  },
  {
  Network: "tcp4",
  Addr: "8.8.8.8:443",
  Err: &net.AddrError{
- Addr: "tcp4!8.8.8.8:443",
- Err: "disallowed by policy",
+ Addr: "8.8.8.8:443",
+ Err: "disallowed by policy: not loopback, link-local, or private",
  },
  },
  {
  Network: "tcp6",
  Addr: "[2000::]:443",
  Err: &net.AddrError{
- Addr: "tcp6![2000::]:443",
- Err: "disallowed by policy",
+ Addr: "[2000::]:443",
+ Err: "disallowed by policy: not loopback, link-local, or private",
  },
  },
  }
+ // CtlLocalOnly doesn't emit logs, don't bother with zlog.
+ ctx := context.Background()
  for _, tc := range tt {
  t.Logf("%s!%s", tc.Network, tc.Addr)
- var nErr *net.AddrError
- got := ctlLocalOnly(tc.Network, tc.Addr, nil)
- if errors.As(got, &nErr) {
- if tc.Err.Err != nErr.Err || tc.Err.Addr != nErr.Addr {
- t.Errorf("got: %v, want: %v", got, tc.Err)
- }
+ var got *net.AddrError
+ err := ctlLocalOnly(ctx, tc.Network, tc.Addr, nil)
+ switch {
+ case err == nil:
+ case !errors.As(err, &got):
+ t.Errorf("returned error not *net.AddrError, is %T", got)
+ continue
+ }
+ if want := tc.Err; !cmp.Equal(got, want) {
+ t.Error(cmp.Diff(got, want))
  }
  }
 }