On Mon, 3 Apr 2023 18:13:19 GMT, Matthew Donovan <d...@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
