-
Notifications
You must be signed in to change notification settings - Fork 300
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
Support AWS KMS for signing and verifying pipelines #2960
Changes from 11 commits
1b79aeb
d6b1752
a53fdaa
34392d7
f75e0c3
8f736eb
31c09ee
abad0d0
ce9aa9a
c97f8a6
757786a
059b464
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -18,10 +18,14 @@ import ( | |||||||||||||||||||||||||
"syscall" | ||||||||||||||||||||||||||
"time" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
"github.com/aws/aws-sdk-go-v2/aws" | ||||||||||||||||||||||||||
"github.com/aws/aws-sdk-go-v2/config" | ||||||||||||||||||||||||||
"github.com/aws/aws-sdk-go-v2/service/kms" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/agent" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/api" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/core" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/internal/agentapi" | ||||||||||||||||||||||||||
awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/internal/experiments" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/internal/job/hook" | ||||||||||||||||||||||||||
"github.com/buildkite/agent/v3/internal/job/shell" | ||||||||||||||||||||||||||
|
@@ -83,8 +87,10 @@ type AgentStartConfig struct { | |||||||||||||||||||||||||
RedactedVars []string `cli:"redacted-vars" normalize:"list"` | ||||||||||||||||||||||||||
CancelSignal string `cli:"cancel-signal"` | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
SigningJWKSFile string `cli:"signing-jwks-file" normalize:"filepath"` | ||||||||||||||||||||||||||
SigningJWKSKeyID string `cli:"signing-jwks-key-id"` | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
SigningJWKSFile string `cli:"signing-jwks-file" normalize:"filepath"` | ||||||||||||||||||||||||||
SigningAWSKMSKey string `cli:"signing-aws-kms-key"` | ||||||||||||||||||||||||||
DebugSigning bool `cli:"debug-signing"` | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
VerificationJWKSFile string `cli:"verification-jwks-file" normalize:"filepath"` | ||||||||||||||||||||||||||
|
@@ -658,6 +664,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.StringFlag{ | ||||||||||||||||||||||||||
Name: "signing-aws-kms-key", | ||||||||||||||||||||||||||
Usage: "The KMS KMS key ID, or key alias used when signing and verifying the pipeline.", | ||||||||||||||||||||||||||
EnvVar: "BUILDKITE_AGENT_SIGNING_AWS_KMS_KEY", | ||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||
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", | ||||||||||||||||||||||||||
|
@@ -878,12 +889,42 @@ var AgentStartCommand = cli.Command{ | |||||||||||||||||||||||||
defer shutdown() | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
var verificationJWKS jwk.Set | ||||||||||||||||||||||||||
if cfg.VerificationJWKSFile != "" { | ||||||||||||||||||||||||||
var err error | ||||||||||||||||||||||||||
verificationJWKS, err = parseAndValidateJWKS(ctx, "verification", cfg.VerificationJWKSFile) | ||||||||||||||||||||||||||
// if the agent is provided a KMS key ID, it should use the KMS signer, otherwise | ||||||||||||||||||||||||||
// it should load the JWKS from the file | ||||||||||||||||||||||||||
var verificationJWKS any | ||||||||||||||||||||||||||
switch { | ||||||||||||||||||||||||||
case cfg.SigningAWSKMSKey != "": | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
var logMode aws.ClientLogMode | ||||||||||||||||||||||||||
// log requests and retries if we are debugging signing | ||||||||||||||||||||||||||
// see https://aws.github.io/aws-sdk-go-v2/docs/configuring-sdk/logging/ | ||||||||||||||||||||||||||
if cfg.DebugSigning { | ||||||||||||||||||||||||||
logMode = aws.LogRetries | aws.LogRequest | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
// this is currently loaded here to ensure it is ONLY loaded if the agent is using KMS for signing | ||||||||||||||||||||||||||
// this will limit the possible impact of this new SDK on the rest of the agent users | ||||||||||||||||||||||||||
awscfg, err := config.LoadDefaultConfig( | ||||||||||||||||||||||||||
context.Background(), | ||||||||||||||||||||||||||
config.WithClientLogMode(logMode), | ||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||
return fmt.Errorf("failed to load AWS config: %w", err) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
// assign a crypto signer which uses the KMS key to sign the pipeline | ||||||||||||||||||||||||||
verificationJWKS, err = awssigner.NewKMS(kms.NewFromConfig(awscfg), cfg.SigningAWSKMSKey) | ||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||
l.Fatal("Verification JWKS failed validation: %v", err) | ||||||||||||||||||||||||||
return fmt.Errorf("couldn't create KMS signer: %w", err) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
case cfg.VerificationJWKSFile != "": | ||||||||||||||||||||||||||
if cfg.VerificationJWKSFile != "" { | ||||||||||||||||||||||||||
var err error | ||||||||||||||||||||||||||
verificationJWKS, err = parseAndValidateJWKS(ctx, "verification", cfg.VerificationJWKSFile) | ||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||
l.Fatal("Verification JWKS failed validation: %v", err) | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant check is redundant:
Suggested change
|
||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
@@ -958,6 +999,7 @@ var AgentStartCommand = cli.Command{ | |||||||||||||||||||||||||
|
||||||||||||||||||||||||||
SigningJWKSFile: cfg.SigningJWKSFile, | ||||||||||||||||||||||||||
SigningJWKSKeyID: cfg.SigningJWKSKeyID, | ||||||||||||||||||||||||||
SigningAWSKMSKey: cfg.SigningAWSKMSKey, | ||||||||||||||||||||||||||
DebugSigning: cfg.DebugSigning, | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
VerificationJWKS: verificationJWKS, | ||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,9 +14,12 @@ import ( | |||||||||
"strings" | ||||||||||
"time" | ||||||||||
|
||||||||||
"github.com/aws/aws-sdk-go-v2/config" | ||||||||||
"github.com/aws/aws-sdk-go-v2/service/kms" | ||||||||||
"github.com/buildkite/agent/v3/agent" | ||||||||||
"github.com/buildkite/agent/v3/api" | ||||||||||
"github.com/buildkite/agent/v3/env" | ||||||||||
awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws" | ||||||||||
"github.com/buildkite/agent/v3/internal/experiments" | ||||||||||
"github.com/buildkite/agent/v3/internal/redact" | ||||||||||
"github.com/buildkite/agent/v3/internal/replacer" | ||||||||||
|
@@ -76,9 +79,10 @@ type PipelineUploadConfig struct { | |||||||||
RejectSecrets bool `cli:"reject-secrets"` | ||||||||||
|
||||||||||
// Used for signing | ||||||||||
JWKSFile string `cli:"jwks-file"` | ||||||||||
JWKSKeyID string `cli:"jwks-key-id"` | ||||||||||
DebugSigning bool `cli:"debug-signing"` | ||||||||||
JWKSFile string `cli:"jwks-file"` | ||||||||||
JWKSKeyID string `cli:"jwks-key-id"` | ||||||||||
SigningAWSKMSKey string `cli:"signing-aws-kms-key"` | ||||||||||
DebugSigning bool `cli:"debug-signing"` | ||||||||||
|
||||||||||
// Global flags | ||||||||||
Debug bool `cli:"debug"` | ||||||||||
|
@@ -144,6 +148,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.StringFlag{ | ||||||||||
Name: "signing-aws-kms-key", | ||||||||||
Usage: "The AWS KMS key identifier which is used to sign pipelines.", | ||||||||||
EnvVar: "BUILDKITE_AGENT_AWS_KMS_KEY", | ||||||||||
}, | ||||||||||
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", | ||||||||||
|
@@ -275,11 +284,33 @@ var PipelineUploadCommand = cli.Command{ | |||||||||
searchForSecrets(l, &cfg, environ, result, src) | ||||||||||
} | ||||||||||
|
||||||||||
if cfg.JWKSFile != "" { | ||||||||||
key, err := jwkutil.LoadKey(cfg.JWKSFile, cfg.JWKSKeyID) | ||||||||||
var ( | ||||||||||
key signature.Key | ||||||||||
) | ||||||||||
|
||||||||||
switch { | ||||||||||
case cfg.SigningAWSKMSKey != "": | ||||||||||
awscfg, err := config.LoadDefaultConfig( | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a small risk that this could resolve credentials in a different order to the how the rest of the agent does it. |
||||||||||
context.Background(), | ||||||||||
) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't load AWS config: %w", err) | ||||||||||
} | ||||||||||
|
||||||||||
// assign a crypto signer which uses the KMS key to sign the pipeline | ||||||||||
key, err = awssigner.NewKMS(kms.NewFromConfig(awscfg), cfg.SigningAWSKMSKey) | ||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't create KMS signer: %w", err) | ||||||||||
} | ||||||||||
|
||||||||||
case cfg.JWKSFile != "": | ||||||||||
key, err = jwkutil.LoadKey(cfg.JWKSFile, cfg.JWKSKeyID) | ||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't read the signing key file: %w", err) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
if key != nil { | ||||||||||
|
||||||||||
err = signature.SignSteps( | ||||||||||
ctx, | ||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,15 +9,17 @@ import ( | |||||||||
"os" | ||||||||||
"strings" | ||||||||||
|
||||||||||
"github.com/aws/aws-sdk-go-v2/config" | ||||||||||
"github.com/aws/aws-sdk-go-v2/service/kms" | ||||||||||
"github.com/buildkite/agent/v3/internal/bkgql" | ||||||||||
awssigner "github.com/buildkite/agent/v3/internal/cryptosigner/aws" | ||||||||||
"github.com/buildkite/agent/v3/internal/stdin" | ||||||||||
"github.com/buildkite/agent/v3/logger" | ||||||||||
"github.com/buildkite/go-pipeline" | ||||||||||
"github.com/buildkite/go-pipeline/jwkutil" | ||||||||||
"github.com/buildkite/go-pipeline/signature" | ||||||||||
"github.com/buildkite/go-pipeline/warning" | ||||||||||
"github.com/buildkite/interpolate" | ||||||||||
"github.com/lestrrat-go/jwx/v2/jwk" | ||||||||||
"github.com/urfave/cli" | ||||||||||
"gopkg.in/yaml.v3" | ||||||||||
) | ||||||||||
|
@@ -31,9 +33,14 @@ type ToolSignConfig struct { | |||||||||
NoConfirm bool `cli:"no-confirm"` | ||||||||||
|
||||||||||
// Used for signing | ||||||||||
JWKSFile string `cli:"jwks-file"` | ||||||||||
JWKSKeyID string `cli:"jwks-key-id"` | ||||||||||
DebugSigning bool `cli:"debug-signing"` | ||||||||||
JWKSFile string `cli:"jwks-file"` | ||||||||||
JWKSKeyID string `cli:"jwks-key-id"` | ||||||||||
|
||||||||||
// AWS KMS key used for signing pipelines | ||||||||||
AWSKMSKeyID string `cli:"signing-aws-kms-key"` | ||||||||||
|
||||||||||
// Enable debug logging for pipeline signing, this depends on debug logging also being enabled | ||||||||||
DebugSigning bool `cli:"debug-signing"` | ||||||||||
|
||||||||||
// Needed for to use GraphQL API | ||||||||||
OrganizationSlug string `cli:"organization-slug"` | ||||||||||
|
@@ -127,6 +134,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.StringFlag{ | ||||||||||
Name: "signing-aws-kms-key", | ||||||||||
Usage: "The AWS KMS key identifier which is used to sign pipelines.", | ||||||||||
EnvVar: "BUILDKITE_AGENT_AWS_KMS_KEY", | ||||||||||
}, | ||||||||||
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", | ||||||||||
|
@@ -170,9 +182,33 @@ Signing a pipeline from a file: | |||||||||
ctx, cfg, l, _, done := setupLoggerAndConfig[ToolSignConfig](context.Background(), c) | ||||||||||
defer done() | ||||||||||
|
||||||||||
key, err := jwkutil.LoadKey(cfg.JWKSFile, cfg.JWKSKeyID) | ||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't read the signing key file: %w", err) | ||||||||||
var ( | ||||||||||
key signature.Key | ||||||||||
err error | ||||||||||
) | ||||||||||
|
||||||||||
switch { | ||||||||||
case cfg.AWSKMSKeyID != "": | ||||||||||
// load the AWS SDK V2 config | ||||||||||
awscfg, err := config.LoadDefaultConfig( | ||||||||||
context.Background(), | ||||||||||
) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't load AWS config: %w", err) | ||||||||||
} | ||||||||||
|
||||||||||
// assign a crypto signer which uses the KMS key to sign the pipeline | ||||||||||
key, err = awssigner.NewKMS(kms.NewFromConfig(awscfg), cfg.AWSKMSKeyID) | ||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't create KMS signer: %w", err) | ||||||||||
} | ||||||||||
|
||||||||||
default: | ||||||||||
key, err = jwkutil.LoadKey(cfg.JWKSFile, cfg.JWKSKeyID) | ||||||||||
if err != nil { | ||||||||||
return fmt.Errorf("couldn't read the signing key file: %w", err) | ||||||||||
} | ||||||||||
|
||||||||||
} | ||||||||||
|
||||||||||
sign := signWithGraphQL | ||||||||||
|
@@ -209,7 +245,7 @@ func validateNoInterpolations(pipelineString string) error { | |||||||||
return nil | ||||||||||
} | ||||||||||
|
||||||||||
func signOffline(ctx 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 signature.Key, cfg *ToolSignConfig) error { | ||||||||||
if cfg.Repository == "" { | ||||||||||
return ErrUseGraphQL | ||||||||||
} | ||||||||||
|
@@ -289,7 +325,7 @@ func signOffline(ctx context.Context, c *cli.Context, l logger.Logger, key jwk.K | |||||||||
return enc.Encode(parsedPipeline) | ||||||||||
} | ||||||||||
|
||||||||||
func signWithGraphQL(ctx context.Context, c *cli.Context, l logger.Logger, key jwk.Key, cfg *ToolSignConfig) error { | ||||||||||
func signWithGraphQL(ctx context.Context, c *cli.Context, l logger.Logger, key signature.Key, cfg *ToolSignConfig) error { | ||||||||||
orgPipelineSlug := fmt.Sprintf("%s/%s", cfg.OrganizationSlug, cfg.PipelineSlug) | ||||||||||
debugL := l.WithFields(logger.StringField("orgPipelineSlug", orgPipelineSlug)) | ||||||||||
|
||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a
ctx
in this function, may as well use it