Skip to content

Commit

Permalink
Fix ResolveImageConfig to evaluate source policy
Browse files Browse the repository at this point in the history
Before this change, ResolveImageConfig was unaware of source policies.
This means that:

1. Images for denied sources may be resolved
2. Image configs may get pulled for sources that are later converted to
   a different image

The update makes it so the image resolver first runs a given ref through
the source policy and uses any mutated ref for the actual resolve
(instead of the original ref).
It also returns the mutated ref so it can be used correctly by the
frontend (e.g. don't want to do llb.Image(oldRef@resolvedDigest)).

Signed-off-by: Brian Goff <[email protected]>
  • Loading branch information
cpuguy83 committed Jul 10, 2023
1 parent 7a65d56 commit 330cf7a
Show file tree
Hide file tree
Showing 21 changed files with 541 additions and 254 deletions.
100 changes: 78 additions & 22 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2787,7 +2787,7 @@ func testSourceDateEpochClamp(t *testing.T, sb integration.Sandbox) {

var bboxConfig []byte
_, err = c.Build(sb.Context(), SolveOpt{}, "", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
_, bboxConfig, err = c.ResolveImageConfig(ctx, "docker.io/library/busybox:latest", llb.ResolveImageConfigOpt{})
_, _, bboxConfig, err = c.ResolveImageConfig(ctx, "docker.io/library/busybox:latest", llb.ResolveImageConfigOpt{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -9470,32 +9470,88 @@ func testSourcePolicy(t *testing.T, sb integration.Sandbox) {
}

t.Run("Frontend policies", func(t *testing.T) {
denied := "https://raw.githubusercontent.com/moby/buildkit/v0.10.1/README.md"
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
st := llb.Image("busybox:1.34.1-uclibc").File(
llb.Copy(llb.HTTP(denied),
"README.md", "README.md"))
def, err := st.Marshal(sb.Context())
if err != nil {
return nil, err
t.Run("deny http", func(t *testing.T) {
denied := "https://raw.githubusercontent.com/moby/buildkit/v0.10.1/README.md"
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
st := llb.Image("busybox:1.34.1-uclibc").File(
llb.Copy(llb.HTTP(denied),
"README.md", "README.md"))
def, err := st.Marshal(sb.Context())
if err != nil {
return nil, err
}
return c.Solve(ctx, gateway.SolveRequest{
Definition: def.ToPB(),
SourcePolicies: []*sourcepolicypb.Policy{{
Rules: []*sourcepolicypb.Rule{
{
Action: sourcepolicypb.PolicyAction_DENY,
Selector: &sourcepolicypb.Selector{
Identifier: denied,
},
},
},
}},
})
}
return c.Solve(ctx, gateway.SolveRequest{
Definition: def.ToPB(),
SourcePolicies: []*sourcepolicypb.Policy{{
Rules: []*sourcepolicypb.Rule{
{
Action: sourcepolicypb.PolicyAction_DENY,
Selector: &sourcepolicypb.Selector{
Identifier: denied,

_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
})
t.Run("resolve image config", func(t *testing.T) {
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
const (
origRef = "docker.io/library/busybox:1.34.1-uclibc"
updatedRef = "docker.io/library/busybox:latest"
)
pol := []*sourcepolicypb.Policy{
{
Rules: []*sourcepolicypb.Rule{
{
Action: sourcepolicypb.PolicyAction_DENY,
Selector: &sourcepolicypb.Selector{
Identifier: "*",
},
},
{
Action: sourcepolicypb.PolicyAction_ALLOW,
Selector: &sourcepolicypb.Selector{
Identifier: "docker-image://" + updatedRef + "*",
},
},
{
Action: sourcepolicypb.PolicyAction_CONVERT,
Selector: &sourcepolicypb.Selector{
Identifier: "docker-image://" + origRef,
},
Updates: &sourcepolicypb.Update{
Identifier: "docker-image://" + updatedRef,
},
},
},
},
}},
})
}
}

_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
ref, dgst, _, err := c.ResolveImageConfig(ctx, origRef, llb.ResolveImageConfigOpt{
SourcePolicies: pol,
})
if err != nil {
return nil, err
}
require.Equal(t, updatedRef, ref)
st := llb.Image(ref + "@" + dgst.String())
def, err := st.Marshal(sb.Context())
if err != nil {
return nil, err
}
return c.Solve(ctx, gateway.SolveRequest{
Definition: def.ToPB(),
SourcePolicies: pol,
})
}
_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
require.NoError(t, err)
})
})
}

Expand Down
13 changes: 7 additions & 6 deletions client/llb/imagemetaresolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,12 @@ type imageMetaResolver struct {
}

type resolveResult struct {
ref string
config []byte
dgst digest.Digest
}

func (imr *imageMetaResolver) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error) {
func (imr *imageMetaResolver) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (string, digest.Digest, []byte, error) {
imr.locker.Lock(ref)
defer imr.locker.Unlock(ref)

Expand All @@ -86,16 +87,16 @@ func (imr *imageMetaResolver) ResolveImageConfig(ctx context.Context, ref string
k := imr.key(ref, platform)

if res, ok := imr.cache[k]; ok {
return res.dgst, res.config, nil
return res.ref, res.dgst, res.config, nil
}

dgst, config, err := imageutil.Config(ctx, ref, imr.resolver, imr.buffer, nil, platform)
ref, dgst, config, err := imageutil.Config(ctx, ref, imr.resolver, imr.buffer, nil, platform, opt.SourcePolicies)
if err != nil {
return "", nil, err
return "", "", nil, err
}

imr.cache[k] = resolveResult{dgst: dgst, config: config}
return dgst, config, nil
imr.cache[k] = resolveResult{dgst: dgst, config: config, ref: ref}
return ref, dgst, config, nil
}

func (imr *imageMetaResolver) key(ref string, platform *ocispecs.Platform) string {
Expand Down
5 changes: 4 additions & 1 deletion client/llb/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package llb
import (
"context"

spb "github.com/moby/buildkit/sourcepolicy/pb"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
)
Expand Down Expand Up @@ -31,7 +32,7 @@ func WithLayerLimit(l int) ImageOption {

// ImageMetaResolver can resolve image config metadata from a reference
type ImageMetaResolver interface {
ResolveImageConfig(ctx context.Context, ref string, opt ResolveImageConfigOpt) (digest.Digest, []byte, error)
ResolveImageConfig(ctx context.Context, ref string, opt ResolveImageConfigOpt) (string, digest.Digest, []byte, error)
}

type ResolverType int
Expand All @@ -49,6 +50,8 @@ type ResolveImageConfigOpt struct {
LogName string

Store ResolveImageConfigOptStore

SourcePolicies []*spb.Policy
}

type ResolveImageConfigOptStore struct {
Expand Down
6 changes: 3 additions & 3 deletions client/llb/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ type testResolver struct {
platform string
}

func (r *testResolver) ResolveImageConfig(ctx context.Context, ref string, opt ResolveImageConfigOpt) (digest.Digest, []byte, error) {
func (r *testResolver) ResolveImageConfig(ctx context.Context, ref string, opt ResolveImageConfigOpt) (string, digest.Digest, []byte, error) {
var img struct {
Config struct {
Env []string `json:"Env,omitempty"`
Expand All @@ -92,7 +92,7 @@ func (r *testResolver) ResolveImageConfig(ctx context.Context, ref string, opt R

dt, err := json.Marshal(img)
if err != nil {
return "", nil, errors.WithStack(err)
return "", "", nil, errors.WithStack(err)
}
return r.digest, dt, nil
return ref, r.digest, dt, nil
}
8 changes: 6 additions & 2 deletions client/llb/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func Image(ref string, opts ...ImageOption) State {
if p == nil {
p = c.Platform
}
_, dt, err := info.metaResolver.ResolveImageConfig(ctx, ref, ResolveImageConfigOpt{
_, _, dt, err := info.metaResolver.ResolveImageConfig(ctx, ref, ResolveImageConfigOpt{
Platform: p,
ResolveMode: info.resolveMode.String(),
ResolverType: ResolverTypeRegistry,
Expand All @@ -151,14 +151,18 @@ func Image(ref string, opts ...ImageOption) State {
if p == nil {
p = c.Platform
}
dgst, dt, err := info.metaResolver.ResolveImageConfig(context.TODO(), ref, ResolveImageConfigOpt{
ref, dgst, dt, err := info.metaResolver.ResolveImageConfig(context.TODO(), ref, ResolveImageConfigOpt{
Platform: p,
ResolveMode: info.resolveMode.String(),
ResolverType: ResolverTypeRegistry,
})
if err != nil {
return State{}, err
}
r, err := reference.ParseNormalizedNamed(ref)
if err != nil {
return State{}, err
}
if dgst != "" {
r, err = reference.WithDigest(r, dgst)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion frontend/attestations/sbom/sbom.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scan
return nil, nil
}

_, dt, err := resolver.ResolveImageConfig(ctx, scanner, resolveOpt)
scanner, _, dt, err := resolver.ResolveImageConfig(ctx, scanner, resolveOpt)
if err != nil {
return nil, err
}
Expand Down
18 changes: 13 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,15 +401,23 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
prefix += platforms.Format(*platform) + " "
}
prefix += "internal]"
dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, llb.ResolveImageConfigOpt{
Platform: platform,
ResolveMode: opt.ImageResolveMode.String(),
LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName),
ResolverType: llb.ResolverTypeRegistry,
mutRef, dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, llb.ResolveImageConfigOpt{
Platform: platform,
ResolveMode: opt.ImageResolveMode.String(),
LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName),
ResolverType: llb.ResolverTypeRegistry,
SourcePolicies: nil,
})
if err != nil {
return suggest.WrapError(errors.Wrap(err, origName), origName, append(allStageNames, commonImageNames()...), true)
}

if ref.String() != mutRef {
ref, err = reference.ParseNormalizedNamed(mutRef)
if err != nil {
return errors.Wrapf(err, "failed to parse ref %q", mutRef)
}
}
var img image.Image
if err := json.Unmarshal(dt, &img); err != nil {
return errors.Wrap(err, "failed to parse image config")
Expand Down
21 changes: 19 additions & 2 deletions frontend/dockerui/namedcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,36 @@ import (
"github.com/moby/buildkit/exporter/containerimage/image"
"github.com/moby/buildkit/frontend/dockerfile/dockerignore"
"github.com/moby/buildkit/frontend/gateway/client"
"github.com/moby/buildkit/util/imageutil"
"github.com/pkg/errors"
)

const (
contextPrefix = "context:"
inputMetadataPrefix = "input-metadata:"
maxContextRecursion = 10
)

func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatform string, opt ContextOpt) (*llb.State, *image.Image, error) {
return bc.namedContextRecursive(ctx, name, nameWithPlatform, opt, 0)
}

func (bc *Client) namedContextRecursive(ctx context.Context, name string, nameWithPlatform string, opt ContextOpt, count int) (*llb.State, *image.Image, error) {
opts := bc.bopts.Opts
v, ok := opts[contextPrefix+nameWithPlatform]
if !ok {
return nil, nil, nil
}

if count > maxContextRecursion {
return nil, nil, errors.New("context recursion limit exceeded; this may indicate a cycle in the provided source policies: " + v)
}

vv := strings.SplitN(v, ":", 2)
if len(vv) != 2 {
return nil, nil, errors.Errorf("invalid context specifier %s for %s", v, nameWithPlatform)
}

// allow git@ without protocol for SSH URLs for backwards compatibility
if strings.HasPrefix(vv[0], "git@") {
vv[0] = "git"
Expand All @@ -58,15 +69,20 @@ func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatfor

named = reference.TagNameOnly(named)

dgst, data, err := bc.client.ResolveImageConfig(ctx, named.String(), llb.ResolveImageConfigOpt{
ref, dgst, data, err := bc.client.ResolveImageConfig(ctx, named.String(), llb.ResolveImageConfigOpt{
Platform: opt.Platform,
ResolveMode: opt.ResolveMode,
LogName: fmt.Sprintf("[context %s] load metadata for %s", nameWithPlatform, ref),
ResolverType: llb.ResolverTypeRegistry,
})
if err != nil {
e := &imageutil.ResolveToNonImageError{}
if errors.As(err, &e) {
return bc.namedContextRecursive(ctx, e.Updated, name, opt, count+1)
}
return nil, nil, err
}

var img image.Image
if err := json.Unmarshal(data, &img); err != nil {
return nil, nil, err
Expand Down Expand Up @@ -121,7 +137,8 @@ func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatfor
return nil, nil, errors.Wrapf(err, "could not wrap %q with digest", name)
}

dgst, data, err := bc.client.ResolveImageConfig(ctx, dummyRef.String(), llb.ResolveImageConfigOpt{
// TODO: How should source policy be handled here with a dummy ref?
_, dgst, data, err := bc.client.ResolveImageConfig(ctx, dummyRef.String(), llb.ResolveImageConfigOpt{
Platform: opt.Platform,
ResolveMode: opt.ResolveMode,
LogName: fmt.Sprintf("[context %s] load metadata for %s", nameWithPlatform, dummyRef.String()),
Expand Down
2 changes: 1 addition & 1 deletion frontend/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ type Frontend interface {

type FrontendLLBBridge interface {
Solve(ctx context.Context, req SolveRequest, sid string) (*Result, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (string, digest.Digest, []byte, error)
Warn(ctx context.Context, dgst digest.Digest, msg string, opts WarnOpts) error
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/gateway/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func NewResult() *Result {

type Client interface {
Solve(ctx context.Context, req SolveRequest) (*Result, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error)
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (string, digest.Digest, []byte, error)
BuildOpts() BuildOpts
Inputs(ctx context.Context) (map[string]llb.State, error)
NewContainer(ctx context.Context, req NewContainerRequest) (Container, error)
Expand Down
12 changes: 10 additions & 2 deletions frontend/gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,16 @@ func (gf *gatewayFrontend) Solve(ctx context.Context, llbBridge frontend.Fronten
return nil, err
}

dgst, config, err := llbBridge.ResolveImageConfig(ctx, reference.TagNameOnly(sourceRef).String(), llb.ResolveImageConfigOpt{})
ref, dgst, config, err := llbBridge.ResolveImageConfig(ctx, reference.TagNameOnly(sourceRef).String(), llb.ResolveImageConfigOpt{})
if err != nil {
return nil, err
}

sourceRef, err = reference.ParseNormalizedNamed(ref)
if err != nil {
return nil, err
}

mfstDigest = dgst

if err := json.Unmarshal(config, &img); err != nil {
Expand Down Expand Up @@ -563,7 +569,7 @@ func (lbf *llbBridgeForwarder) ResolveImageConfig(ctx context.Context, req *pb.R
OSFeatures: p.OSFeatures,
}
}
dgst, dt, err := lbf.llbBridge.ResolveImageConfig(ctx, req.Ref, llb.ResolveImageConfigOpt{
ref, dgst, dt, err := lbf.llbBridge.ResolveImageConfig(ctx, req.Ref, llb.ResolveImageConfigOpt{
ResolverType: llb.ResolverType(req.ResolverType),
Platform: platform,
ResolveMode: req.ResolveMode,
Expand All @@ -572,11 +578,13 @@ func (lbf *llbBridgeForwarder) ResolveImageConfig(ctx context.Context, req *pb.R
SessionID: req.SessionID,
StoreID: req.StoreID,
},
SourcePolicies: req.SourcePolicies,
})
if err != nil {
return nil, err
}
return &pb.ResolveImageConfigResponse{
Ref: ref,
Digest: dgst,
Config: dt,
}, nil
Expand Down
Loading

0 comments on commit 330cf7a

Please sign in to comment.