Skip to content

Commit

Permalink
Fixes for feedback from review
Browse files Browse the repository at this point in the history
  • Loading branch information
wolfeidau committed Sep 2, 2024
1 parent 757786a commit 640a107
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 24 deletions.
12 changes: 5 additions & 7 deletions clicommand/agent_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -905,7 +905,7 @@ var AgentStartCommand = cli.Command{
// 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(),
ctx,
config.WithClientLogMode(logMode),
)
if err != nil {
Expand All @@ -919,12 +919,10 @@ var AgentStartCommand = cli.Command{
}

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)
}
var err error
verificationJWKS, err = parseAndValidateJWKS(ctx, "verification", cfg.VerificationJWKSFile)
if err != nil {
l.Fatal("Verification JWKS failed validation: %v", err)
}
}

Expand Down
4 changes: 1 addition & 3 deletions clicommand/pipeline_upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,7 @@ var PipelineUploadCommand = cli.Command{

switch {
case cfg.SigningAWSKMSKey != "":
awscfg, err := config.LoadDefaultConfig(
context.Background(),
)
awscfg, err := config.LoadDefaultConfig(ctx)
if err != nil {
return fmt.Errorf("couldn't load AWS config: %w", err)
}
Expand Down
4 changes: 1 addition & 3 deletions clicommand/tool_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ Signing a pipeline from a file:
switch {
case cfg.AWSKMSKeyID != "":
// load the AWS SDK V2 config
awscfg, err := config.LoadDefaultConfig(
context.Background(),
)
awscfg, err := config.LoadDefaultConfig(ctx)
if err != nil {
return fmt.Errorf("couldn't load AWS config: %w", err)
}
Expand Down
23 changes: 12 additions & 11 deletions internal/cryptosigner/aws/kms.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var (
ErrInvalidKeyID = fmt.Errorf(`invalid key ID`)
)

// KMS is a crypto.Signer that uses an AWS KMS key for signing.
type KMS struct {
alg types.SigningAlgorithmSpec
jwaAlg jwa.KeyAlgorithm
Expand Down Expand Up @@ -48,12 +49,12 @@ func NewKMS(client *kms.Client, kmsKeyID string) (*KMS, error) {

// the key must be a sign/verify key see https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/kms/types#KeyUsageType
if keyDesc.KeyUsage != types.KeyUsageTypeSignVerify {
return nil, fmt.Errorf(`invalid key usage. expected SIGN_VERIFY, got %q`, keyDesc.KeyUsage)
return nil, fmt.Errorf("invalid key usage. expected SIGN_VERIFY, got %q", keyDesc.KeyUsage)
}

// there should be at least one signing algorithm available, and for sign/verify keys there should only be one.
if len(keyDesc.SigningAlgorithms) != 1 {
return nil, fmt.Errorf(`expected one signing algorithm for key %q got %q`, kmsKeyID, keyDesc.SigningAlgorithms)
return nil, fmt.Errorf("expected one signing algorithm for key %q got %q", kmsKeyID, keyDesc.SigningAlgorithms)
}

alg := keyDesc.SigningAlgorithms[0]
Expand Down Expand Up @@ -100,10 +101,10 @@ func NewKMS(client *kms.Client, kmsKeyID string) (*KMS, error) {
// Sign generates a signature from the given digest.
func (sv *KMS) Sign(_ io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
if sv.alg == "" {
return nil, fmt.Errorf(`aws.KMS.Sign() requires the types.SigningAlgorithmSpec`)
return nil, fmt.Errorf("aws.KMS.Sign() requires the types.SigningAlgorithmSpec")
}
if sv.kid == "" {
return nil, fmt.Errorf(`aws.KMS.Sign() requires the KMS key ID`)
return nil, fmt.Errorf("aws.KMS.Sign() requires the KMS key ID")
}

input := kms.SignInput{
Expand All @@ -114,7 +115,7 @@ func (sv *KMS) Sign(_ io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte,
}
signed, err := sv.client.Sign(context.Background(), &input)
if err != nil {
return nil, fmt.Errorf(`failed to sign via KMS: %w`, err)
return nil, fmt.Errorf("failed to sign via KMS: %w", err)
}

return signed.Signature, nil
Expand All @@ -130,11 +131,11 @@ func (sv *KMS) Public() crypto.PublicKey {
return pubkey
}

// This method is an escape hatch for those cases where the user needs
// GetPublicKey is an escape hatch for those cases where the user needs
// to debug what went wrong during the GetPublicKey operation.
func (sv *KMS) GetPublicKey() (crypto.PublicKey, error) {
if sv.kid == "" {
return nil, fmt.Errorf(`aws.KMS.Sign() requires the key ID`)
return nil, fmt.Errorf("aws.KMS.Sign() requires the key ID")
}

input := kms.GetPublicKeyInput{
Expand All @@ -143,22 +144,22 @@ func (sv *KMS) GetPublicKey() (crypto.PublicKey, error) {

output, err := sv.client.GetPublicKey(context.Background(), &input)
if err != nil {
return nil, fmt.Errorf(`failed to get public key from KMS: %w`, err)
return nil, fmt.Errorf("failed to get public key from KMS: %w", err)
}

if output.KeyUsage != types.KeyUsageTypeSignVerify {
return nil, fmt.Errorf(`invalid key usage. expected SIGN_VERIFY, got %q`, output.KeyUsage)
return nil, fmt.Errorf("invalid key usage. expected SIGN_VERIFY, got %q", output.KeyUsage)
}

key, err := x509.ParsePKIXPublicKey(output.PublicKey)
if err != nil {
return nil, fmt.Errorf(`failed to parse key: %w`, err)
return nil, fmt.Errorf("failed to parse key: %w", err)
}

return key, nil
}

// jwa.ES256
// Algorithm returns the equivalent of the KMS key's signing algorithm as a JWA key algorithm.
func (sv *KMS) Algorithm() jwa.KeyAlgorithm {
return sv.jwaAlg
}

0 comments on commit 640a107

Please sign in to comment.