RFR: 8182621: JSSE should reject empty TLS plaintexts

Xue-Lei Andrew Fan xuelei at openjdk.org
Fri Apr 7 02:19:48 UTC 2023


On Mon, 3 Apr 2023 18:13:19 GMT, Matthew Donovan <duke at openjdk.org> wrote:

> Added code similar to the suggested patches for empty Handshake messages. I also implemented tests to verify empty Handshake, Alert, and ChangeCipherSpec messages result in expected behavior: for SSLEngineImpl, exceptions are thrown, for SSLSockets the connection is closed.

src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java line 266:

> 264:         //
> 265:         if (contentType == ContentType.HANDSHAKE.id) {
> 266:             if (!fragment.hasRemaining()) {

It may be easier to read if using "contentLen != 0", instead of "fragment.hasRemaining()".  I may add a comment like what is says in RFC 8446, "Implementations MUST NOT send zero-length fragments of Handshake types, even if those fragments contain padding."

src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java line 267:

> 265:         if (contentType == ContentType.HANDSHAKE.id) {
> 266:             if (!fragment.hasRemaining()) {
> 267:                 throw new SSLProtocolException("Handshake packets may not be zero-length");

"may not" -> "must not"

src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java line 287:

> 285:         if (contentType == ContentType.HANDSHAKE.id) {
> 286:             ByteBuffer handshakeFrag = fragment;
> 287:             if (!handshakeFrag.hasRemaining()) {

Same comment as src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160385752
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160385932
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160386380



More information about the security-dev mailing list