Preserve certificateMsgTLS13 original bytes for the TLS 1.3 transcript#409
Open
sam-at-ava wants to merge 1 commit into
Open
Conversation
certificateMsgTLS13 does not implement handshakeMessageWithOriginalBytes, so
transcriptMsg re-marshals the parsed Certificate via marshalCertificate() when
feeding the TLS 1.3 handshake transcript. That re-marshal is not guaranteed to
be byte-identical to the peer's encoding: only leaf OCSP/SCT are re-emitted (in
a fixed order), and any other per-certificate extension or non-canonical length
encoding is lost. When a server's Certificate message doesn't round-trip
exactly, the client's transcript diverges from the server's and the handshake
fails at CertificateVerify with:
tls: invalid signature by the server certificate: crypto/rsa: verification error
This is reproducible against real production HTTPS endpoints whose leaf
certificate message carries an extension beyond OCSP/SCT. It only affects the
uncompressed-certificate path; compressed certificates already transcript their
raw CompressedCertificate bytes.
clientHelloMsg, serverHelloMsg and certificateRequestMsgTLS13 already cache
their original wire bytes for exactly this reason (added in the June 2025
originalBytes work); certificateMsgTLS13 was the one TLS 1.3 transcript message
left out. Capture `original` in unmarshal and return it from originalBytes(),
and nil it in the TestMarshalUnmarshal round-trip like the sibling messages.
Adds a regression test that builds a Certificate whose leaf carries an extension
marshalCertificate drops, and asserts transcriptMsg hashes the original wire
bytes rather than the (divergent) re-marshal.
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.
Problem
TLS 1.3 handshakes can fail at
CertificateVerifywith:against real production HTTPS endpoints — even though the certificate chain validates and the same server works with
crypto/tlsand with a Chrome fingerprint.Root cause
certificateMsgTLS13does not implementhandshakeMessageWithOriginalBytes. When uTLS reads the server Certificate withreadHandshake(nil)(to support RFC 8879 cert compression) and then hashes it into the transcript viatranscriptMsg, the fallback path re-marshals the parsed message withmarshalCertificate():marshalCertificate()is not guaranteed to reproduce the peer's byte encoding: it only re-emits OCSP/SCT (in a fixed order) for the leaf, and drops any other per-certificate extension; non-canonical length encodings are also lost. When the server's Certificate message doesn't round-trip exactly, the client's transcript diverges from the server's, so theCertificateVerifysignature — which is correct — fails to verify against the client's (wrong) transcript hash.This only affects the uncompressed-certificate path; compressed certificates already transcript their raw
CompressedCertificatebytes. It's deterministic and reproducible against production servers whose leaf Certificate carries an extension beyond OCSP/SCT.Fix
clientHelloMsg,serverHelloMsgandcertificateRequestMsgTLS13already cache their original wire bytes for exactly this reason (the June-2025originalByteswork).certificateMsgTLS13was the one TLS 1.3 transcript message left out. This PR:original []bytetocertificateMsgTLS13, captures it inunmarshal, and returns it fromoriginalBytes();TestMarshalUnmarshalalongside the sibling messages;marshalCertificatedrops, and assertstranscriptMsghashes the original wire bytes rather than the divergent re-marshal.go test ./...passes; the new test fails without thehandshake_messages.gochange.