diff --git a/README.md b/README.md index e1d5b91..cde4333 100644 --- a/README.md +++ b/README.md @@ -21,12 +21,6 @@ buildkite-signed-pipeline upload In a global `environment` hook, you can include the following to ensure that all jobs that are handed to an agent contain the correct signatures: ```bash -# Allow the upload command to be unsigned, as it typically comes from the Buildkite UI and not your agents -if [[ "${BUILDKITE_COMMAND}" == "buildkite-signed-pipeline upload" ]]; then - echo "Allowing pipeline upload" - exit 0 -fi - export SIGNED_PIPELINE_SECRET='my secret' if ! buildkite-signed-pipeline verify ; then @@ -37,6 +31,31 @@ fi This step will fail if the provided signatures aren't in the environment. +#### Allowing initial commands + +By default the tool will allow steps through without a signature, this allows commands to be specified in the Buildkite UI -- for example the initial pipeline upload command. + +The rules for allowing such commands are: + + 1. `BUILDKITE_COMMAND` is (by default) `buildkite-signed-pipline upload`; **and** + 2. `BUILDKITE_PLUGINS` is empty/unset; **and** + 3. `STEP_SIGNATURE` is empty/unset + +Additional commands can be specified with `--allow-unsigned-command`: + +```bash +export SIGNED_PIPELINE_SECRET='my secret' + +buildkite-signed-pipeline verify \ + --allow-unsigned-command="buildkite-signed-pipline upload" \ + --allow-unsigned-command="buildkite-signed-pipeline upload ./test.yml" + +if [[ $? -ne 0 ]] ; then + echo "Step verification failed" + exit 1 +fi +``` + ## Managing signing secrets ### Simple secret diff --git a/main.go b/main.go index de9a441..d05f4f4 100644 --- a/main.go +++ b/main.go @@ -48,9 +48,13 @@ func main() { BoolVar(&uploadCommand.DryRun) verifyCommand := &verifyCommand{} - app.Command("verify", "Verify a job contains a signature").Action(verifyCommand.run) + verifyCommandClause := app.Command("verify", "Verify a job contains a signature").Action(verifyCommand.run) + verifyCommandClause. + Flag("allowed-unsigned-command", "A list of commands that will be allowed without requiring a signature"). + Default("buildkite-signed-pipeline upload"). + StringsVar(&verifyCommand.AllowedUnsignedCommands) - app.PreAction(func(c *kingpin.ParseContext) error { + app.Action(func(c *kingpin.ParseContext) error { if sharedSecret == "" && awsSharedSecretId == "" { return errors.New("One of --shared-secret or --aws-sm-shared-secret-id must be provided") } @@ -125,7 +129,8 @@ func (l *uploadCommand) run(c *kingpin.ParseContext) error { } type verifyCommand struct { - Signer *SharedSecretSigner + Signer *SharedSecretSigner + AllowedUnsignedCommands []string } func (v *verifyCommand) run(c *kingpin.ParseContext) error { @@ -138,7 +143,7 @@ func (v *verifyCommand) run(c *kingpin.ParseContext) error { return nil } - err := v.Signer.Verify(command, pluginJSON, Signature(sig)) + err := v.Signer.Verify(command, pluginJSON, v.AllowedUnsignedCommands, Signature(sig)) if err != nil { log.Fatalln(err) diff --git a/signer.go b/signer.go index 081da2b..96c86f7 100644 --- a/signer.go +++ b/signer.go @@ -189,17 +189,44 @@ func (s SharedSecretSigner) signData(command string, pluginJSON string) (Signatu return Signature(fmt.Sprintf("sha256:%x", h.Sum(nil))), nil } -func (s SharedSecretSigner) Verify(command string, pluginJSON string, expected Signature) error { - signature, err := s.signData(command, pluginJSON) +func (s SharedSecretSigner) Verify(command string, pluginJSON string, allowedUnsignedCommands []string, expected Signature) error { + // Allow any command on the unsigned list to be verified provided it has no signature and plugins. + // Checking there's no signature is important to ensure plugins aren't being injected with a + // command on the built-in allow list + if contains(allowedUnsignedCommands, command) { + log.Println("Command is on the allowed list of unsigned commands") + if expected == "" && pluginJSON == "" { + log.Println("✅ Allowing allow-listed command as no signature is specified, and no plugins are referenced") + return nil + } + log.Println("❗️Forcing signature check as pstep has an expected signature or referenced plugins") + } + + // allow signerFunc to be overwritten in tests + signerFunc := s.signerFunc + if signerFunc == nil { + signerFunc = s.signData + } + + signature, err := signerFunc(command, pluginJSON) if err != nil { return err } if signature != expected { - return errors.New("🚨 Signature mismatch." + + return errors.New("🚨 Signature mismatch. " + "Perhaps check the shared secret is the same across agents?") } return nil } + +func contains(hayStack []string, needle string) bool { + for _, item := range hayStack { + if item == needle { + return true + } + } + return false +} diff --git a/signer_test.go b/signer_test.go index 4c699c8..144d67c 100644 --- a/signer_test.go +++ b/signer_test.go @@ -173,3 +173,112 @@ func TestPipelines(t *testing.T) { }) } } + +func TestVerifyCommand(t *testing.T) { + const expectedPluginJSON = "" + const expectedCommand = `echo hello world` + const expectedSignature = Signature("llamas") + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return expectedSignature, nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{}, expectedSignature) + assert.Nil(t, err) +} + +func TestVerifyCommandAndPlugins(t *testing.T) { + const expectedPluginJSON = `[{"github.com/buildkite-plugins/docker-buildkite-plugin#v123":{"image":"node8"}}]` + const expectedCommand = `echo hello world` + const expectedSignature = Signature("llamas") + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return expectedSignature, nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{}, expectedSignature) + assert.Nil(t, err) +} + +func TestVerifyCommandAndPluginsRejectsSignature(t *testing.T) { + const expectedPluginJSON = `[{"github.com/buildkite-plugins/docker-buildkite-plugin#v123":{"image":"node8"}}]` + const expectedCommand = `echo hello world` + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return Signature("llamas"), nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{}, "oh no") + assert.NotNil(t, err) +} + +func TestVerifyPluginsAndNoCommand(t *testing.T) { + const expectedPluginJSON = `[{"github.com/buildkite-plugins/docker-buildkite-plugin#v123":{"image":"node8"}}]` + const expectedCommand = "" + const expectedSignature = Signature("llamas") + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return expectedSignature, nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{}, expectedSignature) + assert.Nil(t, err) +} + +func TestVerifyPluginAndAllowedUnsignedCommand(t *testing.T) { + const expectedPluginJSON = `[{"github.com/buildkite-plugins/docker-buildkite-plugin#v123":{"image":"node8"}}]` + const expectedCommand = "buildkite-signed-pipeline upload" + const expectedSignature = Signature("llamas") + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return expectedSignature, nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{expectedCommand}, expectedSignature) + assert.Nil(t, err) +} + +func TestVerifyPluginAndAllowedUnsignedCommandRejectsSignature(t *testing.T) { + const expectedPluginJSON = `[{"github.com/buildkite-plugins/docker-buildkite-plugin#v123":{"image":"node8"}}]` + const expectedCommand = "buildkite-signed-pipeline upload" + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return Signature("llamas"), nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{expectedCommand}, "oh no") + assert.NotNil(t, err) +} + +func TestVerifyAllowedUnsignedCommand(t *testing.T) { + const expectedPluginJSON = "" + const expectedCommand = "buildkite-signed-pipeline upload" + + signer := NewSharedSecretSigner("secret-llamas") + signer.signerFunc = func(command, plugins string) (Signature, error) { + assert.Equal(t, expectedCommand, command) + assert.Equal(t, expectedPluginJSON, plugins) + return Signature("llamas"), nil + } + + err := signer.Verify(expectedCommand, expectedPluginJSON, []string{expectedCommand}, "") + assert.Nil(t, err) +}