Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v0.11 backport] Fix ResolveImageConfig to evaluate source policy #4186

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 78 additions & 23 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import (
"github.com/moby/buildkit/solver/result"
"github.com/moby/buildkit/sourcepolicy"
sourcepolicypb "github.com/moby/buildkit/sourcepolicy/pb"
spb "github.com/moby/buildkit/sourcepolicy/pb"
"github.com/moby/buildkit/util/attestation"
binfotypes "github.com/moby/buildkit/util/buildinfo/types"
"github.com/moby/buildkit/util/contentutil"
Expand Down Expand Up @@ -2601,7 +2600,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 @@ -8999,32 +8998,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: []*spb.Policy{{
Rules: []*spb.Rule{
{
Action: spb.PolicyAction_DENY,
Selector: &spb.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 @@ -71,11 +71,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 @@ -87,16 +88,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 @@ -131,7 +131,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 @@ -147,14 +147,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, llb.ResolveImageConfigOpt{})
scanner, _, dt, err := resolver.ResolveImageConfig(ctx, scanner, llb.ResolveImageConfigOpt{})
if err != nil {
return nil, err
}
Expand Down
17 changes: 12 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,15 +430,22 @@ 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
if err := json.Unmarshal(dt, &img); err != nil {
return errors.Wrap(err, "failed to parse image config")
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 @@ -142,10 +142,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 @@ -540,7 +546,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 @@ -549,11 +555,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
27 changes: 17 additions & 10 deletions frontend/gateway/grpcclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ func (c *grpcClient) Solve(ctx context.Context, creq client.SolveRequest) (res *
return res, nil
}

func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error) {
func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (string, digest.Digest, []byte, error) {
var p *opspb.Platform
if platform := opt.Platform; platform != nil {
p = &opspb.Platform{
Expand All @@ -490,18 +490,25 @@ func (c *grpcClient) ResolveImageConfig(ctx context.Context, ref string, opt llb
}
}
resp, err := c.client.ResolveImageConfig(ctx, &pb.ResolveImageConfigRequest{
ResolverType: int32(opt.ResolverType),
Ref: ref,
Platform: p,
ResolveMode: opt.ResolveMode,
LogName: opt.LogName,
SessionID: opt.Store.SessionID,
StoreID: opt.Store.StoreID,
ResolverType: int32(opt.ResolverType),
Ref: ref,
Platform: p,
ResolveMode: opt.ResolveMode,
LogName: opt.LogName,
SessionID: opt.Store.SessionID,
StoreID: opt.Store.StoreID,
SourcePolicies: opt.SourcePolicies,
})
if err != nil {
return "", nil, err
return "", "", nil, err
}
return resp.Digest, resp.Config, nil
newRef := resp.Ref
if newRef == "" {
// No ref returned, use the original one.
// This could occur if the version of buildkitd is too old.
newRef = ref
}
return newRef, resp.Digest, resp.Config, nil
}

func (c *grpcClient) BuildOpts() client.BuildOpts {
Expand Down
Loading
Loading