Skip to content

Commit 330cf7a

Browse files
cpuguy83cpuguy83
committed
Fix ResolveImageConfig to evaluate source policy
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]>
1 parent 7a65d56 commit 330cf7a

File tree

21 files changed

+541
-254
lines changed

21 files changed

+541
-254
lines changed

‎client/client_test.go

Lines changed: 78 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2787,7 +2787,7 @@ func testSourceDateEpochClamp(t *testing.T, sb integration.Sandbox) {
27872787

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

94729472
t.Run("Frontend policies", func(t *testing.T) {
9473-
denied := "https://raw.githubusercontent.com/moby/buildkit/v0.10.1/README.md"
9474-
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
9475-
st := llb.Image("busybox:1.34.1-uclibc").File(
9476-
llb.Copy(llb.HTTP(denied),
9477-
"README.md", "README.md"))
9478-
def, err := st.Marshal(sb.Context())
9479-
if err != nil {
9480-
return nil, err
9473+
t.Run("deny http", func(t *testing.T) {
9474+
denied := "https://raw.githubusercontent.com/moby/buildkit/v0.10.1/README.md"
9475+
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
9476+
st := llb.Image("busybox:1.34.1-uclibc").File(
9477+
llb.Copy(llb.HTTP(denied),
9478+
"README.md", "README.md"))
9479+
def, err := st.Marshal(sb.Context())
9480+
if err != nil {
9481+
return nil, err
9482+
}
9483+
return c.Solve(ctx, gateway.SolveRequest{
9484+
Definition: def.ToPB(),
9485+
SourcePolicies: []*sourcepolicypb.Policy{{
9486+
Rules: []*sourcepolicypb.Rule{
9487+
{
9488+
Action: sourcepolicypb.PolicyAction_DENY,
9489+
Selector: &sourcepolicypb.Selector{
9490+
Identifier: denied,
9491+
},
9492+
},
9493+
},
9494+
}},
9495+
})
94819496
}
9482-
return c.Solve(ctx, gateway.SolveRequest{
9483-
Definition: def.ToPB(),
9484-
SourcePolicies: []*sourcepolicypb.Policy{{
9485-
Rules: []*sourcepolicypb.Rule{
9486-
{
9487-
Action: sourcepolicypb.PolicyAction_DENY,
9488-
Selector: &sourcepolicypb.Selector{
9489-
Identifier: denied,
9497+
9498+
_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
9499+
require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
9500+
})
9501+
t.Run("resolve image config", func(t *testing.T) {
9502+
frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
9503+
const (
9504+
origRef = "docker.io/library/busybox:1.34.1-uclibc"
9505+
updatedRef = "docker.io/library/busybox:latest"
9506+
)
9507+
pol := []*sourcepolicypb.Policy{
9508+
{
9509+
Rules: []*sourcepolicypb.Rule{
9510+
{
9511+
Action: sourcepolicypb.PolicyAction_DENY,
9512+
Selector: &sourcepolicypb.Selector{
9513+
Identifier: "*",
9514+
},
9515+
},
9516+
{
9517+
Action: sourcepolicypb.PolicyAction_ALLOW,
9518+
Selector: &sourcepolicypb.Selector{
9519+
Identifier: "docker-image://" + updatedRef + "*",
9520+
},
9521+
},
9522+
{
9523+
Action: sourcepolicypb.PolicyAction_CONVERT,
9524+
Selector: &sourcepolicypb.Selector{
9525+
Identifier: "docker-image://" + origRef,
9526+
},
9527+
Updates: &sourcepolicypb.Update{
9528+
Identifier: "docker-image://" + updatedRef,
9529+
},
94909530
},
94919531
},
94929532
},
9493-
}},
9494-
})
9495-
}
9533+
}
94969534

9497-
_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
9498-
require.ErrorContains(t, err, sourcepolicy.ErrSourceDenied.Error())
9535+
ref, dgst, _, err := c.ResolveImageConfig(ctx, origRef, llb.ResolveImageConfigOpt{
9536+
SourcePolicies: pol,
9537+
})
9538+
if err != nil {
9539+
return nil, err
9540+
}
9541+
require.Equal(t, updatedRef, ref)
9542+
st := llb.Image(ref + "@" + dgst.String())
9543+
def, err := st.Marshal(sb.Context())
9544+
if err != nil {
9545+
return nil, err
9546+
}
9547+
return c.Solve(ctx, gateway.SolveRequest{
9548+
Definition: def.ToPB(),
9549+
SourcePolicies: pol,
9550+
})
9551+
}
9552+
_, err = c.Build(sb.Context(), SolveOpt{}, "", frontend, nil)
9553+
require.NoError(t, err)
9554+
})
94999555
})
95009556
}
95019557

‎client/llb/imagemetaresolver/resolver.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,12 @@ type imageMetaResolver struct {
7070
}
7171

7272
type resolveResult struct {
73+
ref string
7374
config []byte
7475
dgst digest.Digest
7576
}
7677

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

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

8889
if res, ok := imr.cache[k]; ok {
89-
return res.dgst, res.config, nil
90+
return res.ref, res.dgst, res.config, nil
9091
}
9192

92-
dgst, config, err := imageutil.Config(ctx, ref, imr.resolver, imr.buffer, nil, platform)
93+
ref, dgst, config, err := imageutil.Config(ctx, ref, imr.resolver, imr.buffer, nil, platform, opt.SourcePolicies)
9394
if err != nil {
94-
return "", nil, err
95+
return "", "", nil, err
9596
}
9697

97-
imr.cache[k] = resolveResult{dgst: dgst, config: config}
98-
return dgst, config, nil
98+
imr.cache[k] = resolveResult{dgst: dgst, config: config, ref: ref}
99+
return ref, dgst, config, nil
99100
}
100101

101102
func (imr *imageMetaResolver) key(ref string, platform *ocispecs.Platform) string {

‎client/llb/resolver.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package llb
33
import (
44
"context"
55

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

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

3738
type ResolverType int
@@ -49,6 +50,8 @@ type ResolveImageConfigOpt struct {
4950
LogName string
5051

5152
Store ResolveImageConfigOptStore
53+
54+
SourcePolicies []*spb.Policy
5255
}
5356

5457
type ResolveImageConfigOptStore struct {

‎client/llb/resolver_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ type testResolver struct {
7474
platform string
7575
}
7676

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

9393
dt, err := json.Marshal(img)
9494
if err != nil {
95-
return "", nil, errors.WithStack(err)
95+
return "", "", nil, errors.WithStack(err)
9696
}
97-
return r.digest, dt, nil
97+
return ref, r.digest, dt, nil
9898
}

‎client/llb/source.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func Image(ref string, opts ...ImageOption) State {
135135
if p == nil {
136136
p = c.Platform
137137
}
138-
_, dt, err := info.metaResolver.ResolveImageConfig(ctx, ref, ResolveImageConfigOpt{
138+
_, _, dt, err := info.metaResolver.ResolveImageConfig(ctx, ref, ResolveImageConfigOpt{
139139
Platform: p,
140140
ResolveMode: info.resolveMode.String(),
141141
ResolverType: ResolverTypeRegistry,
@@ -151,14 +151,18 @@ func Image(ref string, opts ...ImageOption) State {
151151
if p == nil {
152152
p = c.Platform
153153
}
154-
dgst, dt, err := info.metaResolver.ResolveImageConfig(context.TODO(), ref, ResolveImageConfigOpt{
154+
ref, dgst, dt, err := info.metaResolver.ResolveImageConfig(context.TODO(), ref, ResolveImageConfigOpt{
155155
Platform: p,
156156
ResolveMode: info.resolveMode.String(),
157157
ResolverType: ResolverTypeRegistry,
158158
})
159159
if err != nil {
160160
return State{}, err
161161
}
162+
r, err := reference.ParseNormalizedNamed(ref)
163+
if err != nil {
164+
return State{}, err
165+
}
162166
if dgst != "" {
163167
r, err = reference.WithDigest(r, dgst)
164168
if err != nil {

‎frontend/attestations/sbom/sbom.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func CreateSBOMScanner(ctx context.Context, resolver llb.ImageMetaResolver, scan
3838
return nil, nil
3939
}
4040

41-
_, dt, err := resolver.ResolveImageConfig(ctx, scanner, resolveOpt)
41+
scanner, _, dt, err := resolver.ResolveImageConfig(ctx, scanner, resolveOpt)
4242
if err != nil {
4343
return nil, err
4444
}

‎frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -401,15 +401,23 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
401401
prefix += platforms.Format(*platform) + " "
402402
}
403403
prefix += "internal]"
404-
dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, llb.ResolveImageConfigOpt{
405-
Platform: platform,
406-
ResolveMode: opt.ImageResolveMode.String(),
407-
LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName),
408-
ResolverType: llb.ResolverTypeRegistry,
404+
mutRef, dgst, dt, err := metaResolver.ResolveImageConfig(ctx, d.stage.BaseName, llb.ResolveImageConfigOpt{
405+
Platform: platform,
406+
ResolveMode: opt.ImageResolveMode.String(),
407+
LogName: fmt.Sprintf("%s load metadata for %s", prefix, d.stage.BaseName),
408+
ResolverType: llb.ResolverTypeRegistry,
409+
SourcePolicies: nil,
409410
})
410411
if err != nil {
411412
return suggest.WrapError(errors.Wrap(err, origName), origName, append(allStageNames, commonImageNames()...), true)
412413
}
414+
415+
if ref.String() != mutRef {
416+
ref, err = reference.ParseNormalizedNamed(mutRef)
417+
if err != nil {
418+
return errors.Wrapf(err, "failed to parse ref %q", mutRef)
419+
}
420+
}
413421
var img image.Image
414422
if err := json.Unmarshal(dt, &img); err != nil {
415423
return errors.Wrap(err, "failed to parse image config")

‎frontend/dockerui/namedcontext.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,36 @@ import (
1313
"github.com/moby/buildkit/exporter/containerimage/image"
1414
"github.com/moby/buildkit/frontend/dockerfile/dockerignore"
1515
"github.com/moby/buildkit/frontend/gateway/client"
16+
"github.com/moby/buildkit/util/imageutil"
1617
"github.com/pkg/errors"
1718
)
1819

1920
const (
2021
contextPrefix = "context:"
2122
inputMetadataPrefix = "input-metadata:"
23+
maxContextRecursion = 10
2224
)
2325

2426
func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatform string, opt ContextOpt) (*llb.State, *image.Image, error) {
27+
return bc.namedContextRecursive(ctx, name, nameWithPlatform, opt, 0)
28+
}
29+
30+
func (bc *Client) namedContextRecursive(ctx context.Context, name string, nameWithPlatform string, opt ContextOpt, count int) (*llb.State, *image.Image, error) {
2531
opts := bc.bopts.Opts
2632
v, ok := opts[contextPrefix+nameWithPlatform]
2733
if !ok {
2834
return nil, nil, nil
2935
}
3036

37+
if count > maxContextRecursion {
38+
return nil, nil, errors.New("context recursion limit exceeded; this may indicate a cycle in the provided source policies: " + v)
39+
}
40+
3141
vv := strings.SplitN(v, ":", 2)
3242
if len(vv) != 2 {
3343
return nil, nil, errors.Errorf("invalid context specifier %s for %s", v, nameWithPlatform)
3444
}
45+
3546
// allow git@ without protocol for SSH URLs for backwards compatibility
3647
if strings.HasPrefix(vv[0], "git@") {
3748
vv[0] = "git"
@@ -58,15 +69,20 @@ func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatfor
5869

5970
named = reference.TagNameOnly(named)
6071

61-
dgst, data, err := bc.client.ResolveImageConfig(ctx, named.String(), llb.ResolveImageConfigOpt{
72+
ref, dgst, data, err := bc.client.ResolveImageConfig(ctx, named.String(), llb.ResolveImageConfigOpt{
6273
Platform: opt.Platform,
6374
ResolveMode: opt.ResolveMode,
6475
LogName: fmt.Sprintf("[context %s] load metadata for %s", nameWithPlatform, ref),
6576
ResolverType: llb.ResolverTypeRegistry,
6677
})
6778
if err != nil {
79+
e := &imageutil.ResolveToNonImageError{}
80+
if errors.As(err, &e) {
81+
return bc.namedContextRecursive(ctx, e.Updated, name, opt, count+1)
82+
}
6883
return nil, nil, err
6984
}
85+
7086
var img image.Image
7187
if err := json.Unmarshal(data, &img); err != nil {
7288
return nil, nil, err
@@ -121,7 +137,8 @@ func (bc *Client) namedContext(ctx context.Context, name string, nameWithPlatfor
121137
return nil, nil, errors.Wrapf(err, "could not wrap %q with digest", name)
122138
}
123139

124-
dgst, data, err := bc.client.ResolveImageConfig(ctx, dummyRef.String(), llb.ResolveImageConfigOpt{
140+
// TODO: How should source policy be handled here with a dummy ref?
141+
_, dgst, data, err := bc.client.ResolveImageConfig(ctx, dummyRef.String(), llb.ResolveImageConfigOpt{
125142
Platform: opt.Platform,
126143
ResolveMode: opt.ResolveMode,
127144
LogName: fmt.Sprintf("[context %s] load metadata for %s", nameWithPlatform, dummyRef.String()),

‎frontend/frontend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ type Frontend interface {
2222

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

‎frontend/gateway/client/client.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func NewResult() *Result {
2727

2828
type Client interface {
2929
Solve(ctx context.Context, req SolveRequest) (*Result, error)
30-
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (digest.Digest, []byte, error)
30+
ResolveImageConfig(ctx context.Context, ref string, opt llb.ResolveImageConfigOpt) (string, digest.Digest, []byte, error)
3131
BuildOpts() BuildOpts
3232
Inputs(ctx context.Context) (map[string]llb.State, error)
3333
NewContainer(ctx context.Context, req NewContainerRequest) (Container, error)

0 commit comments

Comments
 (0)