Preserve certificateMsgTLS13 original bytes for the TLS 1.3 transcript

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) {

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) {

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")
+	}
+}