fix: reject unknown ACM cert ARN at listener API boundary (#289)

An unresolvable certificate ARN was accepted into a listener record and
only failed later at config-render time, which aborted updateStoredConfig
wholesale and silently froze the load balancer's data-plane convergence.

Validate every certificate ARN against the ACM store (existence + account
ownership) in CreateListener, ModifyListener and AddListenerCertificates,
returning CertificateNotFound up front. Skipped when the ACM store is
unavailable, matching the nil-safe resolve path.
This commit is contained in:
Josh Siviour
2026-06-04 11:20:52 +10:00
committed by GitHub
parent 470954bbe8
commit 61d73d1d4e
4 changed files with 58 additions and 0 deletions
@@ -6,6 +6,7 @@ import (
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_acm "github.com/mulgadc/spinifex/spinifex/handlers/acm"
"github.com/mulgadc/spinifex/spinifex/lbagent"
"github.com/mulgadc/spinifex/spinifex/utils"
@@ -126,6 +127,30 @@ func TestResolveCertPEM_ConcatOrderAndOwnership(t *testing.T) {
require.Error(t, err)
}
func TestValidateListenerCerts_RejectsUnknownAndWrongAccount(t *testing.T) {
svc := setupTestService(t)
putTestCert(t, svc, certTestArn, testAccountID, "LEAF", "", "KEY")
// Known, owned cert → accepted.
require.NoError(t, svc.validateListenerCerts(
[]ListenerCertificate{{CertificateArn: certTestArn}}, testAccountID))
// Unknown ARN → CertificateNotFound, rejected at the API boundary.
err := svc.validateListenerCerts(
[]ListenerCertificate{{CertificateArn: "arn:aws:acm:ap-southeast-2:123456789012:certificate/missing"}}, testAccountID)
require.EqualError(t, err, awserrors.ErrorELBv2CertificateNotFound)
// Cert owned by another account → not found.
err = svc.validateListenerCerts(
[]ListenerCertificate{{CertificateArn: certTestArn}}, "999999999999")
require.EqualError(t, err, awserrors.ErrorELBv2CertificateNotFound)
// No ACM store (degraded) → validation skipped, never blocks.
svc.acmStore = nil
require.NoError(t, svc.validateListenerCerts(
[]ListenerCertificate{{CertificateArn: certTestArn}}, testAccountID))
}
func TestResolveCertPEM_NoChain(t *testing.T) {
svc := setupTestService(t)
putTestCert(t, svc, certTestArn, testAccountID, "LEAF", "", "KEY")
+25
View File
@@ -670,6 +670,25 @@ func (s *ELBv2ServiceImpl) resolveCertPEM(arn, accountID string) (string, error)
return b.String(), nil
}
// validateListenerCerts confirms every certificate ARN resolves in the ACM
// store and is owned by the account, rejecting an unknown ARN at the API
// boundary (CreateListener/ModifyListener) with ErrorELBv2CertificateNotFound.
// Without this, an unresolvable ARN is accepted into the listener and only
// fails later at config-render time, which aborts updateStoredConfig wholesale
// and silently freezes the LB's data-plane convergence. Skipped when the ACM
// store is unavailable (cannot validate; matches the nil-safe resolve path).
func (s *ELBv2ServiceImpl) validateListenerCerts(certs []ListenerCertificate, accountID string) error {
if s.acmStore == nil {
return nil
}
for _, c := range certs {
if _, err := s.resolveCertPEM(c.CertificateArn, accountID); err != nil {
return errors.New(awserrors.ErrorELBv2CertificateNotFound)
}
}
return nil
}
// configCertHash hashes the rendered config plus every delivered cert file
// (path + content, in sorted path order) so a cert rotation changes the hash
// and triggers an agent reload even when the config text is unchanged.
@@ -2016,6 +2035,9 @@ func (s *ELBv2ServiceImpl) CreateListener(input *elbv2.CreateListenerInput, acco
if err != nil {
return nil, err
}
if err := s.validateListenerCerts(certs, accountID); err != nil {
return nil, err
}
listenerID := utils.GenerateResourceID("lst")
listenerArn := buildListenerArn(s.region, accountID, lb.Name, lb.LoadBalancerID, listenerID, lb.Type)
@@ -2256,6 +2278,9 @@ func (s *ELBv2ServiceImpl) ModifyListener(input *elbv2.ModifyListenerInput, acco
if certErr != nil {
return nil, certErr
}
if certErr := s.validateListenerCerts(certs, accountID); certErr != nil {
return nil, certErr
}
updated.Certificates = certs
updated.SslPolicy = policy
}
@@ -63,6 +63,10 @@ func (s *ELBv2ServiceImpl) AddListenerCertificates(input *elbv2.AddListenerCerti
}
updated.Certificates = certs
if err := s.validateListenerCerts(certs, accountID); err != nil {
return nil, err
}
if err := s.store.PutListener(&updated); err != nil {
slog.Error("AddListenerCertificates: failed to persist record", "listenerId", updated.ListenerID, "err", err)
return nil, errors.New(awserrors.ErrorServerInternal)
@@ -22,6 +22,10 @@ func setupTestService(t *testing.T) *ELBv2ServiceImpl {
svc, err := NewELBv2ServiceImplWithNATS(nil, nc)
require.NoError(t, err)
// Seed the certificate ARNs the listener tests attach so the fail-closed
// cert validation in Create/Modify/AddListenerCertificates resolves them.
putTestCert(t, svc, testCertArn, testAccountID, "LEAF", "", "KEY")
putTestCert(t, svc, testCertArn2, testAccountID, "LEAF", "", "KEY")
return svc
}