Skip to content
Merged
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
28 changes: 23 additions & 5 deletions frontend/dockerfile/dockerfile2llb/convert_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,48 @@ import (
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/moby/buildkit/frontend/dockerfile/shell"
"github.com/moby/buildkit/util/system"
"github.com/pkg/errors"
)

func dispatchSecret(d *dispatchState, m *instructions.Mount, loc []parser.Range) (llb.RunOption, error) {
isWindows := d.platform != nil && d.platform.OS == "windows"
targetPath := m.Target
if isWindows {
// Normalize backslashes so C:\path\to\secret resolves to an absolute path.
targetPath = system.ToSlash(targetPath, "windows")
}
Comment thread
rzlink marked this conversation as resolved.

id := m.CacheID
if m.Source != "" {
id = m.Source
}

if id == "" {
if m.Target == "" {
if targetPath == "" {
return nil, errors.New("one of source, target required")
}
id = path.Base(m.Target)
id = path.Base(targetPath)
}

// Reject a non-absolute target (e.g. drive-relative "C:secret.txt") that would
// otherwise mount to an unexpected path.
if isWindows && targetPath != "" && !system.IsAbs(targetPath, "windows") {
return nil, errors.Errorf("secret target %q must be an absolute path with forward slashes on Windows, e.g. --mount=type=secret,id=%s,target=C:/path/to/secret", targetPath, id)
}

var target *string
if m.Target != "" {
target = &m.Target
if targetPath != "" {
target = &targetPath
}

if m.Env == nil {
dest := m.Target
dest := targetPath
if dest == "" {
// Windows has no default secret location like POSIX /run/secrets.
if isWindows {
return nil, errors.Errorf("secret target is required on Windows, e.g. --mount=type=secret,id=%s,target=C:/path/to/secret", id)
}
dest = "/run/secrets/" + path.Base(id)
}
target = &dest
Expand Down
75 changes: 75 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert_secrets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package dockerfile2llb

import (
"testing"

"github.com/moby/buildkit/client/llb"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/stretchr/testify/require"
)

func TestDispatchSecretTarget(t *testing.T) {
newState := func(os string) *dispatchState {
return &dispatchState{
platform: &ocispecs.Platform{OS: os},
outline: newOutlineCapture(),
}
}

secretDest := func(t *testing.T, opt llb.RunOption) string {
t.Helper()
ei := &llb.ExecInfo{}
opt.SetRunOption(ei)
require.Len(t, ei.Secrets, 1)
require.NotNil(t, ei.Secrets[0].Target)
return *ei.Secrets[0].Target
}

t.Run("windows requires explicit target", func(t *testing.T) {
d := newState("windows")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret"}
_, err := dispatchSecret(d, m, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "secret target is required on Windows")
})

t.Run("windows explicit target ok", func(t *testing.T) {
d := newState("windows")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret", Target: "C:/secret.txt"}
_, err := dispatchSecret(d, m, nil)
require.NoError(t, err)
})

t.Run("windows normalizes backslash target", func(t *testing.T) {
d := newState("windows")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret", Target: "C:\\dir\\secret.txt"}
opt, err := dispatchSecret(d, m, nil)
require.NoError(t, err)
require.Equal(t, "C:/dir/secret.txt", secretDest(t, opt))
require.Equal(t, "C:\\dir\\secret.txt", m.Target, "m.Target must not be mutated")
})

t.Run("windows rejects drive-relative target", func(t *testing.T) {
d := newState("windows")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret", Target: "C:secret.txt"}
_, err := dispatchSecret(d, m, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "must be an absolute path")
})

t.Run("linux preserves backslash target", func(t *testing.T) {
d := newState("linux")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret", Target: "/a\\b"}
opt, err := dispatchSecret(d, m, nil)
require.NoError(t, err)
require.Equal(t, "/a\\b", secretDest(t, opt))
})

t.Run("linux defaults target", func(t *testing.T) {
d := newState("linux")
m := &instructions.Mount{Type: instructions.MountTypeSecret, CacheID: "mysecret"}
_, err := dispatchSecret(d, m, nil)
require.NoError(t, err)
})
}
4 changes: 2 additions & 2 deletions frontend/dockerfile/dockerfile_outline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,11 @@ FROM nanoserver AS first
RUN --mount=type=secret,target=/etc/passwd,required=true --mount=type=ssh exit 0

FROM nanoserver AS second
RUN --mount=type=secret,id=unused --mount=type=ssh,id=ssh2 exit 0
RUN --mount=type=secret,id=unused,target=C:/unused --mount=type=ssh,id=ssh2 exit 0

FROM nanoserver AS third
ARG BAR
RUN --mount=type=secret,id=second${BAR} exit 0
RUN --mount=type=secret,id=second${BAR},target=C:/second exit 0

FROM third AS target
COPY --from=first /License.txt /
Expand Down
58 changes: 53 additions & 5 deletions frontend/dockerfile/dockerfile_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var secretsTests = integration.TestFuncs(
testSecretRequiredWithoutValue,
testSecretAsEnviron,
testSecretAsEnvironWithFileMount,
testSecretFileMount,
)

func init() {
Expand Down Expand Up @@ -73,7 +74,7 @@ func testSecretRequiredWithoutValue(t *testing.T, sb integration.Sandbox) {

`FROM nanoserver
USER ContainerAdministrator
RUN --mount=type=secret,required,id=mysecret foo`,
RUN --mount=type=secret,required,id=mysecret,target=C:/mysecret foo`,
))

dir := integration.Tmpdir(
Expand Down Expand Up @@ -170,14 +171,61 @@ RUN --mount=type=secret,id=mysecret,env=SECRET_ENV if %SECRET_ENV% NEQ pw (exit
// testSecretAsEnvironWithFileMount verifies that a secret with both env= and
// target= is accessible as an environment variable and as a file.
func testSecretAsEnvironWithFileMount(t *testing.T, sb integration.Sandbox) {
// target= triggers a tmpfs-backed file mount; Windows only accepts "windows-layer" mounts.
integration.SkipOnPlatform(t, "windows", "secret file mounts use tmpfs which is unsupported on Windows")
f := getFrontend(t, sb)

dockerfile := []byte(`
// Forward slashes in the Windows path because the Dockerfile parser
// consumes backslashes as escapes.
dockerfile := []byte(integration.UnixOrWindows(
`
FROM busybox
RUN --mount=type=secret,id=mysecret,target=/run/secrets/secret,env=SECRET_ENV [ "$SECRET_ENV" == "pw" ] && [ -f /run/secrets/secret ] || false
`)
`,
`
FROM nanoserver
USER ContainerAdministrator
RUN --mount=type=secret,id=mysecret,target=C:/run/secrets/secret,env=SECRET_ENV if %SECRET_ENV% NEQ pw (exit 1) & if not exist C:/run/secrets/secret (exit 1)
`,
))

dir := integration.Tmpdir(
t,
fstest.CreateFile("Dockerfile", dockerfile, 0600),
)

c, err := client.New(sb.Context(), sb.Address())
require.NoError(t, err)
defer c.Close()

_, err = f.Solve(sb.Context(), c, client.SolveOpt{
LocalMounts: map[string]fsutil.FS{
dockerui.DefaultLocalNameDockerfile: dir,
dockerui.DefaultLocalNameContext: dir,
},
Session: []session.Attachable{secretsprovider.FromMap(map[string][]byte{
"mysecret": []byte("pw"),
})},
}, nil)
require.NoError(t, err)
}

// testSecretFileMount verifies a secret mounted as a file (target=) is readable
// inside the RUN step on both Linux and Windows.
func testSecretFileMount(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)

// Forward slashes in the Windows path because the Dockerfile parser
// consumes backslashes as escapes.
dockerfile := []byte(integration.UnixOrWindows(
`
FROM busybox
RUN --mount=type=secret,id=mysecret,target=/secret.txt [ "$(cat /secret.txt)" = "pw" ] || false
`,
`
FROM nanoserver
USER ContainerAdministrator
RUN --mount=type=secret,id=mysecret,target=C:/secret.txt findstr pw C:\secret.txt
`,
))

dir := integration.Tmpdir(
t,
Expand Down
17 changes: 17 additions & 0 deletions frontend/dockerfile/docs/reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,23 @@ an environment variable by setting the `env` option.
| `uid` | User ID for secret file. Default `0`. |
| `gid` | Group ID for secret file. Default `0`. |

> [!NOTE]
> On Windows containers, the secret is delivered as a single-file, read-only
> bind mount (there is no `tmpfs`). An explicit `target` is required because
> there is no default `/run/secrets/` location, e.g.
> `--mount=type=secret,id=mysecret,target=C:/path/to/secret`. Use forward
> slashes in the target: the Dockerfile escape character (default `\`) otherwise
> consumes the backslashes during parsing. The `mode`, `uid`, and `gid` options
> are not supported on Windows and are ignored. The secret is written to a
> temporary file restricted (via an explicit ACL) to SYSTEM, the Administrators
> group, and the BuildKit daemon account, then removed after the step; it is
> written in clear text, so consider BitLocker for at-rest encryption. Because
> the file is not granted to the container's default user, the `RUN` step must
> execute as an administrator (e.g. `USER ContainerAdministrator`) to read the
> secret. The secret value is not persisted in the image; however, an empty
> placeholder directory may remain at the `target` path in the resulting
> layer, which is an inherent property of Windows bind mounts.

#### Example: access to S3

```dockerfile
Expand Down
81 changes: 0 additions & 81 deletions solver/llbsolver/mounts/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ package mounts
import (
"context"
"fmt"
"os"
"path/filepath"
"strings"
"sync"
"time"

"github.com/containerd/containerd/v2/core/mount"
"github.com/moby/buildkit/cache"
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/identity"
"github.com/moby/buildkit/session"
"github.com/moby/buildkit/session/secrets"
"github.com/moby/buildkit/session/sshforward"
Expand All @@ -22,7 +19,6 @@ import (
"github.com/moby/buildkit/util/grpcerrors"
"github.com/moby/locker"
"github.com/moby/sys/user"
"github.com/moby/sys/userns"
"github.com/pkg/errors"
"google.golang.org/grpc/codes"
)
Expand Down Expand Up @@ -286,83 +282,6 @@ type secretMountInstance struct {
idmap *user.IdentityMapping
}

func (sm *secretMountInstance) Mount() ([]mount.Mount, func() error, error) {
dir, err := os.MkdirTemp("", "buildkit-secrets")
if err != nil {
return nil, nil, errors.Wrap(err, "failed to create temp dir")
}
cleanupDir := func() error {
return os.RemoveAll(dir)
}

if err := os.Chmod(dir, 0711); err != nil {
cleanupDir()
return nil, nil, err
}

var mountOpts []string
if sm.sm.mount.SecretOpt.Mode&0o111 == 0 {
mountOpts = append(mountOpts, "noexec")
}

tmpMount := mount.Mount{
Type: "tmpfs",
Source: "tmpfs",
Options: append([]string{"nodev", "nosuid", fmt.Sprintf("uid=%d,gid=%d", os.Geteuid(), os.Getegid())}, mountOpts...),
}

if userns.RunningInUserNS() {
tmpMount.Options = nil
}

if err := mount.All([]mount.Mount{tmpMount}, dir); err != nil {
cleanupDir()
return nil, nil, errors.Wrap(err, "unable to setup secret mount")
}
sm.root = dir

cleanup := func() error {
if err := mount.Unmount(dir, 0); err != nil {
return err
}
return cleanupDir()
}

randID := identity.NewID()
fp := filepath.Join(dir, randID)
if err := os.WriteFile(fp, sm.sm.data, 0600); err != nil {
cleanup()
return nil, nil, err
}

uid := int(sm.sm.mount.SecretOpt.Uid)
gid := int(sm.sm.mount.SecretOpt.Gid)

if sm.idmap != nil {
uid, gid, err = sm.idmap.ToHost(uid, gid)
if err != nil {
cleanup()
return nil, nil, err
}
}

if err := os.Chown(fp, uid, gid); err != nil {
cleanup()
return nil, nil, err
}

if err := os.Chmod(fp, os.FileMode(sm.sm.mount.SecretOpt.Mode&0777)); err != nil {
cleanup()
return nil, nil, err
}

return []mount.Mount{{
Type: "bind",
Source: fp,
Options: append([]string{"ro", "rbind", "nodev", "nosuid"}, mountOpts...),
}}, cleanup, nil
}

func (sm *secretMountInstance) IdentityMapping() *user.IdentityMapping {
return sm.idmap
}
Expand Down
Loading