diff --git a/driver/remote/driver.go b/driver/remote/driver.go index 495a487e04d1..87aac15e892a 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "net" + "net/url" "os" "strings" "sync" @@ -17,6 +18,7 @@ import ( "github.com/moby/buildkit/client/connhelper" "github.com/moby/buildkit/util/tracing/delegated" "github.com/pkg/errors" + "google.golang.org/grpc" ) type Driver struct { @@ -93,13 +95,40 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client. }), client.WithTracerDelegate(delegated.DefaultExporter), }, opts...) - c, err := client.New(ctx, "", opts...) + // The remote driver establishes the connection itself through a custom + // dialer (including TLS), so the buildkit client cannot derive the gRPC + // ":authority" pseudo-header from the connection and would fall back to + // "localhost". Set it explicitly from the configured endpoint so HTTP/2 + // reverse proxies (e.g. Envoy) can route on it. Passing the endpoint + // address also keeps the gRPC dial target meaningful; the actual dial + // target is unaffected as it still goes through the dialer above. + if authority := d.clientAuthority(); authority != "" { + opts = append(opts, client.WithGRPCDialOption(grpc.WithAuthority(authority))) + } + c, err := client.New(ctx, d.EndpointAddr, opts...) d.client = c d.err = err }) return d.client, d.err } +// clientAuthority returns the value to use for the gRPC ":authority" +// pseudo-header when connecting to the remote endpoint. A configured +// servername takes precedence, since it is also used for TLS SNI and +// certificate validation; otherwise the authority is the endpoint host. This +// mirrors how the buildkit client derives the authority when TLS credentials +// are supplied. Endpoints without a host (e.g. unix sockets) have no authority. +func (d *Driver) clientAuthority() string { + if d.tlsOpts != nil && d.serverName != "" { + return d.serverName + } + u, err := url.Parse(d.EndpointAddr) + if err != nil { + return "" + } + return u.Host +} + func (d *Driver) Dial(ctx context.Context) (net.Conn, error) { addr := d.EndpointAddr ch, err := connhelper.GetConnectionHelper(addr) diff --git a/driver/remote/driver_test.go b/driver/remote/driver_test.go new file mode 100644 index 000000000000..7e54bae871f4 --- /dev/null +++ b/driver/remote/driver_test.go @@ -0,0 +1,108 @@ +package remote + +import ( + "context" + "net" + "testing" + "time" + + "github.com/docker/buildx/driver" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +// TestClientAuthorityValue exercises the authority derivation logic: the +// endpoint host is used by default, and a configured servername takes +// precedence (it is also used for TLS SNI). See docker/buildx#3880. +func TestClientAuthorityValue(t *testing.T) { + tests := []struct { + name string + endpoint string + tls *tlsOpts + expected string + }{ + { + name: "tcp endpoint without tls", + endpoint: "tcp://my-buildkit.example.com:443", + expected: "my-buildkit.example.com:443", + }, + { + name: "servername takes precedence over endpoint host", + endpoint: "tcp://10.0.0.5:443", + tls: &tlsOpts{serverName: "my-buildkit.example.com"}, + expected: "my-buildkit.example.com", + }, + { + name: "unix endpoint has no authority", + endpoint: "unix:///run/buildkit/buildkitd.sock", + expected: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &Driver{ + InitConfig: driver.InitConfig{EndpointAddr: tt.endpoint}, + tlsOpts: tt.tls, + } + require.Equal(t, tt.expected, d.clientAuthority()) + }) + } +} + +// TestClientAuthority verifies end-to-end that the remote driver sends the +// configured endpoint address as the gRPC ":authority" pseudo-header instead +// of defaulting to "localhost" (see docker/buildx#3880). It stands up an +// in-process gRPC server on a loopback listener and asserts the authority of +// the request it receives matches the endpoint host. +func TestClientAuthority(t *testing.T) { + ctx, cancel := context.WithTimeoutCause(context.Background(), 10*time.Second, context.DeadlineExceeded) + defer cancel() + + lc := net.ListenConfig{} + lis, err := lc.Listen(ctx, "tcp", "127.0.0.1:0") + require.NoError(t, err) + defer lis.Close() + + authorityCh := make(chan string, 1) + srv := grpc.NewServer(grpc.UnknownServiceHandler(func(_ any, stream grpc.ServerStream) error { + authority := "" + if md, ok := metadata.FromIncomingContext(stream.Context()); ok { + if a := md.Get(":authority"); len(a) > 0 { + authority = a[0] + } + } + select { + case authorityCh <- authority: + default: + } + return status.Error(codes.Unimplemented, "unimplemented") + })) + go func() { + _ = srv.Serve(lis) + }() + defer srv.Stop() + + d := &Driver{ + InitConfig: driver.InitConfig{ + EndpointAddr: "tcp://" + lis.Addr().String(), + }, + } + + c, err := d.Client(ctx) + require.NoError(t, err) + defer c.Close() + + // Any RPC will do: it fails server-side with Unimplemented, but the server + // records the ":authority" it received from the client before responding. + _, _ = c.ListWorkers(ctx) + + select { + case authority := <-authorityCh: + require.Equal(t, lis.Addr().String(), authority) + case <-ctx.Done(): + t.Fatal("timed out waiting for request to reach the server") + } +}