diff --git a/client/policy_test.go b/client/policy_test.go index b53cf8ff32a7..518c30fbd8b2 100644 --- a/client/policy_test.go +++ b/client/policy_test.go @@ -96,6 +96,47 @@ func testProxyNetworkNoRootless(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) require.Equal(t, int32(0), leakHit.Load()) + cleanupDestDir := t.TempDir() + cleanupFailure := llb.Image("alpine:latest"). + Run( + llb.Shlex( + `sh -c 'touch /exec-completed && dd if=/dev/zero bs=1048576 count=11 >> /etc/ssl/certs/ca-certificates.crt 2>/dev/null'`, + ), + llb.IgnoreCache, + ). + Root() + def, err = cleanupFailure.Marshal(ctx) + require.NoError(t, err) + _, err = c.Solve(ctx, def, SolveOpt{ + ProxyNetwork: true, + Exports: []ExportEntry{{ + Type: ExporterLocal, + OutputDir: cleanupDestDir, + }}, + }, nil) + require.Error(t, err) + require.ErrorContains(t, err, "failed to clean up proxy CA") + require.ErrorContains(t, err, "exceeds 10485760 bytes") + require.NoFileExists(t, filepath.Join(cleanupDestDir, "exec-completed")) + + processAndCleanupFailure := llb.Image("alpine:latest"). + Run( + llb.Shlex( + `sh -c 'dd if=/dev/zero bs=1048576 count=11 >> /etc/ssl/certs/ca-certificates.crt 2>/dev/null && exit 42'`, + ), + llb.IgnoreCache, + ). + Root() + def, err = processAndCleanupFailure.Marshal(ctx) + require.NoError(t, err) + _, err = c.Solve(ctx, def, SolveOpt{ + ProxyNetwork: true, + }, nil) + require.Error(t, err) + require.ErrorContains(t, err, "exit code: 42") + require.ErrorContains(t, err, "failed to clean up proxy CA") + require.ErrorContains(t, err, "exceeds 10485760 bytes") + var checked atomic.Int32 denyProvider := policysession.NewPolicyProvider(func(ctx context.Context, req *policysession.CheckPolicyRequest) (*policysession.DecisionResponse, *pb.ResolveSourceMetaRequest, error) { if req.Source.Source.Identifier != httpURL+"/allowed" { diff --git a/docs/proxy.md b/docs/proxy.md index 932db2889fae..83b2fcce6725 100644 --- a/docs/proxy.md +++ b/docs/proxy.md @@ -91,6 +91,16 @@ BuildKit also injects a generated CA certificate into common Linux trust bundle locations for the duration of the exec. This lets HTTPS requests using the system trust store pass through the BuildKit proxy. +BuildKit removes its injected CA after the exec while preserving other changes +to the selected bundle. If BuildKit cannot read or rewrite the bundle during +cleanup, the exec and solve fail; the result is not stored as a successful cache +record or passed to exporters. + +Cleanup is deferred and therefore does not run if `buildkitd` terminates during +the exec. An interrupted exec does not become a successful cache result, and its +incomplete active reference is not exported after restart and remains eligible +for garbage collection. + ## Request capture and logs The proxy records network requests made by exec steps. Build output includes a @@ -129,3 +139,7 @@ host networking. Applications that ignore `HTTP_PROXY` and `HTTPS_PROXY`, use custom trust stores, or open raw TCP connections cannot bypass the proxy. That traffic is blocked instead of being captured. + +Cleanup removes only BuildKit's temporary mutation from the selected system +bundle. It does not remove copies of that bundle or certificates imported by +build code into other system or application-specific trust stores. diff --git a/executor/containerdexecutor/executor.go b/executor/containerdexecutor/executor.go index 69d0f447c887..577e8f99438c 100644 --- a/executor/containerdexecutor/executor.go +++ b/executor/containerdexecutor/executor.go @@ -2,6 +2,7 @@ package containerdexecutor import ( "context" + stderrors "errors" "io" "os" "path/filepath" @@ -193,11 +194,18 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M defer namespace.Close() if proxyNS, ok := namespace.(network.ProxyNamespace); ok { meta.Env = append(meta.Env, proxyNS.ProxyEnv()...) - cleanProxyCA, err := executor.InjectProxyCA(details.rootfsPath, proxyNS.ProxyCACert()) - if err != nil { - return nil, err + cleanProxyCA, injectErr := executor.InjectProxyCA(details.rootfsPath, proxyNS.ProxyCACert()) + if injectErr != nil { + return nil, injectErr } - defer cleanProxyCA() + defer func() { + if cleanupErr := cleanProxyCA(); cleanupErr != nil { + err = stderrors.Join( + err, + errors.Wrap(cleanupErr, "failed to clean up proxy CA"), + ) + } + }() } spec, releaseSpec, err := w.createOCISpec(ctx, id, resolvConf, hostsFile, namespace, mounts, meta, details) diff --git a/executor/proxyca_linux.go b/executor/proxyca_linux.go index 159e1c30f570..71c9b442a194 100644 --- a/executor/proxyca_linux.go +++ b/executor/proxyca_linux.go @@ -67,9 +67,6 @@ func InjectProxyCA(rootfsPath string, caPEM []byte) (func() error, error) { return func() error { return nil }, nil } next := append([]byte{}, original...) - if len(next) > 0 && next[len(next)-1] != '\n' { - next = append(next, '\n') - } next = append(next, proxyCABegin...) next = append(next, caPEM...) if len(next) > 0 && next[len(next)-1] != '\n' { diff --git a/executor/proxyca_linux_test.go b/executor/proxyca_linux_test.go index 33570ce31858..d164f7d46531 100644 --- a/executor/proxyca_linux_test.go +++ b/executor/proxyca_linux_test.go @@ -18,33 +18,82 @@ import ( ) func TestInjectProxyCACleanupPreservesContainerChanges(t *testing.T) { - rootfs := t.TempDir() - root, err := os.OpenRoot(rootfs) - require.NoError(t, err) - t.Cleanup(func() { - require.NoError(t, root.Close()) - }) - const bundle = "etc/ssl/certs/ca-certificates.crt" - require.NoError(t, root.MkdirAll(filepath.Dir(bundle), 0o755)) original := []byte("original bundle\n") - require.NoError(t, root.WriteFile(bundle, original, 0o644)) + bundle, cleanup := injectTestProxyCA(t, original) + + dt, err := os.ReadFile(bundle) + require.NoError(t, err) + + change := []byte("container change\n") + require.NoError(t, os.WriteFile(bundle, append(dt, change...), 0o644)) + require.NoError(t, cleanup()) + + dt, err = os.ReadFile(bundle) + require.NoError(t, err) + expected := append(append([]byte{}, original...), change...) + require.Equal(t, expected, dt) +} + +func TestInjectProxyCARestoresBundleWithoutTrailingNewline(t *testing.T) { + original := []byte("original bundle") + bundle, cleanup := injectTestProxyCA(t, original) + + require.NoError(t, cleanup()) + + dt, err := os.ReadFile(bundle) + require.NoError(t, err) + require.Equal(t, original, dt) +} + +func TestInjectProxyCARestoresEmptyBundle(t *testing.T) { + original := []byte{} + bundle, cleanup := injectTestProxyCA(t, original) - caPEM := testCertPEM(t) - cleanup, err := InjectProxyCA(rootfs, caPEM) + require.NoError(t, cleanup()) + + dt, err := os.ReadFile(bundle) require.NoError(t, err) + require.Equal(t, original, dt) +} + +func TestInjectProxyCARestoresBundleEndingInNewline(t *testing.T) { + original := []byte("original bundle\n") + bundle, cleanup := injectTestProxyCA(t, original) + + require.NoError(t, cleanup()) - dt, err := root.ReadFile(bundle) + dt, err := os.ReadFile(bundle) require.NoError(t, err) - require.Contains(t, string(dt), string(caPEM)) + require.Equal(t, original, dt) +} + +func TestInjectProxyCACleanupAllowsDeletedBundle(t *testing.T) { + bundle, cleanup := injectTestProxyCA(t, []byte("original bundle\n")) + require.NoError(t, os.Remove(bundle)) - require.NoError(t, root.WriteFile(bundle, append(dt, []byte("container change\n")...), 0o644)) require.NoError(t, cleanup()) + require.NoFileExists(t, bundle) +} + +func TestInjectProxyCACleanupRejectsOversizedBundle(t *testing.T) { + bundle, cleanup := injectTestProxyCA(t, []byte("original bundle\n")) + require.NoError(t, os.Truncate(bundle, maxCertBundleBytes+1)) + + err := cleanup() + require.Error(t, err) + require.ErrorContains(t, err, "exceeds 10485760 bytes") +} + +func injectTestProxyCA(t *testing.T, original []byte) (string, func() error) { + t.Helper() + rootfs := t.TempDir() + bundle := filepath.Join(rootfs, "etc/ssl/certs/ca-certificates.crt") + require.NoError(t, os.MkdirAll(filepath.Dir(bundle), 0o755)) + require.NoError(t, os.WriteFile(bundle, original, 0o644)) - dt, err = root.ReadFile(bundle) + cleanup, err := InjectProxyCA(rootfs, testCertPEM(t)) require.NoError(t, err) - require.NotContains(t, string(dt), string(caPEM)) - require.Contains(t, string(dt), string(original)) - require.Contains(t, string(dt), "container change\n") + return bundle, cleanup } func testCertPEM(t *testing.T) []byte { diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index 46e70ef02d81..8f9881e4baa8 100644 --- a/executor/runcexecutor/executor.go +++ b/executor/runcexecutor/executor.go @@ -5,6 +5,7 @@ package runcexecutor import ( "context" "encoding/json" + stderrors "errors" "io" "os" "os/exec" @@ -279,11 +280,18 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root executor.Mount, defer executor.MountStubsCleaner(context.WithoutCancel(ctx), rootFSPath, mounts, meta.RemoveMountStubsRecursive)() if proxyNS, ok := namespace.(network.ProxyNamespace); ok { - cleanProxyCA, err := executor.InjectProxyCA(rootFSPath, proxyNS.ProxyCACert()) - if err != nil { - return nil, err + cleanProxyCA, injectErr := executor.InjectProxyCA(rootFSPath, proxyNS.ProxyCACert()) + if injectErr != nil { + return nil, injectErr } - defer cleanProxyCA() + defer func() { + if cleanupErr := cleanProxyCA(); cleanupErr != nil { + err = stderrors.Join( + err, + errors.Wrap(cleanupErr, "failed to clean up proxy CA"), + ) + } + }() } uid, gid, sgids, err := oci.GetUser(rootFSPath, meta.User) diff --git a/solver/llbsolver/vertex.go b/solver/llbsolver/vertex.go index 61dd3e3c4190..af0a673a079f 100644 --- a/solver/llbsolver/vertex.go +++ b/solver/llbsolver/vertex.go @@ -20,6 +20,8 @@ import ( "golang.org/x/sync/errgroup" ) +const proxyNetworkCacheSalt = "\x00buildkit.proxy-network.v1" + type vertex struct { sys any options solver.VertexOptions @@ -331,7 +333,7 @@ func recomputeDigests(ctx context.Context, all map[digest.Digest]*op, visited ma return "", err } if op.ProxyNetwork { - dt = append(dt, []byte("\x00buildkit.proxy-network.v0")...) + dt = append(dt, []byte(proxyNetworkCacheSalt)...) } newDgst := digest.FromBytes(dt)