Merge pull request #2853 from buildkite/debug-signature

Log public signing key thumbprint and signed step payload
buildkite · Jul 2, 2024 · aa55778 · aa55778
2 parents f4b9942 + f1b8d85
commit aa55778
Show file tree

Hide file tree

Showing 10 changed files with 68 additions and 16 deletions.
diff --git a/agent/agent_configuration.go b/agent/agent_configuration.go
@@ -39,6 +39,7 @@ type AgentConfiguration struct {
 
 	SigningJWKSFile  string // Where to find the key to sign pipeline uploads with (passed through to jobs, they might be uploading pipelines)
 	SigningJWKSKeyID string // The key ID to sign pipeline uploads with
+	DebugSigning     bool   // Whether to print step payloads when signing them
 
 	VerificationJWKS             jwk.Set // The set of keys to verify jobs with
 	VerificationFailureBehaviour string  // What to do if job verification fails (one of `block` or `warn`)

diff --git a/agent/integration/job_verification_integration_test.go b/agent/integration/job_verification_integration_test.go
@@ -568,7 +568,7 @@ func TestJobVerification(t *testing.T) {
 				RepositoryURL: tc.repositoryURL,
 			}
 
-			tc.job.Step = signStep(t, tc.signingKey, pipelineUploadEnv, stepWithInvariants)
+			tc.job.Step = signStep(t, ctx, tc.signingKey, pipelineUploadEnv, stepWithInvariants)
 			err := runJob(t, ctx, testRunJobConfig{
 				job:              &tc.job,
 				server:           server,
@@ -659,6 +659,7 @@ func jwksFromKeys(t *testing.T, jwkes ...jwk.Key) jwk.Set {
 
 func signStep(
 	t *testing.T,
+	ctx context.Context,
 	key jwk.Key,
 	env map[string]string,
 	stepWithInvariants signature.CommandStepWithInvariants,
@@ -670,7 +671,7 @@ func signStep(
 		return stepWithInvariants.CommandStep
 	}
 
-	signature, err := signature.Sign(key, env, &stepWithInvariants)
+	signature, err := signature.Sign(ctx, key, &stepWithInvariants, signature.WithEnv(env))
 	if err != nil {
 		t.Fatalf("signing step: %v", err)
 	}

diff --git a/agent/job_runner.go b/agent/job_runner.go
@@ -523,6 +523,10 @@ func (r *JobRunner) createEnvironment(ctx context.Context) ([]string, error) {
 		env["BUILDKITE_AGENT_JWKS_KEY_ID"] = r.conf.AgentConfiguration.SigningJWKSKeyID
 	}
 
+	if r.conf.AgentConfiguration.DebugSigning {
+		env["BUILDKITE_AGENT_DEBUG_SIGNING"] = "true"
+	}
+
 	enablePluginValidation := r.conf.AgentConfiguration.PluginValidation
 	// Allow BUILDKITE_PLUGIN_VALIDATION to be enabled from env for easier
 	// per-pipeline testing

diff --git a/agent/run_job.go b/agent/run_job.go
@@ -101,7 +101,7 @@ func (r *JobRunner) Run(ctx context.Context) error {
 
 	if r.conf.JWKS != nil {
 		ise := &invalidSignatureError{}
-		switch err := r.verifyJob(r.conf.JWKS); {
+		switch err := r.verifyJob(ctx, r.conf.JWKS); {
 		case errors.Is(err, ErrNoSignature) || errors.As(err, &ise):
 			r.verificationFailureLogs(r.VerificationFailureBehavior, err)
 			if r.VerificationFailureBehavior == VerificationBehaviourBlock {

diff --git a/agent/verify_job.go b/agent/verify_job.go
@@ -2,6 +2,7 @@ package agent
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -34,7 +35,7 @@ func (e *invalidSignatureError) Unwrap() error {
 	return e.underlying
 }
 
-func (r *JobRunner) verifyJob(keySet jwk.Set) error {
+func (r *JobRunner) verifyJob(ctx context.Context, keySet jwk.Set) error {
 	step := r.conf.Job.Step
 
 	if step.Signature == nil {
@@ -48,7 +49,15 @@ func (r *JobRunner) verifyJob(keySet jwk.Set) error {
 	}
 
 	// Verify the signature
-	if err := signature.Verify(step.Signature, r.conf.JWKS, r.conf.Job.Env, stepWithInvariants); err != nil {
+	err := signature.Verify(
+		ctx,
+		step.Signature,
+		r.conf.JWKS, stepWithInvariants,
+		signature.WithEnv(r.conf.Job.Env),
+		signature.WithLogger(r.agentLogger),
+		signature.WithDebugSigning(r.conf.AgentConfiguration.DebugSigning),
+	)
+	if err != nil {
 		r.agentLogger.Debug("verifyJob: step.Signature.Verify(Job.Env, stepWithInvariants, JWKS) = %v", err)
 		return newInvalidSignatureError(ErrVerificationFailed)
 	}

diff --git a/clicommand/agent_start.go b/clicommand/agent_start.go
@@ -84,6 +84,7 @@ type AgentStartConfig struct {
 
 	SigningJWKSFile  string `cli:"signing-jwks-file" normalize:"filepath"`
 	SigningJWKSKeyID string `cli:"signing-jwks-key-id"`
+	DebugSigning     bool   `cli:"debug-signing"`
 
 	VerificationJWKSFile        string `cli:"verification-jwks-file" normalize:"filepath"`
 	VerificationFailureBehavior string `cli:"verification-failure-behavior"`
@@ -655,6 +656,11 @@ var AgentStartCommand = cli.Command{
 			Usage:  "The JWKS key ID to use when signing the pipeline. If omitted, and the signing JWKS contains only one key, that key will be used.",
 			EnvVar: "BUILDKITE_AGENT_SIGNING_JWKS_KEY_ID",
 		},
+		cli.BoolFlag{
+			Name:   "debug-signing",
+			Usage:  "Enable debug logging for pipeline signing. This can potentially leak secrets to the logs as it prints each step in full before signing. Requires debug logging to be enabled",
+			EnvVar: "BUILDKITE_AGENT_DEBUG_SIGNING",
+		},
 		cli.StringFlag{
 			Name:   "verification-failure-behavior",
 			Value:  agent.VerificationBehaviourBlock,
@@ -961,6 +967,7 @@ var AgentStartCommand = cli.Command{
 
 			SigningJWKSFile:  cfg.SigningJWKSFile,
 			SigningJWKSKeyID: cfg.SigningJWKSKeyID,
+			DebugSigning:     cfg.DebugSigning,
 
 			VerificationJWKS: verificationJWKS,
 

diff --git a/clicommand/pipeline_upload.go b/clicommand/pipeline_upload.go
@@ -74,8 +74,9 @@ type PipelineUploadConfig struct {
 	RejectSecrets   bool     `cli:"reject-secrets"`
 
 	// Used for signing
-	JWKSFile  string `cli:"jwks-file"`
-	JWKSKeyID string `cli:"jwks-key-id"`
+	JWKSFile     string `cli:"jwks-file"`
+	JWKSKeyID    string `cli:"jwks-key-id"`
+	DebugSigning bool   `cli:"debug-signing"`
 
 	// Global flags
 	Debug       bool     `cli:"debug"`
@@ -141,6 +142,11 @@ var PipelineUploadCommand = cli.Command{
 			Usage:  "The JWKS key ID to use when signing the pipeline. Required when using a JWKS",
 			EnvVar: "BUILDKITE_AGENT_JWKS_KEY_ID",
 		},
+		cli.BoolFlag{
+			Name:   "debug-signing",
+			Usage:  "Enable debug logging for pipeline signing. This can potentially leak secrets to the logs as it prints each step in full before signing. Requires debug logging to be enabled",
+			EnvVar: "BUILDKITE_AGENT_DEBUG_SIGNING",
+		},
 
 		// API Flags
 		AgentAccessTokenFlag,
@@ -282,7 +288,16 @@ var PipelineUploadCommand = cli.Command{
 				return fmt.Errorf("couldn't read the signing key file: %w", err)
 			}
 
-			if err := signature.SignPipeline(result, key, os.Getenv("BUILDKITE_REPO")); err != nil {
+			err = signature.SignSteps(
+				ctx,
+				result.Steps,
+				key,
+				os.Getenv("BUILDKITE_REPO"),
+				signature.WithEnv(result.Env.ToMap()),
+				signature.WithLogger(l),
+				signature.WithDebugSigning(cfg.DebugSigning),
+			)
+			if err != nil {
 				return fmt.Errorf("couldn't sign pipeline: %w", err)
 			}
 		}

diff --git a/clicommand/tool_sign.go b/clicommand/tool_sign.go
@@ -31,8 +31,9 @@ type ToolSignConfig struct {
 	NoConfirm    bool   `cli:"no-confirm"`
 
 	// Used for signing
-	JWKSFile  string `cli:"jwks-file"`
-	JWKSKeyID string `cli:"jwks-key-id"`
+	JWKSFile     string `cli:"jwks-file"`
+	JWKSKeyID    string `cli:"jwks-key-id"`
+	DebugSigning bool   `cli:"debug-signing"`
 
 	// Needed for to use GraphQL API
 	OrganizationSlug string `cli:"organization-slug"`
@@ -126,6 +127,11 @@ Signing a pipeline from a file:
 			Usage:  "The JWKS key ID to use when signing the pipeline. If none is provided and the JWKS file contains only one key, that key will be used.",
 			EnvVar: "BUILDKITE_AGENT_JWKS_KEY_ID",
 		},
+		cli.BoolFlag{
+			Name:   "debug-signing",
+			Usage:  "Enable debug logging for pipeline signing. This can potentially leak secrets to the logs as it prints each step in full before signing. Requires debug logging to be enabled",
+			EnvVar: "BUILDKITE_AGENT_DEBUG_SIGNING",
+		},
 
 		// These are required for GraphQL
 		cli.StringFlag{
@@ -203,7 +209,7 @@ func validateNoInterpolations(pipelineString string) error {
 	return nil
 }
 
-func signOffline(_ context.Context, c *cli.Context, l logger.Logger, key jwk.Key, cfg *ToolSignConfig) error {
+func signOffline(ctx context.Context, c *cli.Context, l logger.Logger, key jwk.Key, cfg *ToolSignConfig) error {
 	if cfg.Repository == "" {
 		return ErrUseGraphQL
 	}
@@ -263,7 +269,16 @@ func signOffline(_ context.Context, c *cli.Context, l logger.Logger, key jwk.Key
 		l.Debug("Pipeline parsed successfully:\n%v", parsedPipeline)
 	}
 
-	if err := signature.SignPipeline(parsedPipeline, key, cfg.Repository); err != nil {
+	err = signature.SignSteps(
+		ctx,
+		parsedPipeline.Steps,
+		key,
+		cfg.Repository,
+		signature.WithEnv(parsedPipeline.Env.ToMap()),
+		signature.WithLogger(l),
+		signature.WithDebugSigning(cfg.DebugSigning),
+	)
+	if err != nil {
 		return fmt.Errorf("couldn't sign pipeline: %w", err)
 	}
 
@@ -318,7 +333,7 @@ func signWithGraphQL(ctx context.Context, c *cli.Context, l logger.Logger, key j
 		debugL.Debug("Pipeline parsed successfully: %v", parsedPipeline)
 	}
 
-	if err := signature.SignPipeline(parsedPipeline, key, resp.Pipeline.Repository.Url); err != nil {
+	if err := signature.SignSteps(ctx, parsedPipeline.Steps, key, resp.Pipeline.Repository.Url, signature.WithEnv(parsedPipeline.Env.ToMap()), signature.WithLogger(debugL), signature.WithDebugSigning(cfg.DebugSigning)); err != nil {
 		return fmt.Errorf("couldn't sign pipeline: %w", err)
 	}
 

diff --git a/go.mod b/go.mod
@@ -14,7 +14,7 @@ require (
 	github.com/aws/aws-sdk-go v1.54.6
 	github.com/brunoscheufler/aws-ecs-metadata-go v0.0.0-20220812150832-b6b31c6eeeaf
 	github.com/buildkite/bintest/v3 v3.2.0
-	github.com/buildkite/go-pipeline v0.9.0
+	github.com/buildkite/go-pipeline v0.10.0
 	github.com/buildkite/interpolate v0.1.2
 	github.com/buildkite/roko v1.2.0
 	github.com/buildkite/shellwords v0.0.0-20180315084142-c3f497d1e000

diff --git a/go.sum b/go.sum
@@ -62,8 +62,8 @@ github.com/brunoscheufler/aws-ecs-metadata-go v0.0.0-20220812150832-b6b31c6eeeaf
 github.com/brunoscheufler/aws-ecs-metadata-go v0.0.0-20220812150832-b6b31c6eeeaf/go.mod h1:CeKhh8xSs3WZAc50xABMxu+FlfAAd5PNumo7NfOv7EE=
 github.com/buildkite/bintest/v3 v3.2.0 h1:1GqUILGGni5UViGPH/PbSq0MxB9gzY3J/P7vNVqCkX4=
 github.com/buildkite/bintest/v3 v3.2.0/go.mod h1:+AdQZcVlzCiW2UyZWeG63xeH5z011XUBW6kWcRdaMtU=
-github.com/buildkite/go-pipeline v0.9.0 h1:2a2bibJ9dCCyyNReH73jkQVUYyUnhYAxISyf3+mrQNs=
-github.com/buildkite/go-pipeline v0.9.0/go.mod h1:4aqMzJ3iagc0wcI5h8NQpON9xfyq27QGDi4xfnKiCUs=
+github.com/buildkite/go-pipeline v0.10.0 h1:EDffu+LfMY2k5u+iEdo6Jn3obGKsrL5wicc1O/yFeRs=
+github.com/buildkite/go-pipeline v0.10.0/go.mod h1:eMH1kiav5VeiTiu0Mk2/M7nZhKyFeL4iGj7Y7rj4f3w=
 github.com/buildkite/interpolate v0.1.2 h1:mVbMCphpu2MHUr1qLdjq9xc3NjNWYg/w/CbrGS5ckzg=
 github.com/buildkite/interpolate v0.1.2/go.mod h1:UNVe6A+UfiBNKbhAySrBbZFZFxQ+DXr9nWen6WVt/A8=
 github.com/buildkite/roko v1.2.0 h1:hbNURz//dQqNl6Eo9awjQOVOZwSDJ8VEbBDxSfT9rGQ=