# HG changeset patch # User xuelei # Date 1534295807 25200 # Node ID 910f7b56592fcd6cb24cafb6aea04d1c307fa110 # Parent f4b4dfac45b1d61ec7cab27d7441efd92f898490 8207009: TLS 1.3 half-close and synchronization issues Reviewed-by: jnimeh, mullan, wetmore diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/com/sun/crypto/provider/SunJCE.java --- a/src/java.base/share/classes/com/sun/crypto/provider/SunJCE.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/com/sun/crypto/provider/SunJCE.java Tue Aug 14 18:16:47 2018 -0700 @@ -118,7 +118,8 @@ final String BLOCK_PADS = "NOPADDING|PKCS5PADDING|ISO10126PADDING"; AccessController.doPrivileged( - new java.security.PrivilegedAction<>() { + new java.security.PrivilegedAction() { + @Override public Object run() { /* diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/javax/net/ssl/SSLEngine.java --- a/src/java.base/share/classes/javax/net/ssl/SSLEngine.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/javax/net/ssl/SSLEngine.java Tue Aug 14 18:16:47 2018 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2018, 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 @@ -145,7 +145,7 @@ * application messages are encrypted and integrity protected, * and inbound messages reverse the process. * - *
  • Rehandshaking - Either side may request a renegotiation of + *
  • Rehandshaking - Either side may request a renegotiation of * the session at any time during the Application Data phase. New * handshaking data can be intermixed among the application data. * Before starting the rehandshake phase, the application may @@ -156,12 +156,20 @@ * configuration settings will not be used until the next * handshake. * - *
  • Closure - When the connection is no longer needed, the - * application should close the {@code SSLEngine} and should - * send/receive any remaining messages to the peer before - * closing the underlying transport mechanism. Once an engine is - * closed, it is not reusable: a new {@code SSLEngine} must - * be created. + *
  • Closure - When the connection is no longer needed, the client + * and the server applications should each close both sides of their + * respective connections. For {@code SSLEngine} objects, an + * application should call {@link SSLEngine#closeOutbound()} and + * send any remaining messages to the peer. Likewise, an application + * should receive any remaining messages from the peer before calling + * {@link SSLEngine#closeInbound()}. The underlying transport mechanism + * can then be closed after both sides of the {@code SSLEngine} have + * been closed. If the connection is not closed in an orderly manner + * (for example {@link SSLEngine#closeInbound()} is called before the + * peer's write closure notification has been received), exceptions + * will be raised to indicate that an error has occurred. Once an + * engine is closed, it is not reusable: a new {@code SSLEngine} + * must be created. * * An {@code SSLEngine} is created by calling {@link * SSLContext#createSSLEngine()} from an initialized diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/javax/net/ssl/SSLSocket.java --- a/src/java.base/share/classes/javax/net/ssl/SSLSocket.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/javax/net/ssl/SSLSocket.java Tue Aug 14 18:16:47 2018 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, 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 @@ -130,6 +130,21 @@ * socket can not switch between client and server modes, even when * performing renegotiations. * + * @apiNote + * When the connection is no longer needed, the client and server + * applications should each close both sides of their respective connection. + * For {@code SSLSocket} objects, for example, an application can call + * {@link Socket#shutdownOutput()} or {@link java.io.OutputStream#close()} + * for output strean close and call {@link Socket#shutdownInput()} or + * {@link java.io.InputStream#close()} for input stream close. Note that + * in some cases, closing the input stream may depend on the peer's output + * stream being closed first. If the connection is not closed in an orderly + * manner (for example {@link Socket#shutdownInput()} is called before the + * peer's write closure notification has been received), exceptions may + * be raised to indicate that an error has occurred. Once an + * {@code SSLSocket} is closed, it is not reusable: a new {@code SSLSocket} + * must be created. + * * @see java.net.Socket * @see SSLServerSocket * @see SSLSocketFactory diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/Alert.java --- a/src/java.base/share/classes/sun/security/ssl/Alert.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/Alert.java Tue Aug 14 18:16:47 2018 -0700 @@ -235,13 +235,22 @@ Level level = Level.valueOf(am.level); Alert alert = Alert.valueOf(am.id); if (alert == Alert.CLOSE_NOTIFY) { - if (tc.handshakeContext != null) { + tc.isInputCloseNotified = true; + tc.closeInbound(); + + if (tc.peerUserCanceled) { + tc.closeOutbound(); + } else if (tc.handshakeContext != null) { tc.fatal(Alert.UNEXPECTED_MESSAGE, "Received close_notify during handshake"); } - - tc.isInputCloseNotified = true; - tc.closeInbound(); + } else if (alert == Alert.USER_CANCELED) { + if (level == Level.WARNING) { + tc.peerUserCanceled = true; + } else { + tc.fatal(alert, + "Received fatal close_notify alert", true, null); + } } else if ((level == Level.WARNING) && (alert != null)) { // Terminate the connection if an alert with a level of warning // is received during handshaking, except the no_certificate diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/BaseSSLSocketImpl.java --- a/src/java.base/share/classes/sun/security/ssl/BaseSSLSocketImpl.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/BaseSSLSocketImpl.java Tue Aug 14 18:16:47 2018 -0700 @@ -204,30 +204,35 @@ // /** - * The semantics of shutdownInput is not supported in TLS 1.0 - * spec. Thus when the method is called on an SSL socket, an - * UnsupportedOperationException will be thrown. + * Places the input stream for this socket at "end of stream". Any data + * sent to the input stream side of the socket is acknowledged and then + * silently discarded. * - * @throws UnsupportedOperationException + * @see java.net.Socket#shutdownInput */ @Override - public final void shutdownInput() throws IOException { - throw new UnsupportedOperationException("The method shutdownInput()" + - " is not supported in SSLSocket"); + public void shutdownInput() throws IOException { + if (self == this) { + super.shutdownInput(); + } else { + self.shutdownInput(); + } } /** - * The semantics of shutdownOutput is not supported in TLS 1.0 - * spec. Thus when the method is called on an SSL socket, an - * UnsupportedOperationException will be thrown. + * Disables the output stream for this socket. For a TCP socket, any + * previously written data will be sent followed by TCP's normal + * connection termination sequence. * - * @throws UnsupportedOperationException + * @see java.net.Socket#shutdownOutput */ @Override - public final void shutdownOutput() throws IOException { - throw new UnsupportedOperationException("The method shutdownOutput()" + - " is not supported in SSLSocket"); - + public void shutdownOutput() throws IOException { + if (self == this) { + super.shutdownOutput(); + } else { + self.shutdownOutput(); + } } /** @@ -235,7 +240,7 @@ * @see java.net.Socket#isInputShutdown */ @Override - public final boolean isInputShutdown() { + public boolean isInputShutdown() { if (self == this) { return super.isInputShutdown(); } else { @@ -248,7 +253,7 @@ * @see java.net.Socket#isOutputShutdown */ @Override - public final boolean isOutputShutdown() { + public boolean isOutputShutdown() { if (self == this) { return super.isOutputShutdown(); } else { @@ -618,7 +623,7 @@ } @Override - public synchronized void close() throws IOException { + public void close() throws IOException { if (self == this) { super.close(); } else { diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/CipherSuite.java --- a/src/java.base/share/classes/sun/security/ssl/CipherSuite.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/CipherSuite.java Tue Aug 14 18:16:47 2018 -0700 @@ -805,7 +805,7 @@ this.id = id; this.isDefaultEnabled = isDefaultEnabled; this.name = name; - if (aliases.isEmpty()) { + if (!aliases.isEmpty()) { this.aliases = Arrays.asList(aliases.split(",")); } else { this.aliases = Collections.emptyList(); diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/DTLSOutputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -44,7 +44,7 @@ Authenticator prevWriteAuthenticator; SSLWriteCipher prevWriteCipher; - private final LinkedList alertMemos = new LinkedList<>(); + private volatile boolean isCloseWaiting = false; DTLSOutputRecord(HandshakeHash handshakeHash) { super(handshakeHash, SSLWriteCipher.nullDTlsWriteCipher()); @@ -58,6 +58,21 @@ } @Override + public synchronized void close() throws IOException { + if (!isClosed) { + if (fragmenter != null && fragmenter.hasAlert()) { + isCloseWaiting = true; + } else { + super.close(); + } + } + } + + boolean isClosed() { + return isClosed || isCloseWaiting; + } + + @Override void initHandshaker() { // clean up fragmenter = null; @@ -71,6 +86,14 @@ @Override void changeWriteCiphers(SSLWriteCipher writeCipher, boolean useChangeCipherSpec) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "change_cipher_spec message"); + } + return; + } + if (useChangeCipherSpec) { encodeChangeCipherSpec(); } @@ -91,23 +114,31 @@ @Override void encodeAlert(byte level, byte description) throws IOException { - RecordMemo memo = new RecordMemo(); + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "alert message: " + Alert.nameOf(description)); + } + return; + } - memo.contentType = ContentType.ALERT.id; - memo.majorVersion = protocolVersion.major; - memo.minorVersion = protocolVersion.minor; - memo.encodeEpoch = writeEpoch; - memo.encodeCipher = writeCipher; + if (fragmenter == null) { + fragmenter = new DTLSFragmenter(); + } - memo.fragment = new byte[2]; - memo.fragment[0] = level; - memo.fragment[1] = description; - - alertMemos.add(memo); + fragmenter.queueUpAlert(level, description); } @Override void encodeChangeCipherSpec() throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "change_cipher_spec message"); + } + return; + } + if (fragmenter == null) { fragmenter = new DTLSFragmenter(); } @@ -117,6 +148,15 @@ @Override void encodeHandshake(byte[] source, int offset, int length) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "handshake message", + ByteBuffer.wrap(source, offset, length)); + } + return; + } + if (firstMessage) { firstMessage = false; } @@ -132,6 +172,23 @@ Ciphertext encode( ByteBuffer[] srcs, int srcsOffset, int srcsLength, ByteBuffer[] dsts, int dstsOffset, int dstsLength) throws IOException { + + if (isClosed) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "application data or cached messages"); + } + + return null; + } else if (isCloseWaiting) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "application data"); + } + + srcs = null; // use no application data. + } + return encode(srcs, srcsOffset, srcsLength, dsts[0]); } @@ -237,48 +294,6 @@ private Ciphertext acquireCiphertext( ByteBuffer destination) throws IOException { - if (alertMemos != null && !alertMemos.isEmpty()) { - RecordMemo memo = alertMemos.pop(); - - int dstPos = destination.position(); - int dstLim = destination.limit(); - int dstContent = dstPos + headerSize + - writeCipher.getExplicitNonceSize(); - destination.position(dstContent); - - destination.put(memo.fragment); - - destination.limit(destination.position()); - destination.position(dstContent); - - if (SSLLogger.isOn && SSLLogger.isOn("record")) { - SSLLogger.fine( - "WRITE: " + protocolVersion + " " + - ContentType.ALERT.name + - ", length = " + destination.remaining()); - } - - // Encrypt the fragment and wrap up a record. - long recordSN = encrypt(memo.encodeCipher, - ContentType.ALERT.id, - destination, dstPos, dstLim, headerSize, - ProtocolVersion.valueOf(memo.majorVersion, - memo.minorVersion)); - - if (SSLLogger.isOn && SSLLogger.isOn("packet")) { - ByteBuffer temporary = destination.duplicate(); - temporary.limit(temporary.position()); - temporary.position(dstPos); - SSLLogger.fine("Raw write", temporary); - } - - // remain the limit unchanged - destination.limit(dstLim); - - return new Ciphertext(ContentType.ALERT.id, - SSLHandshake.NOT_APPLICABLE.id, recordSN); - } - if (fragmenter != null) { return fragmenter.acquireCiphertext(destination); } @@ -288,16 +303,14 @@ @Override boolean isEmpty() { - return ((fragmenter == null) || fragmenter.isEmpty()) && - ((alertMemos == null) || alertMemos.isEmpty()); + return (fragmenter == null) || fragmenter.isEmpty(); } @Override void launchRetransmission() { // Note: Please don't retransmit if there are handshake messages // or alerts waiting in the queue. - if (((alertMemos == null) || alertMemos.isEmpty()) && - (fragmenter != null) && fragmenter.isRetransmittable()) { + if ((fragmenter != null) && fragmenter.isRetransmittable()) { fragmenter.setRetransmission(); } } @@ -338,29 +351,6 @@ // size is bigger than 256 bytes. private int retransmits = 2; // attemps of retransmits - void queueUpChangeCipherSpec() { - - // Cleanup if a new flight starts. - if (flightIsReady) { - handshakeMemos.clear(); - acquireIndex = 0; - flightIsReady = false; - } - - RecordMemo memo = new RecordMemo(); - - memo.contentType = ContentType.CHANGE_CIPHER_SPEC.id; - memo.majorVersion = protocolVersion.major; - memo.minorVersion = protocolVersion.minor; - memo.encodeEpoch = writeEpoch; - memo.encodeCipher = writeCipher; - - memo.fragment = new byte[1]; - memo.fragment[0] = 1; - - handshakeMemos.add(memo); - } - void queueUpHandshake(byte[] buf, int offset, int length) throws IOException { @@ -401,6 +391,45 @@ } } + void queueUpChangeCipherSpec() { + + // Cleanup if a new flight starts. + if (flightIsReady) { + handshakeMemos.clear(); + acquireIndex = 0; + flightIsReady = false; + } + + RecordMemo memo = new RecordMemo(); + + memo.contentType = ContentType.CHANGE_CIPHER_SPEC.id; + memo.majorVersion = protocolVersion.major; + memo.minorVersion = protocolVersion.minor; + memo.encodeEpoch = writeEpoch; + memo.encodeCipher = writeCipher; + + memo.fragment = new byte[1]; + memo.fragment[0] = 1; + + handshakeMemos.add(memo); + } + + void queueUpAlert(byte level, byte description) throws IOException { + RecordMemo memo = new RecordMemo(); + + memo.contentType = ContentType.ALERT.id; + memo.majorVersion = protocolVersion.major; + memo.minorVersion = protocolVersion.minor; + memo.encodeEpoch = writeEpoch; + memo.encodeCipher = writeCipher; + + memo.fragment = new byte[2]; + memo.fragment[0] = level; + memo.fragment[1] = description; + + handshakeMemos.add(memo); + } + Ciphertext acquireCiphertext(ByteBuffer dstBuf) throws IOException { if (isEmpty()) { if (isRetransmittable()) { @@ -500,8 +529,13 @@ return new Ciphertext(hsMemo.contentType, hsMemo.handshakeType, recordSN); } else { + if (isCloseWaiting && + memo.contentType == ContentType.ALERT.id) { + close(); + } + acquireIndex++; - return new Ciphertext(ContentType.CHANGE_CIPHER_SPEC.id, + return new Ciphertext(memo.contentType, SSLHandshake.NOT_APPLICABLE.id, recordSN); } } @@ -552,6 +586,16 @@ return false; } + boolean hasAlert() { + for (RecordMemo memo : handshakeMemos) { + if (memo.contentType == ContentType.ALERT.id) { + return true; + } + } + + return false; + } + boolean isRetransmittable() { return (flightIsReady && !handshakeMemos.isEmpty() && (acquireIndex >= handshakeMemos.size())); diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/HandshakeContext.java --- a/src/java.base/share/classes/sun/security/ssl/HandshakeContext.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/HandshakeContext.java Tue Aug 14 18:16:47 2018 -0700 @@ -47,7 +47,6 @@ import sun.security.ssl.SupportedGroupsExtension.NamedGroupType; import static sun.security.ssl.SupportedGroupsExtension.NamedGroupType.*; import sun.security.ssl.SupportedGroupsExtension.SupportedGroups; -import sun.security.ssl.PskKeyExchangeModesExtension.PskKeyExchangeMode; abstract class HandshakeContext implements ConnectionContext { // System properties diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/HandshakeOutStream.java --- a/src/java.base/share/classes/sun/security/ssl/HandshakeOutStream.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/HandshakeOutStream.java Tue Aug 14 18:16:47 2018 -0700 @@ -27,6 +27,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.ByteBuffer; /** * Output stream for handshake data. This is used only internally @@ -57,7 +58,14 @@ } if (outputRecord != null) { - outputRecord.encodeHandshake(buf, 0, count); + if (!outputRecord.isClosed()) { + outputRecord.encodeHandshake(buf, 0, count); + } else { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "handshake messages", ByteBuffer.wrap(buf, 0, count)); + } + } // reset the byte array output stream reset(); diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/KeyUpdate.java --- a/src/java.base/share/classes/sun/security/ssl/KeyUpdate.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/KeyUpdate.java Tue Aug 14 18:16:47 2018 -0700 @@ -223,8 +223,8 @@ Authenticator.valueOf(hc.conContext.protocolVersion), hc.conContext.protocolVersion, key, ivSpec, hc.sslContext.getSecureRandom()); + rc.baseSecret = nplus1; hc.conContext.inputRecord.changeReadCiphers(rc); - hc.conContext.inputRecord.readCipher.baseSecret = nplus1; if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { SSLLogger.fine("KeyUpdate: read key updated"); } @@ -303,13 +303,12 @@ return null; } - // Output the handshake message. - km.write(hc.handshakeOutput); - hc.handshakeOutput.flush(); - - // change write cipher - hc.conContext.outputRecord.changeWriteCiphers(wc, false); - hc.conContext.outputRecord.writeCipher.baseSecret = nplus1; + // Output the handshake message and change the write cipher. + // + // The KeyUpdate handshake message SHALL be delivered in the + // changeWriteCiphers() implementation. + wc.baseSecret = nplus1; + hc.conContext.outputRecord.changeWriteCiphers(wc, km.status.id); if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { SSLLogger.fine("KeyUpdate: write key updated"); } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/OutputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/OutputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/OutputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -66,7 +66,7 @@ int fragmentSize; // closed or not? - boolean isClosed; + volatile boolean isClosed; /* * Mappings from V3 cipher suite encodings to their pure V2 equivalents. @@ -76,6 +76,8 @@ {-1, -1, -1, 0x02, 0x01, -1, 0x04, 0x05, -1, 0x06, 0x07}; private static final int[] V3toV2CipherMap3 = {-1, -1, -1, 0x80, 0x80, -1, 0x80, 0x80, -1, 0x40, 0xC0}; + private static final byte[] HANDSHAKE_MESSAGE_KEY_UPDATE = + {SSLHandshake.KEY_UPDATE.id, 0x00, 0x00, 0x01, 0x00}; OutputRecord(HandshakeHash handshakeHash, SSLWriteCipher writeCipher) { this.writeCipher = writeCipher; @@ -87,7 +89,7 @@ // Please set packetSize and protocolVersion in the implementation. } - void setVersion(ProtocolVersion protocolVersion) { + synchronized void setVersion(ProtocolVersion protocolVersion) { this.protocolVersion = protocolVersion; } @@ -106,7 +108,7 @@ return false; } - boolean seqNumIsHuge() { + synchronized boolean seqNumIsHuge() { return (writeCipher.authenticator != null) && writeCipher.authenticator.seqNumIsHuge(); } @@ -145,8 +147,17 @@ throw new UnsupportedOperationException(); } - void changeWriteCiphers(SSLWriteCipher writeCipher, + // Change write ciphers, may use change_cipher_spec record. + synchronized void changeWriteCiphers(SSLWriteCipher writeCipher, boolean useChangeCipherSpec) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "change_cipher_spec message"); + } + return; + } + if (useChangeCipherSpec) { encodeChangeCipherSpec(); } @@ -165,15 +176,39 @@ this.isFirstAppOutputRecord = true; } - void changePacketSize(int packetSize) { + // Change write ciphers using key_update handshake message. + synchronized void changeWriteCiphers(SSLWriteCipher writeCipher, + byte keyUpdateRequest) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "key_update handshake message"); + } + return; + } + + // encode the handshake message, KeyUpdate + byte[] hm = HANDSHAKE_MESSAGE_KEY_UPDATE.clone(); + hm[hm.length - 1] = keyUpdateRequest; + encodeHandshake(hm, 0, hm.length); + flush(); + + // Dispose of any intermediate state in the underlying cipher. + writeCipher.dispose(); + + this.writeCipher = writeCipher; + this.isFirstAppOutputRecord = true; + } + + synchronized void changePacketSize(int packetSize) { this.packetSize = packetSize; } - void changeFragmentSize(int fragmentSize) { + synchronized void changeFragmentSize(int fragmentSize) { this.fragmentSize = fragmentSize; } - int getMaxPacketSize() { + synchronized int getMaxPacketSize() { return packetSize; } @@ -194,13 +229,15 @@ @Override public synchronized void close() throws IOException { - if (!isClosed) { - isClosed = true; - writeCipher.dispose(); + if (isClosed) { + return; } + + isClosed = true; + writeCipher.dispose(); } - synchronized boolean isClosed() { + boolean isClosed() { return isClosed; } @@ -241,7 +278,7 @@ } } - static long d13Encrypt( + private static long d13Encrypt( SSLWriteCipher encCipher, byte contentType, ByteBuffer destination, int headerOffset, int dstLim, int headerSize, ProtocolVersion protocolVersion) { @@ -282,7 +319,7 @@ return Authenticator.toLong(sequenceNumber); } - static long t13Encrypt( + private static long t13Encrypt( SSLWriteCipher encCipher, byte contentType, ByteBuffer destination, int headerOffset, int dstLim, int headerSize, ProtocolVersion protocolVersion) { @@ -321,7 +358,7 @@ return Authenticator.toLong(sequenceNumber); } - static long t10Encrypt( + private static long t10Encrypt( SSLWriteCipher encCipher, byte contentType, ByteBuffer destination, int headerOffset, int dstLim, int headerSize, ProtocolVersion protocolVersion) { @@ -362,7 +399,7 @@ private static final byte[] zeros = new byte[16]; } - long t13Encrypt( + private long t13Encrypt( SSLWriteCipher encCipher, byte contentType, int headerSize) { if (!encCipher.isNullCipher()) { // inner plaintext @@ -375,9 +412,10 @@ int contentLen = count - position; // ensure the capacity - int packetSize = encCipher.calculatePacketSize(contentLen, headerSize); - if (packetSize > buf.length) { - byte[] newBuf = new byte[packetSize]; + int requiredPacketSize = + encCipher.calculatePacketSize(contentLen, headerSize); + if (requiredPacketSize > buf.length) { + byte[] newBuf = new byte[requiredPacketSize]; System.arraycopy(buf, 0, newBuf, 0, count); buf = newBuf; } @@ -406,16 +444,17 @@ return Authenticator.toLong(sequenceNumber); } - long t10Encrypt( + private long t10Encrypt( SSLWriteCipher encCipher, byte contentType, int headerSize) { byte[] sequenceNumber = encCipher.authenticator.sequenceNumber(); int position = headerSize + writeCipher.getExplicitNonceSize(); int contentLen = count - position; // ensure the capacity - int packetSize = encCipher.calculatePacketSize(contentLen, headerSize); - if (packetSize > buf.length) { - byte[] newBuf = new byte[packetSize]; + int requiredPacketSize = + encCipher.calculatePacketSize(contentLen, headerSize); + if (requiredPacketSize > buf.length) { + byte[] newBuf = new byte[requiredPacketSize]; System.arraycopy(buf, 0, newBuf, 0, count); buf = newBuf; } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java --- a/src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java Tue Aug 14 18:16:47 2018 -0700 @@ -96,6 +96,10 @@ static final boolean useCompatibilityMode = Utilities.getBooleanProperty( "jdk.tls.client.useCompatibilityMode", true); + // Respond a close_notify alert if receiving close_notify alert. + static final boolean acknowledgeCloseNotify = Utilities.getBooleanProperty( + "jdk.tls.acknowledgeCloseNotify", false); + // TODO: Please remove after TLS 1.3 draft interop testing // delete me static int tls13VN; diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java --- a/src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java Tue Aug 14 18:16:47 2018 -0700 @@ -155,6 +155,7 @@ ByteBuffer[] srcs, int srcsOffset, int srcsLength, ByteBuffer[] dsts, int dstsOffset, int dstsLength) throws IOException { + // May need to deliver cached records. if (isOutboundDone()) { return new SSLEngineResult( Status.CLOSED, getHandshakeStatus(), 0, 0); @@ -162,8 +163,9 @@ HandshakeContext hc = conContext.handshakeContext; HandshakeStatus hsStatus = null; - if (!conContext.isNegotiated && - !conContext.isClosed() && !conContext.isBroken) { + if (!conContext.isNegotiated && !conContext.isBroken && + !conContext.isInboundClosed() && + !conContext.isOutboundClosed()) { conContext.kickstart(); hsStatus = getHandshakeStatus(); @@ -315,7 +317,8 @@ } // Is the sequence number is nearly overflow? - if (conContext.outputRecord.seqNumIsHuge()) { + if (conContext.outputRecord.seqNumIsHuge() || + conContext.outputRecord.writeCipher.atKeyLimit()) { hsStatus = tryKeyUpdate(hsStatus); } @@ -343,25 +346,29 @@ } /** - * Try renegotiation or key update for sequence number wrap. + * Try key update for sequence number wrap or key usage limit. * * Note that in order to maintain the handshake status properly, we check - * the sequence number after the last record reading/writing process. As - * we request renegotiation or close the connection for wrapped sequence + * the sequence number and key usage limit after the last record + * reading/writing process. + * + * As we request renegotiation or close the connection for wrapped sequence * number when there is enough sequence number space left to handle a few * more records, so the sequence number of the last record cannot be * wrapped. */ private HandshakeStatus tryKeyUpdate( HandshakeStatus currentHandshakeStatus) throws IOException { - // Don't bother to kickstart the renegotiation or key update when the - // local is asking for it. + // Don't bother to kickstart if handshaking is in progress, or if the + // connection is not duplex-open. if ((conContext.handshakeContext == null) && - !conContext.isClosed() && !conContext.isBroken) { + !conContext.isOutboundClosed() && + !conContext.isInboundClosed() && + !conContext.isBroken) { if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.finest("key update to wrap sequence number"); + SSLLogger.finest("trigger key update"); } - conContext.keyUpdate(); + beginHandshake(); return conContext.getHandshakeStatus(); } @@ -471,8 +478,9 @@ } HandshakeStatus hsStatus = null; - if (!conContext.isNegotiated && - !conContext.isClosed() && !conContext.isBroken) { + if (!conContext.isNegotiated && !conContext.isBroken && + !conContext.isInboundClosed() && + !conContext.isOutboundClosed()) { conContext.kickstart(); /* @@ -677,7 +685,8 @@ } // Is the sequence number is nearly overflow? - if (conContext.inputRecord.seqNumIsHuge()) { + if (conContext.inputRecord.seqNumIsHuge() || + conContext.inputRecord.readCipher.atKeyLimit()) { pt.handshakeStatus = tryKeyUpdate(pt.handshakeStatus); } @@ -700,16 +709,42 @@ @Override public synchronized void closeInbound() throws SSLException { + if (isInboundDone()) { + return; + } + + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.finest("Closing inbound of SSLEngine"); + } + + // Is it ready to close inbound? + // + // No need to throw exception if the initial handshake is not started. + if (!conContext.isInputCloseNotified && + (conContext.isNegotiated || conContext.handshakeContext != null)) { + + conContext.fatal(Alert.INTERNAL_ERROR, + "closing inbound before receiving peer's close_notify"); + } + conContext.closeInbound(); } @Override public synchronized boolean isInboundDone() { - return conContext.isInboundDone(); + return conContext.isInboundClosed(); } @Override public synchronized void closeOutbound() { + if (conContext.isOutboundClosed()) { + return; + } + + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.finest("Closing outbound of SSLEngine"); + } + conContext.closeOutbound(); } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLEngineInputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -34,8 +34,6 @@ import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLProtocolException; import sun.security.ssl.SSLCipher.SSLReadCipher; -import sun.security.ssl.KeyUpdate.KeyUpdateMessage; -import sun.security.ssl.KeyUpdate.KeyUpdateRequest; /** * {@code InputRecord} implementation for {@code SSLEngine}. @@ -300,7 +298,7 @@ handshakeBuffer.put(handshakeFrag); handshakeBuffer.rewind(); break; - } if (remaining == handshakeMessageLen) { + } else if (remaining == handshakeMessageLen) { if (handshakeHash.isHashable(handshakeType)) { handshakeHash.receive(handshakeFrag); } @@ -333,20 +331,6 @@ return plaintexts.toArray(new Plaintext[0]); } - // KeyLimit check during application data. - // atKeyLimit() inactive when limits not checked, tc set when limits - // are active. - - if (readCipher.atKeyLimit()) { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("KeyUpdate: triggered, read side."); - } - - PostHandshakeContext p = new PostHandshakeContext(tc); - KeyUpdate.handshakeProducer.produce(p, - new KeyUpdateMessage(p, KeyUpdateRequest.REQUESTED)); - } - return new Plaintext[] { new Plaintext(contentType, majorVersion, minorVersion, -1, -1L, fragment) diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLEngineOutputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/SSLEngineOutputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLEngineOutputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -31,8 +31,6 @@ import javax.net.ssl.SSLHandshakeException; import sun.security.ssl.SSLCipher.SSLWriteCipher; -import sun.security.ssl.KeyUpdate.KeyUpdateMessage; -import sun.security.ssl.KeyUpdate.KeyUpdateRequest; /** * {@code OutputRecord} implementation for {@code SSLEngine}. @@ -43,7 +41,7 @@ private boolean isTalkingToV2 = false; // SSLv2Hello private ByteBuffer v2ClientHello = null; // SSLv2Hello - private boolean isCloseWaiting = false; + private volatile boolean isCloseWaiting = false; SSLEngineOutputRecord(HandshakeHash handshakeHash) { super(handshakeHash, SSLWriteCipher.nullTlsWriteCipher()); @@ -63,8 +61,20 @@ } } + boolean isClosed() { + return isClosed || isCloseWaiting; + } + @Override void encodeAlert(byte level, byte description) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "alert message: " + Alert.nameOf(description)); + } + return; + } + if (fragmenter == null) { fragmenter = new HandshakeFragment(); } @@ -75,6 +85,14 @@ @Override void encodeHandshake(byte[] source, int offset, int length) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "handshake message", + ByteBuffer.wrap(source, offset, length)); + } + return; + } if (fragmenter == null) { fragmenter = new HandshakeFragment(); @@ -114,6 +132,14 @@ @Override void encodeChangeCipherSpec() throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "change_cipher_spec message"); + } + return; + } + if (fragmenter == null) { fragmenter = new HandshakeFragment(); } @@ -129,6 +155,23 @@ Ciphertext encode( ByteBuffer[] srcs, int srcsOffset, int srcsLength, ByteBuffer[] dsts, int dstsOffset, int dstsLength) throws IOException { + + if (isClosed) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "application data or cached messages"); + } + + return null; + } else if (isCloseWaiting) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "application data"); + } + + srcs = null; // use no application data. + } + return encode(srcs, srcsOffset, srcsLength, dsts[0]); } @@ -248,25 +291,14 @@ if (isFirstAppOutputRecord) { isFirstAppOutputRecord = false; } - - // atKeyLimit() inactive when limits not checked, tc set when limits - // are active. - if (writeCipher.atKeyLimit()) { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("KeyUpdate: triggered, write side."); - } - - PostHandshakeContext p = new PostHandshakeContext(tc); - KeyUpdate.handshakeProducer.produce(p, - new KeyUpdateMessage(p, KeyUpdateRequest.REQUESTED)); - } } return new Ciphertext(ContentType.APPLICATION_DATA.id, SSLHandshake.NOT_APPLICABLE.id, recordSN); } - private Ciphertext acquireCiphertext(ByteBuffer destination) throws IOException { + private Ciphertext acquireCiphertext( + ByteBuffer destination) throws IOException { if (isTalkingToV2) { // SSLv2Hello // We don't support SSLv2. Send an SSLv2 error message // so that the connection can be closed gracefully. @@ -517,7 +549,7 @@ boolean hasAlert() { for (RecordMemo memo : handshakeMemos) { - if (memo.contentType == ContentType.ALERT.id) { + if (memo.contentType == ContentType.ALERT.id) { return true; } } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLHandshake.java --- a/src/java.base/share/classes/sun/security/ssl/SSLHandshake.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLHandshake.java Tue Aug 14 18:16:47 2018 -0700 @@ -366,6 +366,7 @@ SSLHandshake(byte id, String name, Map.Entry[] handshakeConsumers, Map.Entry[] handshakeProducers) { + this(id, name, handshakeConsumers, handshakeProducers, (Map.Entry[])( new Map.Entry[0])); @@ -375,6 +376,7 @@ Map.Entry[] handshakeConsumers, Map.Entry[] handshakeProducers, Map.Entry[] handshakeAbsence) { + this.id = id; this.name = name; this.handshakeConsumers = handshakeConsumers; @@ -404,7 +406,12 @@ ProtocolVersion protocolVersion; if ((hc.negotiatedProtocol == null) || (hc.negotiatedProtocol == ProtocolVersion.NONE)) { - protocolVersion = hc.maximumActiveProtocol; + if (hc.conContext.isNegotiated && + hc.conContext.protocolVersion != ProtocolVersion.NONE) { + protocolVersion = hc.conContext.protocolVersion; + } else { + protocolVersion = hc.maximumActiveProtocol; + } } else { protocolVersion = hc.negotiatedProtocol; } @@ -444,7 +451,12 @@ ProtocolVersion protocolVersion; if ((hc.negotiatedProtocol == null) || (hc.negotiatedProtocol == ProtocolVersion.NONE)) { - protocolVersion = hc.maximumActiveProtocol; + if (hc.conContext.isNegotiated && + hc.conContext.protocolVersion != ProtocolVersion.NONE) { + protocolVersion = hc.conContext.protocolVersion; + } else { + protocolVersion = hc.maximumActiveProtocol; + } } else { protocolVersion = hc.negotiatedProtocol; } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java --- a/src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java Tue Aug 14 18:16:47 2018 -0700 @@ -82,7 +82,7 @@ private String peerHost; private boolean autoClose; private boolean isConnected = false; - private boolean tlsIsClosed = false; + private volatile boolean tlsIsClosed = false; /* * Is the local name service trustworthy? @@ -325,7 +325,7 @@ } @Override - public synchronized SSLSession getSession() { + public SSLSession getSession() { try { // start handshaking, if failed, the connection will be closed. ensureNegotiated(); @@ -343,7 +343,11 @@ @Override public synchronized SSLSession getHandshakeSession() { if (conContext.handshakeContext != null) { - return conContext.handshakeContext.handshakeSession; + synchronized (this) { + if (conContext.handshakeContext != null) { + return conContext.handshakeContext.handshakeSession; + } + } } return null; @@ -370,23 +374,39 @@ } @Override - public synchronized void startHandshake() throws IOException { - checkWrite(); - try { - conContext.kickstart(); + public void startHandshake() throws IOException { + if (!isConnected) { + throw new SocketException("Socket is not connected"); + } + + if (conContext.isBroken || conContext.isInboundClosed() || + conContext.isOutboundClosed()) { + throw new SocketException("Socket has been closed or broken"); + } + + synchronized (conContext) { // handshake lock + // double check the context status + if (conContext.isBroken || conContext.isInboundClosed() || + conContext.isOutboundClosed()) { + throw new SocketException("Socket has been closed or broken"); + } - // All initial handshaking goes through this operation until we - // have a valid SSL connection. - // - // Handle handshake messages only, need no application data. - if (!conContext.isNegotiated) { - readRecord(); + try { + conContext.kickstart(); + + // All initial handshaking goes through this operation until we + // have a valid SSL connection. + // + // Handle handshake messages only, need no application data. + if (!conContext.isNegotiated) { + readHandshakeRecord(); + } + } catch (IOException ioe) { + conContext.fatal(Alert.HANDSHAKE_FAILURE, + "Couldn't kickstart handshaking", ioe); + } catch (Exception oe) { // including RuntimeException + handleException(oe); } - } catch (IOException ioe) { - conContext.fatal(Alert.HANDSHAKE_FAILURE, - "Couldn't kickstart handshaking", ioe); - } catch (Exception oe) { // including RuntimeException - handleException(oe); } } @@ -437,44 +457,264 @@ } @Override - public synchronized boolean isClosed() { - return tlsIsClosed && conContext.isClosed(); + public boolean isClosed() { + return tlsIsClosed; } + // Please don't synchronized this method. Otherwise, the read and close + // locks may be deadlocked. @Override - public synchronized void close() throws IOException { + public void close() throws IOException { + if (tlsIsClosed) { + return; + } + + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("duplex close of SSLSocket"); + } + try { - conContext.close(); + // shutdown output bound, which may have been closed previously. + if (!isOutputShutdown()) { + duplexCloseOutput(); + } + + // shutdown input bound, which may have been closed previously. + if (!isInputShutdown()) { + duplexCloseInput(); + } + + if (!isClosed()) { + // close the connection directly + closeSocket(false); + } } catch (IOException ioe) { // ignore the exception if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.warning("connection context closure failed", ioe); + SSLLogger.warning("SSLSocket duplex close failed", ioe); } } finally { tlsIsClosed = true; } } + /** + * Duplex close, start from closing outbound. + * + * For TLS 1.2 [RFC 5246], unless some other fatal alert has been + * transmitted, each party is required to send a close_notify alert + * before closing the write side of the connection. The other party + * MUST respond with a close_notify alert of its own and close down + * the connection immediately, discarding any pending writes. It is + * not required for the initiator of the close to wait for the responding + * close_notify alert before closing the read side of the connection. + * + * For TLS 1.3, Each party MUST send a close_notify alert before + * closing its write side of the connection, unless it has already sent + * some error alert. This does not have any effect on its read side of + * the connection. Both parties need not wait to receive a close_notify + * alert before closing their read side of the connection, though doing + * so would introduce the possibility of truncation. + * + * In order to support user initiated duplex-close for TLS 1.3 connections, + * the user_canceled alert is used together with the close_notify alert. + */ + private void duplexCloseOutput() throws IOException { + boolean useUserCanceled = false; + boolean hasCloseReceipt = false; + if (conContext.isNegotiated) { + if (!conContext.protocolVersion.useTLS13PlusSpec()) { + hasCloseReceipt = true; + } else { + // Use a user_canceled alert for TLS 1.3 duplex close. + useUserCanceled = true; + } + } else if (conContext.handshakeContext != null) { // initial handshake + // Use user_canceled alert regardless the protocol versions. + useUserCanceled = true; + + // The protocol version may have been negotiated. + ProtocolVersion pv = conContext.handshakeContext.negotiatedProtocol; + if (pv == null || (!pv.useTLS13PlusSpec())) { + hasCloseReceipt = true; + } + } + + // Need a lock here so that the user_canceled alert and the + // close_notify alert can be delivered together. + try { + synchronized (conContext.outputRecord) { + // send a user_canceled alert if needed. + if (useUserCanceled) { + conContext.warning(Alert.USER_CANCELED); + } + + // send a close_notify alert + conContext.warning(Alert.CLOSE_NOTIFY); + } + } finally { + if (!conContext.isOutboundClosed()) { + conContext.outputRecord.close(); + } + + if ((autoClose || !isLayered()) && !super.isOutputShutdown()) { + super.shutdownOutput(); + } + } + + if (!isInputShutdown()) { + bruteForceCloseInput(hasCloseReceipt); + } + } + + /** + * Duplex close, start from closing inbound. + * + * This method should only be called when the outbound has been closed, + * but the inbound is still open. + */ + private void duplexCloseInput() throws IOException { + boolean hasCloseReceipt = false; + if (conContext.isNegotiated && + !conContext.protocolVersion.useTLS13PlusSpec()) { + hasCloseReceipt = true; + } // No close receipt if handshake has no completed. + + bruteForceCloseInput(hasCloseReceipt); + } + + /** + * Brute force close the input bound. + * + * This method should only be called when the outbound has been closed, + * but the inbound is still open. + */ + private void bruteForceCloseInput( + boolean hasCloseReceipt) throws IOException { + if (hasCloseReceipt) { + // It is not required for the initiator of the close to wait for + // the responding close_notify alert before closing the read side + // of the connection. However, if the application protocol using + // TLS provides that any data may be carried over the underlying + // transport after the TLS connection is closed, the TLS + // implementation MUST receive a "close_notify" alert before + // indicating end-of-data to the application-layer. + try { + this.shutdown(); + } finally { + if (!isInputShutdown()) { + shutdownInput(false); + } + } + } else { + if (!conContext.isInboundClosed()) { + conContext.inputRecord.close(); + } + + if ((autoClose || !isLayered()) && !super.isInputShutdown()) { + super.shutdownInput(); + } + } + } + + // Please don't synchronized this method. Otherwise, the read and close + // locks may be deadlocked. + @Override + public void shutdownInput() throws IOException { + shutdownInput(true); + } + + // It is not required to check the close_notify receipt unless an + // application call shutdownInput() explicitly. + private void shutdownInput( + boolean checkCloseNotify) throws IOException { + if (isInputShutdown()) { + return; + } + + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("close inbound of SSLSocket"); + } + + // Is it ready to close inbound? + // + // No need to throw exception if the initial handshake is not started. + if (checkCloseNotify && !conContext.isInputCloseNotified && + (conContext.isNegotiated || conContext.handshakeContext != null)) { + + conContext.fatal(Alert.INTERNAL_ERROR, + "closing inbound before receiving peer's close_notify"); + } + + conContext.closeInbound(); + if ((autoClose || !isLayered()) && !super.isInputShutdown()) { + super.shutdownInput(); + } + } + + @Override + public boolean isInputShutdown() { + return conContext.isInboundClosed() && + ((autoClose || !isLayered()) ? super.isInputShutdown(): true); + } + + // Please don't synchronized this method. Otherwise, the read and close + // locks may be deadlocked. + @Override + public void shutdownOutput() throws IOException { + if (isOutputShutdown()) { + return; + } + + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("close outbound of SSLSocket"); + } + conContext.closeOutbound(); + + if ((autoClose || !isLayered()) && !super.isOutputShutdown()) { + super.shutdownOutput(); + } + } + + @Override + public boolean isOutputShutdown() { + return conContext.isOutboundClosed() && + ((autoClose || !isLayered()) ? super.isOutputShutdown(): true); + } + @Override public synchronized InputStream getInputStream() throws IOException { - if (isClosed() || conContext.isInboundDone()) { - throw new SocketException("Socket or inbound is closed"); + if (isClosed()) { + throw new SocketException("Socket is closed"); } if (!isConnected) { throw new SocketException("Socket is not connected"); } + if (conContext.isInboundClosed() || isInputShutdown()) { + throw new SocketException("Socket input is already shutdown"); + } + return appInput; } - private synchronized void ensureNegotiated() throws IOException { - if (conContext.isNegotiated || - conContext.isClosed() || conContext.isBroken) { + private void ensureNegotiated() throws IOException { + if (conContext.isNegotiated || conContext.isBroken || + conContext.isInboundClosed() || conContext.isOutboundClosed()) { return; } - startHandshake(); + synchronized (conContext) { // handshake lock + // double check the context status + if (conContext.isNegotiated || conContext.isBroken || + conContext.isInboundClosed() || + conContext.isOutboundClosed()) { + return; + } + + startHandshake(); + } } /** @@ -489,7 +729,7 @@ private ByteBuffer buffer; // Is application data available in the stream? - private boolean appDataIsAvailable; + private volatile boolean appDataIsAvailable; AppInputStream() { this.appDataIsAvailable = false; @@ -514,7 +754,7 @@ * Read a single byte, returning -1 on non-fault EOF status. */ @Override - public synchronized int read() throws IOException { + public int read() throws IOException { int n = read(oneByte, 0, 1); if (n <= 0) { // EOF return -1; @@ -536,7 +776,7 @@ * and returning "-1" on non-fault EOF status. */ @Override - public synchronized int read(byte[] b, int off, int len) + public int read(byte[] b, int off, int len) throws IOException { if (b == null) { throw new NullPointerException("the target buffer is null"); @@ -553,75 +793,54 @@ } // start handshaking if the connection has not been negotiated. - if (!conContext.isNegotiated && - !conContext.isClosed() && !conContext.isBroken) { + if (!conContext.isNegotiated && !conContext.isBroken && + !conContext.isInboundClosed() && + !conContext.isOutboundClosed()) { ensureNegotiated(); } + // Check if the Socket is invalid (error or closed). + if (!conContext.isNegotiated || + conContext.isBroken || conContext.isInboundClosed()) { + throw new SocketException("Connection or inbound has closed"); + } + // Read the available bytes at first. - int remains = available(); - if (remains > 0) { - int howmany = Math.min(remains, len); - buffer.get(b, off, howmany); - - return howmany; - } + // + // Note that the receiving and processing of post-handshake message + // are also synchronized with the read lock. + synchronized (this) { + int remains = available(); + if (remains > 0) { + int howmany = Math.min(remains, len); + buffer.get(b, off, howmany); - appDataIsAvailable = false; - int volume = 0; - try { - /* - * Read data if needed ... notice that the connection - * guarantees that handshake, alert, and change cipher spec - * data streams are handled as they arrive, so we never - * see them here. - */ - while (volume == 0) { - // Clear the buffer for a new record reading. - buffer.clear(); + return howmany; + } - // grow the buffer if needed - int inLen = conContext.inputRecord.bytesInCompletePacket(); - if (inLen < 0) { // EOF - handleEOF(null); - - // if no exception thrown + appDataIsAvailable = false; + try { + ByteBuffer bb = readApplicationRecord(buffer); + if (bb == null) { // EOF return -1; + } else { + // The buffer may be reallocated for bigger capacity. + buffer = bb; } - // Is this packet bigger than SSL/TLS normally allows? - if (inLen > SSLRecord.maxLargeRecordSize) { - throw new SSLProtocolException( - "Illegal packet size: " + inLen); - } - - if (inLen > buffer.remaining()) { - buffer = ByteBuffer.allocate(inLen); - } + bb.flip(); + int volume = Math.min(len, bb.remaining()); + buffer.get(b, off, volume); + appDataIsAvailable = true; - volume = readRecord(buffer); - buffer.flip(); - if (volume < 0) { // EOF - // treat like receiving a close_notify warning message. - conContext.isInputCloseNotified = true; - conContext.closeInbound(); - return -1; - } else if (volume > 0) { - appDataIsAvailable = true; - break; - } + return volume; + } catch (Exception e) { // including RuntimeException + // shutdown and rethrow (wrapped) exception as appropriate + handleException(e); + + // dummy for compiler + return -1; } - - // file the destination buffer - int howmany = Math.min(len, volume); - buffer.get(b, off, howmany); - return howmany; - } catch (Exception e) { // including RuntimeException - // shutdown and rethrow (wrapped) exception as appropriate - handleException(e); - - // dummy for compiler - return -1; } } @@ -658,20 +877,53 @@ SSLLogger.finest("Closing input stream"); } - conContext.closeInbound(); + try { + shutdownInput(false); + } catch (IOException ioe) { + // ignore the exception + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("input stream close failed", ioe); + } + } + } + + /** + * Return whether we have reached end-of-file. + * + * If the socket is not connected, has been shutdown because of an error + * or has been closed, throw an Exception. + */ + private boolean checkEOF() throws IOException { + if (conContext.isInboundClosed()) { + return true; + } else if (conContext.isInputCloseNotified || conContext.isBroken) { + if (conContext.closeReason == null) { + return true; + } else { + throw new SSLException( + "Connection has closed: " + conContext.closeReason, + conContext.closeReason); + } + } + + return false; } } @Override public synchronized OutputStream getOutputStream() throws IOException { - if (isClosed() || conContext.isOutboundDone()) { - throw new SocketException("Socket or outbound is closed"); + if (isClosed()) { + throw new SocketException("Socket is closed"); } if (!isConnected) { throw new SocketException("Socket is not connected"); } + if (conContext.isOutboundDone() || isOutputShutdown()) { + throw new SocketException("Socket output is already shutdown"); + } + return appOutput; } @@ -691,7 +943,7 @@ } @Override - public synchronized void write(byte[] b, + public void write(byte[] b, int off, int len) throws IOException { if (b == null) { throw new NullPointerException("the source buffer is null"); @@ -700,25 +952,47 @@ "buffer length: " + b.length + ", offset; " + off + ", bytes to read:" + len); } else if (len == 0) { + // + // Don't bother to really write empty records. We went this + // far to drive the handshake machinery, for correctness; not + // writing empty records improves performance by cutting CPU + // time and network resource usage. However, some protocol + // implementations are fragile and don't like to see empty + // records, so this also increases robustness. + // return; } - // start handshaking if the connection has not been negotiated. - if (!conContext.isNegotiated && - !conContext.isClosed() && !conContext.isBroken) { + // Start handshaking if the connection has not been negotiated. + if (!conContext.isNegotiated && !conContext.isBroken && + !conContext.isInboundClosed() && + !conContext.isOutboundClosed()) { ensureNegotiated(); } - // check if the Socket is invalid (error or closed) - checkWrite(); + // Check if the Socket is invalid (error or closed). + if (!conContext.isNegotiated || + conContext.isBroken || conContext.isOutboundClosed()) { + throw new SocketException("Connection or outbound has closed"); + } + + // // Delegate the writing to the underlying socket. try { - writeRecord(b, off, len); - checkWrite(); - } catch (IOException ioe) { - // shutdown and rethrow (wrapped) exception as appropriate - handleException(ioe); + conContext.outputRecord.deliver(b, off, len); + } catch (SSLHandshakeException she) { + // may be record sequence number overflow + conContext.fatal(Alert.HANDSHAKE_FAILURE, she); + } catch (IOException e) { + conContext.fatal(Alert.UNEXPECTED_MESSAGE, e); + } + + // Is the sequence number is nearly overflow, or has the key usage + // limit been reached? + if (conContext.outputRecord.seqNumIsHuge() || + conContext.outputRecord.writeCipher.atKeyLimit()) { + tryKeyUpdate(); } } @@ -728,7 +1002,14 @@ SSLLogger.finest("Closing output stream"); } - conContext.closeOutbound(); + try { + shutdownOutput(); + } catch (IOException ioe) { + // ignore the exception + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("output stream close failed", ioe); + } + } } } @@ -773,39 +1054,11 @@ return conContext.sslConfig.socketAPSelector; } - private synchronized void writeRecord(byte[] source, - int offset, int length) throws IOException { - if (conContext.isOutboundDone()) { - throw new SocketException("Socket or outbound closed"); - } - - // - // Don't bother to really write empty records. We went this - // far to drive the handshake machinery, for correctness; not - // writing empty records improves performance by cutting CPU - // time and network resource usage. However, some protocol - // implementations are fragile and don't like to see empty - // records, so this also increases robustness. - // - if (length > 0) { - try { - conContext.outputRecord.deliver(source, offset, length); - } catch (SSLHandshakeException she) { - // may be record sequence number overflow - conContext.fatal(Alert.HANDSHAKE_FAILURE, she); - } catch (IOException e) { - conContext.fatal(Alert.UNEXPECTED_MESSAGE, e); - } - } - - // Is the sequence number is nearly overflow? - if (conContext.outputRecord.seqNumIsHuge()) { - tryKeyUpdate(); - } - } - - private synchronized int readRecord() throws IOException { - while (!conContext.isInboundDone()) { + /** + * Read the initial handshake records. + */ + private int readHandshakeRecord() throws IOException { + while (!conContext.isInboundClosed()) { try { Plaintext plainText = decode(null); if ((plainText.contentType == ContentType.HANDSHAKE.id) && @@ -816,7 +1069,7 @@ throw ssle; } catch (IOException ioe) { if (!(ioe instanceof SSLException)) { - throw new SSLException("readRecord", ioe); + throw new SSLException("readHandshakeRecord", ioe); } else { throw ioe; } @@ -826,8 +1079,20 @@ return -1; } - private synchronized int readRecord(ByteBuffer buffer) throws IOException { - while (!conContext.isInboundDone()) { + /** + * Read application data record. Used by AppInputStream only, but defined + * here so as to use the socket level synchronization. + * + * Note that the connection guarantees that handshake, alert, and change + * cipher spec data streams are handled as they arrive, so we never see + * them here. + * + * Note: Please be careful about the synchronization, and don't use this + * method other than in the AppInputStream class! + */ + private ByteBuffer readApplicationRecord( + ByteBuffer buffer) throws IOException { + while (!conContext.isInboundClosed()) { /* * clean the buffer and check if it is too small, e.g. because * the AppInputStream did not have the chance to see the @@ -841,23 +1106,33 @@ handleEOF(null); // if no exception thrown - return -1; + return null; } - if (buffer.remaining() < inLen) { - return 0; + // Is this packet bigger than SSL/TLS normally allows? + if (inLen > SSLRecord.maxLargeRecordSize) { + throw new SSLProtocolException( + "Illegal packet size: " + inLen); + } + + if (inLen > buffer.remaining()) { + buffer = ByteBuffer.allocate(inLen); } try { - Plaintext plainText = decode(buffer); - if (plainText.contentType == ContentType.APPLICATION_DATA.id) { - return buffer.position(); + Plaintext plainText; + synchronized (this) { + plainText = decode(buffer); + } + if (plainText.contentType == ContentType.APPLICATION_DATA.id && + buffer.position() > 0) { + return buffer; } } catch (SSLException ssle) { throw ssle; } catch (IOException ioe) { if (!(ioe instanceof SSLException)) { - throw new SSLException("readRecord", ioe); + throw new SSLException("readApplicationRecord", ioe); } else { throw ioe; } @@ -867,7 +1142,7 @@ // // couldn't read, due to some kind of error // - return -1; + return null; } private Plaintext decode(ByteBuffer destination) throws IOException { @@ -887,7 +1162,8 @@ // Is the sequence number is nearly overflow? if (plainText != Plaintext.PLAINTEXT_NULL && - conContext.inputRecord.seqNumIsHuge()) { + (conContext.inputRecord.seqNumIsHuge() || + conContext.inputRecord.readCipher.atKeyLimit())) { tryKeyUpdate(); } @@ -895,69 +1171,28 @@ } /** - * Try renegotiation or key update for sequence number wrap. + * Try key update for sequence number wrap or key usage limit. * * Note that in order to maintain the handshake status properly, we check - * the sequence number after the last record reading/writing process. As - * we request renegotiation or close the connection for wrapped sequence + * the sequence number and key usage limit after the last record + * reading/writing process. + * + * As we request renegotiation or close the connection for wrapped sequence * number when there is enough sequence number space left to handle a few * more records, so the sequence number of the last record cannot be * wrapped. */ private void tryKeyUpdate() throws IOException { - // Don't bother to kickstart the renegotiation or key update when the - // local is asking for it. + // Don't bother to kickstart if handshaking is in progress, or if the + // connection is not duplex-open. if ((conContext.handshakeContext == null) && - !conContext.isClosed() && !conContext.isBroken) { + !conContext.isOutboundClosed() && + !conContext.isInboundClosed() && + !conContext.isBroken) { if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.finest("key update to wrap sequence number"); + SSLLogger.finest("trigger key update"); } - conContext.keyUpdate(); - } - } - - private void closeSocket(boolean selfInitiated) throws IOException { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("close the ssl connection " + - (selfInitiated ? "(initiative)" : "(passive)")); - } - - if (autoClose || !isLayered()) { - super.close(); - } else if (selfInitiated) { - // wait for close_notify alert to clear input stream. - waitForClose(); - } - } - - /** - * Wait for close_notify alert for a graceful closure. - * - * [RFC 5246] If the application protocol using TLS provides that any - * data may be carried over the underlying transport after the TLS - * connection is closed, the TLS implementation must receive the responding - * close_notify alert before indicating to the application layer that - * the TLS connection has ended. If the application protocol will not - * transfer any additional data, but will only close the underlying - * transport connection, then the implementation MAY choose to close the - * transport without waiting for the responding close_notify. - */ - private void waitForClose() throws IOException { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("wait for close_notify or alert"); - } - - while (!conContext.isInboundDone()) { - try { - Plaintext plainText = decode(null); - // discard and continue - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.finest( - "discard plaintext while waiting for close", plainText); - } - } catch (Exception e) { // including RuntimeException - handleException(e); - } + startHandshake(); } } @@ -1041,41 +1276,6 @@ } /** - * Return whether we have reached end-of-file. - * - * If the socket is not connected, has been shutdown because of an error - * or has been closed, throw an Exception. - */ - synchronized boolean checkEOF() throws IOException { - if (conContext.isClosed()) { - return true; - } else if (conContext.isInputCloseNotified || conContext.isBroken) { - if (conContext.closeReason == null) { - return true; - } else { - throw new SSLException( - "Connection has been shutdown: " + conContext.closeReason, - conContext.closeReason); - } - } - - return false; - } - - /** - * Check if we can write data to this socket. - */ - synchronized void checkWrite() throws IOException { - if (checkEOF() || conContext.isOutboundClosed()) { - // we are at EOF, write must throw Exception - throw new SocketException("Connection closed"); - } - if (!isConnected) { - throw new SocketException("Socket is not connected"); - } - } - - /** * Handle an exception. * * This method is called by top level exception handlers (in read(), @@ -1132,7 +1332,7 @@ } else { // treat as if we had received a close_notify conContext.isInputCloseNotified = true; - conContext.transport.shutdown(); + shutdownInput(); return Plaintext.PLAINTEXT_NULL; } @@ -1174,4 +1374,51 @@ } } } + + private void closeSocket(boolean selfInitiated) throws IOException { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("close the SSL connection " + + (selfInitiated ? "(initiative)" : "(passive)")); + } + + if (autoClose || !isLayered()) { + super.close(); + } else if (selfInitiated) { + if (!conContext.isInboundClosed() && !isInputShutdown()) { + // wait for close_notify alert to clear input stream. + waitForClose(); + } + } + } + + /** + * Wait for close_notify alert for a graceful closure. + * + * [RFC 5246] If the application protocol using TLS provides that any + * data may be carried over the underlying transport after the TLS + * connection is closed, the TLS implementation must receive the responding + * close_notify alert before indicating to the application layer that + * the TLS connection has ended. If the application protocol will not + * transfer any additional data, but will only close the underlying + * transport connection, then the implementation MAY choose to close the + * transport without waiting for the responding close_notify. + */ + private void waitForClose() throws IOException { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.fine("wait for close_notify or alert"); + } + + while (!conContext.isInboundClosed()) { + try { + Plaintext plainText = decode(null); + // discard and continue + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.finest( + "discard plaintext while waiting for close", plainText); + } + } catch (Exception e) { // including RuntimeException + handleException(e); + } + } + } } diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLSocketInputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -38,8 +38,6 @@ import javax.net.ssl.SSLProtocolException; import sun.security.ssl.SSLCipher.SSLReadCipher; -import sun.security.ssl.KeyUpdate.KeyUpdateMessage; -import sun.security.ssl.KeyUpdate.KeyUpdateRequest; /** * {@code InputRecord} implementation for {@code SSLSocket}. @@ -348,20 +346,6 @@ return plaintexts.toArray(new Plaintext[0]); } - // KeyLimit check during application data. - // atKeyLimit() inactive when limits not checked, tc set when limits - // are active. - - if (readCipher.atKeyLimit()) { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("KeyUpdate: triggered, read side."); - } - - PostHandshakeContext p = new PostHandshakeContext(tc); - KeyUpdate.handshakeProducer.produce(p, - new KeyUpdateMessage(p, KeyUpdateRequest.REQUESTED)); - } - return new Plaintext[] { new Plaintext(contentType, majorVersion, minorVersion, -1, -1L, fragment) diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/SSLSocketOutputRecord.java --- a/src/java.base/share/classes/sun/security/ssl/SSLSocketOutputRecord.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/SSLSocketOutputRecord.java Tue Aug 14 18:16:47 2018 -0700 @@ -28,12 +28,10 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.OutputStream; +import java.net.SocketException; import java.nio.ByteBuffer; import javax.net.ssl.SSLHandshakeException; -import sun.security.ssl.KeyUpdate.KeyUpdateMessage; -import sun.security.ssl.KeyUpdate.KeyUpdateRequest; - /** * {@code OutputRecord} implementation for {@code SSLSocket}. */ @@ -53,7 +51,16 @@ } @Override - void encodeAlert(byte level, byte description) throws IOException { + synchronized void encodeAlert( + byte level, byte description) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "alert message: " + Alert.nameOf(description)); + } + return; + } + // use the buf of ByteArrayOutputStream int position = headerSize + writeCipher.getExplicitNonceSize(); count = position; @@ -63,6 +70,7 @@ if (SSLLogger.isOn && SSLLogger.isOn("record")) { SSLLogger.fine("WRITE: " + protocolVersion + " " + ContentType.ALERT.name + + "(" + Alert.nameOf(description) + ")" + ", length = " + (count - headerSize)); } @@ -83,8 +91,17 @@ } @Override - void encodeHandshake(byte[] source, + synchronized void encodeHandshake(byte[] source, int offset, int length) throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "handshake message", + ByteBuffer.wrap(source, offset, length)); + } + return; + } + if (firstMessage) { firstMessage = false; @@ -182,7 +199,14 @@ } @Override - void encodeChangeCipherSpec() throws IOException { + synchronized void encodeChangeCipherSpec() throws IOException { + if (isClosed()) { + if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { + SSLLogger.warning("outbound has closed, ignore outbound " + + "change_cipher_spec message"); + } + return; + } // use the buf of ByteArrayOutputStream int position = headerSize + writeCipher.getExplicitNonceSize(); @@ -207,7 +231,7 @@ } @Override - public void flush() throws IOException { + public synchronized void flush() throws IOException { int position = headerSize + writeCipher.getExplicitNonceSize(); if (count <= position) { return; @@ -237,7 +261,12 @@ } @Override - void deliver(byte[] source, int offset, int length) throws IOException { + synchronized void deliver( + byte[] source, int offset, int length) throws IOException { + if (isClosed()) { + throw new SocketException("Connection or outbound has been closed"); + } + if (writeCipher.authenticator.seqNumOverflow()) { if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { SSLLogger.fine( @@ -304,23 +333,11 @@ } offset += fragLen; - - // atKeyLimit() inactive when limits not checked, tc set when limits - // are active. - if (writeCipher.atKeyLimit()) { - if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { - SSLLogger.fine("KeyUpdate: triggered, write side."); - } - - PostHandshakeContext p = new PostHandshakeContext(tc); - KeyUpdate.handshakeProducer.produce(p, - new KeyUpdateMessage(p, KeyUpdateRequest.REQUESTED)); - } } } @Override - void setDeliverStream(OutputStream outputStream) { + synchronized void setDeliverStream(OutputStream outputStream) { this.deliverStream = outputStream; } @@ -347,7 +364,7 @@ * This avoids issues in the outbound direction. For a full fix, * the peer must have similar protections. */ - boolean needToSplitPayload() { + private boolean needToSplitPayload() { return (!protocolVersion.useTLS11PlusSpec()) && writeCipher.isCBCMode() && !isFirstAppOutputRecord && Record.enableCBCProtection; diff -r f4b4dfac45b1 -r 910f7b56592f src/java.base/share/classes/sun/security/ssl/TransportContext.java --- a/src/java.base/share/classes/sun/security/ssl/TransportContext.java Tue Aug 14 19:52:34 2018 -0400 +++ b/src/java.base/share/classes/sun/security/ssl/TransportContext.java Tue Aug 14 18:16:47 2018 -0700 @@ -25,7 +25,6 @@ package sun.security.ssl; -import java.io.Closeable; import java.io.IOException; import java.security.AccessControlContext; import java.security.AccessController; @@ -45,7 +44,7 @@ /** * SSL/(D)TLS transportation context. */ -class TransportContext implements ConnectionContext, Closeable { +class TransportContext implements ConnectionContext { final SSLTransport transport; // registered plaintext consumers @@ -62,7 +61,7 @@ boolean isNegotiated = false; boolean isBroken = false; boolean isInputCloseNotified = false; - boolean isOutputCloseNotified = false; + boolean peerUserCanceled = false; Exception closeReason = null; // negotiated security parameters @@ -229,10 +228,6 @@ } } - void keyUpdate() throws IOException { - kickstart(); - } - boolean isPostHandshakeContext() { return handshakeContext != null && (handshakeContext instanceof PostHandshakeContext); @@ -348,7 +343,7 @@ // // If we haven't even started handshaking yet, or we are the recipient // of a fatal alert, no need to generate a fatal close alert. - if (!recvFatalAlert && !isOutboundDone() && !isBroken && + if (!recvFatalAlert && !isOutboundClosed() && !isBroken && (isNegotiated || handshakeContext != null)) { try { outputRecord.encodeAlert(Alert.Level.FATAL.level, alert.id); @@ -436,35 +431,26 @@ return outputRecord.isClosed(); } - boolean isInboundDone() { + boolean isInboundClosed() { return inputRecord.isClosed(); } - boolean isClosed() { - return isOutboundClosed() && isInboundDone(); - } - - @Override - public void close() throws IOException { - if (!isOutboundDone()) { - closeOutbound(); - } - - if (!isInboundDone()) { - closeInbound(); - } - } - - void closeInbound() { - if (isInboundDone()) { + // Close inbound, no more data should be delivered to the underlying + // transportation connection. + void closeInbound() throws SSLException { + if (isInboundClosed()) { return; } try { - if (isInputCloseNotified) { // passive close + // Important note: check if the initial handshake is started at + // first so that the passiveInboundClose() implementation need not + // to consider the case any more. + if (!isInputCloseNotified) { + // the initial handshake is not started + initiateInboundClose(); + } else { passiveInboundClose(); - } else { // initiative close - initiateInboundClose(); } } catch (IOException ioe) { if (SSLLogger.isOn && SSLLogger.isOn("ssl")) { @@ -473,8 +459,58 @@ } } + // Close the connection passively. The closure could be kickoff by + // receiving a close_notify alert or reaching end_of_file of the socket. + // + // Note that this method is called only if the initial handshake has + // started or completed. + private void passiveInboundClose() throws IOException { + if (!isInboundClosed()) { + inputRecord.close(); + } + + // For TLS 1.2 and prior version, it is required to respond with + // a close_notify alert of its own and close down the connection + // immediately, discarding any pending writes. + if (!isOutboundClosed()) { + boolean needCloseNotify = SSLConfiguration.acknowledgeCloseNotify; + if (!needCloseNotify) { + if (isNegotiated) { + if (!protocolVersion.useTLS13PlusSpec()) { + needCloseNotify = true; + } + } else if (handshakeContext != null) { // initial handshake + ProtocolVersion pv = handshakeContext.negotiatedProtocol; + if (pv == null || (!pv.useTLS13PlusSpec())) { + needCloseNotify = true; + } + } + } + + if (needCloseNotify) { + synchronized (outputRecord) { + try { + // send a close_notify alert + warning(Alert.CLOSE_NOTIFY); + } finally { + outputRecord.close(); + } + } + } + } + } + + // Initiate a inbound close when the handshake is not started. + private void initiateInboundClose() throws IOException { + if (!isInboundClosed()) { + inputRecord.close(); + } + } + + // Close outbound, no more data should be received from the underlying + // transportation connection. void closeOutbound() { - if (isOutboundDone()) { + if (isOutboundClosed()) { return; } @@ -487,116 +523,59 @@ } } - // Close the connection passively. The closure could be kickoff by - // receiving a close_notify alert or reaching end_of_file of the socket. - private void passiveInboundClose() throws IOException { - if (!isInboundDone()) { - inputRecord.close(); + // Initiate a close by sending a close_notify alert. + private void initiateOutboundClose() throws IOException { + boolean useUserCanceled = false; + if (!isNegotiated && (handshakeContext != null) && !peerUserCanceled) { + // initial handshake + useUserCanceled = true; } - // For TLS 1.2 and prior version, it is required to respond with - // a close_notify alert of its own and close down the connection - // immediately, discarding any pending writes. - if (!isOutboundDone() && !isOutputCloseNotified) { + // Need a lock here so that the user_canceled alert and the + // close_notify alert can be delivered together. + synchronized (outputRecord) { try { - // send a close_notify alert - warning(Alert.CLOSE_NOTIFY); - } finally { - // any data received after a closure alert is ignored. - isOutputCloseNotified = true; - outputRecord.close(); - } - } + // send a user_canceled alert if needed. + if (useUserCanceled) { + warning(Alert.USER_CANCELED); + } - transport.shutdown(); - } - - // Initiate a close by sending a close_notify alert. - private void initiateInboundClose() throws IOException { - // TLS 1.3 does not define how to initiate and close a TLS connection - // gracefully. We will always send a close_notify alert, and close - // the underlying transportation layer if needed. - if (!isInboundDone() && !isInputCloseNotified) { - try { // send a close_notify alert warning(Alert.CLOSE_NOTIFY); } finally { - // any data received after a closure alert is ignored. - isInputCloseNotified = true; - inputRecord.close(); - } - } - - // For TLS 1.3, input closure is independent from output closure. Both - // parties need not wait to receive a "close_notify" alert before - // closing their read side of the connection. - // - // For TLS 1.2 and prior version, it is not required for the initiator - // of the close to wait for the responding close_notify alert before - // closing the read side of the connection. - try { - transport.shutdown(); - } finally { - if (!isOutboundDone()) { outputRecord.close(); } } } - // Initiate a close by sending a close_notify alert. - private void initiateOutboundClose() throws IOException { - if (!isOutboundDone() && !isOutputCloseNotified) { - try { // close outputRecord - // send a close_notify alert - warning(Alert.CLOSE_NOTIFY); - } finally { - // any data received after a closure alert is ignored. - isOutputCloseNotified = true; - outputRecord.close(); - } - } - - // It is not required for the initiator of the close to wait for the - // responding close_notify alert before closing the read side of the - // connection. However, if the application protocol using TLS - // provides that any data may be carried over the underlying transport - // after the TLS connection is closed, the TLS implementation MUST - // receive a "close_notify" alert before indicating end-of-data to the - // application-layer. - try { - transport.shutdown(); - } finally { - if (!isInboundDone()) { - inputRecord.close(); - } - } - } - // Note; HandshakeStatus.FINISHED status is retrieved in other places. HandshakeStatus getHandshakeStatus() { if (!outputRecord.isEmpty()) { // If no handshaking, special case to wrap alters or // post-handshake messages. return HandshakeStatus.NEED_WRAP; + } else if (isOutboundClosed() && isInboundClosed()) { + return HandshakeStatus.NOT_HANDSHAKING; } else if (handshakeContext != null) { if (!handshakeContext.delegatedActions.isEmpty()) { return HandshakeStatus.NEED_TASK; - } else if (sslContext.isDTLS() && - !inputRecord.isEmpty()) { - return HandshakeStatus.NEED_UNWRAP_AGAIN; - } else { - return HandshakeStatus.NEED_UNWRAP; + } else if (!isInboundClosed()) { + if (sslContext.isDTLS() && + !inputRecord.isEmpty()) { + return HandshakeStatus.NEED_UNWRAP_AGAIN; + } else { + return HandshakeStatus.NEED_UNWRAP; + } + } else if (!isOutboundClosed()) { + // Special case that the inbound was closed, but outbound open. + return HandshakeStatus.NEED_WRAP; } - } else if (isOutboundDone() && !isInboundDone()) { - /* - * Special case where we're closing, but - * still need the close_notify before we - * can officially be closed. - * - * Note isOutboundDone is taken care of by - * hasOutboundData() above. - */ + } else if (isOutboundClosed() && !isInboundClosed()) { + // Special case that the outbound was closed, but inbound open. return HandshakeStatus.NEED_UNWRAP; + } else if (!isOutboundClosed() && isInboundClosed()) { + // Special case that the inbound was closed, but outbound open. + return HandshakeStatus.NEED_WRAP; } return HandshakeStatus.NOT_HANDSHAKING; @@ -607,8 +586,10 @@ outputRecord.tc = this; inputRecord.tc = this; cipherSuite = handshakeContext.negotiatedCipherSuite; - inputRecord.readCipher.baseSecret = handshakeContext.baseReadSecret; - outputRecord.writeCipher.baseSecret = handshakeContext.baseWriteSecret; + inputRecord.readCipher.baseSecret = + handshakeContext.baseReadSecret; + outputRecord.writeCipher.baseSecret = + handshakeContext.baseWriteSecret; } handshakeContext = null; diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/java/net/httpclient/CookieHeaderTest.java --- a/test/jdk/java/net/httpclient/CookieHeaderTest.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/java/net/httpclient/CookieHeaderTest.java Tue Aug 14 18:16:47 2018 -0700 @@ -35,6 +35,7 @@ * @build Http2TestServer * @build jdk.testlibrary.SimpleSSLContext * @run testng/othervm + * -Djdk.tls.acknowledgeCloseNotify=true * -Djdk.httpclient.HttpClient.log=trace,headers,requests * CookieHeaderTest */ diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/java/net/httpclient/EncodedCharsInURI.java --- a/test/jdk/java/net/httpclient/EncodedCharsInURI.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/java/net/httpclient/EncodedCharsInURI.java Tue Aug 14 18:16:47 2018 -0700 @@ -33,6 +33,7 @@ * java.net.http/jdk.internal.net.http.frame * java.net.http/jdk.internal.net.http.hpack * @run testng/othervm + * -Djdk.tls.acknowledgeCloseNotify=true * -Djdk.internal.httpclient.debug=true * -Djdk.httpclient.HttpClient.log=headers,errors EncodedCharsInURI */ diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/java/net/httpclient/ServerCloseTest.java --- a/test/jdk/java/net/httpclient/ServerCloseTest.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/java/net/httpclient/ServerCloseTest.java Tue Aug 14 18:16:47 2018 -0700 @@ -31,7 +31,7 @@ * java.net.http/jdk.internal.net.http.common * java.net.http/jdk.internal.net.http.frame * java.net.http/jdk.internal.net.http.hpack - * @run testng/othervm ServerCloseTest + * @run testng/othervm -Djdk.tls.acknowledgeCloseNotify=true ServerCloseTest */ //* -Djdk.internal.httpclient.debug=true diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/javax/net/ssl/ALPN/SSLEngineAlpnTest.java --- a/test/jdk/javax/net/ssl/ALPN/SSLEngineAlpnTest.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/javax/net/ssl/ALPN/SSLEngineAlpnTest.java Tue Aug 14 18:16:47 2018 -0700 @@ -191,6 +191,7 @@ if (debug) { System.setProperty("javax.net.debug", "all"); } + System.setProperty("jdk.tls.acknowledgeCloseNotify", "true"); System.out.println("Test args: " + Arrays.toString(args)); // Validate parameters @@ -358,6 +359,7 @@ log("\tClosing clientEngine's *OUTBOUND*..."); clientEngine.closeOutbound(); + // serverEngine.closeOutbound(); dataDone = true; } } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/javax/net/ssl/SSLEngine/Arrays.java --- a/test/jdk/javax/net/ssl/SSLEngine/Arrays.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/javax/net/ssl/SSLEngine/Arrays.java Tue Aug 14 18:16:47 2018 -0700 @@ -25,7 +25,14 @@ * @test * @bug 5019096 * @summary Add scatter/gather APIs for SSLEngine - * + * @run main/othervm Arrays SSL + * @run main/othervm Arrays TLS + * @run main/othervm Arrays SSLv3 + * @run main/othervm Arrays TLSv1 + * @run main/othervm Arrays TLSv1.1 + * @run main/othervm Arrays TLSv1.2 + * @run main/othervm Arrays TLSv1.3 + * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true Arrays TLSv1.3 */ import javax.net.ssl.*; @@ -37,6 +44,8 @@ public class Arrays { private static boolean debug = false; + private static boolean acknowledgeCloseNotify = + "true".equals(System.getProperty("jdk.tls.acknowledgeCloseNotify")); private SSLContext sslc; private SSLEngine ssle1; // client @@ -131,11 +140,13 @@ for (int i = 0; i < appOutArray1.length; i++) { if (appOutArray1[i].remaining() != 0) { + log("1st out not done"); done = false; } } if (appOut2.remaining() != 0) { + log("2nd out not done"); done = false; } @@ -145,6 +156,19 @@ appOutArray1[i].rewind(); } ssle1.closeOutbound(); + String protocol = ssle2.getSession().getProtocol(); + if (!acknowledgeCloseNotify) { + switch (ssle2.getSession().getProtocol()) { + case "SSLv3": + case "TLSv1": + case "TLSv1.1": + case "TLSv1.2": + break; + default: // TLSv1.3 + // TLS 1.3, half-close only. + ssle2.closeOutbound(); + } + } dataDone = true; } } @@ -155,7 +179,9 @@ checkTransfer(appInArray1, appOut2); } + private static String contextVersion; public static void main(String args[]) throws Exception { + contextVersion = args[0]; Arrays test; @@ -165,7 +191,7 @@ test.runTest(); - System.out.println("Test Passed."); + System.err.println("Test Passed."); } /* @@ -198,7 +224,7 @@ TrustManagerFactory tmf = TrustManagerFactory.getInstance("SunX509"); tmf.init(ts); - SSLContext sslCtx = SSLContext.getInstance("TLS"); + SSLContext sslCtx = SSLContext.getInstance(contextVersion); sslCtx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); @@ -288,7 +314,7 @@ private static void log(String str) { if (debug) { - System.out.println(str); + System.err.println(str); } } } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/javax/net/ssl/SSLEngine/ExtendedKeyEngine.java --- a/test/jdk/javax/net/ssl/SSLEngine/ExtendedKeyEngine.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/javax/net/ssl/SSLEngine/ExtendedKeyEngine.java Tue Aug 14 18:16:47 2018 -0700 @@ -26,6 +26,8 @@ * @bug 4981697 * @summary Rework the X509KeyManager to avoid incompatibility issues * @author Brad R. Wetmore + * + * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true ExtendedKeyEngine */ import javax.net.ssl.*; diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/javax/net/ssl/TLSCommon/SSLEngineTestCase.java --- a/test/jdk/javax/net/ssl/TLSCommon/SSLEngineTestCase.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/javax/net/ssl/TLSCommon/SSLEngineTestCase.java Tue Aug 14 18:16:47 2018 -0700 @@ -936,6 +936,7 @@ case SUPPORTED_NON_KRB_NON_SHA_CIPHERS: case SUPPORTED_KRB_CIPHERS: case ENABLED_NON_KRB_NOT_ANON_CIPHERS: + case TLS13_CIPHERS: if (error != null) { System.out.println("Test Failed: " + cs); System.err.println("Test Exception for " + cs); diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/javax/net/ssl/TLSv12/TLSEnginesClosureTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/javax/net/ssl/TLSv12/TLSEnginesClosureTest.java Tue Aug 14 18:16:47 2018 -0700 @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8207009 + * @summary Testing TLS engines closing using each of the supported + * cipher suites. + * @library /sun/security/krb5/auto /javax/net/ssl/TLSCommon + * @modules java.security.jgss + * jdk.security.auth + * java.security.jgss/sun.security.jgss.krb5 + * java.security.jgss/sun.security.krb5:+open + * java.security.jgss/sun.security.krb5.internal:+open + * java.security.jgss/sun.security.krb5.internal.ccache + * java.security.jgss/sun.security.krb5.internal.crypto + * java.security.jgss/sun.security.krb5.internal.ktab + * java.base/sun.security.util + * @run main/othervm -Dtest.security.protocol=TLSv1.2 + * -Dtest.mode=norm TLSEnginesClosureTest + * @run main/othervm -Dtest.security.protocol=TLSv1.2 + * -Dtest.mode=norm_sni TLSEnginesClosureTest + * @run main/othervm -Dtest.security.protocol=TLSv1.2 + * -Dtest.mode=krb TLSEnginesClosureTest + */ + +/** + * Testing TLS engines closing using each of the supported cipher suites. + */ +public class TLSEnginesClosureTest { + public static void main(String[] args) { + EnginesClosureTest.main(args); + } +} diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/AppInputStream/ReadBlocksClose.java --- a/test/jdk/sun/security/ssl/AppInputStream/ReadBlocksClose.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/AppInputStream/ReadBlocksClose.java Tue Aug 14 18:16:47 2018 -0700 @@ -21,14 +21,16 @@ * questions. */ +// +// SunJSSE does not support dynamic system properties, no way to re-use +// system properties in samevm/agentvm mode. +// + /* * @test * @bug 4814140 * @summary AppInputStream: read can block a close - * @run main/othervm ReadBlocksClose - * - * SunJSSE does not support dynamic system properties, no way to re-use - * system properties in samevm/agentvm mode. + * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true ReadBlocksClose * @author Brad Wetmore */ @@ -141,7 +143,8 @@ System.out.println("Closing Thread started"); Thread.sleep(3000); System.out.println("Closing Thread closing"); - sslIS.close(); + sslOS.close(); + System.out.println("Closing Thread closed"); } catch (Exception e) { RuntimeException rte = new RuntimeException("Check this out"); diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLEngineImpl/CloseStart.java --- a/test/jdk/sun/security/ssl/SSLEngineImpl/CloseStart.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLEngineImpl/CloseStart.java Tue Aug 14 18:16:47 2018 -0700 @@ -68,11 +68,6 @@ } } - private static void runTest1(SSLEngine ssle) throws Exception { - ssle.closeInbound(); - checkDone(ssle); - } - private static void runTest2(SSLEngine ssle) throws Exception { ssle.closeOutbound(); checkDone(ssle); @@ -81,10 +76,16 @@ public static void main(String args[]) throws Exception { SSLEngine ssle = createSSLEngine(keyFilename, trustFilename); - runTest1(ssle); + ssle.closeInbound(); + if (!ssle.isInboundDone()) { + throw new Exception("isInboundDone isn't done"); + } ssle = createSSLEngine(keyFilename, trustFilename); - runTest2(ssle); + ssle.closeOutbound(); + if (!ssle.isOutboundDone()) { + throw new Exception("isOutboundDone isn't done"); + } System.out.println("Test Passed."); } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDeadlock.java --- a/test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDeadlock.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineDeadlock.java Tue Aug 14 18:16:47 2018 -0700 @@ -280,6 +280,7 @@ log("\tClosing clientEngine's *OUTBOUND*..."); clientEngine.closeOutbound(); + serverEngine.closeOutbound(); dataDone = true; } } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineKeyLimit.java --- a/test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineKeyLimit.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLEngineImpl/SSLEngineKeyLimit.java Tue Aug 14 18:16:47 2018 -0700 @@ -127,8 +127,7 @@ output.shouldNotContain("KeyUpdate: write key updated"); output.shouldNotContain("KeyUpdate: read key updated"); } else { - output.shouldContain("KeyUpdate: triggered, read side"); - output.shouldContain("KeyUpdate: triggered, write side"); + output.shouldContain("trigger key update"); output.shouldContain("KeyUpdate: write key updated"); output.shouldContain("KeyUpdate: read key updated"); } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLEngineImpl/TLS13BeginHandshake.java --- a/test/jdk/sun/security/ssl/SSLEngineImpl/TLS13BeginHandshake.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLEngineImpl/TLS13BeginHandshake.java Tue Aug 14 18:16:47 2018 -0700 @@ -106,11 +106,11 @@ checkTransfer(clientOut, serverIn); System.out.println("\tClosing..."); clientEngine.closeOutbound(); + serverEngine.closeOutbound(); done++; continue; } } - } private static boolean isEngineClosed(SSLEngine engine) { diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLSocketImpl/AsyncSSLSocketClose.java --- a/test/jdk/sun/security/ssl/SSLSocketImpl/AsyncSSLSocketClose.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLSocketImpl/AsyncSSLSocketClose.java Tue Aug 14 18:16:47 2018 -0700 @@ -36,6 +36,7 @@ import javax.net.ssl.*; import java.io.*; +import java.net.SocketException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -123,6 +124,9 @@ os.write(ba); System.out.println(count + " bytes written"); } + } catch (SocketException se) { + // the closing may be in progress + System.out.println("interrupted? " + se); } catch (Exception e) { if (socket.isClosed() || socket.isOutputShutdown()) { System.out.println("interrupted, the socket is closed"); diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketKeyLimit.java --- a/test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketKeyLimit.java Tue Aug 14 19:52:34 2018 -0400 +++ b/test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketKeyLimit.java Tue Aug 14 18:16:47 2018 -0700 @@ -119,7 +119,7 @@ System.setProperty("test.java.opts", "-Dtest.src=" + System.getProperty("test.src") + " -Dtest.jdk=" + System.getProperty("test.jdk") + - " -Djavax.net.debug=ssl,handshake " + + " -Djavax.net.debug=ssl,handshake" + " -Djava.security.properties=" + f.getName()); System.out.println("test.java.opts: " + @@ -134,8 +134,7 @@ output.shouldNotContain("KeyUpdate: write key updated"); output.shouldNotContain("KeyUpdate: read key updated"); } else { - output.shouldContain("KeyUpdate: triggered, read side"); - output.shouldContain("KeyUpdate: triggered, write side"); + output.shouldContain("trigger key update"); output.shouldContain("KeyUpdate: write key updated"); output.shouldContain("KeyUpdate: read key updated"); } diff -r f4b4dfac45b1 -r 910f7b56592f test/jdk/sun/security/ssl/SSLSocketImpl/ServerRenegoWithTwoVersions.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/sun/security/ssl/SSLSocketImpl/ServerRenegoWithTwoVersions.java Tue Aug 14 18:16:47 2018 -0700 @@ -0,0 +1,329 @@ +/* + * Copyright (c) 2018, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +// +// SunJSSE does not support dynamic system properties, no way to re-use +// system properties in samevm/agentvm mode. +// + +/* + * @test + * @bug 8208642 + * @summary Server initiated TLSv1.2 renegotiation fails if Java + * client allows TLSv1.3 + * @run main/othervm ServerRenegoWithTwoVersions TLSv1 SSLv3 + * @run main/othervm ServerRenegoWithTwoVersions TLSv1 TLSv1.1 + * @run main/othervm ServerRenegoWithTwoVersions TLSv1.2 TLSv1.1 + * @run main/othervm ServerRenegoWithTwoVersions TLSv1.3 TLSv1.2 + */ + +import java.io.*; +import java.net.*; +import java.security.Security; +import javax.net.ssl.*; + +public class ServerRenegoWithTwoVersions implements + HandshakeCompletedListener { + + static byte handshakesCompleted = 0; + + /* + * Define what happens when handshaking is completed + */ + public void handshakeCompleted(HandshakeCompletedEvent event) { + synchronized (this) { + handshakesCompleted++; + System.out.println("Session: " + event.getSession().toString()); + System.out.println("Seen handshake completed #" + + handshakesCompleted); + } + } + + /* + * ============================================================= + * Set the various variables needed for the tests, then + * specify what tests to run on each side. + */ + + /* + * Should we run the client or server in a separate thread? + * Both sides can throw exceptions, but do you have a preference + * as to which side should be the main thread. + */ + static boolean separateServerThread = false; + + /* + * Where do we find the keystores? + */ + static String pathToStores = "../../../../javax/net/ssl/etc"; + static String keyStoreFile = "keystore"; + static String trustStoreFile = "truststore"; + static String passwd = "passphrase"; + + /* + * Is the server ready to serve? + */ + volatile static boolean serverReady = false; + + /* + * Turn on SSL debugging? + */ + static boolean debug = false; + + /* + * If the client or server is doing some kind of object creation + * that the other side depends on, and that thread prematurely + * exits, you may experience a hang. The test harness will + * terminate all hung threads after its timeout has expired, + * currently 3 minutes by default, but you might try to be + * smart about it.... + */ + + /* + * Define the server side of the test. + * + * If the server prematurely exits, serverReady will be set to true + * to avoid infinite hangs. + */ + void doServerSide() throws Exception { + SSLServerSocketFactory sslssf = + (SSLServerSocketFactory)SSLServerSocketFactory.getDefault(); + SSLServerSocket sslServerSocket = + (SSLServerSocket) sslssf.createServerSocket(serverPort); + + serverPort = sslServerSocket.getLocalPort(); + + /* + * Signal Client, we're ready for his connect. + */ + serverReady = true; + + SSLSocket sslSocket = (SSLSocket)sslServerSocket.accept(); + sslSocket.setEnabledProtocols(new String[] { serverProtocol }); + sslSocket.addHandshakeCompletedListener(this); + InputStream sslIS = sslSocket.getInputStream(); + OutputStream sslOS = sslSocket.getOutputStream(); + + for (int i = 0; i < 10; i++) { + sslIS.read(); + sslOS.write(85); + sslOS.flush(); + } + + System.out.println("invalidating"); + sslSocket.getSession().invalidate(); + System.out.println("starting new handshake"); + sslSocket.startHandshake(); + + for (int i = 0; i < 10; i++) { + System.out.println("sending/receiving data, iteration: " + i); + sslIS.read(); + sslOS.write(85); + sslOS.flush(); + } + + sslSocket.close(); + } + + /* + * Define the client side of the test. + * + * If the server prematurely exits, serverReady will be set to true + * to avoid infinite hangs. + */ + void doClientSide() throws Exception { + + /* + * Wait for server to get started. + */ + while (!serverReady) { + Thread.sleep(50); + } + + SSLSocketFactory sslsf = + (SSLSocketFactory)SSLSocketFactory.getDefault(); + SSLSocket sslSocket = (SSLSocket) + sslsf.createSocket("localhost", serverPort); + sslSocket.setEnabledProtocols( + new String[] { serverProtocol, clientProtocol }); + + InputStream sslIS = sslSocket.getInputStream(); + OutputStream sslOS = sslSocket.getOutputStream(); + + for (int i = 0; i < 10; i++) { + sslOS.write(280); + sslOS.flush(); + sslIS.read(); + } + + for (int i = 0; i < 10; i++) { + sslOS.write(280); + sslOS.flush(); + sslIS.read(); + } + + sslSocket.close(); + } + + /* + * ============================================================= + * The remainder is just support stuff + */ + + // use any free port by default + volatile int serverPort = 0; + + volatile Exception serverException = null; + volatile Exception clientException = null; + + // the specified protocol + private static String clientProtocol; + private static String serverProtocol; + + public static void main(String[] args) throws Exception { + String keyFilename = + System.getProperty("test.src", "./") + "/" + pathToStores + + "/" + keyStoreFile; + String trustFilename = + System.getProperty("test.src", "./") + "/" + pathToStores + + "/" + trustStoreFile; + + System.setProperty("javax.net.ssl.keyStore", keyFilename); + System.setProperty("javax.net.ssl.keyStorePassword", passwd); + System.setProperty("javax.net.ssl.trustStore", trustFilename); + System.setProperty("javax.net.ssl.trustStorePassword", passwd); + + if (debug) { + System.setProperty("javax.net.debug", "all"); + } + + Security.setProperty("jdk.tls.disabledAlgorithms", ""); + + clientProtocol = args[0]; + serverProtocol = args[1]; + + /* + * Start the tests. + */ + new ServerRenegoWithTwoVersions(); + } + + Thread clientThread = null; + Thread serverThread = null; + + /* + * Primary constructor, used to drive remainder of the test. + * + * Fork off the other side, then do your work. + */ + ServerRenegoWithTwoVersions() throws Exception { + if (separateServerThread) { + startServer(true); + startClient(false); + } else { + startClient(true); + startServer(false); + } + + /* + * Wait for other side to close down. + */ + if (separateServerThread) { + serverThread.join(); + } else { + clientThread.join(); + } + + /* + * When we get here, the test is pretty much over. + * + * If the main thread excepted, that propagates back + * immediately. If the other thread threw an exception, we + * should report back. + */ + if (serverException != null) { + System.out.print("Server Exception:"); + throw serverException; + } + if (clientException != null) { + System.out.print("Client Exception:"); + throw clientException; + } + + /* + * Give the Handshaker Thread a chance to run + */ + Thread.sleep(1000); + + synchronized (this) { + if (handshakesCompleted != 2) { + throw new Exception("Didn't see 2 handshake completed events."); + } + } + } + + void startServer(boolean newThread) throws Exception { + if (newThread) { + serverThread = new Thread() { + public void run() { + try { + doServerSide(); + } catch (Exception e) { + /* + * Our server thread just died. + * + * Release the client, if not active already... + */ + System.err.println("Server died..."); + serverReady = true; + serverException = e; + } + } + }; + serverThread.start(); + } else { + doServerSide(); + } + } + + void startClient(boolean newThread) throws Exception { + if (newThread) { + clientThread = new Thread() { + public void run() { + try { + doClientSide(); + } catch (Exception e) { + /* + * Our client thread just died. + */ + System.err.println("Client died..."); + clientException = e; + } + } + }; + clientThread.start(); + } else { + doClientSide(); + } + } +}