Skip to content

Commit

Permalink
provenance: ensure URLs are redacted before written
Browse files Browse the repository at this point in the history 
HTTP and Git URLs may contain inlined credentials in some cases.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 287d2c328dfe7303adf39911ef49153010534214)

# Conflicts:
#	frontend/dockerfile/dockerfile_buildinfo_test.go

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
  • Loading branch information
@tonistiigi @crazy-max
tonistiigi authored and crazy-max committed Feb 28, 2023
1 parent c327eb8 commit 7d45f99
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 19 deletions.
17 changes: 12 additions & 5 deletions frontend/dockerfile/dockerfile_buildinfo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func testBuildInfoSources(t *testing.T, sb integration.Sandbox) {
dockerfile := `
FROM alpine:latest@sha256:21a3deaa0d32a8057914f36584b5288d2e5ecc984380bc0118285c70fa8c9300 AS alpine
FROM busybox:latest
ADD https://raw.githubusercontent.com/moby/moby/v20.10.21/README.md /
ADD https://user2:pw2@raw.githubusercontent.com/moby/moby/v20.10.21/README.md /
COPY --from=alpine /bin/busybox /alpine-busybox
`

Expand Down Expand Up @@ -93,10 +93,15 @@ COPY --from=alpine /bin/busybox /alpine-busybox
}}
}

expectedURL := strings.Replace(server.URL, "http://", "http://xxxxx:xxxxx@", 1)
require.NotEqual(t, expectedURL, server.URL)
server.URL = strings.Replace(server.URL, "http://", "http://user:pass@", 1)

res, err := f.Solve(sb.Context(), c, client.SolveOpt{
Exports: exports,
FrontendAttrs: map[string]string{
builder.DefaultLocalNameContext: server.URL + "/.git#buildinfo",
builder.DefaultLocalNameContext: server.URL + "/.git#buildinfo",
builder.DefaultLocalNameContext + ":foo": "https://foo:bar@example.invalid/foo.html",
},
}, nil)
require.NoError(t, err)
Expand All @@ -110,7 +115,9 @@ COPY --from=alpine /bin/busybox /alpine-busybox
require.NoError(t, err)

require.Contains(t, bi.Attrs, "context")
require.Equal(t, server.URL+"/.git#buildinfo", *bi.Attrs["context"])
require.Equal(t, expectedURL+"/.git#buildinfo", *bi.Attrs["context"])

require.Equal(t, "https://xxxxx:xxxxx@example.invalid/foo.html", *bi.Attrs["context:foo"])

_, isGateway := f.(*gatewayFrontend)

Expand All @@ -131,11 +138,11 @@ COPY --from=alpine /bin/busybox /alpine-busybox
assert.NotEmpty(t, sources[1].Pin)

assert.Equal(t, binfotypes.SourceTypeGit, sources[2].Type)
assert.Equal(t, server.URL+"/.git#buildinfo", sources[2].Ref)
assert.Equal(t, expectedURL+"/.git#buildinfo", sources[2].Ref)
assert.NotEmpty(t, sources[2].Pin)

assert.Equal(t, binfotypes.SourceTypeHTTP, sources[3].Type)
assert.Equal(t, "https://raw.githubusercontent.com/moby/moby/v20.10.21/README.md", sources[3].Ref)
assert.Equal(t, "https://xxxxx:xxxxx@raw.githubusercontent.com/moby/moby/v20.10.21/README.md", sources[3].Ref)
assert.Equal(t, "sha256:419455202b0ef97e480d7f8199b26a721a417818bc0e2d106975f74323f25e6c", sources[3].Pin)
}

Expand Down
36 changes: 22 additions & 14 deletions frontend/dockerfile/dockerfile_provenance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,13 @@ RUN echo "ok" > /foo
builder.DefaultLocalNameContext: dir,
},
FrontendAttrs: map[string]string{
"attest:provenance": provReq,
"build-arg:FOO": "bar",
"label:lbl": "abc",
"vcs:source": "https://example.invalid/repo.git",
"vcs:revision": "123456",
"filename": "Dockerfile",
"attest:provenance": provReq,
"build-arg:FOO": "bar",
"label:lbl": "abc",
"vcs:source": "https://user:pass@example.invalid/repo.git",
"vcs:revision": "123456",
"filename": "Dockerfile",
builder.DefaultLocalNameContext + ":foo": "https://foo:bar@example.invalid/foo.html",
},
Exports: []client.ExportEntry{
{
Expand Down Expand Up @@ -137,30 +138,32 @@ RUN echo "ok" > /foo
require.Equal(t, "gateway.v0", pred.Invocation.Parameters.Frontend)

if mode == "max" || mode == "" {
require.Equal(t, 3, len(args), "%v", args)
require.Equal(t, 4, len(args), "%v", args)
require.True(t, pred.Metadata.Completeness.Parameters)

require.Equal(t, "bar", args["build-arg:FOO"])
require.Equal(t, "abc", args["label:lbl"])
require.Contains(t, args["source"], "buildkit_test/")
} else {
require.False(t, pred.Metadata.Completeness.Parameters)
require.Equal(t, 1, len(args), "%v", args)
require.Equal(t, 2, len(args), "%v", args)
require.Contains(t, args["source"], "buildkit_test/")
}
require.Equal(t, "https://xxxxx:xxxxx@example.invalid/foo.html", args["context:foo"])
} else {
require.Equal(t, "dockerfile.v0", pred.Invocation.Parameters.Frontend)

if mode == "max" || mode == "" {
require.Equal(t, 2, len(args))
require.Equal(t, 3, len(args))
require.True(t, pred.Metadata.Completeness.Parameters)

require.Equal(t, "bar", args["build-arg:FOO"])
require.Equal(t, "abc", args["label:lbl"])
} else {
require.False(t, pred.Metadata.Completeness.Parameters)
require.Equal(t, 0, len(args), "%v", args)
require.Equal(t, 1, len(args), "%v", args)
}
require.Equal(t, "https://xxxxx:xxxxx@example.invalid/foo.html", args["context:foo"])
}

expectedBase := "pkg:docker/busybox@latest?platform=" + url.PathEscape(platforms.Format(platforms.Normalize(platforms.DefaultSpec())))
Expand All @@ -177,7 +180,7 @@ RUN echo "ok" > /foo

if !isClient {
require.Equal(t, "Dockerfile", pred.Invocation.ConfigSource.EntryPoint)
require.Equal(t, "https://example.invalid/repo.git", pred.Metadata.BuildKitMetadata.VCS["source"])
require.Equal(t, "https://xxxxx:xxxxx@example.invalid/repo.git", pred.Metadata.BuildKitMetadata.VCS["source"])
require.Equal(t, "123456", pred.Metadata.BuildKitMetadata.VCS["revision"])
}

Expand Down Expand Up @@ -265,6 +268,11 @@ COPY myapp.Dockerfile /

target := registry + "/buildkit/testwithprovenance:git"

// inject dummy credentials to test that they are masked
expectedURL := strings.Replace(server.URL, "http://", "http://xxxxx:xxxxx@", 1)
require.NotEqual(t, expectedURL, server.URL)
server.URL = strings.Replace(server.URL, "http://", "http://user:pass@", 1)

_, err = f.Solve(sb.Context(), c, client.SolveOpt{
FrontendAttrs: map[string]string{
"context": server.URL + "/.git#v1",
Expand Down Expand Up @@ -318,7 +326,7 @@ COPY myapp.Dockerfile /
require.Equal(t, "", pred.Invocation.ConfigSource.EntryPoint)
} else {
require.NotEmpty(t, pred.Invocation.Parameters.Frontend)
require.Equal(t, server.URL+"/.git#v1", pred.Invocation.ConfigSource.URI)
require.Equal(t, expectedURL+"/.git#v1", pred.Invocation.ConfigSource.URI)
require.Equal(t, "myapp.Dockerfile", pred.Invocation.ConfigSource.EntryPoint)
}

Expand All @@ -332,15 +340,15 @@ COPY myapp.Dockerfile /
require.Equal(t, expBase, pred.Materials[1].URI)
require.NotEmpty(t, pred.Materials[1].Digest["sha256"])

require.Equal(t, server.URL+"/.git#v1", pred.Materials[2].URI)
require.Equal(t, expectedURL+"/.git#v1", pred.Materials[2].URI)
require.Equal(t, strings.TrimSpace(string(expectedGitSHA)), pred.Materials[2].Digest["sha1"])
} else {
require.Equal(t, 2, len(pred.Materials), "%+v", pred.Materials)

require.Equal(t, expBase, pred.Materials[0].URI)
require.NotEmpty(t, pred.Materials[0].Digest["sha256"])

require.Equal(t, server.URL+"/.git#v1", pred.Materials[1].URI)
require.Equal(t, expectedURL+"/.git#v1", pred.Materials[1].URI)
require.Equal(t, strings.TrimSpace(string(expectedGitSHA)), pred.Materials[1].Digest["sha1"])
}

Expand Down
3 changes: 3 additions & 0 deletions solver/llbsolver/provenance/capture.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

distreference "github.com/docker/distribution/reference"
"github.com/moby/buildkit/solver/result"
"github.com/moby/buildkit/util/urlutil"
digest "github.com/opencontainers/go-digest"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
)
Expand Down Expand Up @@ -190,6 +191,7 @@ func (c *Capture) AddLocal(l LocalSource) {
}

func (c *Capture) AddGit(g GitSource) {
g.URL = urlutil.RedactCredentials(g.URL)
for _, v := range c.Sources.Git {
if v.URL == g.URL {
return
Expand All @@ -199,6 +201,7 @@ func (c *Capture) AddGit(g GitSource) {
}

func (c *Capture) AddHTTP(h HTTPSource) {
h.URL = urlutil.RedactCredentials(h.URL)
for _, v := range c.Sources.HTTP {
if v.URL == h.URL {
return
Expand Down
13 changes: 13 additions & 0 deletions solver/llbsolver/provenance/predicate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
slsa "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/common"
slsa02 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v0.2"
"github.com/moby/buildkit/util/purl"
"github.com/moby/buildkit/util/urlutil"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/package-url/packageurl-go"
)
Expand Down Expand Up @@ -151,6 +152,7 @@ func NewPredicate(c *Capture) (*ProvenancePredicate, error) {
} else {
inv.ConfigSource.URI = v
}
inv.ConfigSource.URI = urlutil.RedactCredentials(inv.ConfigSource.URI)
delete(c.Args, contextKey)
}

Expand All @@ -162,6 +164,9 @@ func NewPredicate(c *Capture) (*ProvenancePredicate, error) {
vcs := make(map[string]string)
for k, v := range c.Args {
if strings.HasPrefix(k, "vcs:") {
if k == "vcs:source" {
v = urlutil.RedactCredentials(v)
}
delete(c.Args, k)
if v != "" {
vcs[strings.TrimPrefix(k, "vcs:")] = v
Expand Down Expand Up @@ -231,6 +236,11 @@ func FilterArgs(m map[string]string) map[string]string {
"platform": {},
"cache-imports": {},
}
const defaultContextKey = "context"
contextKey := defaultContextKey
if v, ok := m["contextkey"]; ok && v != "" {
contextKey = v
}
out := make(map[string]string)
for k, v := range m {
if _, ok := hostSpecificArgs[k]; ok {
Expand All @@ -239,6 +249,9 @@ func FilterArgs(m map[string]string) map[string]string {
if strings.HasPrefix(k, "attest:") {
continue
}
if k == contextKey || strings.HasPrefix(k, defaultContextKey+":") {
v = urlutil.RedactCredentials(v)
}
out[k] = v
}
return out
Expand Down

0 comments on commit 7d45f99

Please sign in to comment.