8215712: Parsing extension failure may alert decode_error

Reviewed-by: jnimeh

src/java.base/share/classes/sun/security/ssl/AlpnExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -72,29 +72,33 @@ private AlpnSpec(String[] applicationProtocols) {
                     Arrays.asList(applicationProtocols));
         }
 
-        private AlpnSpec(ByteBuffer buffer) throws IOException {
+        private AlpnSpec(HandshakeContext hc,
+                ByteBuffer buffer) throws IOException {
             // ProtocolName protocol_name_list<2..2^16-1>, RFC 7301.
             if (buffer.remaining() < 2) {
-                throw new SSLProtocolException(
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
                     "Invalid application_layer_protocol_negotiation: " +
-                    "insufficient data (length=" + buffer.remaining() + ")");
+                    "insufficient data (length=" + buffer.remaining() + ")"));
             }
 
             int listLen = Record.getInt16(buffer);
             if (listLen < 2 || listLen != buffer.remaining()) {
-                throw new SSLProtocolException(
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
                     "Invalid application_layer_protocol_negotiation: " +
-                    "incorrect list length (length=" + listLen + ")");
+                    "incorrect list length (length=" + listLen + ")"));
             }
 
             List<String> protocolNames = new LinkedList<>();
             while (buffer.hasRemaining()) {
                 // opaque ProtocolName<1..2^8-1>, RFC 7301.
                 byte[] bytes = Record.getBytes8(buffer);
                 if (bytes.length == 0) {
-                    throw new SSLProtocolException(
+                    throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                            new SSLProtocolException(
                         "Invalid application_layer_protocol_negotiation " +
-                        "extension: empty application protocol name");
+                        "extension: empty application protocol name"));
                 }
 
                 String appProtocol = new String(bytes, StandardCharsets.UTF_8);
@@ -113,9 +117,9 @@ public String toString() {
 
     private static final class AlpnStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new AlpnSpec(buffer)).toString();
+                return (new AlpnSpec(hc, buffer)).toString();
             } catch (IOException ioe) {
                 // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -282,12 +286,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            AlpnSpec spec;
-            try {
-                spec = new AlpnSpec(buffer);
-            } catch (IOException ioe) {
-                throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            AlpnSpec spec = new AlpnSpec(shc, buffer);
 
             // Update the context.
             if (noAPSelector) {     // noAlpnProtocols is false
@@ -463,12 +462,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            AlpnSpec spec;
-            try {
-                spec = new AlpnSpec(buffer);
-            } catch (IOException ioe) {
-                throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            AlpnSpec spec = new AlpnSpec(chc, buffer);
 
             // Only one application protocol is allowed.
             if (spec.applicationProtocols.size() != 1) {

src/java.base/share/classes/sun/security/ssl/CertSignAlgsExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -56,9 +56,10 @@
     private static final
             class CertSignatureSchemesStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new SignatureSchemesSpec(buffer)).toString();
+                return (new SignatureSchemesSpec(hc, buffer))
+                        .toString();
             } catch (IOException ioe) {
                 // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -149,12 +150,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            SignatureSchemesSpec spec;
-            try {
-                spec = new SignatureSchemesSpec(buffer);
-            } catch (IOException ioe) {
-                throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            SignatureSchemesSpec spec = new SignatureSchemesSpec(shc, buffer);
 
             // Update the context.
             shc.handshakeExtensions.put(
@@ -292,12 +288,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            SignatureSchemesSpec spec;
-            try {
-                spec = new SignatureSchemesSpec(buffer);
-            } catch (IOException ioe) {
-                throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            SignatureSchemesSpec spec = new SignatureSchemesSpec(chc, buffer);
 
             // Update the context.
             chc.handshakeExtensions.put(

src/java.base/share/classes/sun/security/ssl/CertStatusExtension.java

@@ -121,7 +121,8 @@ private CertStatusRequestSpec(CertStatusRequest statusRequest) {
             this.statusRequest = statusRequest;
         }
 
-        private CertStatusRequestSpec(ByteBuffer buffer) throws IOException {
+        private CertStatusRequestSpec(HandshakeContext hc,
+                ByteBuffer buffer) throws IOException {
             // Is it a empty extension_data?
             if (buffer.remaining() == 0) {
                 // server response
@@ -130,8 +131,9 @@ private CertStatusRequestSpec(ByteBuffer buffer) throws IOException {
             }
 
             if (buffer.remaining() < 1) {
-                throw new SSLProtocolException(
-                    "Invalid status_request extension: insufficient data");
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
+                    "Invalid status_request extension: insufficient data"));
             }
 
             byte statusType = (byte)Record.getInt8(buffer);
@@ -178,10 +180,12 @@ private CertStatusResponseSpec(CertStatusResponse resp) {
             this.statusResponse = resp;
         }
 
-        private CertStatusResponseSpec(ByteBuffer buffer) throws IOException {
+        private CertStatusResponseSpec(HandshakeContext hc,
+                ByteBuffer buffer) throws IOException {
             if (buffer.remaining() < 2) {
-                throw new SSLProtocolException(
-                    "Invalid status_request extension: insufficient data");
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
+                    "Invalid status_request extension: insufficient data"));
             }
 
             // Get the status type (1 byte) and response data (vector)
@@ -212,9 +216,9 @@ public String toString() {
     private static final
             class CertStatusRequestStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new CertStatusRequestSpec(buffer)).toString();
+                return (new CertStatusRequestSpec(hc, buffer)).toString();
             } catch (IOException ioe) {
                 // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -225,9 +229,9 @@ public String toString(ByteBuffer buffer) {
     private static final
             class CertStatusRespStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new CertStatusResponseSpec(buffer)).toString();
+                return (new CertStatusResponseSpec(hc, buffer)).toString();
             } catch (IOException ioe) {
                  // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -599,12 +603,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            CertStatusRequestSpec spec;
-            try {
-                spec = new CertStatusRequestSpec(buffer);
-            } catch (IOException ioe) {
-                throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            CertStatusRequestSpec spec = new CertStatusRequestSpec(shc, buffer);
 
             // Update the context.
             shc.handshakeExtensions.put(SSLExtension.CH_STATUS_REQUEST, spec);
@@ -776,7 +775,8 @@ private CertStatusRequestV2Spec(CertStatusRequest[] certStatusRequests) {
             this.certStatusRequests = certStatusRequests;
         }
 
-        private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
+        private CertStatusRequestV2Spec(HandshakeContext hc,
+                ByteBuffer message) throws IOException {
             // Is it a empty extension_data?
             if (message.remaining() == 0) {
                 // server response
@@ -787,15 +787,17 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
             if (message.remaining() < 5) {  //  2: certificate_status_req_list
                                             // +1: status_type
                                             // +2: request_length
-                throw new SSLProtocolException(
-                    "Invalid status_request_v2 extension: insufficient data");
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
+                    "Invalid status_request_v2 extension: insufficient data"));
             }
 
             int listLen = Record.getInt16(message);
             if (listLen <= 0) {
-                throw new SSLProtocolException(
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
                     "certificate_status_req_list length must be positive " +
-                    "(received length: " + listLen + ")");
+                    "(received length: " + listLen + ")"));
             }
 
             int remaining = listLen;
@@ -805,10 +807,12 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
                 int requestLen = Record.getInt16(message);
 
                 if (message.remaining() < requestLen) {
-                    throw new SSLProtocolException(
+                        throw hc.conContext.fatal(
+                                Alert.DECODE_ERROR,
+                                new SSLProtocolException(
                             "Invalid status_request_v2 extension: " +
                             "insufficient data (request_length=" + requestLen +
-                            ", remining=" + message.remaining() + ")");
+                            ", remining=" + message.remaining() + ")"));
                 }
 
                 byte[] encoded = new byte[requestLen];
@@ -823,9 +827,11 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
                     if (encoded.length < 4) {
                                         //  2: length of responder_id_list
                                         // +2: length of request_extensions
-                        throw new SSLProtocolException(
+                        throw hc.conContext.fatal(
+                                Alert.DECODE_ERROR,
+                                new SSLProtocolException(
                             "Invalid status_request_v2 extension: " +
-                            "insufficient data");
+                            "insufficient data"));
                     }
                     statusRequests.add(
                             new OCSPStatusRequest(statusType, encoded));
@@ -874,9 +880,9 @@ public String toString() {
     private static final
             class CertStatusRequestsStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new CertStatusRequestV2Spec(buffer)).toString();
+                return (new CertStatusRequestV2Spec(hc, buffer)).toString();
             } catch (IOException ioe) {
                 // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -957,12 +963,7 @@ public void consume(ConnectionContext context,
             }
 
             // Parse the extension.
-            CertStatusRequestV2Spec spec;
-            try {
-                spec = new CertStatusRequestV2Spec(buffer);
-            } catch (IOException ioe) {
-                throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
+            CertStatusRequestV2Spec spec = new CertStatusRequestV2Spec(shc, buffer);
 
             // Update the context.
             shc.handshakeExtensions.put(SSLExtension.CH_STATUS_REQUEST_V2,
@@ -1185,12 +1186,7 @@ public void consume(ConnectionContext context,
             ClientHandshakeContext chc = (ClientHandshakeContext)context;
 
             // Parse the extension.
-            CertStatusResponseSpec spec;
-            try {
-                spec = new CertStatusResponseSpec(buffer);
-            } catch (IOException ioe) {
-                throw chc.conContext.fatal(Alert.DECODE_ERROR, ioe);
-            }
+            CertStatusResponseSpec spec = new CertStatusResponseSpec(chc, buffer);
 
             if (chc.sslContext.isStaplingEnabled(true)) {
                 // Activate stapling

src/java.base/share/classes/sun/security/ssl/CookieExtension.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -63,11 +63,13 @@
     static class CookieSpec implements SSLExtensionSpec {
         final byte[] cookie;
 
-        private CookieSpec(ByteBuffer m) throws IOException {
+        private CookieSpec(HandshakeContext hc,
+                ByteBuffer m) throws IOException {
             // opaque cookie<1..2^16-1>;
             if (m.remaining() < 3) {
-                throw new SSLProtocolException(
-                    "Invalid cookie extension: insufficient data");
+                throw hc.conContext.fatal(Alert.DECODE_ERROR,
+                        new SSLProtocolException(
+                    "Invalid cookie extension: insufficient data"));
             }
 
             this.cookie = Record.getBytes16(m);
@@ -90,9 +92,9 @@ public String toString() {
 
     private static final class CookieStringizer implements SSLStringizer {
         @Override
-        public String toString(ByteBuffer buffer) {
+        public String toString(HandshakeContext hc, ByteBuffer buffer) {
             try {
-                return (new CookieSpec(buffer)).toString();
+                return (new CookieSpec(hc, buffer)).toString();
             } catch (IOException ioe) {
                 // For debug logging only, so please swallow exceptions.
                 return ioe.getMessage();
@@ -159,13 +161,7 @@ public void consume(ConnectionContext context,
                 return;     // ignore the extension
             }
 
-            CookieSpec spec;
-            try {
-                spec = new CookieSpec(buffer);
-            } catch (IOException ioe) {
-                throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
-
+            CookieSpec spec = new CookieSpec(shc, buffer);
             shc.handshakeExtensions.put(SSLExtension.CH_COOKIE, spec);
 
             // No impact on session resumption.
@@ -264,13 +260,7 @@ public void consume(ConnectionContext context,
                 return;     // ignore the extension
             }
 
-            CookieSpec spec;
-            try {
-                spec = new CookieSpec(buffer);
-            } catch (IOException ioe) {
-                throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
-            }
-
+            CookieSpec spec = new CookieSpec(chc, buffer);
             chc.handshakeExtensions.put(SSLExtension.HRR_COOKIE, spec);
         }
     }