From 265f6fad1962dd51297c99199e309079b2cfa1d2 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:25:01 +1100 Subject: [PATCH] fix: report ACM RSA key algorithms as RSA-, as AWS does (#1178) --- spinifex/handlers/acm/keyalgorithm_test.go | 108 +++++++++++++++++++++ spinifex/handlers/acm/service_impl.go | 16 ++- spinifex/handlers/acm/store.go | 2 + 3 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 spinifex/handlers/acm/keyalgorithm_test.go diff --git a/spinifex/handlers/acm/keyalgorithm_test.go b/spinifex/handlers/acm/keyalgorithm_test.go new file mode 100644 index 000000000..16f1e7a35 --- /dev/null +++ b/spinifex/handlers/acm/keyalgorithm_test.go @@ -0,0 +1,108 @@ +package handlers_acm + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/acm" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// genRSACert returns a self-signed RSA leaf with only a CN, the shape of the +// certificate the AWS conformance audit imported. +func genRSACert(t *testing.T, cn string, bits int) (certPEM, keyPEM []byte) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, bits) + require.NoError(t, err) + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(time.Now().UnixNano()), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + certPEM = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) + keyPEM = pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + return certPEM, keyPEM +} + +// describeAndListKeyAlgorithm returns the KeyAlgorithm that DescribeCertificate +// and the ListCertificates summary report for certArn. +func describeAndListKeyAlgorithm(t *testing.T, svc *ACMServiceImpl, certArn string) (described, listed string) { + t.Helper() + desc, err := svc.DescribeCertificate(context.Background(), &acm.DescribeCertificateInput{ + CertificateArn: aws.String(certArn), + }, testAccountID) + require.NoError(t, err) + + list, err := svc.ListCertificates(context.Background(), &acm.ListCertificatesInput{}, testAccountID) + require.NoError(t, err) + for _, s := range list.CertificateSummaryList { + if aws.StringValue(s.CertificateArn) == certArn { + return aws.StringValue(desc.Certificate.KeyAlgorithm), aws.StringValue(s.KeyAlgorithm) + } + } + t.Fatalf("ListCertificates did not return %s", certArn) + return "", "" +} + +// AWS returns RSA-2048 with a hyphen from both Describe and List, although the +// API enum (and the ListCertificates keyTypes filter) spells it RSA_2048. +func TestImportCertificate_RSAKeyAlgorithmIsHyphenated(t *testing.T) { + svc := setupACMService(t) + certPEM, keyPEM := genRSACert(t, "rsa.example.com", 2048) + + out, err := svc.ImportCertificate(context.Background(), &acm.ImportCertificateInput{ + Certificate: certPEM, + PrivateKey: keyPEM, + }, testAccountID) + require.NoError(t, err) + + described, listed := describeAndListKeyAlgorithm(t, svc, aws.StringValue(out.CertificateArn)) + assert.Equal(t, "RSA-2048", described) + assert.Equal(t, "RSA-2048", listed) + + // The read path normalises, so check the stored spelling separately. + raw, _, err := svc.store.store.Get(t.Context(), certKey(aws.StringValue(out.CertificateArn))) + require.NoError(t, err) + assert.Equal(t, "RSA-2048", raw.KeyAlgorithm) +} + +// Records written before the spelling fix hold RSA_; they must read back +// in the AWS spelling without a migration, and other values must pass through. +func TestKeyAlgorithm_StoredUnderscoreSpellingReadsBackHyphenated(t *testing.T) { + cases := []struct { + stored, want string + }{ + {stored: "RSA_2048", want: "RSA-2048"}, + {stored: "RSA_4096", want: "RSA-4096"}, + {stored: "RSA-3072", want: "RSA-3072"}, + {stored: "EC_P-256", want: "EC_P-256"}, + } + svc := setupACMService(t) + for i, tc := range cases { + t.Run(tc.stored, func(t *testing.T) { + certArn := "arn:aws:acm:ap-southeast-2:" + testAccountID + ":certificate/legacy-" + string(rune('a'+i)) + require.NoError(t, svc.store.PutCert(t.Context(), &CertRecord{ + CertificateArn: certArn, + AccountID: testAccountID, + DomainName: "legacy.example.com", + KeyAlgorithm: tc.stored, + })) + + described, listed := describeAndListKeyAlgorithm(t, svc, certArn) + assert.Equal(t, tc.want, described) + assert.Equal(t, tc.want, listed) + }) + } +} diff --git a/spinifex/handlers/acm/service_impl.go b/spinifex/handlers/acm/service_impl.go index a5226f67b..5467cc422 100644 --- a/spinifex/handlers/acm/service_impl.go +++ b/spinifex/handlers/acm/service_impl.go @@ -764,15 +764,25 @@ func leafDomain(leaf *x509.Certificate) string { return "" } -// keyAlgorithm maps the leaf public key to an ACM-style algorithm string -// (RSA_2048, EC_prime256v1, ...). +// keyAlgorithm maps the leaf public key to the KeyAlgorithm ACM returns: RSA-, +// hyphenated as AWS returns it, unlike the RSA_2048 API enum. EC is EC_ plus Go's +// curve name (EC_P-256), a spelling not yet checked against AWS. func keyAlgorithm(leaf *x509.Certificate) string { switch pub := leaf.PublicKey.(type) { case *rsa.PublicKey: - return fmt.Sprintf("RSA_%d", pub.N.BitLen()) + return fmt.Sprintf("RSA-%d", pub.N.BitLen()) case *ecdsa.PublicKey: return "EC_" + pub.Curve.Params().Name default: return "UNKNOWN" } } + +// normaliseKeyAlgorithm rewrites the RSA_ spelling stored by earlier +// builds to the RSA- that keyAlgorithm now writes. +func normaliseKeyAlgorithm(alg string) string { + if bits, ok := strings.CutPrefix(alg, "RSA_"); ok { + return "RSA-" + bits + } + return alg +} diff --git a/spinifex/handlers/acm/store.go b/spinifex/handlers/acm/store.go index f4a5043c1..4b7efc998 100644 --- a/spinifex/handlers/acm/store.go +++ b/spinifex/handlers/acm/store.go @@ -273,6 +273,7 @@ func (s *Store) getCert(ctx context.Context, certArn string, decrypt bool) (*Cer if err != nil { return nil, err } + rec.KeyAlgorithm = normaliseKeyAlgorithm(rec.KeyAlgorithm) if !decrypt { rec.PrivateKey = "" return rec, nil @@ -410,6 +411,7 @@ func (s *Store) listCerts(ctx context.Context, keep func(*CertRecord) bool, decr if err := json.Unmarshal(entry.Value(), &rec); err != nil { continue } + rec.KeyAlgorithm = normaliseKeyAlgorithm(rec.KeyAlgorithm) if !keep(&rec) { continue }