8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers

Reviewed-by: ascarpino

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

@@ -105,6 +105,14 @@ final class ChangeCipherSpec {
                 throw new SSLException("Algorithm missing:  ", gse);
             }
 
+            if (writeCipher == null) {
+                hc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + ncs +
+                    ") and protocol version (" + hc.negotiatedProtocol + ")");
+
+                return null;
+            }
+
             if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) {
                 SSLLogger.fine("Produced ChangeCipherSpec message");
             }
@@ -195,6 +203,16 @@ final class ChangeCipherSpec {
                     // unlikely
                     throw new SSLException("Algorithm missing:  ", gse);
                 }
+
+                if (readCipher == null) {
+                    hc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + hc.negotiatedCipherSuite +
+                        ") and protocol version (" + hc.negotiatedProtocol +
+                        ")");
+
+                    return;
+                }
+
                 tc.inputRecord.changeReadCiphers(readCipher);
             } else {
                 throw new UnsupportedOperationException("Not supported.");

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

@@ -118,7 +118,7 @@ final class Finished {
             } catch (IOException ioe) {
                 context.conContext.fatal(Alert.ILLEGAL_PARAMETER,
                         "Failed to generate verify_data", ioe);
-                return;     // make the compiler happy
+                return;
             }
             if (!MessageDigest.isEqual(myVerifyData, verifyData)) {
                 context.conContext.fatal(Alert.ILLEGAL_PARAMETER,
@@ -681,7 +681,7 @@ final class Finished {
                 // unlikely
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                     "no key derivation");
-                return null;    // make the compiler happy
+                return null;
             }
 
             SSLTrafficKeyDerivation kdg =
@@ -691,7 +691,7 @@ final class Finished {
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         chc.negotiatedProtocol);
-                return null;    // make the compiler happy
+                return null;
             }
 
             try {
@@ -713,6 +713,15 @@ final class Finished {
                                 chc.negotiatedProtocol, writeKey, writeIv,
                                 chc.sslContext.getSecureRandom());
 
+                if (writeCipher == null) {
+                    chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + chc.negotiatedCipherSuite +
+                        ") and protocol version (" + chc.negotiatedProtocol +
+                        ")");
+
+                    return null;
+                }
+
                 chc.baseWriteSecret = writeSecret;
                 chc.conContext.outputRecord.changeWriteCiphers(
                         writeCipher, false);
@@ -720,15 +729,16 @@ final class Finished {
             } catch (GeneralSecurityException gse) {
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Failure to derive application secrets", gse);
-                return null;    // make the compiler happy
+                return null;
             }
 
             // The resumption master secret is stored in the session so
             // it can be used after the handshake is completed.
             SSLSecretDerivation sd = ((SSLSecretDerivation) kd).forContext(chc);
             SecretKey resumptionMasterSecret = sd.deriveKey(
-            "TlsResumptionMasterSecret", null);
-            chc.handshakeSession.setResumptionMasterSecret(resumptionMasterSecret);
+                    "TlsResumptionMasterSecret", null);
+            chc.handshakeSession.setResumptionMasterSecret(
+                    resumptionMasterSecret);
 
             chc.conContext.conSession = chc.handshakeSession.finish();
             chc.conContext.protocolVersion = chc.negotiatedProtocol;
@@ -764,7 +774,7 @@ final class Finished {
                 // unlikely
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                     "no key derivation");
-                return null;    // make the compiler happy
+                return null;
             }
 
             SSLTrafficKeyDerivation kdg =
@@ -774,7 +784,7 @@ final class Finished {
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         shc.negotiatedProtocol);
-                return null;    // make the compiler happy
+                return null;
             }
 
             // derive salt secret
@@ -810,6 +820,15 @@ final class Finished {
                                 shc.negotiatedProtocol, writeKey, writeIv,
                                 shc.sslContext.getSecureRandom());
 
+                if (writeCipher == null) {
+                    shc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + shc.negotiatedCipherSuite +
+                        ") and protocol version (" + shc.negotiatedProtocol +
+                        ")");
+
+                    return null;
+                }
+
                 shc.baseWriteSecret = writeSecret;
                 shc.conContext.outputRecord.changeWriteCiphers(
                         writeCipher, false);
@@ -819,7 +838,7 @@ final class Finished {
             } catch (GeneralSecurityException gse) {
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Failure to derive application secrets", gse);
-                return null;    // make the compiler happy
+                return null;
             }
 
             /*
@@ -894,7 +913,7 @@ final class Finished {
                 // unlikely
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                     "no key derivation");
-                return;    // make the compiler happy
+                return;
             }
 
             SSLTrafficKeyDerivation kdg =
@@ -904,7 +923,7 @@ final class Finished {
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         chc.negotiatedProtocol);
-                return;    // make the compiler happy
+                return;
             }
 
             // save the session
@@ -947,6 +966,15 @@ final class Finished {
                                 chc.negotiatedProtocol, readKey, readIv,
                                 chc.sslContext.getSecureRandom());
 
+                if (readCipher == null) {
+                    chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + chc.negotiatedCipherSuite +
+                        ") and protocol version (" + chc.negotiatedProtocol +
+                        ")");
+
+                    return;
+                }
+
                 chc.baseReadSecret = readSecret;
                 chc.conContext.inputRecord.changeReadCiphers(readCipher);
 
@@ -955,7 +983,7 @@ final class Finished {
             } catch (GeneralSecurityException gse) {
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Failure to derive application secrets", gse);
-                return;    // make the compiler happy
+                return;
             }
 
             //
@@ -1005,7 +1033,7 @@ final class Finished {
                 // unlikely
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                     "no key derivation");
-                return;    // make the compiler happy
+                return;
             }
 
             SSLTrafficKeyDerivation kdg =
@@ -1015,7 +1043,7 @@ final class Finished {
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         shc.negotiatedProtocol);
-                return;    // make the compiler happy
+                return;
             }
 
             // save the session
@@ -1044,20 +1072,31 @@ final class Finished {
                                 shc.negotiatedProtocol, readKey, readIv,
                                 shc.sslContext.getSecureRandom());
 
+                if (readCipher == null) {
+                    shc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + shc.negotiatedCipherSuite +
+                        ") and protocol version (" + shc.negotiatedProtocol +
+                        ")");
+
+                    return;
+                }
+
                 shc.baseReadSecret = readSecret;
                 shc.conContext.inputRecord.changeReadCiphers(readCipher);
 
                 // The resumption master secret is stored in the session so
                 // it can be used after the handshake is completed.
                 shc.handshakeHash.update();
-                SSLSecretDerivation sd = ((SSLSecretDerivation)kd).forContext(shc);
+                SSLSecretDerivation sd =
+                        ((SSLSecretDerivation)kd).forContext(shc);
                 SecretKey resumptionMasterSecret = sd.deriveKey(
                 "TlsResumptionMasterSecret", null);
-                shc.handshakeSession.setResumptionMasterSecret(resumptionMasterSecret);
+                shc.handshakeSession.setResumptionMasterSecret(
+                        resumptionMasterSecret);
             } catch (GeneralSecurityException gse) {
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Failure to derive application secrets", gse);
-                return;    // make the compiler happy
+                return;
             }
 
             //  update connection context

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

@@ -223,6 +223,16 @@ final class KeyUpdate {
                         Authenticator.valueOf(hc.conContext.protocolVersion),
                         hc.conContext.protocolVersion, key, ivSpec,
                         hc.sslContext.getSecureRandom());
+
+                if (rc == null) {
+                    hc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                        "Illegal cipher suite (" + hc.negotiatedCipherSuite +
+                        ") and protocol version (" + hc.negotiatedProtocol +
+                        ")");
+
+                    return;
+                }
+
                 rc.baseSecret = nplus1;
                 hc.conContext.inputRecord.changeReadCiphers(rc);
                 if (SSLLogger.isOn && SSLLogger.isOn("ssl")) {
@@ -303,6 +313,14 @@ final class KeyUpdate {
                 return null;
             }
 
+            if (wc == null) {
+                hc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + hc.negotiatedCipherSuite +
+                    ") and protocol version (" + hc.negotiatedProtocol + ")");
+
+                return null;
+            }
+
             // Output the handshake message and change the write cipher.
             //
             // The KeyUpdate handshake message SHALL be delivered in the

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

@@ -296,7 +296,7 @@ final class ServerHello {
                     shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                             "no cipher suites in common");
 
-                    return null;    // make the compiler happy
+                    return null;
                 }
                 shc.negotiatedCipherSuite = credentials.cipherSuite;
                 shc.handshakeKeyExchange = credentials.keyExchange;
@@ -461,7 +461,7 @@ final class ServerHello {
             shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                     "no cipher suites in common");
 
-            return null;    // make the compiler happy.
+            return null;
         }
 
         private static final class KeyExchangeProperties {
@@ -526,7 +526,7 @@ final class ServerHello {
                 if (cipherSuite == null) {
                     shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                             "no cipher suites in common");
-                    return null;    // make the compiler happy
+                    return null;
                 }
                 shc.negotiatedCipherSuite = cipherSuite;
                 shc.handshakeSession.setSuite(cipherSuite);
@@ -594,7 +594,7 @@ final class ServerHello {
                 // unlikely
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not negotiated key shares");
-                return null;    // make the compiler happy
+                return null;
             }
 
             SSLKeyDerivation handshakeKD = ke.createKeyDerivation(shc);
@@ -608,7 +608,7 @@ final class ServerHello {
                 shc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         shc.negotiatedProtocol);
-                return null;    // make the compiler happy
+                return null;
             }
 
             SSLKeyDerivation kd =
@@ -636,7 +636,16 @@ final class ServerHello {
                 // unlikely
                 shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Missing cipher algorithm", gse);
-                return null;    // make the compiler happy
+                return null;
+            }
+
+            if (readCipher == null) {
+                shc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + shc.negotiatedCipherSuite +
+                    ") and protocol version (" + shc.negotiatedProtocol +
+                    ")");
+
+                return null;
             }
 
             shc.baseReadSecret = readSecret;
@@ -664,7 +673,16 @@ final class ServerHello {
                 // unlikely
                 shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Missing cipher algorithm", gse);
-                return null;    //  make the compiler happy
+                return null;
+            }
+
+            if (writeCipher == null) {
+                shc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + shc.negotiatedCipherSuite +
+                    ") and protocol version (" + shc.negotiatedProtocol +
+                    ")");
+
+                return null;
             }
 
             shc.baseWriteSecret = writeSecret;
@@ -748,7 +766,7 @@ final class ServerHello {
             if (cipherSuite == null) {
                 shc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "no cipher suites in common for hello retry request");
-                return null;    // make the compiler happy
+                return null;
             }
 
             ServerHelloMessage hhrm = new ServerHelloMessage(shc,
@@ -1244,7 +1262,7 @@ final class ServerHello {
                 // unlikely
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not negotiated key shares");
-                return;     // make the compiler happy
+                return;
             }
 
             SSLKeyDerivation handshakeKD = ke.createKeyDerivation(chc);
@@ -1257,7 +1275,7 @@ final class ServerHello {
                 chc.conContext.fatal(Alert.INTERNAL_ERROR,
                         "Not supported key derivation: " +
                         chc.negotiatedProtocol);
-                return;     // make the compiler happy
+                return;
             }
 
             SSLKeyDerivation secretKD =
@@ -1286,7 +1304,16 @@ final class ServerHello {
                 // unlikely
                 chc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Missing cipher algorithm", gse);
-                return;     // make the compiler happy
+                return;
+            }
+
+            if (readCipher == null) {
+                chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + chc.negotiatedCipherSuite +
+                    ") and protocol version (" + chc.negotiatedProtocol +
+                    ")");
+
+                return;
             }
 
             chc.baseReadSecret = readSecret;
@@ -1314,7 +1341,16 @@ final class ServerHello {
                 // unlikely
                 chc.conContext.fatal(Alert.HANDSHAKE_FAILURE,
                         "Missing cipher algorithm", gse);
-                return;     //  make the compiler happy
+                return;
+            }
+
+            if (writeCipher == null) {
+                chc.conContext.fatal(Alert.ILLEGAL_PARAMETER,
+                    "Illegal cipher suite (" + chc.negotiatedCipherSuite +
+                    ") and protocol version (" + chc.negotiatedProtocol +
+                    ")");
+
+                return;
             }
 
             chc.baseWriteSecret = writeSecret;