Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8263779: SSLEngine reports NEED_WRAP continuously without producing a… 
…ny further output

Reviewed-by: wetmore
  • Loading branch information
@XueleiFan
XueleiFan committed Apr 28, 2021
1 parent 889d246 commit 1a37bce
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 44 deletions.
22 changes: 11 additions & 11 deletions src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java
View file
Expand Up @@ -174,7 +174,7 @@ private SSLEngineResult writeRecord(
// May need to deliver cached records.
if (isOutboundDone()) {
return new SSLEngineResult(
Status.CLOSED, getHandshakeStatus(), 0, 0);
Status.CLOSED, conContext.getHandshakeStatus(), 0, 0);
}

HandshakeContext hc = conContext.handshakeContext;
Expand All @@ -184,7 +184,7 @@ private SSLEngineResult writeRecord(
!conContext.isOutboundClosed()) {
conContext.kickstart();

hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
if (hsStatus == HandshakeStatus.NEED_UNWRAP) {
/*
* For DTLS, if the handshake state is
Expand All @@ -202,7 +202,7 @@ private SSLEngineResult writeRecord(
}

if (hsStatus == null) {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
}

/*
Expand All @@ -226,7 +226,7 @@ private SSLEngineResult writeRecord(
// now, force it to be large enough to handle any valid record.
if (dstsRemains < conContext.conSession.getPacketBufferSize()) {
return new SSLEngineResult(
Status.BUFFER_OVERFLOW, getHandshakeStatus(), 0, 0);
Status.BUFFER_OVERFLOW, conContext.getHandshakeStatus(), 0, 0);
}

int srcsRemains = 0;
Expand Down Expand Up @@ -266,7 +266,7 @@ private SSLEngineResult writeRecord(
if (ciphertext != null && ciphertext.handshakeStatus != null) {
hsStatus = ciphertext.handshakeStatus;
} else {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
if (ciphertext == null && !conContext.isNegotiated &&
conContext.isInboundClosed() &&
hsStatus == HandshakeStatus.NEED_WRAP) {
Expand Down Expand Up @@ -536,7 +536,7 @@ private SSLEngineResult readRecord(
*/
if (isInboundDone()) {
return new SSLEngineResult(
Status.CLOSED, getHandshakeStatus(), 0, 0);
Status.CLOSED, conContext.getHandshakeStatus(), 0, 0);
}

HandshakeStatus hsStatus = null;
Expand All @@ -549,14 +549,14 @@ private SSLEngineResult readRecord(
* If there's still outbound data to flush, we
* can return without trying to unwrap anything.
*/
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
if (hsStatus == HandshakeStatus.NEED_WRAP) {
return new SSLEngineResult(Status.OK, hsStatus, 0, 0);
}
}

if (hsStatus == null) {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
}

/*
Expand Down Expand Up @@ -586,7 +586,7 @@ private SSLEngineResult readRecord(
if (plainText.handshakeStatus != null) {
hsStatus = plainText.handshakeStatus;
} else {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
}

return new SSLEngineResult(
Expand Down Expand Up @@ -625,7 +625,7 @@ private SSLEngineResult readRecord(

Status status = (isInboundDone() ? Status.CLOSED : Status.OK);
if (hsStatus == null) {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
}

return new SSLEngineResult(status, hsStatus, srcsRemains, 0, -1L);
Expand Down Expand Up @@ -712,7 +712,7 @@ private SSLEngineResult readRecord(
if (plainText.handshakeStatus != null) {
hsStatus = plainText.handshakeStatus;
} else {
hsStatus = getHandshakeStatus();
hsStatus = conContext.getHandshakeStatus();
}

int deltaNet = srcsRemains;
Expand Down
33 changes: 12 additions & 21 deletions src/java.base/share/classes/sun/security/ssl/SSLEngineOutputRecord.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1996, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -55,7 +55,7 @@ public void close() throws IOException {
recordLock.lock();
try {
if (!isClosed) {
if (fragmenter != null && fragmenter.hasAlert()) {
if (fragmenter != null && !fragmenter.isEmpty()) {
isCloseWaiting = true;
} else {
super.close();
Expand Down Expand Up @@ -533,33 +533,24 @@ Ciphertext acquireCiphertext(ByteBuffer dstBuf) throws IOException {
dstBuf.limit(dstLim);

// Reset the fragmentation offset.
if (hsMemo != null) {
return new Ciphertext(hsMemo.contentType,
hsMemo.handshakeType, recordSN);
} else {
if (isCloseWaiting &&
memo.contentType == ContentType.ALERT.id) {
try {
if (hsMemo != null) {
return new Ciphertext(hsMemo.contentType,
hsMemo.handshakeType, recordSN);
} else {
return new Ciphertext(memo.contentType,
SSLHandshake.NOT_APPLICABLE.id, recordSN);
}
} finally {
if (isCloseWaiting && isEmpty()) {
close();
}

return new Ciphertext(memo.contentType,
SSLHandshake.NOT_APPLICABLE.id, recordSN);
}
}

boolean isEmpty() {
return handshakeMemos.isEmpty();
}

boolean hasAlert() {
for (RecordMemo memo : handshakeMemos) {
if (memo.contentType == ContentType.ALERT.id) {
return true;
}
}

return false;
}
}

/*
Expand Down
20 changes: 13 additions & 7 deletions src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java
View file
Expand Up @@ -1417,8 +1417,10 @@ private int readHandshakeRecord() throws IOException {
conContext.isNegotiated) {
return 0;
}
} catch (SSLException | InterruptedIOException | SocketException se) {
// don't change exception in case of timeouts or interrupts or SocketException
} catch (SSLException |
InterruptedIOException | SocketException se) {
// Don't change exception in case of timeouts or interrupts
// or SocketException.
throw se;
} catch (IOException ioe) {
throw new SSLException("readHandshakeRecord", ioe);
Expand Down Expand Up @@ -1474,16 +1476,19 @@ private ByteBuffer readApplicationRecord(
buffer.position() > 0) {
return buffer;
}
} catch (SSLException | InterruptedIOException | SocketException se) {
// don't change exception in case of timeouts or interrupts or SocketException.
} catch (SSLException |
InterruptedIOException | SocketException se) {
// Don't change exception in case of timeouts or interrupts
// or SocketException.
throw se;
} catch (IOException ioe) {
throw new SSLException("readApplicationRecord", ioe);
}
}

//
// couldn't read, due to some kind of error
// Couldn't read, due to some kind of error or inbound
// has been closed.
//
return null;
}
Expand Down Expand Up @@ -1686,7 +1691,7 @@ private void handleException(Exception cause) throws IOException {

if (cause instanceof SocketException) {
try {
conContext.fatal(alert, cause);
throw conContext.fatal(alert, cause);
} catch (Exception e) {
// Just delivering the fatal alert, re-throw the socket exception instead.
}
Expand Down Expand Up @@ -1748,7 +1753,8 @@ public void shutdown() throws IOException {
// If conContext.isInputCloseNotified is false, close the
// connection, no wait for more peer response. Otherwise,
// may wait for peer close_notify.
closeSocket(!conContext.isInputCloseNotified);
closeSocket(conContext.isNegotiated &&
!conContext.isInputCloseNotified);
} finally {
tlsIsClosed = true;
}
Expand Down
9 changes: 4 additions & 5 deletions src/java.base/share/classes/sun/security/ssl/TransportContext.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -584,10 +584,10 @@ private void initiateOutboundClose() throws IOException {
closeNotify(useUserCanceled);
}

// Note; HandshakeStatus.FINISHED status is retrieved in other places.
// Note: HandshakeStatus.FINISHED status is retrieved in other places.
HandshakeStatus getHandshakeStatus() {
if (!outputRecord.isEmpty()) {
// If no handshaking, special case to wrap alters or
// If not handshaking, special case to wrap alerts or
// post-handshake messages.
return HandshakeStatus.NEED_WRAP;
} else if (isOutboundClosed() && isInboundClosed()) {
Expand All @@ -596,8 +596,7 @@ HandshakeStatus getHandshakeStatus() {
if (!handshakeContext.delegatedActions.isEmpty()) {
return HandshakeStatus.NEED_TASK;
} else if (!isInboundClosed()) {
if (sslContext.isDTLS() &&
!inputRecord.isEmpty()) {
if (sslContext.isDTLS() && !inputRecord.isEmpty()) {
return HandshakeStatus.NEED_UNWRAP_AGAIN;
} else {
return HandshakeStatus.NEED_UNWRAP;
Expand Down

1 comment on commit 1a37bce

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.