amadan
console: SES ingest — drop the cert chain build, it cannot work
Second reversal on this paragraph, and the one that stuck. Round 3's chain-verification fix asserted the right subject but still tried to build an Intermediates pool from the response and verify it against system roots — and that also fails closed on every real message, for a different reason than round 3's own bug: AWS serves exactly ONE certificate in a real SigningCertURL response (a bare leaf; no bundled intermediate), and that leaf's issuer is distributed via its AIA extension, which Go's x509.Verify never fetches. An Intermediates pool built only from the response is always empty, so verification against system roots always fails with "certificate signed by unknown authority" — proven against a real captured certificate. Every reference implementation (aws-sdk-ruby, robbiet480/go.sns, AWS's own openssl procedure) agrees: decode one leaf, verify no chain. fetchVerifiedCertKey now builds no chain at all. Trust comes from three things instead: the fetch rides on HTTPS, so the Go http client's own TLS handshake validates AWS's TLS chain against system roots — that is the chain check, at the layer where it can actually succeed; the host is pinned to this deployment's own sns.<region>.amazonaws.com; and redirects are refused. The certificate's own subject is still asserted against sns.amazonaws.com on top, as a sanity check rather than a trust decision. Also closes the gap the round-3 review found: nothing exercised fetchVerifiedCertKey itself on a happy path, which is how three separate regressions (dropping Intermediates, asserting the regional host instead of the fixed subject, and a full revert of the round-2 fix) all left the suite green. A DialContext hijack now drives the real function, through a TLS connection presenting a certificate shaped exactly like AWS's real one — single bare leaf, subject sns.amazonaws.com — and asserts it is accepted, with a paired test proving a wrong-subject certificate over the same otherwise-legitimate path is still refused.
2 files changed,
+203
−218
internal/console/sesevents.go+98 −82internal/console/sesevents_test.go+105 −136
diff --git a/internal/console/sesevents.go b/internal/console/sesevents.go| index 3fbe7611a..76c352466 100644 |
| --- a/internal/console/sesevents.go |
| +++ b/internal/console/sesevents.go |
| @@ -10,16 +10,38 @@ |
| // where the verifying certificate may come from, and verify the signature |
| // before trusting any field the message carries. |
| // |
| +// THE TRUST MODEL, IN ONE PARAGRAPH (read this before touching anything |
| +// below — four review rounds of history follow it, and the history is |
| +// there so the model doesn't get "corrected" back to something that |
| +// already failed): this door does NOT verify a certificate chain for SNS's |
| +// signing certificate, and that is deliberate. Trust instead comes from |
| +// THREE things together — the request is HTTPS (so the fetch itself rides |
| +// on a TLS connection whose OWN chain the Go http client already validates |
| +// against system roots — that is the chain check, done at the transport |
| +// layer, which is the only layer where it can actually succeed); the host |
| +// is PINNED to this deployment's own sns.<region>.amazonaws.com (or a |
| +// region-shaped fallback), so the TLS-validated connection is going |
| +// somewhere that is provably SNS's own infrastructure and not a stranger's; |
| +// and REDIRECTS are refused, so a 3xx response from that pinned, validated |
| +// host cannot hand control to a different, unvalidated one. On top of that, |
| +// the downloaded certificate's own subject is checked against |
| +// snsCertDNSName as a sanity check (catches a misconfigured or |
| +// wrong-endpoint response), NOT as a trust decision — there is no chain |
| +// verification of the certificate BODY at all, because (see Round 4 below) |
| +// there is no chain a client can build for it. |
| +// |
| // SECURITY: validSigningCertURL and fetchVerifiedCertKey TOGETHER are the |
| // entirety of SNS signature security. SNS tells the receiver where to fetch |
| // the certificate that verifies the message — so an unvalidated URL, or an |
| -// unverified certificate, lets anyone who can reach this endpoint host |
| +// unvalidated certificate, lets anyone who can reach this endpoint host |
| // their own certificate, sign their own payload, and have it verify. The |
| // consequence here is not merely bad data: forged bounce events trip the |
| // automatic pause, so it is a remote off-switch for any tenant's email. |
| // |
| // A host allowlist is NOT a signer identity by itself, and a parsed |
| -// certificate is NOT a verified one — two review rounds' worth of fixes |
| +// certificate is NOT a verified one — but ALSO, a certificate chain is not |
| +// always buildable, and code that insists on building one anyway can fail |
| +// closed on every legitimate message. Four review rounds' worth of fixes |
| // live in this file because each of those was true in turn: |
| // |
| // - Round 1: the original check accepted any *.amazonaws.com host, which |
| @@ -33,23 +55,45 @@ |
| // not. |
| // - Round 2 (C2): the certificate was parsed and its public key trusted |
| // with no chain verification and no subject check at all — a |
| -// self-signed certificate would have verified. fetchVerifiedCertKey now |
| -// checks the chain against system roots AND the subject SNS's real |
| -// certificate carries. |
| -// - Round 3 (C2 fixed-closed bug): the FIRST attempt at C2 asserted |
| -// DNSName against the FETCH host (e.g. sns.eu-west-1.amazonaws.com) and |
| -// decoded only the first PEM block, discarding any intermediate — but a |
| -// genuine SNS signing certificate names "sns.amazonaws.com" (one |
| -// identity for the whole service, not one per region) and does not |
| -// chain to a system root without its intermediate. That combination |
| -// failed closed on every real message. fetchVerifiedCertKey now decodes |
| -// every PEM block (leaf plus intermediates) and asserts the FIXED |
| -// subject SNS actually uses, not the fetch host. |
| +// self-signed certificate would have verified. |
| +// - Round 3 (C2 fixed-closed bug, attempt 1): the FIRST attempt at C2 |
| +// asserted DNSName against the FETCH host (e.g. |
| +// sns.eu-west-1.amazonaws.com) and decoded only the first PEM block, |
| +// discarding any intermediate — but a genuine SNS signing certificate |
| +// names "sns.amazonaws.com" (one identity for the whole service, not |
| +// one per region), so this failed closed on every real message. |
| // - Round 3 (redirect guard): both outbound fetches used |
| // http.DefaultClient, which follows redirects — for the SubscribeURL |
| // fetch specifically, a redirect target is never re-validated against |
| // the host pin, so following one defeats it outright. Both fetches now |
| -// use a client that refuses to follow a redirect at all. |
| +// use snsFetchClient, which refuses to follow a redirect at all. (This |
| +// fix stands; it is not touched by Round 4.) |
| +// - Round 4 (C2 fixed-closed bug, attempt 2 — the SECOND reversal on this |
| +// paragraph, and the one that stuck): attempt 1's fix DID assert the |
| +// right subject, but it also tried to build an Intermediates pool from |
| +// the response and verify the chain against system roots. THIS ALSO |
| +// FAILS CLOSED, for a completely different reason than attempt 1's: |
| +// AWS serves EXACTLY ONE certificate in a real SigningCertURL response |
| +// — a bare leaf, no bundled intermediate (verified: `grep -c 'BEGIN |
| +// CERTIFICATE'` on a real captured SimpleNotificationService-*.pem is |
| +// 1). That leaf's issuer is an intermediate CA (e.g. "Amazon RSA 2048 |
| +// M04") distributed via the leaf's AIA (Authority Information Access) |
| +// extension — which Go's x509.Verify never fetches; it does not chase |
| +// AIA. System root stores hold only the top-level Amazon Root CA |
| +// certificates, several hops above that intermediate. So an |
| +// Intermediates pool built only from what the response contains is |
| +// ALWAYS empty against a real message, and cert.Verify against system |
| +// roots always returns "certificate signed by unknown authority" — |
| +// proven directly against a real captured certificate, with |
| +// CurrentTime set inside its validity window so expiry could not mask |
| +// the result. Every reference implementation agrees there is no chain |
| +// to build here: aws-sdk-ruby's message_verifier.rb, robbiet480/go.sns, |
| +// and AWS's own documented `openssl x509 -pubkey -noout` procedure all |
| +// decode exactly ONE certificate block and verify no chain at all. |
| +// fetchVerifiedCertKey now does the same: decode one leaf, assert its |
| +// subject, done — no x509.CertPool, no Intermediates, no cert.Verify. |
| +// DO NOT reintroduce a chain build here without re-deriving why it was |
| +// removed twice. |
| package console |
| import ( |
| @@ -458,69 +502,40 @@ func (s *Server) verifiedCertKey(ctx context.Context, certURL string) (*rsa.Publ |
| // what remains unproven until a real message arrives. |
| const snsCertDNSName = "sns.amazonaws.com" |
| -// decodeCertChain splits a PEM-encoded response body into its leaf |
| -// certificate (the first CERTIFICATE block) and every subsequent |
| -// CERTIFICATE block as an intermediates pool. |
| +// fetchVerifiedCertKey downloads the certificate at certURL and returns its |
| +// RSA public key. certURL is assumed already pinned (validSigningCertURL) |
| +// and fetched with redirects refused (snsFetchClient) — those two, plus the |
| +// TLS chain validation the http client performs automatically as part of |
| +// making an HTTPS request, are what make the BYTES this function receives |
| +// trustworthy. See the package comment's "TRUST MODEL" paragraph for the |
| +// full picture. |
| // |
| -// ROUND-3 FIX: the previous version decoded only the FIRST PEM block and |
| -// discarded anything after it. A genuine Amazon leaf certificate does not |
| -// chain to a system root on its own — proven live, during the round-2 |
| -// review, against the actual TLS leaf of sns.eu-west-1.amazonaws.com, which |
| -// fails chain verification until its intermediate ("Amazon RSA 2048 M01") |
| -// is supplied — so a signing-cert response that legitimately bundles an |
| -// intermediate was being silently truncated to just the part that can never |
| -// verify alone. This is what fixes-closed-on-every-real-message meant in |
| -// the round-3 review. |
| -func decodeCertChain(certPEM []byte) (leaf *x509.Certificate, intermediates *x509.CertPool, err error) { |
| - intermediates = x509.NewCertPool() |
| - rest := certPEM |
| - for { |
| - var block *pem.Block |
| - block, rest = pem.Decode(rest) |
| - if block == nil { |
| - break |
| - } |
| - if block.Type != "CERTIFICATE" { |
| - continue |
| - } |
| - c, perr := x509.ParseCertificate(block.Bytes) |
| - if perr != nil { |
| - return nil, nil, fmt.Errorf("sns: parsing signing cert: %w", perr) |
| - } |
| - if leaf == nil { |
| - leaf = c |
| - } else { |
| - intermediates.AddCert(c) |
| - } |
| - } |
| - if leaf == nil { |
| - return nil, nil, fmt.Errorf("sns: signing cert response contained no PEM certificate") |
| - } |
| - return leaf, intermediates, nil |
| -} |
| - |
| -// fetchVerifiedCertKey downloads the certificate (and any bundled |
| -// intermediates) at certURL and returns its RSA public key, but only once |
| -// it has passed BOTH of two independent checks (CRITICAL-2 fix) — neither |
| -// is sufficient alone: |
| +// NO CHAIN IS BUILT OR VERIFIED FOR THE CERTIFICATE ITSELF, DELIBERATELY. |
| +// This reverses an EARLIER fix in this same file — twice (see the package |
| +// comment, "Round 4") — and the removal is deliberate, not an oversight to |
| +// "fix" back: |
| // |
| -// - Its chain verifies against the system root pool, using any |
| -// intermediate certificates the response bundled alongside the leaf. |
| -// Without this, a certificate that merely PARSES is trusted — a |
| -// self-signed certificate generated on an attacker's laptop, or any |
| -// other certificate not actually issued by a CA anyone trusts, would |
| -// verify a forged signature. |
| -// - It is valid for snsCertDNSName — the fixed subject every genuine SNS |
| -// signing certificate carries, NOT the regional host certURL was |
| -// fetched from (that was round 2's C2 fix, and it was wrong: SNS names |
| -// one identity for the whole service). Without this, a validly-issued |
| -// certificate for a completely different (but still |
| -// amazonaws.com-hosted) service could be swapped in for SNS's own — |
| -// chain verification alone says "some CA vouches for this cert", not |
| -// "this cert speaks for SNS". |
| +// - AWS serves EXACTLY ONE certificate in a real SigningCertURL response |
| +// — a bare leaf, with no bundled intermediate. |
| +// - That leaf's issuer is an intermediate CA distributed via the leaf's |
| +// AIA extension, which Go's x509.Verify never fetches (no AIA |
| +// chasing). System root stores hold only the top-level Amazon Root CA |
| +// certificates, several hops above that intermediate. |
| +// - So a chain built ONLY from what the response contains is ALWAYS |
| +// empty against a real message, and verifying it against system roots |
| +// ALWAYS fails with "certificate signed by unknown authority" — |
| +// proven directly against a real captured certificate. Every |
| +// reference implementation (aws-sdk-ruby's message_verifier.rb, |
| +// robbiet480/go.sns, AWS's own documented `openssl x509 -pubkey |
| +// -noout` procedure) agrees: decode one leaf, verify no chain. |
| // |
| -// Uses snsFetchClient, never http.DefaultClient, so this fetch cannot be |
| -// redirected to an unpinned host either. |
| +// What remains is the ONE check that both means something and can actually |
| +// succeed: the certificate's own subject must be snsCertDNSName — SNS's |
| +// fixed identity, not the regional host it happened to be fetched from. |
| +// This is a SANITY check, not a trust decision on its own — it catches a |
| +// misconfigured or wrong-endpoint response, and it is deliberately paired |
| +// with the host pin and redirect guard elsewhere, which are what make the |
| +// FETCH itself trustworthy in the first place. |
| func fetchVerifiedCertKey(ctx context.Context, certURL string) (*rsa.PublicKey, error) { |
| fctx, cancel := context.WithTimeout(ctx, sesFetchTimeout) |
| defer cancel() |
| @@ -540,15 +555,16 @@ func fetchVerifiedCertKey(ctx context.Context, certURL string) (*rsa.PublicKey, |
| if err != nil { |
| return nil, fmt.Errorf("sns: reading signing cert: %w", err) |
| } |
| - leaf, intermediates, err := decodeCertChain(certPEM) |
| + block, _ := pem.Decode(certPEM) |
| + if block == nil { |
| + return nil, fmt.Errorf("sns: signing cert is not PEM") |
| + } |
| + leaf, err := x509.ParseCertificate(block.Bytes) |
| if err != nil { |
| - return nil, err |
| + return nil, fmt.Errorf("sns: parsing signing cert: %w", err) |
| } |
| - // Chain (system roots, plus any bundled intermediates) and subject, in |
| - // one call: nil Roots defaults to the system pool, and DNSName |
| - // additionally requires the leaf to be valid for SNS's own identity. |
| - if _, err := leaf.Verify(x509.VerifyOptions{DNSName: snsCertDNSName, Intermediates: intermediates}); err != nil { |
| - return nil, fmt.Errorf("sns: signing cert failed chain/subject verification: %w", err) |
| + if err := leaf.VerifyHostname(snsCertDNSName); err != nil { |
| + return nil, fmt.Errorf("sns: signing cert subject is not %s: %w", snsCertDNSName, err) |
| } |
| pub, ok := leaf.PublicKey.(*rsa.PublicKey) |
| if !ok { |
diff --git a/internal/console/sesevents_test.go b/internal/console/sesevents_test.go| index 38f77becb..af1c90a6d 100644 |
| --- a/internal/console/sesevents_test.go |
| +++ b/internal/console/sesevents_test.go |
| @@ -16,12 +16,14 @@ import ( |
| "crypto/rsa" |
| "crypto/sha1" |
| "crypto/sha256" |
| + "crypto/tls" |
| "crypto/x509" |
| "crypto/x509/pkix" |
| "encoding/base64" |
| "encoding/pem" |
| "errors" |
| "math/big" |
| + "net" |
| "net/http" |
| "net/http/httptest" |
| "strings" |
| @@ -437,18 +439,25 @@ func TestVerifySNSSignatureBytesRefusesAnUnknownVersion(t *testing.T) { |
| } |
| } |
| -// ---- fetchVerifiedCertKey: the certificate chain/subject check ----------- |
| - |
| -// selfSignedTestCertPEM builds a certificate that is internally well-formed |
| -// — it parses, and its SAN matches snsCertDNSName exactly — but is signed |
| -// by nothing anyone trusts: exactly what an attacker running |
| -// `openssl req -x509` on their own laptop produces. The SAN is deliberately |
| -// the FIXED constant, not whatever host happens to serve it (round-3 fix to |
| -// this fixture): fetchVerifiedCertKey no longer checks the fetch host at |
| -// all, so a fixture whose SAN was the serving host's address would trip the |
| -// DNSName check before chain verification is even reached, proving nothing |
| -// about the chain check specifically. See TestFetchVerifiedCertKeyRefusesASelfSignedCert. |
| -func selfSignedTestCertPEM(t *testing.T) []byte { |
| +// ---- fetchVerifiedCertKey: the trust model that survived four rounds ---- |
| +// |
| +// There is no chain to verify for SNS's own signing certificate (see |
| +// sesevents.go's package comment, "Round 4") — what's left is: fetched over |
| +// HTTPS (TLS chain validated by the http client itself), from a pinned |
| +// host, with no redirect followed, and the certificate's own subject |
| +// checked against snsCertDNSName. The tests below exercise exactly that |
| +// combination through fetchVerifiedCertKey ITSELF — not through a |
| +// hand-rolled x509.Verify call with its own options, which is what let |
| +// three mutations (dropping the Intermediates pool, asserting the regional |
| +// host, and fully reverting the round-2 cert fix) all pass unnoticed in an |
| +// earlier round: nothing exercised the real function on a happy path, so a |
| +// broken happy path had nothing to break loudly. |
| + |
| +// genLeafCert builds a single self-signed leaf certificate for cn — usable |
| +// as BOTH a TLS server certificate and as the body fetchVerifiedCertKey |
| +// parses — mirroring AWS's actual wire format exactly: one bare leaf, never |
| +// an intermediate. |
| +func genLeafCert(t *testing.T, cn string) (der []byte, cert *x509.Certificate, key *rsa.PrivateKey) { |
| t.Helper() |
| key, err := rsa.GenerateKey(rand.Reader, 2048) |
| if err != nil { |
| @@ -456,40 +465,103 @@ func selfSignedTestCertPEM(t *testing.T) []byte { |
| } |
| tmpl := &x509.Certificate{ |
| SerialNumber: big.NewInt(1), |
| - Subject: pkix.Name{CommonName: snsCertDNSName}, |
| - DNSNames: []string{snsCertDNSName}, |
| + Subject: pkix.Name{CommonName: cn}, |
| + DNSNames: []string{cn}, |
| NotBefore: time.Now().Add(-time.Hour), |
| NotAfter: time.Now().Add(time.Hour), |
| - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, |
| + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageCertSign, |
| + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, |
| BasicConstraintsValid: true, |
| IsCA: true, |
| } |
| - der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) |
| + der, err = x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cert, err = x509.ParseCertificate(der) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) |
| + return der, cert, key |
| +} |
| + |
| +// hijackFetchClient points the package-level snsFetchClient at a local TLS |
| +// listener for the duration of the calling test: a custom Transport whose |
| +// DialContext ignores the requested address entirely and always dials |
| +// listenerAddr, while trusting ONLY trust (added directly to a fresh |
| +// RootCAs pool as its own anchor). The request's URL host is left |
| +// untouched, so Go's normal TLS hostname verification still runs for real — |
| +// against the certificate the local listener actually presents — the same |
| +// as it would over a genuine network path to the genuine pinned host. |
| +// Restored automatically via t.Cleanup. |
| +func hijackFetchClient(t *testing.T, listenerAddr string, trust *x509.Certificate) { |
| + t.Helper() |
| + roots := x509.NewCertPool() |
| + roots.AddCert(trust) |
| + orig := snsFetchClient |
| + snsFetchClient = &http.Client{ |
| + CheckRedirect: orig.CheckRedirect, |
| + Transport: &http.Transport{ |
| + DialContext: func(ctx context.Context, network, _ string) (net.Conn, error) { |
| + return (&net.Dialer{}).DialContext(ctx, network, listenerAddr) |
| + }, |
| + TLSClientConfig: &tls.Config{RootCAs: roots}, |
| + }, |
| + } |
| + t.Cleanup(func() { snsFetchClient = orig }) |
| } |
| -// CRITICAL-2 regression test: a certificate whose SUBJECT matches exactly |
| -// what fetchVerifiedCertKey asserts (snsCertDNSName) must still be REFUSED |
| -// if nothing trusted signed it. Because the SAN matches, the DNSName check |
| -// inside cert.Verify passes trivially — what refuses this certificate is |
| -// ONLY the chain check, which is the property this test needs to isolate |
| -// (see the round-3 fix note on selfSignedTestCertPEM: the previous fixture |
| -// used a SAN that did not match the check at all, so it was refused by the |
| -// hostname mismatch and never reached the chain logic — a mutation that |
| -// deleted chain verification entirely still passed). |
| -func TestFetchVerifiedCertKeyRefusesASelfSignedCert(t *testing.T) { |
| - certPEM := selfSignedTestCertPEM(t) |
| - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| +// newHijackedCertServer starts a local httptest TLS server presenting |
| +// serverDER as its own TLS certificate, whose handler serves certPEM as the |
| +// response body — modelling a real SigningCertURL fetch, where the TLS |
| +// certificate the far end presents and the PEM content it serves need not |
| +// be (and for this fixture, are) the same bytes. |
| +func newHijackedCertServer(t *testing.T, der []byte, key *rsa.PrivateKey, certPEM []byte) *httptest.Server { |
| + t.Helper() |
| + ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| w.Write(certPEM) |
| })) |
| - defer ts.Close() |
| + ts.TLS = &tls.Config{Certificates: []tls.Certificate{{Certificate: [][]byte{der}, PrivateKey: key}}} |
| + ts.StartTLS() |
| + t.Cleanup(ts.Close) |
| + return ts |
| +} |
| + |
| +// THE HAPPY PATH, missing until this round: a certificate shaped exactly |
| +// like AWS's real one — a single bare leaf, subject sns.amazonaws.com, no |
| +// intermediate, fetched over a TLS connection whose chain the http client |
| +// validates itself — must be ACCEPTED by fetchVerifiedCertKey, and the |
| +// public key it returns must be the fixture's own. |
| +func TestFetchVerifiedCertKeyAcceptsTheRealShape(t *testing.T) { |
| + der, leaf, key := genLeafCert(t, snsCertDNSName) |
| + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) |
| + ts := newHijackedCertServer(t, der, key, certPEM) |
| + hijackFetchClient(t, ts.Listener.Addr().String(), leaf) |
| + |
| + pub, err := fetchVerifiedCertKey(context.Background(), "https://"+snsCertDNSName+"/SimpleNotificationService.pem") |
| + if err != nil { |
| + t.Fatalf("a real-shaped SNS certificate (single bare leaf, subject %s, no intermediate) was refused: %v", snsCertDNSName, err) |
| + } |
| + if pub.N.Cmp(key.PublicKey.N) != 0 { |
| + t.Fatal("fetchVerifiedCertKey returned a different key than the fixture's own") |
| + } |
| +} |
| - _, err := fetchVerifiedCertKey(context.Background(), ts.URL) |
| - if err == nil { |
| - t.Fatal("a self-signed certificate (with a matching subject) passed chain verification") |
| +// The subject assertion is now the ONLY check standing between an |
| +// authentically-fetched (valid TLS, pinned dial target, no redirect) but |
| +// WRONG-content certificate and treating it as SNS's own — there is no |
| +// chain check left to catch this (see fetchVerifiedCertKey's doc comment). |
| +// A certificate whose subject is NOT sns.amazonaws.com must still be |
| +// refused even though everything about how it was fetched is legitimate. |
| +func TestFetchVerifiedCertKeyRefusesAWrongSubject(t *testing.T) { |
| + const wrongCN = "not-sns.example.com" |
| + der, leaf, key := genLeafCert(t, wrongCN) |
| + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) |
| + ts := newHijackedCertServer(t, der, key, certPEM) |
| + hijackFetchClient(t, ts.Listener.Addr().String(), leaf) |
| + |
| + if _, err := fetchVerifiedCertKey(context.Background(), "https://"+wrongCN+"/x.pem"); err == nil { |
| + t.Fatal("a certificate with the wrong subject was accepted") |
| } |
| } |
| @@ -514,106 +586,3 @@ func TestVerifiedCertKeyServesFromCache(t *testing.T) { |
| t.Fatal("verifiedCertKey returned a different key than the one cached") |
| } |
| } |
| - |
| -// ---- decodeCertChain: leaf vs. intermediates ----------------------------- |
| - |
| -// testCA builds either a self-signed root (parent == nil) or a certificate |
| -// issued by parent/parentKey — enough machinery to build a real, |
| -// independently-verifiable two-level chain entirely offline. |
| -func testCA(t *testing.T, cn string, parent *x509.Certificate, parentKey *rsa.PrivateKey) (*rsa.PrivateKey, *x509.Certificate) { |
| - t.Helper() |
| - key, err := rsa.GenerateKey(rand.Reader, 2048) |
| - if err != nil { |
| - t.Fatal(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(time.Hour), |
| - KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, |
| - BasicConstraintsValid: true, |
| - IsCA: true, |
| - } |
| - signerCert, signerKey := tmpl, key |
| - if parent != nil { |
| - signerCert, signerKey = parent, parentKey |
| - } |
| - der, err := x509.CreateCertificate(rand.Reader, tmpl, signerCert, &key.PublicKey, signerKey) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - cert, err := x509.ParseCertificate(der) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - return key, cert |
| -} |
| - |
| -// testLeaf builds a non-CA certificate for dnsName, issued by parent. |
| -func testLeaf(t *testing.T, dnsName string, parent *x509.Certificate, parentKey *rsa.PrivateKey) *x509.Certificate { |
| - t.Helper() |
| - key, err := rsa.GenerateKey(rand.Reader, 2048) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - tmpl := &x509.Certificate{ |
| - SerialNumber: big.NewInt(time.Now().UnixNano()), |
| - Subject: pkix.Name{CommonName: dnsName}, |
| - DNSNames: []string{dnsName}, |
| - NotBefore: time.Now().Add(-time.Hour), |
| - NotAfter: time.Now().Add(time.Hour), |
| - KeyUsage: x509.KeyUsageDigitalSignature, |
| - } |
| - der, err := x509.CreateCertificate(rand.Reader, tmpl, parent, &key.PublicKey, parentKey) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - cert, err := x509.ParseCertificate(der) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - return cert |
| -} |
| - |
| -func pemEncodeCert(c *x509.Certificate) []byte { |
| - return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.Raw}) |
| -} |
| - |
| -// ROUND-3 FIX regression test, entirely offline: decodeCertChain must treat |
| -// the FIRST PEM block as the leaf and every subsequent block as an |
| -// intermediate — and the intermediate must actually be NECESSARY for the |
| -// leaf to chain, mirroring the real SNS leaf, which the round-2 review |
| -// proved does not chain to a system root without its own intermediate |
| -// ("Amazon RSA 2048 M01"). A root that is trusted only within this test |
| -// (never the real system pool) makes the whole chain independently |
| -// verifiable without any network access or system trust store changes. |
| -func TestDecodeCertChainSeparatesLeafFromIntermediates(t *testing.T) { |
| - rootKey, rootCert := testCA(t, "Test Root CA", nil, nil) |
| - interKey, interCert := testCA(t, "Test Intermediate CA", rootCert, rootKey) |
| - leafCert := testLeaf(t, snsCertDNSName, interCert, interKey) |
| - |
| - blob := append(pemEncodeCert(leafCert), pemEncodeCert(interCert)...) |
| - leaf, intermediates, err := decodeCertChain(blob) |
| - if err != nil { |
| - t.Fatalf("decodeCertChain: %v", err) |
| - } |
| - if leaf.Subject.CommonName != leafCert.Subject.CommonName { |
| - t.Fatalf("leaf = %q, want %q — the first block must be the leaf", leaf.Subject.CommonName, leafCert.Subject.CommonName) |
| - } |
| - |
| - roots := x509.NewCertPool() |
| - roots.AddCert(rootCert) |
| - |
| - if _, err := leaf.Verify(x509.VerifyOptions{DNSName: snsCertDNSName, Intermediates: intermediates, Roots: roots}); err != nil { |
| - t.Fatalf("leaf+intermediate did not verify against its own trusted root: %v", err) |
| - } |
| - // Prove the intermediate is NECESSARY, not incidental: the same leaf, |
| - // against the same root, with no intermediate supplied, must fail. This |
| - // is exactly the shape of the real Amazon leaf per the round-2 review — |
| - // if this assertion did not hold, the fixture would prove nothing about |
| - // needing decodeCertChain to carry the intermediate through at all. |
| - if _, err := leaf.Verify(x509.VerifyOptions{DNSName: snsCertDNSName, Roots: roots}); err == nil { |
| - t.Fatal("leaf verified against its root with NO intermediate — the fixture proves nothing about needing one") |
| - } |
| -} |