crypto/tls: reject change_cipher_spec record after handshake in TLS 1.3

[RPRX]

golang/go#58912

[RPRX]

该补丁对 uTLS 的特殊意义：uTLS 模仿的指纹大多使用 BoringSSL，或其它对 TLSv1.3 的正确实现，它们没有 Golang TLS 的这个问题。比如，中间人怀疑某个 Chrome TLSv1.3 连接实际上是 uTLS，插入 CCS 包后未触发 alert，则可以确定它为 uTLS。

该行为对已经建立的 TLSv1.3 连接可能是破坏性的，中间人不一定会这样操作，但需要防患于未然。

---

The special significance of this patch for uTLS: most of the fingerprints emulated by uTLS use BoringSSL, or other correct implementations of TLSv1.3, which do not have this problem with Golang TLS. For example, an intermediary who suspects that a Chrome TLSv1.3 connection is actually uTLS, and inserts a CCS packet without triggering alert, can determine that it is uTLS.

This behavior can be destructive to already established TLSv1.3 connections, and the man-in-the-middle will not necessarily do this, but it needs to be prevented before it happens.

Translated with www.DeepL.com/Translator (free version)

[gaukas]

A nice catch @RPRX! I agree this would allow an attacker distinguish crypto/tls from BoringSSL/OpenSSL trivially.

I wonder would it be easy to verify the alleged behavior that Real world TLS 1.3 implementation sends alert after seeing ChangeCipherSpec but not uTLS? Experimental data might be more significant than the result from code analysis.

[gaukas]

Btw I am assuming crypto/tls will not be accepting it since it is an undefined behavior in (instead of a violation of) the RFC document.

[RPRX]

I wonder would it be easy to verify the alleged behavior that Real world TLS 1.3 implementation sends alert after seeing ChangeCipherSpec but not uTLS? Experimental data might be more significant than the result from code analysis.

我使用 golang/go#58912 中的方法，即在每个真正的 application_data record 前插入 change_cipher_spec record，并作为服务端：

• 使用未修复 bug 的 Go crypto/tls 库或 uTLS 库作为客户端时，连接无异常。
• 使用最新的 Chrome、Edge、Firefox 作为客户端时，连接均异常。

为了测试这些浏览器有无发送 alert，我在服务端 func readRecordOrCCS() 的 case recordTypeAlert: 内输出了 data：

• 使用最新的 Chrome、Edge 作为客户端时，服务端收到了 alertLevelError 和 alertBadRecordMAC（解密后）。
• 使用最新的 Firefox 作为客户端时，服务端收到了 alertLevelError 和 alertUnexpectedMessage（解密后）。

（对于握手完毕的 TLSv1.3 连接，消息外观均为 application_data record，所以我们发 alertUnexpectedMessage 没有问题。）

---

I use the method in golang/go#58912, where I insert the change_cipher_spec record in front of each real application_data record, and as a server side.

No connection exceptions when using the Go crypto/tls library or uTLS library with no bug fixes as a client.
When using the latest Chrome, Edge, Firefox as clients, the connection is abnormal.
To test if these browsers send alerts, I output data in the case recordTypeAlert: of the server-side func readRecordOrCCS().

When using the latest Chrome and Edge as clients, the server receives alertLevelError and alertBadRecordMAC (after decryption).
When using the latest Firefox as a client, the server receives alertLevelError and alertUnexpectedMessage (after decryption).
(For a TLSv1.3 connection with a completed handshake, the message appearance is application_data record, so we have no problem sending alertUnexpectedMessage.)

Translated with DeepL (https://www.deepl.com/app/?utm_source=ios&utm_medium=app&utm_campaign=share-translation

[RPRX]

Btw I am assuming crypto/tls will not accepting it since it is an undefined behavior in (instead of a violation of) the RFC document.

RFC 8446 写的是 CCS 可以在握手过程中出现（特例），也就是说除此之外应走常规流程，即所有消息均加密、接收方先解密。

OpenSSL/BoringSSL 的做法和 Go crypto/tls 的现有注释也体现了这些，所以它其实是 Go crypto/tls 的一个 bug，应当被修复。

---

RFC 8446 states that the CCS can occur during the handshake (a special case), meaning that otherwise the normal process should be followed, i.e. all messages are encrypted and decrypted by the receiver first.

This is reflected in the OpenSSL/BoringSSL approach and in the existing comments for Go crypto/tls, so it is actually a bug in Go crypto/tls that should be fixed.

Translated with DeepL (https://www.deepl.com/app/?utm_source=ios&utm_medium=app&utm_campaign=share-translation

[gaukas]

也就是说除此之外应走常规流程，即所有消息均加密、接收方先解密。
meaning that otherwise the normal process should be followed, i.e. all messages are encrypted and decrypted by the receiver first.

Sounds like you were saying that "RFC implies ...". But if this restriction is not described using "MUST/SHOULD [NOT]", it is considered undefined.

E.g., the behavior when the package received NOT having a TLS header is not defined by RFC even though everyone know it is considered as an "illegal behavior". Therefore the handling routines vary by a good amount.

[RPRX]

Sounds like you were saying that "RFC implies ...". But if this restriction is not described using "MUST/SHOULD [NOT]", it is considered undefined.

这是个好问题，我发现 RFC 8446, Section 5 对这种情况有强制规定：

If an implementation detects a change_cipher_spec record received before the first ClientHello message or after the peer's Finished message, it MUST be treated as an unexpected record type ... If a TLS implementation receives an unexpected record type, it MUST terminate the connection with an "unexpected_message" alert.

并且 RFC 8446, Appendix E.2 写道：

Confidentiality: An attacker should not be able to determine the plaintext contents of a given record.

所以该 bug 是一个切实的安全漏洞，攻击者可以通过插 16 个 CCS 包的方式来探测特定 application_data record 是否实际为空。

---

This is a good question, and I found RFC 8446, Section 5 to be mandatory for this case.

If an implementation detects a change_cipher_spec record received before the first ClientHello message or after the peer's Finished message, it MUST be treated as an unexpected record type ... If a TLS implementation receives an unexpected record type, it MUST terminate the connection with an "unexpected_message" alert.

And RFC 8446, Appendix E.2 reads.

Confidentiality: An attacker should not be able to determine the plaintext contents of a given record.

So the bug is a real security vulnerability that allows an attacker to detect whether a specific application_data record is actually empty by inserting 16 CCS packets.

[gaukas]

Good find. Perhaps also append the related section to your PR for golang/go. I don't think they would care though. I had an impression that golang/go doesn't accept community contributions.

Actually such finding could be considered a CVE and I would encourage you to submit it.

[gaukas]

I was about to publish a security advisory but only to find myself not being an admin of this repo 😓.