Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log public signing key thumbprint and signed step payload #2853

Merged
merged 4 commits into from
Jul 2, 2024
Merged
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
1 change: 1 addition & 0 deletions agent/agent_configuration.go
Original file line number Diff line number Diff line change
@@ -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`)
5 changes: 3 additions & 2 deletions agent/integration/job_verification_integration_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
4 changes: 4 additions & 0 deletions agent/job_runner.go
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion agent/run_job.go
Original file line number Diff line number Diff line change
@@ -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 {
13 changes: 11 additions & 2 deletions agent/verify_job.go
Original file line number Diff line number Diff line change
@@ -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)
}
7 changes: 7 additions & 0 deletions clicommand/agent_start.go
Original file line number Diff line number Diff line change
@@ -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,

21 changes: 18 additions & 3 deletions clicommand/pipeline_upload.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
25 changes: 20 additions & 5 deletions clicommand/tool_sign.go
Original file line number Diff line number Diff line change
@@ -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"`
@@ -125,6 +126,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{
@@ -196,7 +202,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
}
@@ -256,7 +262,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)
}

@@ -311,7 +326,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)
}

8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ require (
github.com/aws/aws-sdk-go v1.54.0
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
@@ -28,7 +28,7 @@ require (
github.com/google/go-cmp v0.6.0
github.com/google/go-querystring v1.1.0
github.com/gowebpki/jcs v1.0.1
github.com/lestrrat-go/jwx/v2 v2.0.21
github.com/lestrrat-go/jwx/v2 v2.1.0
github.com/mattn/go-zglob v0.0.4
github.com/mitchellh/go-homedir v1.1.0
github.com/oleiade/reflections v1.0.1
@@ -79,12 +79,12 @@ require (
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.4 // indirect
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 // indirect
github.com/ebitengine/purego v0.6.0-alpha.5 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/go-logr/logr v1.4.1 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/goccy/go-json v0.10.2 // indirect
github.com/goccy/go-json v0.10.3 // indirect
github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.4 // indirect
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
@@ -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=
@@ -86,8 +86,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 h1:8UrgZ3GkP4i/CLijOJx79Yu+etlyjdBU4sfcs2WYQMs=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 h1:rpfIENRNNilwHwZeG5+P150SMrnNEcHYvcCuK6dPZSg=
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0/go.mod h1:v57UDF4pDQJcEfFUCRop3lJL149eHGSe9Jvczhzjo/0=
github.com/denisbrodbeck/machineid v1.0.1 h1:geKr9qtkB876mXguW2X6TU4ZynleN6ezuMSRhl4D7AQ=
github.com/denisbrodbeck/machineid v1.0.1/go.mod h1:dJUwb7PTidGDeYyUBmXZ2GphQBbjJCrnectwCyxcUSI=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 h1:tdlZCpZ/P9DhczCTSixgIKmwPv6+wP5DGjqLYw5SUiA=
@@ -118,8 +118,8 @@ github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ=
github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU=
github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I=
github.com/goccy/go-json v0.10.3 h1:KZ5WoDbxAIgm2HNbYckL0se1fHD6rz5j4ywS6ebzDqA=
github.com/goccy/go-json v0.10.3/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M=
github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
@@ -196,8 +196,8 @@ github.com/lestrrat-go/httprc v1.0.5 h1:bsTfiH8xaKOJPrg1R+E3iE/AWZr/x0Phj9PBTG/O
github.com/lestrrat-go/httprc v1.0.5/go.mod h1:mwwz3JMTPBjHUkkDv/IGJ39aALInZLrhBp0X7KGUZlo=
github.com/lestrrat-go/iter v1.0.2 h1:gMXo1q4c2pHmC3dn8LzRhJfP1ceCbgSiT9lUydIzltI=
github.com/lestrrat-go/iter v1.0.2/go.mod h1:Momfcq3AnRlRjI5b5O8/G5/BvpzrhoFTZcn06fEOPt4=
github.com/lestrrat-go/jwx/v2 v2.0.21 h1:jAPKupy4uHgrHFEdjVjNkUgoBKtVDgrQPB/h55FHrR0=
github.com/lestrrat-go/jwx/v2 v2.0.21/go.mod h1:09mLW8zto6bWL9GbwnqAli+ArLf+5M33QLQPDggkUWM=
github.com/lestrrat-go/jwx/v2 v2.1.0 h1:0zs7Ya6+39qoit7gwAf+cYm1zzgS3fceIdo7RmQ5lkw=
github.com/lestrrat-go/jwx/v2 v2.1.0/go.mod h1:Xpw9QIaUGiIUD1Wx0NcY1sIHwFf8lDuZn/cmxtXYRys=
github.com/lestrrat-go/option v1.0.1 h1:oAzP2fvZGQKWkvHa1/SAcFolBEca1oN+mQ7eooNBEYU=
github.com/lestrrat-go/option v1.0.1/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I=
github.com/mattn/go-zglob v0.0.4 h1:LQi2iOm0/fGgu80AioIJ/1j9w9Oh+9DZ39J4VAGzHQM=