fix(tl): emit DER C2SP checkpoint signatures#38
Merged
Conversation
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 <csnitker@godaddy.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns the TL checkpoint primary (C2SP / sumdb-note) signature wire format with production by emitting ASN.1 DER-encoded ECDSA signatures (wrapped as <keyhash:4> || <sig>), updates verification to prefer ecdsa.VerifyASN1 with a legacy P1363 fallback, and removes deprecated chi RealIP middleware usage.
Changes:
- Emit DER ECDSA bytes for the primary checkpoint signature and verify with
ecdsa.VerifyASN1(legacy P1363 verification retained). - Add/adjust regression tests covering DER signer output, sumdb/note wrapping, DER verification, and legacy P1363 verification.
- Remove deprecated
middleware.RealIPcalls from TL/RA HTTP router setup.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/tl/service/checkpoint.go | Updates checkpoint signature metadata/comments and C2SP verification description to reflect DER-first ECDSA verification. |
| internal/tl/service/checkpoint_helpers_test.go | Updates parser test fixture to use a DER-shaped signature blob. |
| internal/tl/logstore/signer_errors_test.go | Updates negative-path test commentary for DER/P1363 handling. |
| internal/tl/logstore/c2spsigner.go | Switches primary checkpoint signer output to DER and updates verifier to DER-first with P1363 fallback. |
| internal/tl/logstore/c2spsigner_test.go | Adds tests asserting DER output and DER/P1363 verification behavior. |
| internal/crypto/p1363.go | Updates documentation to reflect DER↔P1363 conversion responsibilities for JWS/COSE callers. |
| cmd/ans-tl/main.go | Removes deprecated chi RealIP middleware. |
| cmd/ans-ra/main.go | Removes deprecated chi RealIP middleware. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
90
to
+92
| 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 |
Comment on lines
95
to
+101
| 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 |
bdittmer-godaddy
approved these changes
Jun 4, 2026
kperry-godaddy
approved these changes
Jun 4, 2026
nicknacnic
added a commit
to nicknacnic/ans
that referenced
this pull request
Jun 7, 2026
Updates signTestCheckpoint in walk_test.go to produce ASN.1 DER ECDSA signatures, matching the production wire shape restored in godaddy/ans PR godaddy#38 ("fix(tl): emit DER C2SP checkpoint signatures"). VerifyC2SPECDSA on main still accepts IEEE P1363 r||s as a legacy fallback for older local-dev checkpoints, so the previous P1363 fixture continued to pass — but the tests should pin the format verifiers will see in production, not the deprecated one. Walker production code is unchanged: verifyCheckpointNote delegates ECDSA verification to logstore.VerifyC2SPECDSA, which after PR godaddy#38 transparently accepts DER (primary) and P1363 (legacy). No other adjustments needed. Signed-off-by: Layer8 <NWillAU900@gmail.com>
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<keyhash:4> || <DER signature>.ecdsa.VerifyASN1, while retaining a legacy P1363 fallback for existing local-dev checkpoints.sumdb/notekeyhash wrapping, DER verification, and legacy P1363 verification.RealIPmiddleware calls surfaced bymake check; the repo does not currently consumeRemoteAddror chi client-IP context.Wire Shape
Canonical contract reference:
spec/api-spec-tl-v2.yamldocumentsCheckpointSignature.rawSignatureas base64-encoded raw signature bytes with shape<keyhash:4> || <sig>.This PR changes the local primary C2SP checkpoint signer from:
to production parity:
JWS checkpoint signatures, COSE receipts, and status tokens remain P1363.
Validation
go test ./internal/tl/logstore ./internal/tl/servicemake check