RFR: 8182621: JSSE should reject empty TLS plaintexts [v2]

[Matthew Donovan]

On Fri, 7 Apr 2023 02:14:59 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Matthew Donovan has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - added comment referring to relevant RFC
>>  - clarified if-statements; fixed exception message wording
>
> 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."

I updated the if-logic and added the comment

> 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"

Fixed

> 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

Also fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716144
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716277
PR Review Comment: https://git.openjdk.org/jdk/pull/13306#discussion_r1160716381