From 4b62f6a99bf66b50d587c7f70bb7c965e1c24771 Mon Sep 17 00:00:00 2001 From: Connor Snitker Date: Thu, 4 Jun 2026 11:24:07 -0500 Subject: [PATCH] fix(tl): emit DER C2SP checkpoint signatures Return the KeyManager DER signature directly for the primary C2SP checkpoint signer and verify DER with ecdsa.VerifyASN1, while keeping a legacy P1363 verifier fallback for local checkpoints. Update checkpoint signature comments and add regression coverage for signer output, sumdb-note keyhash wrapping, DER verification, and P1363 compatibility. Remove deprecated chi RealIP middleware calls surfaced by make check; the repo does not currently consume RemoteAddr/client-IP state. Signed-off-by: Connor Snitker --- cmd/ans-ra/main.go | 1 - cmd/ans-tl/main.go | 1 - internal/crypto/p1363.go | 6 +- internal/tl/logstore/c2spsigner.go | 41 ++--- internal/tl/logstore/c2spsigner_test.go | 150 ++++++++++++++++++ internal/tl/logstore/signer_errors_test.go | 8 +- internal/tl/service/checkpoint.go | 16 +- .../tl/service/checkpoint_helpers_test.go | 16 +- 8 files changed, 197 insertions(+), 42 deletions(-) create mode 100644 internal/tl/logstore/c2spsigner_test.go diff --git a/cmd/ans-ra/main.go b/cmd/ans-ra/main.go index 9c734d6..b61a5b7 100644 --- a/cmd/ans-ra/main.go +++ b/cmd/ans-ra/main.go @@ -178,7 +178,6 @@ func run(cfgPath string) error { r := chi.NewRouter() r.Use(middleware.Recoverer) r.Use(middleware.RequestID) - r.Use(middleware.RealIP) r.Use(middleware.Timeout(30 * time.Second)) r.Use(middleware.AllowContentType("application/json")) r.Use(authProvider.Middleware()) diff --git a/cmd/ans-tl/main.go b/cmd/ans-tl/main.go index 4ab015d..611b5fe 100644 --- a/cmd/ans-tl/main.go +++ b/cmd/ans-tl/main.go @@ -209,7 +209,6 @@ func run(cfgPath string) error { r := chi.NewRouter() r.Use(middleware.Recoverer) r.Use(middleware.RequestID) - r.Use(middleware.RealIP) r.Use(middleware.Timeout(30 * time.Second)) r.Use(authProvider.Middleware()) diff --git a/internal/crypto/p1363.go b/internal/crypto/p1363.go index 73a4a29..b9bd550 100644 --- a/internal/crypto/p1363.go +++ b/internal/crypto/p1363.go @@ -17,9 +17,9 @@ import ( // COSE (RFC 8152), and WebCrypto. // // Interop between these two worlds is the whole reason this file -// exists. The reference TL converts at the KMS boundary; we do the -// same at the KeyManager boundary so the rest of our code works in -// JWS-native P1363 form. +// exists. Callers that emit JWS or COSE convert DER signatures at +// the KeyManager boundary so those wire formats stay in their +// required P1363 form. // ErrInvalidP1363Length is returned when a P1363 signature is not // exactly 2*coordinateSize bytes. diff --git a/internal/tl/logstore/c2spsigner.go b/internal/tl/logstore/c2spsigner.go index 92505e0..02a17cf 100644 --- a/internal/tl/logstore/c2spsigner.go +++ b/internal/tl/logstore/c2spsigner.go @@ -23,12 +23,12 @@ import ( // return, base64-encodes the combined blob, and writes it as the // `— ` signature line. // -// Sign(msg) returns a raw ECDSA signature in IEEE P1363 form over +// Sign(msg) returns an ASN.1 DER ECDSA signature over // SHA-256(msg). The wire shape on the checkpoint note is therefore: // -// || +// || // -// which is exactly what the reference TL's sigstore.Signer emits. +// which is exactly what the production TL emits. // // Mirrors the reference's sigstore.LoadSigner / // `merkle.TesseraJWSSigner`'s primary signer, minus the KMS @@ -86,34 +86,39 @@ func (s *C2SPECDSASigner) Name() string { return s.origin } // signature line agrees with the one advertised on /root-keys. func (s *C2SPECDSASigner) KeyHash() uint32 { return s.keyhash } -// Sign implements note.Signer. Returns raw ECDSA P1363 signature +// Sign implements note.Signer. Returns ASN.1 DER ECDSA signature // bytes over SHA-256(msg) — no JWS framing, no extra envelope. The -// verifier recomputes SHA-256 over the same body bytes and verifies -// against the key advertised at /root-keys. +// note package prepends the 4-byte keyhash before writing the +// checkpoint signature line. The verifier recomputes SHA-256 over +// the same body bytes and verifies against the key advertised at +// /root-keys. func (s *C2SPECDSASigner) Sign(msg []byte) ([]byte, error) { digest := sha256.Sum256(msg) rawSig, err := s.km.Sign(context.Background(), s.keyID, digest[:]) if err != nil { return nil, fmt.Errorf("logstore: c2sp sign: %w", err) } - // KeyManager returns ASN.1 DER; C2SP wants IEEE P1363 (r||s). - // P-256 coordinates are 32 bytes. - coordBytes := (s.pub.Curve.Params().BitSize + 7) / 8 - p1363, err := anscrypto.DERToP1363(rawSig, coordBytes) - if err != nil { - return nil, fmt.Errorf("logstore: c2sp sig format: %w", err) - } - return p1363, nil + return rawSig, nil } -// VerifyC2SPECDSA verifies a raw ECDSA P1363 signature over SHA-256 -// of the given checkpoint body. Used by the checkpoint-read path to -// set `valid` on C2SP signature entries. +// VerifyC2SPECDSA verifies an ASN.1 DER ECDSA signature over +// SHA-256 of the given checkpoint body. Used by the checkpoint-read +// path to set `valid` on C2SP signature entries. +// +// Legacy local-dev checkpoints were emitted as IEEE P1363 r||s +// signatures, so verification accepts that form as a compatibility +// fallback. New checkpoint signatures should be DER. func VerifyC2SPECDSA(pub *ecdsa.PublicKey, body, sig []byte) bool { - if pub == nil || len(sig) == 0 { + if pub == nil || pub.Curve == nil || len(sig) == 0 { return false } digest := sha256.Sum256(body) + if ecdsa.VerifyASN1(pub, digest[:], sig) { + return true + } + if len(sig) != 2*anscrypto.CoordinateBytes(pub) { + return false + } r, s, err := anscrypto.P1363ToScalars(sig) if err != nil { return false diff --git a/internal/tl/logstore/c2spsigner_test.go b/internal/tl/logstore/c2spsigner_test.go new file mode 100644 index 0000000..8bf06b0 --- /dev/null +++ b/internal/tl/logstore/c2spsigner_test.go @@ -0,0 +1,150 @@ +package logstore_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/asn1" + "encoding/base64" + "encoding/binary" + "math/big" + "path/filepath" + "strings" + "testing" + + "golang.org/x/mod/sumdb/note" + + "github.com/godaddy/ans/internal/adapter/keymanager" + anscrypto "github.com/godaddy/ans/internal/crypto" + "github.com/godaddy/ans/internal/port" + "github.com/godaddy/ans/internal/tl/logstore" +) + +type ecdsaDERForTest struct { + R, S *big.Int +} + +func TestC2SPSignerSignReturnsDER(t *testing.T) { + t.Parallel() + + ctx := context.Background() + signer := newC2SPTestSigner(ctx, t, "ans-test") + body := []byte("ans-test\n1\nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n") + + sig, err := signer.Sign(body) + if err != nil { + t.Fatalf("Sign: %v", err) + } + assertDERSignature(t, signer.PublicKey(), body, sig) +} + +func TestC2SPSignerNoteSignatureLineWrapsDER(t *testing.T) { + t.Parallel() + + ctx := context.Background() + origin := "ans-test" + signer := newC2SPTestSigner(ctx, t, origin) + body := "ans-test\n1\nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n" + + signed, err := note.Sign(¬e.Note{Text: body}, signer) + if err != nil { + t.Fatalf("note.Sign: %v", err) + } + + raw := decodeLastNoteSignature(t, signed) + if len(raw) <= 4 { + t.Fatalf("note signature length: got %d, want keyhash plus DER signature", len(raw)) + } + if got := binary.BigEndian.Uint32(raw[:4]); got != signer.KeyHash() { + t.Fatalf("keyhash: got 0x%08x, want 0x%08x", got, signer.KeyHash()) + } + assertDERSignature(t, signer.PublicKey(), []byte(body), raw[4:]) +} + +func TestVerifyC2SPECDSAAcceptsDERAndLegacyP1363(t *testing.T) { + t.Parallel() + + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + body := []byte("ans-test\n1\nAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\n") + digest := sha256.Sum256(body) + der, err := ecdsa.SignASN1(rand.Reader, priv, digest[:]) + if err != nil { + t.Fatalf("SignASN1: %v", err) + } + p1363, err := anscrypto.DERToP1363(der, anscrypto.CoordinateBytes(&priv.PublicKey)) + if err != nil { + t.Fatalf("DERToP1363: %v", err) + } + + if !logstore.VerifyC2SPECDSA(&priv.PublicKey, body, der) { + t.Fatal("DER signature did not verify") + } + if !logstore.VerifyC2SPECDSA(&priv.PublicKey, body, p1363) { + t.Fatal("legacy P1363 signature did not verify") + } + if logstore.VerifyC2SPECDSA(&priv.PublicKey, []byte("tampered\n"), der) { + t.Fatal("signature verified against the wrong checkpoint body") + } +} + +func newC2SPTestSigner(ctx context.Context, t *testing.T, origin string) *logstore.C2SPECDSASigner { + t.Helper() + + dir := t.TempDir() + km, err := keymanager.NewFileKeyManager(filepath.Join(dir, "keys")) + if err != nil { + t.Fatalf("NewFileKeyManager: %v", err) + } + if _, err := km.EnsureKey(ctx, "tl-sign", port.AlgorithmECDSAP256); err != nil { + t.Fatalf("EnsureKey: %v", err) + } + signer, err := logstore.NewC2SPECDSASigner(ctx, km, "tl-sign", origin) + if err != nil { + t.Fatalf("NewC2SPECDSASigner: %v", err) + } + return signer +} + +func assertDERSignature(t *testing.T, pub *ecdsa.PublicKey, body, sig []byte) { + t.Helper() + + var parsed ecdsaDERForTest + rest, err := asn1.Unmarshal(sig, &parsed) + if err != nil { + t.Fatalf("signature is not ASN.1 DER: %v", err) + } + if len(rest) != 0 { + t.Fatalf("DER signature has trailing bytes: %x", rest) + } + if parsed.R == nil || parsed.S == nil || parsed.R.Sign() <= 0 || parsed.S.Sign() <= 0 { + t.Fatalf("DER signature has invalid ECDSA scalars: %+v", parsed) + } + + digest := sha256.Sum256(body) + if !ecdsa.VerifyASN1(pub, digest[:], sig) { + t.Fatal("DER signature failed VerifyASN1") + } +} + +func decodeLastNoteSignature(t *testing.T, signed []byte) []byte { + t.Helper() + + lines := strings.Split(strings.TrimSpace(string(signed)), "\n") + if len(lines) == 0 { + t.Fatal("signed note has no lines") + } + fields := strings.Fields(lines[len(lines)-1]) + if len(fields) != 3 || fields[0] != "\u2014" { + t.Fatalf("signature line: got %q, want em-dash signer base64", lines[len(lines)-1]) + } + raw, err := base64.StdEncoding.DecodeString(fields[2]) + if err != nil { + t.Fatalf("decode signature: %v", err) + } + return raw +} diff --git a/internal/tl/logstore/signer_errors_test.go b/internal/tl/logstore/signer_errors_test.go index dddb922..0f75a75 100644 --- a/internal/tl/logstore/signer_errors_test.go +++ b/internal/tl/logstore/signer_errors_test.go @@ -127,9 +127,8 @@ func TestNewJWSCheckpointSigner_NonECDSAKey(t *testing.T) { // ----- VerifyC2SPECDSA negative paths ----- -// VerifyC2SPECDSA's three reject branches: nil pubkey, empty -// signature, and failure to split the P1363 signature into r/s -// scalars. Each returns false instead of an error. +// VerifyC2SPECDSA's malformed-input branches return false instead of +// an error. func TestVerifyC2SPECDSA_NilPubKey(t *testing.T) { if logstore.VerifyC2SPECDSA(nil, []byte("body"), []byte{0x01, 0x02}) { t.Error("expected false for nil public key") @@ -145,7 +144,8 @@ func TestVerifyC2SPECDSA_EmptySig(t *testing.T) { func TestVerifyC2SPECDSA_MalformedSig(t *testing.T) { pub, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - // 5-byte sig isn't a valid P1363 signature (need 64 bytes for P-256). + // 5 bytes is neither a valid DER signature nor a legacy P-256 + // P1363 signature. if logstore.VerifyC2SPECDSA(&pub.PublicKey, []byte("body"), []byte{1, 2, 3, 4, 5}) { t.Error("expected false for malformed signature") } diff --git a/internal/tl/service/checkpoint.go b/internal/tl/service/checkpoint.go index 26bc555..f8d810b 100644 --- a/internal/tl/service/checkpoint.go +++ b/internal/tl/service/checkpoint.go @@ -89,7 +89,7 @@ type CheckpointView struct { type CheckpointSignatureView struct { SignerName string SignatureType string // "C2SP" (sumdb note) or "JWS" (additional signer) - Algorithm string // "ED25519" for C2SP, "ES256" for JWS + Algorithm string // "ES256" for the single ECDSA checkpoint signing key KeyHash string // 4-byte hex, matches the keyhash in /root-keys RawSignature string // base64-encoded raw signature bytes JwsSignature string // full compact-JWS "header.payload.signature" @@ -205,7 +205,7 @@ func (s *CheckpointService) viewFromRecord(rec *sqlitetl.CheckpointRecord) *Chec // amortized at page-render time. // // For the primary (sumdb-note) signer: verify the signature block -// against the configured ed25519 verifier over the checkpoint body. +// against the configured ECDSA verifier over the checkpoint body. // // Signature classification labels — match the reference TL wire and // the production /v1/log/checkpoint response. Lowercase by design. @@ -235,8 +235,10 @@ func (s *CheckpointService) enrichSignatures(body string, sigs []CheckpointSigna // enrichC2SPSignature verifies a raw C2SP ECDSA signature against the // configured signing key. The signature line's base64 body is -// ``; we hand the signature bytes to -// logstore.VerifyC2SPECDSA which re-hashes the checkpoint body. +// `` for current checkpoints; legacy local +// dev checkpoints may still carry P1363 bytes. We hand the signature +// bytes to logstore.VerifyC2SPECDSA which re-hashes the checkpoint +// body and handles both encodings. func (s *CheckpointService) enrichC2SPSignature(body string, sv *CheckpointSignatureView) { if s.signingKey == nil { return @@ -319,7 +321,7 @@ func treeHeight(size uint64) int { // // This doesn't re-verify anything — it just decomposes the stored // text into fields the REST response wants. Consumers that need -// cryptographic verification should use the ed25519 verifier served +// cryptographic verification should use the ECDSA verifier served // from /root-keys. func splitNoteBody(raw, origin string) (string, []CheckpointSignatureView) { // The sumdb-note separator per golang.org/x/mod/sumdb/note is @@ -389,8 +391,8 @@ func keyhashFromSumdbSig(b64 string) string { // // Detection: after the 4-byte keyhash prefix, a JWS starts with the // base64 of `{"alg":` — which in URL-safe base64 is `eyJhbGciOi`. -// The primary C2SP signature is raw P1363 ECDSA (64 bytes), so that -// prefix will not appear. Misclassification risk is ~1 in 2^80. +// The primary C2SP signature is ASN.1 DER ECDSA, so that JWS prefix +// will not appear. Misclassification risk is ~1 in 2^80. // // Labels match the reference TL production wire: lowercase // "c2sp" / "jws". diff --git a/internal/tl/service/checkpoint_helpers_test.go b/internal/tl/service/checkpoint_helpers_test.go index 00c5856..367e128 100644 --- a/internal/tl/service/checkpoint_helpers_test.go +++ b/internal/tl/service/checkpoint_helpers_test.go @@ -2,7 +2,6 @@ package service import ( "encoding/base64" - "encoding/binary" "strings" "testing" ) @@ -36,13 +35,14 @@ func TestTreeHeight(t *testing.T) { func TestSplitNoteBody_SingleSumdbSig(t *testing.T) { t.Parallel() - // Construct a sumdb-note-style body. - // 4 keyhash bytes + 64-byte-ish ed25519 sig. - sigBytes := make([]byte, 4+64) - binary.BigEndian.PutUint32(sigBytes[:4], 0xdeadbeef) - // Fill the signature with some bytes — not validated here. - for i := 4; i < len(sigBytes); i++ { - sigBytes[i] = byte(i) + // Construct a sumdb-note-style body: 4 keyhash bytes followed by + // an ASN.1 DER ECDSA signature-shaped blob. The signature is not + // cryptographically validated in this parser test. + sigBytes := []byte{ + 0xde, 0xad, 0xbe, 0xef, + 0x30, 0x06, + 0x02, 0x01, 0x01, + 0x02, 0x01, 0x02, } b64Sig := base64.StdEncoding.EncodeToString(sigBytes) note := "ans-demo\n5\nhashhex\n\n\u2014 ans-demo " + b64Sig + "\n"