From 9c0e2d1d9530c2527f10e6b3fd91872bd6f3b5e7 Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Tue, 25 May 2021 18:56:16 -0700 Subject: [PATCH] buildkit secrets support - Secrets are never exposed in plaintext in the Cue tree. `dagger query` won't dump secrets anymore, Cue errors won't contain them either. - BuildKit-native secrets support through a new `mount` type. This ensures secrets will never be part of containerd layers, buildkit cache and generally speaking will never be saved to disk in plaintext. - Updated netlify as an example - Added tests - Changed the Cue definition of a secret to: ``` @dagger(secret) id: string } ``` This is to ensure both that setting the wrong input type on a secret (e.g. `dagger input text`) will fail, and attempting to misuse the secret (e.g. interpolating, passing as an env variable, etc) will also fail properly. Signed-off-by: Andrea Luzzardi --- client/client.go | 32 ++++--------- environment/environment.go | 6 +-- environment/pipeline.go | 19 ++++++++ solver/secretsprovider.go | 47 +++++++++++++++++++ solver/solver.go | 30 +++++++++++- state/input.go | 40 +++++++++------- stdlib/dagger/dagger.cue | 5 +- stdlib/dagger/op/op.cue | 2 +- stdlib/netlify/netlify.cue | 6 +-- stdlib/netlify/netlify.sh.cue | 2 + stdlib/os/container.cue | 11 +++-- tests/compute.bats | 27 +++++++++++ tests/compute/secrets/invalid/env/env.cue | 21 +++++++++ .../compute/secrets/invalid/string/string.cue | 21 +++++++++ tests/compute/secrets/simple/simple.cue | 34 ++++++++++++++ 15 files changed, 244 insertions(+), 59 deletions(-) create mode 100644 solver/secretsprovider.go create mode 100644 tests/compute/secrets/invalid/env/env.cue create mode 100644 tests/compute/secrets/invalid/string/string.cue create mode 100644 tests/compute/secrets/simple/simple.cue diff --git a/client/client.go b/client/client.go index 15506007..81d348ca 100644 --- a/client/client.go +++ b/client/client.go @@ -2,7 +2,6 @@ package client import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -87,13 +86,13 @@ func (c *Client) Do(ctx context.Context, state *state.State, fn DoFunc) (*enviro // Spawn build function eg.Go(func() error { - return c.buildfn(gctx, environment, fn, events) + return c.buildfn(gctx, state, environment, fn, events) }) return environment, eg.Wait() } -func (c *Client) buildfn(ctx context.Context, env *environment.Environment, fn DoFunc, ch chan *bk.SolveStatus) error { +func (c *Client) buildfn(ctx context.Context, st *state.State, env *environment.Environment, fn DoFunc, ch chan *bk.SolveStatus) error { lg := log.Ctx(ctx) // Scan local dirs to grant access @@ -109,10 +108,13 @@ func (c *Client) buildfn(ctx context.Context, env *environment.Environment, fn D // buildkit auth provider (registry) auth := solver.NewRegistryAuthProvider() + // secrets + secrets := solver.NewSecretsProvider(st) + // Setup solve options opts := bk.SolveOpt{ LocalDirs: localdirs, - Session: []session.Attachable{auth}, + Session: []session.Attachable{auth, secrets}, } // Call buildkit solver @@ -127,6 +129,7 @@ func (c *Client) buildfn(ctx context.Context, env *environment.Environment, fn D Gateway: gw, Events: ch, Auth: auth, + Secrets: secrets, NoCache: c.noCache, }) @@ -165,7 +168,7 @@ func (c *Client) buildfn(ctx context.Context, env *environment.Environment, fn D return res, nil }, ch) if err != nil { - return fmt.Errorf("buildkit solve: %w", bkCleanError(err)) + return solver.CleanError(err) } for k, v := range resp.ExporterResponse { // FIXME consume exporter response @@ -243,22 +246,3 @@ func (c *Client) logSolveStatus(ctx context.Context, ch chan *bk.SolveStatus) er }, ) } - -// A helper to remove noise from buildkit error messages. -// FIXME: Obviously a cleaner solution would be nice. -func bkCleanError(err error) error { - noise := []string{ - "executor failed running ", - "buildkit-runc did not terminate successfully", - "rpc error: code = Unknown desc = ", - "failed to solve: ", - } - - msg := err.Error() - - for _, s := range noise { - msg = strings.ReplaceAll(msg, s, "") - } - - return errors.New(msg) -} diff --git a/environment/environment.go b/environment/environment.go index cc732f4c..7cb0a45a 100644 --- a/environment/environment.go +++ b/environment/environment.go @@ -44,7 +44,7 @@ func New(st *state.State) (*Environment, error) { // Prepare inputs for key, input := range st.Inputs { - v, err := input.Compile(st) + v, err := input.Compile(key, st) if err != nil { return nil, err } @@ -86,7 +86,7 @@ func (e *Environment) LoadPlan(ctx context.Context, s solver.Solver) error { span, ctx := opentracing.StartSpanFromContext(ctx, "environment.LoadPlan") defer span.Finish() - planSource, err := e.state.PlanSource().Compile(e.state) + planSource, err := e.state.PlanSource().Compile("", e.state) if err != nil { return err } @@ -157,7 +157,7 @@ func (e *Environment) LocalDirs() map[string]string { } // 2. Scan the plan - plan, err := e.state.PlanSource().Compile(e.state) + plan, err := e.state.PlanSource().Compile("", e.state) if err != nil { panic(err) } diff --git a/environment/pipeline.go b/environment/pipeline.go index 226e0830..687371b6 100644 --- a/environment/pipeline.go +++ b/environment/pipeline.go @@ -490,6 +490,25 @@ func (p *Pipeline) mount(ctx context.Context, dest string, mnt *compiler.Value) return nil, fmt.Errorf("invalid mount source: %q", s) } } + // eg. mount: "/foo": secret: mysecret + if secret := mnt.Lookup("secret"); secret.Exists() { + if !secret.HasAttr("secret") { + return nil, fmt.Errorf("invalid secret %q: not a secret", secret.Path().String()) + } + idValue := secret.Lookup("id") + if !idValue.Exists() { + return nil, fmt.Errorf("invalid secret %q: no id field", secret.Path().String()) + } + id, err := idValue.String() + if err != nil { + return nil, fmt.Errorf("invalid secret id: %w", err) + } + return llb.AddSecret(dest, + llb.SecretID(id), + llb.SecretFileOpt(0, 0, 0400), // uid, gid, mask) + ), nil + } + // eg. mount: "/foo": { from: www.source } from := NewPipeline(mnt.Lookup("from"), p.s) if err := from.Run(ctx); err != nil { diff --git a/solver/secretsprovider.go b/solver/secretsprovider.go new file mode 100644 index 00000000..67f8436e --- /dev/null +++ b/solver/secretsprovider.go @@ -0,0 +1,47 @@ +package solver + +import ( + "context" + "strings" + + "github.com/moby/buildkit/session" + "github.com/moby/buildkit/session/secrets" + "github.com/moby/buildkit/session/secrets/secretsprovider" + "github.com/rs/zerolog/log" + "go.dagger.io/dagger/state" +) + +func NewSecretsProvider(st *state.State) session.Attachable { + return secretsprovider.NewSecretProvider(&inputStore{st}) +} + +type inputStore struct { + st *state.State +} + +func (s *inputStore) GetSecret(ctx context.Context, id string) ([]byte, error) { + lg := log.Ctx(ctx) + + const secretPrefix = "secret=" + + if !strings.HasPrefix(id, secretPrefix) { + return nil, secrets.ErrNotFound + } + + id = strings.TrimPrefix(id, secretPrefix) + + input, ok := s.st.Inputs[id] + if !ok { + return nil, secrets.ErrNotFound + } + if input.Secret == nil { + return nil, secrets.ErrNotFound + } + + lg. + Debug(). + Str("id", id). + Msg("injecting secret") + + return []byte(input.Secret.PlainText()), nil +} diff --git a/solver/solver.go b/solver/solver.go index 4a33ac62..7ec67cf8 100644 --- a/solver/solver.go +++ b/solver/solver.go @@ -3,7 +3,9 @@ package solver import ( "context" "encoding/json" + "errors" "fmt" + "strings" bk "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/llb" @@ -25,6 +27,7 @@ type Opts struct { Gateway bkgw.Client Events chan *bk.SolveStatus Auth *RegistryAuthProvider + Secrets session.Attachable NoCache bool } @@ -100,7 +103,11 @@ func (s Solver) ResolveImageConfig(ctx context.Context, ref string, opts llb.Res // Solve will block until the state is solved and returns a Reference. func (s Solver) SolveRequest(ctx context.Context, req bkgw.SolveRequest) (*bkgw.Result, error) { - return s.opts.Gateway.Solve(ctx, req) + res, err := s.opts.Gateway.Solve(ctx, req) + if err != nil { + return nil, CleanError(err) + } + return res, nil } // Solve will block until the state is solved and returns a Reference. @@ -150,7 +157,7 @@ func (s Solver) Export(ctx context.Context, st llb.State, img *dockerfile2llb.Im opts := bk.SolveOpt{ Exports: []bk.ExportEntry{output}, - Session: []session.Attachable{s.opts.Auth}, + Session: []session.Attachable{s.opts.Auth, s.opts.Secrets}, } ch := make(chan *bk.SolveStatus) @@ -204,3 +211,22 @@ func dumpLLB(def *bkpb.Definition) ([]byte, error) { } return json.Marshal(ops) } + +// A helper to remove noise from buildkit error messages. +// FIXME: Obviously a cleaner solution would be nice. +func CleanError(err error) error { + noise := []string{ + "executor failed running ", + "buildkit-runc did not terminate successfully", + "rpc error: code = Unknown desc = ", + "failed to solve: ", + } + + msg := err.Error() + + for _, s := range noise { + msg = strings.ReplaceAll(msg, s, "") + } + + return errors.New(msg) +} diff --git a/state/input.go b/state/input.go index c29e5e0d..cdf74e45 100644 --- a/state/input.go +++ b/state/input.go @@ -37,24 +37,24 @@ type Input struct { File *fileInput `yaml:"file,omitempty"` } -func (i Input) Compile(state *State) (*compiler.Value, error) { +func (i Input) Compile(key string, state *State) (*compiler.Value, error) { switch { case i.Dir != nil: - return i.Dir.Compile(state) + return i.Dir.Compile(key, state) case i.Git != nil: - return i.Git.Compile(state) + return i.Git.Compile(key, state) case i.Docker != nil: - return i.Docker.Compile(state) + return i.Docker.Compile(key, state) case i.Text != nil: - return i.Text.Compile(state) + return i.Text.Compile(key, state) case i.Secret != nil: - return i.Secret.Compile(state) + return i.Secret.Compile(key, state) case i.JSON != nil: - return i.JSON.Compile(state) + return i.JSON.Compile(key, state) case i.YAML != nil: - return i.YAML.Compile(state) + return i.YAML.Compile(key, state) case i.File != nil: - return i.File.Compile(state) + return i.File.Compile(key, state) default: return nil, fmt.Errorf("input has not been set") } @@ -75,7 +75,7 @@ type dirInput struct { Include []string `json:"include,omitempty"` } -func (dir dirInput) Compile(state *State) (*compiler.Value, error) { +func (dir dirInput) Compile(_ string, state *State) (*compiler.Value, error) { // FIXME: serialize an intermediate struct, instead of generating cue source // json.Marshal([]string{}) returns []byte("null"), which wreaks havoc @@ -122,7 +122,7 @@ func GitInput(remote, ref, dir string) Input { } } -func (git gitInput) Compile(_ *State) (*compiler.Value, error) { +func (git gitInput) Compile(_ string, _ *State) (*compiler.Value, error) { ref := "HEAD" if git.Ref != "" { ref = git.Ref @@ -148,7 +148,7 @@ type dockerInput struct { Ref string `json:"ref,omitempty"` } -func (i dockerInput) Compile(_ *State) (*compiler.Value, error) { +func (i dockerInput) Compile(_ string, _ *State) (*compiler.Value, error) { panic("NOT IMPLEMENTED") } @@ -162,7 +162,7 @@ func TextInput(data string) Input { type textInput string -func (i textInput) Compile(_ *State) (*compiler.Value, error) { +func (i textInput) Compile(_ string, _ *State) (*compiler.Value, error) { return compiler.Compile("", fmt.Sprintf("%q", i)) } @@ -176,8 +176,12 @@ func SecretInput(data string) Input { type secretInput string -func (i secretInput) Compile(_ *State) (*compiler.Value, error) { - return compiler.Compile("", fmt.Sprintf("%q", i)) +func (i secretInput) Compile(key string, _ *State) (*compiler.Value, error) { + return compiler.Compile("", fmt.Sprintf(`{id:%q}`, "secret="+key)) +} + +func (i secretInput) PlainText() string { + return string(i) } // An input value encoded as JSON @@ -190,7 +194,7 @@ func JSONInput(data string) Input { type jsonInput string -func (i jsonInput) Compile(_ *State) (*compiler.Value, error) { +func (i jsonInput) Compile(_ string, _ *State) (*compiler.Value, error) { return compiler.DecodeJSON("", []byte(i)) } @@ -204,7 +208,7 @@ func YAMLInput(data string) Input { type yamlInput string -func (i yamlInput) Compile(_ *State) (*compiler.Value, error) { +func (i yamlInput) Compile(_ string, _ *State) (*compiler.Value, error) { return compiler.DecodeYAML("", []byte(i)) } @@ -220,7 +224,7 @@ type fileInput struct { Path string `json:"data,omitempty"` } -func (i fileInput) Compile(_ *State) (*compiler.Value, error) { +func (i fileInput) Compile(_ string, _ *State) (*compiler.Value, error) { data, err := ioutil.ReadFile(i.Path) if err != nil { return nil, err diff --git a/stdlib/dagger/dagger.cue b/stdlib/dagger/dagger.cue index 3f9bcdd2..e7afc6d5 100644 --- a/stdlib/dagger/dagger.cue +++ b/stdlib/dagger/dagger.cue @@ -15,9 +15,8 @@ import ( } // Secret value -// FIXME: currently aliased as a string to mark secrets -// this requires proper support. #Secret: { @dagger(secret) - string | bytes + + id: string } diff --git a/stdlib/dagger/op/op.cue b/stdlib/dagger/op/op.cue index 18f1e3ee..af8acd5b 100644 --- a/stdlib/dagger/op/op.cue +++ b/stdlib/dagger/op/op.cue @@ -57,7 +57,7 @@ package op // `true` means also ignoring the mount cache volumes always?: true | *false dir: string | *"/" - mount: [string]: "tmpfs" | "cache" | {from: _, path: string | *"/"} + mount: [string]: "tmpfs" | "cache" | {from: _, path: string | *"/"} | {secret: _} // Map of hostnames to ip hosts?: [string]: string // User to exec with (if left empty, will default to the set user in the image) diff --git a/stdlib/netlify/netlify.cue b/stdlib/netlify/netlify.cue index 61e23f06..79fabdcc 100644 --- a/stdlib/netlify/netlify.cue +++ b/stdlib/netlify/netlify.cue @@ -80,10 +80,10 @@ import ( if customDomain != _|_ { NETLIFY_DOMAIN: customDomain } - NETLIFY_ACCOUNT: account.name - NETLIFY_AUTH_TOKEN: account.token + NETLIFY_ACCOUNT: account.name } dir: "/src" - mount: "/src": from: contents + mount: "/src": from: contents + mount: "/token": secret: account.token } } diff --git a/stdlib/netlify/netlify.sh.cue b/stdlib/netlify/netlify.sh.cue index 2130de71..27667a9e 100644 --- a/stdlib/netlify/netlify.sh.cue +++ b/stdlib/netlify/netlify.sh.cue @@ -1,6 +1,8 @@ package netlify #Site: ctr: command: #""" + export NETLIFY_AUTH_TOKEN="$(cat /token)" + create_site() { url="https://api.netlify.com/api/v1/${NETLIFY_ACCOUNT:-}/sites" diff --git a/stdlib/os/container.cue b/stdlib/os/container.cue index f13185f8..035de1c3 100644 --- a/stdlib/os/container.cue +++ b/stdlib/os/container.cue @@ -52,6 +52,8 @@ import ( mount: [string]: { from: dagger.#Artifact // FIXME: support source path + } | { + secret: dagger.#Secret } // Mount persistent cache directories @@ -94,10 +96,9 @@ import ( // Execute setup commands, without volumes for cmd in setup { op.#Exec & { - args: [shell.path] + shell.args + [cmd] - "env": env - "dir": dir - "always": always + args: [shell.path] + shell.args + [cmd] + "env": env + "dir": dir } }, // Execute main command with volumes @@ -109,7 +110,7 @@ import ( "always": always "mount": { for dest, o in mount { - "\(dest)": from: o.from + "\(dest)": o // FIXME: support source path } for dest in cache { diff --git a/tests/compute.bats b/tests/compute.bats index 7f0363dc..97676dd4 100644 --- a/tests/compute.bats +++ b/tests/compute.bats @@ -67,6 +67,33 @@ setup() { assert_line '{"in":"foobar","test":"received: foobar"}' } +@test "compute: secrets" { + # secrets used as environment variables must fail + run "$DAGGER" compute "$TESTDIR"/compute/secrets/invalid/env + assert_failure + assert_line --partial "conflicting values" + + # strings passed as secrets must fail + run "$DAGGER" compute "$TESTDIR"/compute/secrets/invalid/string + assert_failure + + # Setting a text input for a secret value should fail + run "$DAGGER" compute --input-string 'mySecret=SecretValue' "$TESTDIR"/compute/secrets/simple + assert_failure + + # Now test with an actual secret and make sure it works + "$DAGGER" init + dagger_new_with_plan secrets "$TESTDIR"/compute/secrets/simple + "$DAGGER" input secret mySecret SecretValue + run "$DAGGER" up + assert_success + + # Make sure the secret doesn't show in dagger query + run "$DAGGER" query mySecret.id -f text + assert_success + assert_output "secret=mySecret" +} + @test ".daggerignore" { "$DAGGER" compute --input-dir TestData="$TESTDIR"/compute/ignore/testdata "$TESTDIR"/compute/ignore } diff --git a/tests/compute/secrets/invalid/env/env.cue b/tests/compute/secrets/invalid/env/env.cue new file mode 100644 index 00000000..bfded599 --- /dev/null +++ b/tests/compute/secrets/invalid/env/env.cue @@ -0,0 +1,21 @@ +package testing + +import ( + "dagger.io/dagger" + "dagger.io/dagger/op" + "dagger.io/alpine" +) + +mySecret: dagger.#Secret + +TestSecrets: #up: [ + op.#Load & { + from: alpine.#Image & { + package: bash: "=~5.1" + } + }, + + op.#Exec & { + env: foo: mySecret + }, +] diff --git a/tests/compute/secrets/invalid/string/string.cue b/tests/compute/secrets/invalid/string/string.cue new file mode 100644 index 00000000..2015a1d7 --- /dev/null +++ b/tests/compute/secrets/invalid/string/string.cue @@ -0,0 +1,21 @@ +package testing + +import ( + "dagger.io/dagger/op" + "dagger.io/alpine" +) + +mySecret: dagger.#Secret + +TestString: #up: [ + op.#Load & { + from: alpine.#Image & { + package: bash: "=~5.1" + } + }, + + op.#Exec & { + mount: "/secret": secret: mySecret + args: ["true"] + }, +] diff --git a/tests/compute/secrets/simple/simple.cue b/tests/compute/secrets/simple/simple.cue new file mode 100644 index 00000000..4d1f5ca6 --- /dev/null +++ b/tests/compute/secrets/simple/simple.cue @@ -0,0 +1,34 @@ +package testing + +import ( + "dagger.io/dagger" + "dagger.io/dagger/op" + "dagger.io/alpine" +) + +mySecret: dagger.#Secret + +TestSecrets: #up: [ + op.#Load & { + from: alpine.#Image & { + package: bash: "=~5.1" + } + }, + + op.#Exec & { + mount: "/secret": secret: mySecret + env: PLAIN: mySecret.id + args: [ + "/bin/bash", + "--noprofile", + "--norc", + "-eo", + "pipefail", + "-c", + #""" + test "$(cat /secret)" = "SecretValue" + test "$PLAIN" != "SecretValue" + """#, + ] + }, +]