Preserve certificateMsgTLS13 original bytes for the TLS 1.3 transcript#8
Closed
sam-at-ava wants to merge 3 commits into
Closed
Preserve certificateMsgTLS13 original bytes for the TLS 1.3 transcript#8sam-at-ava wants to merge 3 commits into
sam-at-ava wants to merge 3 commits into
Conversation
certificateMsgTLS13 did not implement handshakeMessageWithOriginalBytes, so
transcriptMsg re-marshaled the parsed Certificate via marshalCertificate() to
feed the TLS 1.3 handshake transcript. That re-marshal is not byte-identical to
the server's encoding — leaf extensions other than OCSP/SCT are dropped, and
OCSP/SCT are re-emitted in a fixed order — so the client's transcript diverges
from the server's and CertificateVerify fails with:
tls: invalid signature by the server certificate: crypto/rsa: verification error
clientHelloMsg, serverHelloMsg and certificateRequestMsgTLS13 already cache
their original wire bytes for exactly this reason; certificateMsgTLS13 was the
only TLS 1.3 transcript message missing it. Capture `original` in unmarshal and
return it from originalBytes(), matching the existing pattern. Only the
uncompressed-certificate path was affected; compressed certs already transcript
their raw CompressedCertificate bytes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
certificateMsgTLS13 now caches its original wire bytes (like clientHelloMsg, serverHelloMsg, certificateRequestMsgTLS13), so TestMarshalUnmarshal must nil the field before DeepEqual, matching the existing cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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 fail at
CertificateVerifywith:against real production HTTPS endpoints — even though the certificate chain validates and the same server handshakes fine with
crypto/tlsand with a Chrome fingerprint. It's deterministic (not the proxy, not flaky) and reproducible on a direct connection.Root cause
certificateMsgTLS13does not implementhandshakeMessageWithOriginalBytes. uTLS reads the server Certificate withreadHandshake(nil)(to support RFC 8879 cert compression) and then hashes it into the transcript viatranscriptMsg, whose fallback path re-marshals the parsed message withmarshalCertificate(). That re-marshal is not guaranteed to reproduce the peer's byte encoding: only leaf OCSP/SCT are re-emitted (in a fixed order), any other per-certificate extension is dropped, and non-canonical length encodings are lost. When the server's Certificate doesn't round-trip exactly, the client transcript diverges from the server's, so the (correct)CertificateVerifysignature fails against the client's wrong transcript hash.Only the uncompressed-certificate path is affected; compressed certs already transcript their raw
CompressedCertificatebytes (which is why a Chrome/brotli fingerprint dodges it).Fix
clientHelloMsg,serverHelloMsgandcertificateRequestMsgTLS13already cache their original wire bytes for exactly this reason;certificateMsgTLS13was the one TLS 1.3 transcript message left out. This PR capturesoriginalinunmarshal, returns it fromoriginalBytes(), nils it inTestMarshalUnmarshallike the sibling messages, and adds a regression test (a Certificate whose leaf carries an extensionmarshalCertificatedrops; assertstranscriptMsghashes the original wire bytes, not the divergent re-marshal).go test ./...passes; the new test fails without the fix.Also submitted upstream: refraction-networking/utls#409.