8213782: NullPointerException in sun.security.ssl.OutputRecord.changeWriteCiphers
Reviewed-by: ascarpino
--- a/src/java.base/share/classes/sun/security/ssl/ChangeCipherSpec.java Fri Dec 14 17:32:16 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/ChangeCipherSpec.java Fri Dec 14 17:51:02 2018 -0800
@@ -105,6 +105,14 @@
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 @@
// 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.");
--- a/src/java.base/share/classes/sun/security/ssl/Finished.java Fri Dec 14 17:32:16 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/Finished.java Fri Dec 14 17:51:02 2018 -0800
@@ -118,7 +118,7 @@
} 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 @@
// unlikely
chc.conContext.fatal(Alert.INTERNAL_ERROR,
"no key derivation");
- return null; // make the compiler happy
+ return null;
}
SSLTrafficKeyDerivation kdg =
@@ -691,7 +691,7 @@
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 @@
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 @@
} 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 @@
// unlikely
shc.conContext.fatal(Alert.INTERNAL_ERROR,
"no key derivation");
- return null; // make the compiler happy
+ return null;
}
SSLTrafficKeyDerivation kdg =
@@ -774,7 +784,7 @@
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 @@
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 @@
} 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 @@
// unlikely
chc.conContext.fatal(Alert.INTERNAL_ERROR,
"no key derivation");
- return; // make the compiler happy
+ return;
}
SSLTrafficKeyDerivation kdg =
@@ -904,7 +923,7 @@
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 @@
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 @@
} 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 @@
// unlikely
shc.conContext.fatal(Alert.INTERNAL_ERROR,
"no key derivation");
- return; // make the compiler happy
+ return;
}
SSLTrafficKeyDerivation kdg =
@@ -1015,7 +1043,7 @@
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 @@
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
--- a/src/java.base/share/classes/sun/security/ssl/KeyUpdate.java Fri Dec 14 17:32:16 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/KeyUpdate.java Fri Dec 14 17:51:02 2018 -0800
@@ -223,6 +223,16 @@
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 @@
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
--- a/src/java.base/share/classes/sun/security/ssl/ServerHello.java Fri Dec 14 17:32:16 2018 -0800
+++ b/src/java.base/share/classes/sun/security/ssl/ServerHello.java Fri Dec 14 17:51:02 2018 -0800
@@ -296,7 +296,7 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
// 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 @@
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 @@
// 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 @@
chc.conContext.fatal(Alert.INTERNAL_ERROR,
"Not supported key derivation: " +
chc.negotiatedProtocol);
- return; // make the compiler happy
+ return;
}
SSLKeyDerivation secretKD =
@@ -1286,7 +1304,16 @@
// 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 @@
// 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;