Support unsigned command verification into the tool

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

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)

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
+}

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)
+}