From 98eb67dfee23902156aa87358e60883dcc96229a Mon Sep 17 00:00:00 2001 From: sam-at-ava Date: Tue, 21 Jul 2026 14:20:38 -0500 Subject: [PATCH] Preserve certificateMsgTLS13 original bytes for the TLS 1.3 transcript 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. --- handshake_messages.go | 19 ++++++- handshake_messages_test.go | 2 + u_certificate_originalbytes_test.go | 82 +++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 u_certificate_originalbytes_test.go diff --git a/handshake_messages.go b/handshake_messages.go index c382b3346a9..883838b6aa0 100644 --- a/handshake_messages.go +++ b/handshake_messages.go @@ -1499,6 +1499,7 @@ func (m *certificateMsg) unmarshal(data []byte) bool { } type certificateMsgTLS13 struct { + original []byte // [uTLS] certificate Certificate ocspStapling bool scts bool @@ -1561,7 +1562,7 @@ func marshalCertificate(b *cryptobyte.Builder, certificate Certificate) { } func (m *certificateMsgTLS13) unmarshal(data []byte) bool { - *m = certificateMsgTLS13{} + *m = certificateMsgTLS13{original: data} // [uTLS] s := cryptobyte.String(data) var context cryptobyte.String @@ -1578,6 +1579,22 @@ func (m *certificateMsgTLS13) unmarshal(data []byte) bool { return true } +// [UTLS SECTION BEGINS] +// originalBytes lets transcriptMsg hash this Certificate message exactly as it +// was received, instead of a re-marshal via marshalCertificate(). The 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; other per-certificate extensions +// and non-canonical length encodings are lost), which diverges the TLS 1.3 +// handshake transcript and causes CertificateVerify to fail with +// "crypto/rsa: verification error". clientHelloMsg, serverHelloMsg and +// certificateRequestMsgTLS13 already preserve their original bytes for the same +// reason. +func (m *certificateMsgTLS13) originalBytes() []byte { + return m.original +} + +// [UTLS SECTION ENDS] + func unmarshalCertificate(s *cryptobyte.String, certificate *Certificate) bool { var certList cryptobyte.String if !s.ReadUint24LengthPrefixed(&certList) { diff --git a/handshake_messages_test.go b/handshake_messages_test.go index b71d97ca262..aa916c6b485 100644 --- a/handshake_messages_test.go +++ b/handshake_messages_test.go @@ -100,6 +100,8 @@ func TestMarshalUnmarshal(t *testing.T) { t.original = nil case *certificateRequestMsgTLS13: // [UTLS] t.original = nil // [UTLS] + case *certificateMsgTLS13: // [UTLS] + t.original = nil // [UTLS] } if !reflect.DeepEqual(m1, m) { diff --git a/u_certificate_originalbytes_test.go b/u_certificate_originalbytes_test.go new file mode 100644 index 00000000000..2fd21edddca --- /dev/null +++ b/u_certificate_originalbytes_test.go @@ -0,0 +1,82 @@ +package tls + +import ( + "bytes" + "crypto/sha256" + "testing" + + "golang.org/x/crypto/cryptobyte" +) + +// buildTLS13CertificateWithLeafExtension builds a valid TLS 1.3 Certificate +// handshake message whose leaf certificate carries an extension that +// marshalCertificate does not re-emit (it only re-emits OCSP and SCT). +// Re-marshaling therefore drops the extension, so a transcript computed from +// the re-marshal diverges from the peer's — which makes CertificateVerify fail +// with "crypto/rsa: verification error". +func buildTLS13CertificateWithLeafExtension() []byte { + var b cryptobyte.Builder + b.AddUint8(typeCertificate) + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddUint8(0) // empty certificate_request_context + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + // single leaf certificate + b.AddUint24LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte("dummy-leaf-der")) + }) + // leaf extensions: one extension that unmarshal ignores and + // marshalCertificate never re-emits. + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddUint16(0x1234) // unknown extension type + b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte("ext-payload")) + }) + }) + }) + }) + out, err := b.Bytes() + if err != nil { + panic(err) + } + return out +} + +// TestCertificateMsgTLS13PreservesOriginalBytes ensures certificateMsgTLS13 +// feeds transcriptMsg the exact bytes it was unmarshaled from, not a +// (potentially lossy) re-marshal. +func TestCertificateMsgTLS13PreservesOriginalBytes(t *testing.T) { + wire := buildTLS13CertificateWithLeafExtension() + + var m certificateMsgTLS13 + if !m.unmarshal(wire) { + t.Fatal("unmarshal failed") + } + + wm, ok := handshakeMessage(&m).(handshakeMessageWithOriginalBytes) + if !ok { + t.Fatal("certificateMsgTLS13 does not implement handshakeMessageWithOriginalBytes") + } + if !bytes.Equal(wm.originalBytes(), wire) { + t.Fatalf("originalBytes() != wire\n orig=%x\n wire=%x", wm.originalBytes(), wire) + } + + // Guard: the re-marshal must actually diverge from the wire, otherwise the + // test is no longer exercising the bug. + remarshaled, err := m.marshal() + if err != nil { + t.Fatalf("marshal: %v", err) + } + if bytes.Equal(remarshaled, wire) { + t.Fatal("re-marshal unexpectedly equals wire; test no longer exercises the divergence") + } + + // transcriptMsg must hash the original wire bytes, not the re-marshal. + h := sha256.New() + if err := transcriptMsg(&m, h); err != nil { + t.Fatalf("transcriptMsg: %v", err) + } + want := sha256.Sum256(wire) + if got := h.Sum(nil); !bytes.Equal(got, want[:]) { + t.Fatal("transcriptMsg hashed the re-marshal instead of the original wire bytes") + } +}