From baceaab271dffa85c1155d29cb75df15a1ab7104 Mon Sep 17 00:00:00 2001 From: Valerie Peng Date: Mon, 6 Apr 2015 20:24:27 +0000 Subject: [PATCH] 8074865: General crypto resilience changes Replaced Arrays.equals() with MessageDigest.isEqual() when comparing sensitive values Reviewed-by: mullan, xuelei --- .../com/sun/crypto/provider/AESCrypt.java | 5 +++-- .../com/sun/crypto/provider/CipherCore.java | 4 ++-- .../com/sun/crypto/provider/DESKey.java | 5 +++-- .../com/sun/crypto/provider/DESedeKey.java | 5 +++-- .../com/sun/crypto/provider/PBEKey.java | 5 +++-- .../sun/crypto/provider/PBKDF2KeyImpl.java | 7 ++++--- .../share/classes/java/security/Identity.java | 4 ++-- .../classes/java/security/MessageDigest.java | 6 +++++- .../classes/java/security/Signature.java | 4 ++-- .../javax/crypto/spec/SecretKeySpec.java | 5 +++-- .../sun/security/pkcs12/PKCS12KeyStore.java | 2 +- .../sun/security/rsa/RSASignature.java | 3 +-- .../sun/security/ssl/ClientHandshaker.java | 2 +- .../sun/security/ssl/HandshakeMessage.java | 2 +- .../sun/security/ssl/ServerHandshaker.java | 2 +- .../classes/sun/security/pkcs11/P11Key.java | 4 ++-- .../security/pkcs11/wrapper/Functions.java | 20 ++----------------- .../security/ucrypto/NativeGCMCipher.java | 2 +- 18 files changed, 40 insertions(+), 47 deletions(-) diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java index b6b1d34c450..21acfccded0 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/AESCrypt.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2015, 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 @@ -37,6 +37,7 @@ package com.sun.crypto.provider; import java.security.InvalidKeyException; +import java.security.MessageDigest; import java.util.Arrays; import java.util.Objects; @@ -91,7 +92,7 @@ final class AESCrypt extends SymmetricCipher implements AESConstants key.length + " bytes"); } - if (!Arrays.equals(key, lastKey)) { + if (!MessageDigest.isEqual(key, lastKey)) { // re-generate session key 'sessionK' when cipher key changes makeSessionKey(key); lastKey = key.clone(); // save cipher key diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java index 96a624391f0..4cdfdf8850e 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2015, 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 @@ -568,7 +568,7 @@ final class CipherCore { // check key+iv for encryption in GCM mode requireReinit = Arrays.equals(ivBytes, lastEncIv) && - Arrays.equals(keyBytes, lastEncKey); + MessageDigest.isEqual(keyBytes, lastEncKey); if (requireReinit) { throw new InvalidAlgorithmParameterException ("Cannot reuse iv for GCM encryption"); diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java index 32564ce84bf..d4493808167 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESKey.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, 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 @@ -25,6 +25,7 @@ package com.sun.crypto.provider; +import java.security.MessageDigest; import java.security.KeyRep; import java.security.InvalidKeyException; import javax.crypto.SecretKey; @@ -113,7 +114,7 @@ final class DESKey implements SecretKey { return false; byte[] thatKey = ((SecretKey)obj).getEncoded(); - boolean ret = java.util.Arrays.equals(this.key, thatKey); + boolean ret = MessageDigest.isEqual(this.key, thatKey); java.util.Arrays.fill(thatKey, (byte)0x00); return ret; } diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java index 8f264b8251c..a0de5dc059c 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/DESedeKey.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, 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 @@ -25,6 +25,7 @@ package com.sun.crypto.provider; +import java.security.MessageDigest; import java.security.KeyRep; import java.security.InvalidKeyException; import javax.crypto.SecretKey; @@ -114,7 +115,7 @@ final class DESedeKey implements SecretKey { return false; byte[] thatKey = ((SecretKey)obj).getEncoded(); - boolean ret = java.util.Arrays.equals(this.key, thatKey); + boolean ret = MessageDigest.isEqual(this.key, thatKey); java.util.Arrays.fill(thatKey, (byte)0x00); return ret; } diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java index 10ba5cb79f6..3331f787429 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBEKey.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, 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 @@ -25,6 +25,7 @@ package com.sun.crypto.provider; +import java.security.MessageDigest; import java.security.KeyRep; import java.security.spec.InvalidKeySpecException; import java.util.Locale; @@ -108,7 +109,7 @@ final class PBEKey implements SecretKey { return false; byte[] thatEncoded = that.getEncoded(); - boolean ret = java.util.Arrays.equals(this.key, thatEncoded); + boolean ret = MessageDigest.isEqual(this.key, thatEncoded); java.util.Arrays.fill(thatEncoded, (byte)0x00); return ret; } diff --git a/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java b/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java index 16756fbe5e0..134a60096c2 100644 --- a/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java +++ b/jdk/src/java.base/share/classes/com/sun/crypto/provider/PBKDF2KeyImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2015, 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 @@ -31,6 +31,7 @@ import java.nio.CharBuffer; import java.nio.charset.Charset; import java.util.Arrays; import java.util.Locale; +import java.security.MessageDigest; import java.security.KeyRep; import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; @@ -153,7 +154,7 @@ final class PBKDF2KeyImpl implements javax.crypto.interfaces.PBEKey { SecretKey sk = (SecretKey)obj; return prf.getAlgorithm().equalsIgnoreCase( sk.getAlgorithm()) && - Arrays.equals(password, sk.getEncoded()); + MessageDigest.isEqual(password, sk.getEncoded()); } }; prf.init(macKey); @@ -239,7 +240,7 @@ final class PBKDF2KeyImpl implements javax.crypto.interfaces.PBEKey { if (!(that.getFormat().equalsIgnoreCase("RAW"))) return false; byte[] thatEncoded = that.getEncoded(); - boolean ret = Arrays.equals(key, that.getEncoded()); + boolean ret = MessageDigest.isEqual(key, that.getEncoded()); java.util.Arrays.fill(thatEncoded, (byte)0x00); return ret; } diff --git a/jdk/src/java.base/share/classes/java/security/Identity.java b/jdk/src/java.base/share/classes/java/security/Identity.java index 467e18d1b84..83e10cee128 100644 --- a/jdk/src/java.base/share/classes/java/security/Identity.java +++ b/jdk/src/java.base/share/classes/java/security/Identity.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2015, 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 @@ -261,7 +261,7 @@ public abstract class Identity implements Principal, Serializable { certificates.addElement(certificate); } - private boolean keyEquals(Key aKey, Key anotherKey) { + private boolean keyEquals(PublicKey aKey, PublicKey anotherKey) { String aKeyFormat = aKey.getFormat(); String anotherKeyFormat = anotherKey.getFormat(); if ((aKeyFormat == null) ^ (anotherKeyFormat == null)) diff --git a/jdk/src/java.base/share/classes/java/security/MessageDigest.java b/jdk/src/java.base/share/classes/java/security/MessageDigest.java index cf3e3a3a1c3..5a58f0997d0 100644 --- a/jdk/src/java.base/share/classes/java/security/MessageDigest.java +++ b/jdk/src/java.base/share/classes/java/security/MessageDigest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2015, 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 @@ -440,6 +440,10 @@ public abstract class MessageDigest extends MessageDigestSpi { * @return true if the digests are equal, false otherwise. */ public static boolean isEqual(byte[] digesta, byte[] digestb) { + if (digesta == digestb) return true; + if (digesta == null || digestb == null) { + return false; + } if (digesta.length != digestb.length) { return false; } diff --git a/jdk/src/java.base/share/classes/java/security/Signature.java b/jdk/src/java.base/share/classes/java/security/Signature.java index e23b0dde121..4e26a8f9a58 100644 --- a/jdk/src/java.base/share/classes/java/security/Signature.java +++ b/jdk/src/java.base/share/classes/java/security/Signature.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2015, 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 @@ -1324,7 +1324,7 @@ public abstract class Signature extends SignatureSpi { byte[] out = cipher.doFinal(sigBytes); byte[] dataBytes = data.toByteArray(); data.reset(); - return Arrays.equals(out, dataBytes); + return MessageDigest.isEqual(out, dataBytes); } catch (BadPaddingException e) { // e.g. wrong public key used // return false rather than throwing exception diff --git a/jdk/src/java.base/share/classes/javax/crypto/spec/SecretKeySpec.java b/jdk/src/java.base/share/classes/javax/crypto/spec/SecretKeySpec.java index 94c35e0b80d..767c0d2a6f3 100644 --- a/jdk/src/java.base/share/classes/javax/crypto/spec/SecretKeySpec.java +++ b/jdk/src/java.base/share/classes/javax/crypto/spec/SecretKeySpec.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2015, 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 @@ -25,6 +25,7 @@ package javax.crypto.spec; +import java.security.MessageDigest; import java.security.spec.KeySpec; import java.util.Locale; import javax.crypto.SecretKey; @@ -228,6 +229,6 @@ public class SecretKeySpec implements KeySpec, SecretKey { byte[] thatKey = ((SecretKey)obj).getEncoded(); - return java.util.Arrays.equals(this.key, thatKey); + return MessageDigest.isEqual(this.key, thatKey); } } diff --git a/jdk/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java b/jdk/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java index a8c5fa709ba..162191ec850 100644 --- a/jdk/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java +++ b/jdk/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java @@ -2059,7 +2059,7 @@ public final class PKCS12KeyStore extends KeyStoreSpi { "(MAC algorithm: " + m.getAlgorithm() + ")"); } - if (!Arrays.equals(macData.getDigest(), macResult)) { + if (!MessageDigest.isEqual(macData.getDigest(), macResult)) { throw new SecurityException("Failed PKCS12" + " integrity checking"); } diff --git a/jdk/src/java.base/share/classes/sun/security/rsa/RSASignature.java b/jdk/src/java.base/share/classes/sun/security/rsa/RSASignature.java index 6d43d1e32a3..7c707bbb15e 100644 --- a/jdk/src/java.base/share/classes/sun/security/rsa/RSASignature.java +++ b/jdk/src/java.base/share/classes/sun/security/rsa/RSASignature.java @@ -27,7 +27,6 @@ package sun.security.rsa; import java.io.IOException; import java.nio.ByteBuffer; -import java.util.Arrays; import java.security.*; import java.security.interfaces.*; @@ -194,7 +193,7 @@ public abstract class RSASignature extends SignatureSpi { byte[] decrypted = RSACore.rsa(sigBytes, publicKey); byte[] unpadded = padding.unpad(decrypted); byte[] decodedDigest = decodeSignature(digestOID, unpadded); - return Arrays.equals(digest, decodedDigest); + return MessageDigest.isEqual(digest, decodedDigest); } catch (javax.crypto.BadPaddingException e) { // occurs if the app has used the wrong RSA public key // or if sigBytes is invalid diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/ClientHandshaker.java b/jdk/src/java.base/share/classes/sun/security/ssl/ClientHandshaker.java index 16384007238..077aa54ce8c 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/ClientHandshaker.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/ClientHandshaker.java @@ -542,7 +542,7 @@ final class ClientHandshaker extends Handshaker { 0, clientVerifyData.length); System.arraycopy(serverVerifyData, 0, verifyData, clientVerifyData.length, serverVerifyData.length); - if (!Arrays.equals(verifyData, + if (!MessageDigest.isEqual(verifyData, serverHelloRI.getRenegotiatedConnection())) { fatalSE(Alerts.alert_handshake_failure, "Incorrect verify data in ServerHello " + diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java b/jdk/src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java index f4452faaa2c..a27515f3835 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/HandshakeMessage.java @@ -2040,7 +2040,7 @@ static final class Finished extends HandshakeMessage { */ boolean verify(HandshakeHash handshakeHash, int sender, SecretKey master) { byte[] myFinished = getFinished(handshakeHash, sender, master); - return Arrays.equals(myFinished, verifyData); + return MessageDigest.isEqual(myFinished, verifyData); } /* diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java b/jdk/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java index 5c375c4147f..ba229f914ed 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/ServerHandshaker.java @@ -407,7 +407,7 @@ final class ServerHandshaker extends Handshaker { } // verify the client_verify_data value - if (!Arrays.equals(clientVerifyData, + if (!MessageDigest.isEqual(clientVerifyData, clientHelloRI.getRenegotiatedConnection())) { fatalSE(Alerts.alert_handshake_failure, "Incorrect verify data in ClientHello " + diff --git a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java index 652bf0cbd92..f35d3fed959 100644 --- a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java +++ b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2015, 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 @@ -165,7 +165,7 @@ abstract class P11Key implements Key, Length { } else { otherEnc = other.getEncoded(); } - return Arrays.equals(thisEnc, otherEnc); + return MessageDigest.isEqual(thisEnc, otherEnc); } public int hashCode() { diff --git a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/wrapper/Functions.java b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/wrapper/Functions.java index a478569d296..814b155390a 100644 --- a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/wrapper/Functions.java +++ b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/wrapper/Functions.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved. */ /* Copyright (c) 2002 Graz University of Technology. All rights reserved. @@ -444,22 +444,6 @@ public class Functions { return getId(objectClassIds, name); } - /** - * Check the given arrays for equalitiy. This method considers both arrays as - * equal, if both are null or both have the same length and - * contain exactly the same byte values. - * - * @param array1 The first array. - * @param array2 The second array. - * @return True, if both arrays are null or both have the same - * length and contain exactly the same byte values. False, otherwise. - * @preconditions - * @postconditions - */ - public static boolean equals(byte[] array1, byte[] array2) { - return Arrays.equals(array1, array2); - } - /** * Check the given arrays for equalitiy. This method considers both arrays as * equal, if both are null or both have the same length and @@ -472,7 +456,7 @@ public class Functions { * @preconditions * @postconditions */ - public static boolean equals(char[] array1, char[] array2) { + private static boolean equals(char[] array1, char[] array2) { return Arrays.equals(array1, array2); } diff --git a/jdk/src/jdk.crypto.ucrypto/solaris/classes/com/oracle/security/ucrypto/NativeGCMCipher.java b/jdk/src/jdk.crypto.ucrypto/solaris/classes/com/oracle/security/ucrypto/NativeGCMCipher.java index 8d13d6c6a69..ff12ef73c1f 100644 --- a/jdk/src/jdk.crypto.ucrypto/solaris/classes/com/oracle/security/ucrypto/NativeGCMCipher.java +++ b/jdk/src/jdk.crypto.ucrypto/solaris/classes/com/oracle/security/ucrypto/NativeGCMCipher.java @@ -206,7 +206,7 @@ class NativeGCMCipher extends NativeCipher { } if (doEncrypt) { requireReinit = Arrays.equals(ivBytes, lastEncIv) && - Arrays.equals(keyBytes, lastEncKey); + MessageDigest.isEqual(keyBytes, lastEncKey); if (requireReinit) { throw new InvalidAlgorithmParameterException ("Cannot reuse iv for GCM encryption");