Merge pull request #3961 from tonistiigi/buildkit-default-policy

policy: verify BuildKit builder images

driver/bkimage/bkimage.go

@@ -1,7 +1,14 @@
 package bkimage
 
 const (
-	DefaultImage         = "moby/buildkit:buildx-stable-1" // TODO: make this verified
-	QemuImage            = "tonistiigi/binfmt:latest"      // TODO: make this verified
+	DefaultImage         = "moby/buildkit:buildx-stable-1"
+	QemuImage            = "tonistiigi/binfmt:latest" // TODO: make this verified
 	DefaultRootlessImage = DefaultImage + "-rootless"
+
+	// TrustedRepo is the fully-qualified repository whose tags are verified
+	// against the builtin default policy before a builder is created from
+	// them. Images from other repositories pass through unverified; the
+	// allow-untrusted-image driver-opt is only needed when a TrustedRepo tag
+	// that the policy covers does not verify correctly.
+	TrustedRepo = "docker.io/moby/buildkit"
 )

driver/docker-container/driver.go

@@ -13,12 +13,15 @@ import (
 	"time"
 
 	cerrdefs "github.com/containerd/errdefs"
+	"github.com/containerd/platforms"
+	"github.com/distribution/reference"
 	"github.com/docker/buildx/driver"
 	"github.com/docker/buildx/driver/bkimage"
 	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/util/ghutil"
 	"github.com/docker/buildx/util/imagetools"
 	"github.com/docker/buildx/util/progress"
+	"github.com/docker/buildx/util/sourcemeta"
 	"github.com/docker/cli/cli/context/docker"
 	contextstore "github.com/docker/cli/cli/context/store"
 	"github.com/docker/cli/opts"
@@ -29,6 +32,7 @@ import (
 	"github.com/moby/moby/api/types/mount"
 	dockerclient "github.com/moby/moby/client"
 	"github.com/moby/moby/client/pkg/security"
+	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
 
@@ -43,21 +47,22 @@ type Driver struct {
 
 	// if you add fields, remember to update docs:
 	// https://github.com/docker/docs/blob/main/content/build/drivers/docker-container.md
-	netMode            string
-	image              string
-	memory             opts.MemBytes
-	memorySwap         opts.MemSwapBytes
-	cpuQuota           int64
-	cpuPeriod          int64
-	cpuShares          int64
-	cpusetCpus         string
-	cpusetMems         string
-	cgroupParent       string
-	restartPolicy      container.RestartPolicy
-	env                []string
-	defaultLoad        bool
-	gpus               []container.DeviceRequest
-	writeProvenanceGHA bool
+	netMode             string
+	image               string
+	allowUntrustedImage bool
+	memory              opts.MemBytes
+	memorySwap          opts.MemSwapBytes
+	cpuQuota            int64
+	cpuPeriod           int64
+	cpuShares           int64
+	cpusetCpus          string
+	cpusetMems          string
+	cgroupParent        string
+	restartPolicy       container.RestartPolicy
+	env                 []string
+	defaultLoad         bool
+	gpus                []container.DeviceRequest
+	writeProvenanceGHA  bool
 }
 
 func (d *Driver) IsMobyDriver() bool {
@@ -92,27 +97,59 @@ func (d *Driver) create(ctx context.Context, l progress.SubLogger) error {
 		imageName = d.image
 	}
 
-	if err := l.Wrap("pulling image "+imageName, func() error {
-		ra, err := imagetools.RegistryAuthForRef(imageName, d.Auth)
+	imageRef := imageName
+	pullErr := l.Wrap("pulling image "+imageRef, func() error {
+		ra, err := imagetools.RegistryAuthForRef(imageRef, d.Auth)
 		if err != nil {
 			return err
 		}
-		resp, err := d.DockerAPI.ImagePull(ctx, imageName, dockerclient.ImagePullOptions{
+		resp, err := d.DockerAPI.ImagePull(ctx, imageRef, dockerclient.ImagePullOptions{
 			RegistryAuth: ra,
 		})
 		if err != nil {
 			return err
 		}
 		return resp.Wait(ctx)
-	}); err != nil {
-		// image pulling failed, check if it exists in local image store.
-		// if not, return pulling error. otherwise log it.
-		_, errInspect := d.DockerAPI.ImageInspect(ctx, imageName)
-		found := errInspect == nil
-		if !found {
+	})
+	image, inspectErr := d.DockerAPI.ImageInspect(ctx, imageRef)
+	if inspectErr != nil {
+		if pullErr != nil {
+			return pullErr
+		}
+		return errors.Wrapf(inspectErr, "failed to inspect pulled image %s", imageRef)
+	}
+
+	imageName = imageRef
+	if image.Descriptor != nil && d.ImageVerifier != nil && !d.allowUntrustedImage {
+		named, err := reference.ParseNormalizedNamed(imageRef)
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse image reference %s", imageRef)
+		}
+		if named.Name() == bkimage.TrustedRepo {
+			if _, canonical := named.(reference.Canonical); !canonical {
+				named = reference.TagNameOnly(named)
+			}
+			pinned, err := reference.WithDigest(named, image.Descriptor.Digest)
+			if err != nil {
+				return errors.Wrapf(err, "failed to construct image reference for %s", imageRef)
+			}
+			imageName = pinned.String()
+		}
+	}
+
+	// Policy verification requires the immutable descriptor exposed by the
+	// containerd image store. Classic-store images are allowed without it.
+	if image.Descriptor != nil {
+		var err error
+		imageName, err = d.verifiedImageRef(ctx, l, imageName)
+		if err != nil {
+			return err
+		}
+	}
+	if pullErr != nil {
+		if err := l.Wrap("using local image "+imageName, func() error { return nil }); err != nil {
 			return err
 		}
-		l.Wrap("pulling failed, using local image "+imageName, func() error { return nil })
 	}
 
 	cfg := &container.Config{
@@ -229,6 +266,65 @@ func (d *Driver) create(ctx context.Context, l progress.SubLogger) error {
 	})
 }
 
+// verifiedImageRef evaluates ref against the builtin default policy and
+// returns the canonical reference carrying the digest that verification
+// resolved. Source metadata is resolved through the BuildKit embedded in the
+// Docker daemon that hosts the builder, so registry access follows the
+// daemon configuration. The reference is returned unchanged when the policy
+// does not apply to it: policy disabled, allow-untrusted-image set, the
+// image pinned by digest without a tag, or an image outside the managed
+// moby/buildkit repository (which the default policy passes through).
+func (d *Driver) verifiedImageRef(ctx context.Context, l progress.SubLogger, ref string) (string, error) {
+	if d.ImageVerifier == nil || d.allowUntrustedImage {
+		return ref, nil
+	}
+
+	c, err := d.buildkitClient(ctx)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to connect to BuildKit for image verification")
+	}
+	defer c.Close()
+	mr := sourcemeta.NewResolver(c)
+	defer mr.Close()
+
+	pinned, applied, err := driver.VerifyImageRef(ctx, l, ref, d.daemonPlatform(ctx), mr, d.ImageVerifier)
+	if err != nil {
+		if !applied {
+			return "", err
+		}
+		return "", errors.Wrapf(err, "failed to verify image %s", ref)
+	}
+	if !applied {
+		return ref, nil
+	}
+	return pinned, nil
+}
+
+// buildkitClient returns a client to the BuildKit embedded in the Docker
+// daemon that hosts the builder container.
+func (d *Driver) buildkitClient(ctx context.Context) (*client.Client, error) {
+	return client.New(ctx, "",
+		client.WithContextDialer(func(context.Context, string) (net.Conn, error) {
+			return d.DockerAPI.DialHijack(ctx, "/grpc", "h2c", d.DialMeta)
+		}),
+		client.WithSessionDialer(func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) {
+			return d.DockerAPI.DialHijack(ctx, "/session", proto, meta)
+		}),
+	)
+}
+
+// daemonPlatform returns the platform of the images the daemon pulls,
+// falling back to the client platform when daemon info is unavailable.
+func (d *Driver) daemonPlatform(ctx context.Context) *ocispecs.Platform {
+	if resp, err := d.DockerAPI.Info(ctx, dockerclient.InfoOptions{}); err == nil {
+		if p, err := platforms.Parse(resp.Info.OSType + "/" + resp.Info.Architecture); err == nil {
+			return &p
+		}
+	}
+	p := platforms.Normalize(platforms.DefaultSpec())
+	return &p
+}
+
 func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error {
 	try := 1
 	for {

driver/docker-container/factory.go

@@ -117,6 +117,11 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver
 			if err != nil {
 				return nil, err
 			}
+		case k == "allow-untrusted-image":
+			d.allowUntrustedImage, err = strconv.ParseBool(v)
+			if err != nil {
+				return nil, err
+			}
 		default:
 			return nil, errors.Errorf("invalid driver option %s for docker-container driver", k)
 		}

driver/image.go

@@ -0,0 +1,54 @@
+package driver
+
+import (
+	"context"
+
+	"github.com/distribution/reference"
+	"github.com/docker/buildx/driver/bkimage"
+	"github.com/docker/buildx/policy"
+	"github.com/docker/buildx/util/progress"
+	digest "github.com/opencontainers/go-digest"
+	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
+)
+
+// VerifyImageRef evaluates ref against the builtin default policy through
+// verify and returns the reference pinned to the digest that verification
+// resolved. The boolean result reports whether the policy applied: it is
+// false, with ref returned unchanged, when there is no verifier, when ref is
+// pinned by digest without a tag, or when ref is outside the managed
+// moby/buildkit repository (unmanaged images pass through the default policy
+// unchanged). A tagged canonical reference is still verified because its tag
+// carries the release identity checked by the policy.
+func VerifyImageRef(ctx context.Context, l progress.SubLogger, ref string, platform *ocispecs.Platform, resolver policy.SourceMetadataResolver, verify ImageVerifier) (string, bool, error) {
+	if verify == nil {
+		return ref, false, nil
+	}
+	named, err := reference.ParseNormalizedNamed(ref)
+	if err != nil {
+		return "", false, errors.Wrapf(err, "failed to parse image reference %s", ref)
+	}
+	_, isCanonical := named.(reference.Canonical)
+	_, isTagged := named.(reference.Tagged)
+	if isCanonical && !isTagged {
+		return ref, false, nil
+	}
+	if named.Name() != bkimage.TrustedRepo {
+		return ref, false, nil
+	}
+	named = reference.TagNameOnly(named)
+
+	var dgst digest.Digest
+	if err := l.Wrap("verifying image "+named.String(), func() error {
+		var err error
+		dgst, err = verify(ctx, named.String(), platform, resolver)
+		return err
+	}); err != nil {
+		return "", true, err
+	}
+	canonical, err := reference.WithDigest(named, dgst)
+	if err != nil {
+		return "", true, errors.Wrapf(err, "failed to construct canonical reference for %s", ref)
+	}
+	return canonical.String(), true, nil
+}

driver/image_test.go

@@ -0,0 +1,47 @@
+package driver
+
+import (
+	"context"
+	"testing"
+
+	"github.com/docker/buildx/policy"
+	"github.com/moby/buildkit/client"
+	digest "github.com/opencontainers/go-digest"
+	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/stretchr/testify/require"
+)
+
+type nopSubLogger struct{}
+
+func (nopSubLogger) Wrap(_ string, fn func() error) error { return fn() }
+func (nopSubLogger) Log(int, []byte)                      {}
+func (nopSubLogger) SetStatus(*client.VertexStatus)       {}
+
+func TestVerifyImageRefTaggedCanonical(t *testing.T) {
+	dgst := digest.FromString("buildkit")
+	ref := "moby/buildkit:v0.31.2@" + dgst.String()
+	var verifiedRef string
+
+	pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, ref, nil, nil, func(_ context.Context, ref string, _ *ocispecs.Platform, _ policy.SourceMetadataResolver) (digest.Digest, error) {
+		verifiedRef = ref
+		return dgst, nil
+	})
+	require.NoError(t, err)
+	require.True(t, applied)
+	require.Equal(t, "docker.io/"+ref, verifiedRef)
+	require.Equal(t, "docker.io/"+ref, pinned)
+}
+
+func TestVerifyImageRefDigestOnly(t *testing.T) {
+	dgst := digest.FromString("buildkit")
+	called := false
+
+	pinned, applied, err := VerifyImageRef(context.Background(), nopSubLogger{}, "moby/buildkit@"+dgst.String(), nil, nil, func(_ context.Context, _ string, _ *ocispecs.Platform, _ policy.SourceMetadataResolver) (digest.Digest, error) {
+		called = true
+		return dgst, nil
+	})
+	require.NoError(t, err)
+	require.False(t, applied)
+	require.False(t, called)
+	require.Equal(t, "moby/buildkit@"+dgst.String(), pinned)
+}

driver/manager.go

@@ -5,11 +5,13 @@ import (
 	"sort"
 	"sync"
 
+	"github.com/docker/buildx/policy"
 	"github.com/docker/cli/cli/context/store"
 	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/session/auth/authprovider"
 	"github.com/moby/buildkit/util/tracing/delegated"
 	dockerclient "github.com/moby/moby/client"
+	digest "github.com/opencontainers/go-digest"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
@@ -27,6 +29,15 @@ type BuildkitConfig struct {
 	// Rootless bool
 }
 
+// ImageVerifier validates the builder image ref against the builtin default
+// policy and returns the digest that verification resolved the reference to.
+// Drivers that materialize the builder from an image call it before creating
+// the builder so the verified digest can be used instead of the mutable tag.
+// Source metadata is resolved through the given resolver, which drivers back
+// with a BuildKit instance they have access to so that registry access
+// happens where the image is pulled from.
+type ImageVerifier func(ctx context.Context, ref string, platform *ocispecs.Platform, resolver policy.SourceMetadataResolver) (digest.Digest, error)
+
 type InitConfig struct {
 	Name            string
 	EndpointAddr    string
@@ -36,6 +47,7 @@ type InitConfig struct {
 	Files           map[string][]byte
 	DriverOpts      map[string]string
 	Auth            authprovider.AuthConfigProvider
+	ImageVerifier   ImageVerifier
 	Platforms       []ocispecs.Platform
 	ContextPathHash string
 	DialMeta        map[string][]string