diff --git a/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java b/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java index fe0b6be479c..19b37eb7434 100644 --- a/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java +++ b/jdk/src/share/classes/sun/security/ssl/SSLSocketImpl.java @@ -73,6 +73,16 @@ public final class SSLSocketImpl extends BaseSSLSocketImpl implements SSLTransport { + /** + * ERROR HANDLING GUIDELINES + * (which exceptions to throw and catch and which not to throw and catch) + * + * - if there is an IOException (SocketException) when accessing the + * underlying Socket, pass it through + * + * - do not throw IOExceptions, throw SSLExceptions (or a subclass) + */ + final SSLContextImpl sslContext; final TransportContext conContext; @@ -439,6 +449,8 @@ public void startHandshake() throws IOException { if (!conContext.isNegotiated) { readHandshakeRecord(); } + } catch (SocketException se) { + handleException(se); } catch (IOException ioe) { throw conContext.fatal(Alert.HANDSHAKE_FAILURE, "Couldn't kickstart handshaking", ioe); @@ -1311,7 +1323,8 @@ private int readHandshakeRecord() throws IOException { conContext.isNegotiated) { return 0; } - } catch (SSLException ssle) { + } catch (SSLException | SocketException ssle) { + // don't change exception in case of SocketException throw ssle; } catch (IOException ioe) { if (!(ioe instanceof SSLException)) { @@ -1377,7 +1390,8 @@ private ByteBuffer readApplicationRecord( buffer.position() > 0) { return buffer; } - } catch (SSLException ssle) { + } catch (SSLException | SocketException ssle) { + // don't change exception in case of SocketException. throw ssle; } catch (IOException ioe) { if (!(ioe instanceof SSLException)) { @@ -1569,6 +1583,16 @@ private void handleException(Exception cause) throws IOException { } } + if (cause instanceof SocketException) { + try { + conContext.fatal(alert, cause); + } catch (Exception e) { + // Just delivering the fatal alert, re-throw the socket exception instead. + } + + throw (SocketException)cause; + } + throw conContext.fatal(alert, cause); } diff --git a/jdk/src/share/classes/sun/security/ssl/SSLTransport.java b/jdk/src/share/classes/sun/security/ssl/SSLTransport.java index b3d03b37044..18aa2e62860 100644 --- a/jdk/src/share/classes/sun/security/ssl/SSLTransport.java +++ b/jdk/src/share/classes/sun/security/ssl/SSLTransport.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -28,6 +28,7 @@ import java.io.EOFException; import java.io.IOException; import java.nio.ByteBuffer; +import java.net.SocketException; import javax.crypto.AEADBadTagException; import javax.crypto.BadPaddingException; import javax.net.ssl.SSLHandshakeException; @@ -134,6 +135,9 @@ static Plaintext decode(TransportContext context, } catch (EOFException eofe) { // rethrow EOFException, the call will handle it if neede. throw eofe; + } catch (SocketException se) { + // don't close the Socket in case of SocketException. + throw se; } catch (IOException ioe) { throw context.fatal(Alert.UNEXPECTED_MESSAGE, ioe); } diff --git a/jdk/test/javax/net/ssl/SSLSession/TestEnabledProtocols.java b/jdk/test/javax/net/ssl/SSLSession/TestEnabledProtocols.java index cd8ad678b47..355a290355c 100644 --- a/jdk/test/javax/net/ssl/SSLSession/TestEnabledProtocols.java +++ b/jdk/test/javax/net/ssl/SSLSession/TestEnabledProtocols.java @@ -41,6 +41,7 @@ import java.io.InputStream; import java.io.InterruptedIOException; import java.io.OutputStream; +import java.net.SocketException; import java.security.Security; import java.util.Arrays; @@ -86,10 +87,10 @@ protected void runServerApplication(SSLSocket socket) throws Exception { se.printStackTrace(System.out); } catch (InterruptedIOException ioe) { // must have been interrupted, no harm - } catch (SSLException ssle) { + } catch (SSLException | SocketException se) { // The client side may have closed the socket. System.out.println("Server SSLException:"); - ssle.printStackTrace(System.out); + se.printStackTrace(System.out); } catch (Exception e) { System.out.println("Server exception:"); e.printStackTrace(System.out); diff --git a/jdk/test/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java b/jdk/test/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java index 51fdafff7da..6d705000e86 100644 --- a/jdk/test/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java +++ b/jdk/test/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -131,9 +131,9 @@ protected void runServerApplication(SSLSocket socket) throws Exception { sslIS.read(); sslOS.write('A'); sslOS.flush(); - } catch (SSLException ssle) { + } catch (SSLException | SocketException se) { if (!expectFail) { - throw ssle; + throw se; } // Otherwise, ignore. } } diff --git a/jdk/test/sun/security/ssl/SSLSocketImpl/SSLSocketShouldThrowSocketException.java b/jdk/test/sun/security/ssl/SSLSocketImpl/SSLSocketShouldThrowSocketException.java new file mode 100644 index 00000000000..46cedd8f15e --- /dev/null +++ b/jdk/test/sun/security/ssl/SSLSocketImpl/SSLSocketShouldThrowSocketException.java @@ -0,0 +1,153 @@ +/* + * Copyright (c) 2021, Amazon 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 8214339 8259662 + * @summary When a SocketException is thrown by the underlying layer, It + * should be thrown as is and not be transformed to an SSLException. + * @library /javax/net/ssl/templates + * @run main/othervm SSLSocketShouldThrowSocketException + */ + +import java.io.*; +import java.net.*; +import java.util.*; +import java.security.*; +import javax.net.ssl.*; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class SSLSocketShouldThrowSocketException extends SSLSocketTemplate { + + boolean handshake; + + private final CountDownLatch clientTerminatedCondition = new CountDownLatch(1); + + SSLSocketShouldThrowSocketException(boolean handshake) { + this.handshake = handshake; + } + + @Override + protected boolean isCustomizedClientConnection() { + return true; + } + + @Override + protected void runServerApplication(SSLSocket socket) throws Exception { + clientTerminatedCondition.await(30L, TimeUnit.SECONDS); + } + + @Override + protected void runClientApplication(int serverPort) throws Exception { + Socket baseSocket = new Socket("localhost", serverPort); + + SSLSocketFactory sslsf = + (SSLSocketFactory) SSLSocketFactory.getDefault(); + SSLSocket sslSocket = (SSLSocket) + sslsf.createSocket(baseSocket, "localhost", serverPort, false); + + if (this.handshake) { + testHandshakeClose(baseSocket, sslSocket); + } else { + testDataClose(baseSocket, sslSocket); + } + + clientTerminatedCondition.countDown(); + + } + + private void testHandshakeClose(Socket baseSocket, SSLSocket sslSocket) throws Exception { + Thread aborter = new Thread() { + @Override + public void run() { + + try { + Thread.sleep(10); + System.err.println("Closing the client socket : " + System.nanoTime()); + baseSocket.close(); + } catch (Exception ieo) { + ieo.printStackTrace(); + } + } + }; + + aborter.start(); + + try { + // handshaking + System.err.println("Client starting handshake: " + System.nanoTime()); + sslSocket.startHandshake(); + throw new Exception("Start handshake did not throw an exception"); + } catch (SocketException se) { + System.err.println("Caught Expected SocketException"); + } + + aborter.join(); + } + + private void testDataClose(Socket baseSocket, SSLSocket sslSocket) throws Exception{ + + CountDownLatch handshakeCondition = new CountDownLatch(1); + + Thread aborter = new Thread() { + @Override + public void run() { + + try { + handshakeCondition.await(10L, TimeUnit.SECONDS); + System.err.println("Closing the client socket : " + System.nanoTime()); + baseSocket.close(); + } catch (Exception ieo) { + ieo.printStackTrace(); + } + } + }; + + aborter.start(); + + try { + // handshaking + System.err.println("Client starting handshake: " + System.nanoTime()); + sslSocket.startHandshake(); + handshakeCondition.countDown(); + System.err.println("Reading data from server"); + BufferedReader is = new BufferedReader( + new InputStreamReader(sslSocket.getInputStream())); + String data = is.readLine(); + throw new Exception("Start handshake did not throw an exception"); + } catch (SocketException se) { + System.err.println("Caught Expected SocketException"); + } + + aborter.join(); + } + + public static void main(String[] args) throws Exception { + // SocketException should be throws during a handshake phase. + (new SSLSocketShouldThrowSocketException(true)).run(); + // SocketException should be throw during the application data phase. + (new SSLSocketShouldThrowSocketException(false)).run(); + } +} diff --git a/jdk/test/sun/security/ssl/SSLSocketImpl/SSLExceptionForIOIssue.java b/jdk/test/sun/security/ssl/SSLSocketImpl/SocketExceptionForSocketIssues.java similarity index 94% rename from jdk/test/sun/security/ssl/SSLSocketImpl/SSLExceptionForIOIssue.java rename to jdk/test/sun/security/ssl/SSLSocketImpl/SocketExceptionForSocketIssues.java index 3e626a2577c..58a0c2ca38a 100644 --- a/jdk/test/sun/security/ssl/SSLSocketImpl/SSLExceptionForIOIssue.java +++ b/jdk/test/sun/security/ssl/SSLSocketImpl/SocketExceptionForSocketIssues.java @@ -31,18 +31,19 @@ * @bug 8214339 * @summary SSLSocketImpl erroneously wraps SocketException * @library /javax/net/ssl/templates - * @run main/othervm SSLExceptionForIOIssue + * @run main/othervm SocketExceptionForSocketIssues */ import javax.net.ssl.*; import java.io.*; import java.net.InetAddress; +import java.net.SocketException; -public class SSLExceptionForIOIssue implements SSLContextTemplate { +public class SocketExceptionForSocketIssues implements SSLContextTemplate { public static void main(String[] args) throws Exception { System.err.println("==================================="); - new SSLExceptionForIOIssue().test(); + new SocketExceptionForSocketIssues().test(); } private void test() throws Exception { @@ -79,9 +80,9 @@ private void test() throws Exception { os.flush(); } catch (SSLProtocolException | SSLHandshakeException sslhe) { throw sslhe; - } catch (SSLException ssle) { + } catch (SocketException se) { // the expected exception, ignore it - System.err.println("server exception: " + ssle); + System.err.println("server exception: " + se); } finally { if (listenSocket != null) { listenSocket.close();