fix: report ACM RSA key algorithms as RSA-<bits>, as AWS does (#1178)

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_<bits>; 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)
+		})
+	}
+}

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-<bits>,
+// 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_<bits> spelling stored by earlier
+// builds to the RSA-<bits> that keyAlgorithm now writes.
+func normaliseKeyAlgorithm(alg string) string {
+	if bits, ok := strings.CutPrefix(alg, "RSA_"); ok {
+		return "RSA-" + bits
+	}
+	return alg
+}

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
 		}