From b7bf7fd3fdafccadb803cf327d2255057619856e Mon Sep 17 00:00:00 2001 From: Xue-Lei Andrew Fan Date: Wed, 22 Apr 2015 05:09:54 +0000 Subject: [PATCH] 8076328: Enforce key exchange constraints Reviewed-by: wetmore, igerasim, ahgross, asmotrak --- .../sun/security/ssl/ClientHandshaker.java | 19 ++++++++ .../classes/sun/security/ssl/DHCrypt.java | 25 ++++++++++- .../classes/sun/security/ssl/ECDHCrypt.java | 43 ++++++++++++++++--- .../classes/sun/security/ssl/Handshaker.java | 4 +- .../sun/security/ssl/ServerHandshaker.java | 17 +++++++- .../share/conf/security/java.security | 6 +-- .../sanity/interop/ClientJSSEServerJSSE.java | 9 ++-- jdk/test/sun/security/ec/TestEC.java | 9 ++-- .../pkcs11/sslecc/ClientJSSEServerJSSE.java | 13 +++--- .../ssl/DHKeyExchange/DHEKeySizing.java | 3 +- .../AnonCipherWithWantClientAuth.java | 16 +++++-- 11 files changed, 126 insertions(+), 38 deletions(-) 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 077aa54ce8c..4d8df58aefb 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 @@ -723,6 +723,14 @@ final class ClientHandshaker extends Handshaker { // NOTREACHED } ephemeralServerKey = mesg.getPublicKey(); + + // check constraints of RSA PublicKey + if (!algorithmConstraints.permits( + EnumSet.of(CryptoPrimitive.KEY_AGREEMENT), ephemeralServerKey)) { + + throw new SSLHandshakeException("RSA ServerKeyExchange " + + "does not comply to algorithm constraints"); + } } /* @@ -739,6 +747,9 @@ final class ClientHandshaker extends Handshaker { dh = new DHCrypt(mesg.getModulus(), mesg.getBase(), sslContext.getSecureRandom()); serverDH = mesg.getServerPublicKey(); + + // check algorithm constraints + dh.checkConstraints(algorithmConstraints, serverDH); } private void serverKeyExchange(ECDH_ServerKeyExchange mesg) @@ -749,6 +760,14 @@ final class ClientHandshaker extends Handshaker { ECPublicKey key = mesg.getPublicKey(); ecdh = new ECDHCrypt(key.getParams(), sslContext.getSecureRandom()); ephemeralServerKey = key; + + // check constraints of EC PublicKey + if (!algorithmConstraints.permits( + EnumSet.of(CryptoPrimitive.KEY_AGREEMENT), ephemeralServerKey)) { + + throw new SSLHandshakeException("ECDH ServerKeyExchange " + + "does not comply to algorithm constraints"); + } } /* diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/DHCrypt.java b/jdk/src/java.base/share/classes/sun/security/ssl/DHCrypt.java index 6deae7e2657..7872c081df7 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/DHCrypt.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/DHCrypt.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 @@ -34,6 +34,7 @@ import javax.crypto.SecretKey; import javax.crypto.KeyAgreement; import javax.crypto.interfaces.DHPublicKey; import javax.crypto.spec.*; +import java.util.EnumSet; import sun.security.util.KeyUtil; @@ -216,6 +217,28 @@ final class DHCrypt { } } + // Check constraints of the specified DH public key. + void checkConstraints(AlgorithmConstraints constraints, + BigInteger peerPublicValue) throws SSLHandshakeException { + + try { + KeyFactory kf = JsseJce.getKeyFactory("DiffieHellman"); + DHPublicKeySpec spec = + new DHPublicKeySpec(peerPublicValue, modulus, base); + DHPublicKey publicKey = (DHPublicKey)kf.generatePublic(spec); + + // check constraints of DHPublicKey + if (!constraints.permits( + EnumSet.of(CryptoPrimitive.KEY_AGREEMENT), publicKey)) { + throw new SSLHandshakeException( + "DHPublicKey does not comply to algorithm constraints"); + } + } catch (GeneralSecurityException gse) { + throw (SSLHandshakeException) new SSLHandshakeException( + "Could not generate DHPublicKey").initCause(gse); + } + } + // Generate and validate DHPublicKeySpec private DHPublicKeySpec generateDHPublicKeySpec(KeyPairGenerator kpg) throws GeneralSecurityException { diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/ECDHCrypt.java b/jdk/src/java.base/share/classes/sun/security/ssl/ECDHCrypt.java index c1ce4e93cee..b85c4c5dfae 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/ECDHCrypt.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/ECDHCrypt.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2006, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2006, 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 @@ -29,6 +29,7 @@ import java.security.*; import java.security.interfaces.ECPublicKey; import java.security.spec.*; +import java.util.EnumSet; import javax.crypto.SecretKey; import javax.crypto.KeyAgreement; import javax.net.ssl.SSLHandshakeException; @@ -88,8 +89,11 @@ final class ECDHCrypt { return publicKey; } - // called by ClientHandshaker with either the server's static or ephemeral public key - SecretKey getAgreedSecret(PublicKey peerPublicKey) throws SSLHandshakeException { + // called by ClientHandshaker with either the server's static or + // ephemeral public key + SecretKey getAgreedSecret( + PublicKey peerPublicKey) throws SSLHandshakeException { + try { KeyAgreement ka = JsseJce.getKeyAgreement("ECDH"); ka.init(privateKey); @@ -102,10 +106,13 @@ final class ECDHCrypt { } // called by ServerHandshaker - SecretKey getAgreedSecret(byte[] encodedPoint) throws SSLHandshakeException { + SecretKey getAgreedSecret( + byte[] encodedPoint) throws SSLHandshakeException { + try { ECParameterSpec params = publicKey.getParams(); - ECPoint point = JsseJce.decodePoint(encodedPoint, params.getCurve()); + ECPoint point = + JsseJce.decodePoint(encodedPoint, params.getCurve()); KeyFactory kf = JsseJce.getKeyFactory("EC"); ECPublicKeySpec spec = new ECPublicKeySpec(point, params); PublicKey peerPublicKey = kf.generatePublic(spec); @@ -116,4 +123,30 @@ final class ECDHCrypt { } } + // Check constraints of the specified EC public key. + void checkConstraints(AlgorithmConstraints constraints, + byte[] encodedPoint) throws SSLHandshakeException { + + try { + + ECParameterSpec params = publicKey.getParams(); + ECPoint point = + JsseJce.decodePoint(encodedPoint, params.getCurve()); + ECPublicKeySpec spec = new ECPublicKeySpec(point, params); + + KeyFactory kf = JsseJce.getKeyFactory("EC"); + ECPublicKey publicKey = (ECPublicKey)kf.generatePublic(spec); + + // check constraints of ECPublicKey + if (!constraints.permits( + EnumSet.of(CryptoPrimitive.KEY_AGREEMENT), publicKey)) { + throw new SSLHandshakeException( + "ECPublicKey does not comply to algorithm constraints"); + } + } catch (GeneralSecurityException | java.io.IOException e) { + throw (SSLHandshakeException) new SSLHandshakeException( + "Could not generate ECPublicKey").initCause(e); + } + } + } diff --git a/jdk/src/java.base/share/classes/sun/security/ssl/Handshaker.java b/jdk/src/java.base/share/classes/sun/security/ssl/Handshaker.java index c75bf63d006..24fec1bf52b 100644 --- a/jdk/src/java.base/share/classes/sun/security/ssl/Handshaker.java +++ b/jdk/src/java.base/share/classes/sun/security/ssl/Handshaker.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 @@ -87,7 +87,7 @@ abstract class Handshaker { String identificationProtocol; // The cryptographic algorithm constraints - private AlgorithmConstraints algorithmConstraints = null; + AlgorithmConstraints algorithmConstraints = null; // Local supported signature and algorithms Collection localSupportedSignAlgs; 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 ba229f914ed..ec2e0a0ecbd 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 @@ -32,6 +32,7 @@ import java.security.*; import java.security.cert.*; import java.security.interfaces.*; import java.security.spec.ECParameterSpec; +import java.math.BigInteger; import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; @@ -1571,7 +1572,13 @@ final class ServerHandshaker extends Handshaker { if (debug != null && Debug.isOn("handshake")) { mesg.print(System.out); } - return dh.getAgreedSecret(mesg.getClientPublicKey(), false); + + BigInteger publicKeyValue = mesg.getClientPublicKey(); + + // check algorithm constraints + dh.checkConstraints(algorithmConstraints, publicKeyValue); + + return dh.getAgreedSecret(publicKeyValue, false); } private SecretKey clientKeyExchange(ECDHClientKeyExchange mesg) @@ -1580,7 +1587,13 @@ final class ServerHandshaker extends Handshaker { if (debug != null && Debug.isOn("handshake")) { mesg.print(System.out); } - return ecdh.getAgreedSecret(mesg.getEncodedPoint()); + + byte[] publicPoint = mesg.getEncodedPoint(); + + // check algorithm constraints + ecdh.checkConstraints(algorithmConstraints, publicPoint); + + return ecdh.getAgreedSecret(publicPoint); } /* diff --git a/jdk/src/java.base/share/conf/security/java.security b/jdk/src/java.base/share/conf/security/java.security index 6eda678595e..b1b616c2175 100644 --- a/jdk/src/java.base/share/conf/security/java.security +++ b/jdk/src/java.base/share/conf/security/java.security @@ -541,7 +541,7 @@ jdk.certpath.disabledAlgorithms=MD2, MD5, RSA keySize < 1024 # # Example: # jdk.tls.disabledAlgorithms=MD5, SSLv3, DSA, RSA keySize < 2048 -jdk.tls.disabledAlgorithms=SSLv3, RC4 +jdk.tls.disabledAlgorithms=SSLv3, RC4, DH keySize < 768 # Legacy algorithms for Secure Socket Layer/Transport Layer Security (SSL/TLS) # processing in JSSE implementation. @@ -580,7 +580,7 @@ jdk.tls.disabledAlgorithms=SSLv3, RC4 # 1. JSSE cipher suite name, e.g., TLS_RSA_WITH_AES_128_CBC_SHA # 2. JSSE key exchange algorithm name, e.g., RSA # 3. JSSE cipher (encryption) algorithm name, e.g., AES_128_CBC -# 4. JSSE message digest algorithm name, e.g., SHA-1 +# 4. JSSE message digest algorithm name, e.g., SHA # # See SSL/TLS specifications and "Java Cryptography Architecture Standard # Algorithm Name Documentation" for information about the algorithm names. @@ -598,4 +598,4 @@ jdk.tls.legacyAlgorithms= \ DHE_DSS_EXPORT, DHE_RSA_EXPORT, DH_anon_EXPORT, DH_DSS_EXPORT, \ DH_RSA_EXPORT, RSA_EXPORT, \ DH_anon, ECDH_anon, \ - RC4_128, RC4_40, DES_CBC, DES40_CBC \ No newline at end of file + RC4_128, RC4_40, DES_CBC, DES40_CBC diff --git a/jdk/test/javax/net/ssl/sanity/interop/ClientJSSEServerJSSE.java b/jdk/test/javax/net/ssl/sanity/interop/ClientJSSEServerJSSE.java index abce4ff1417..a837bd3e652 100644 --- a/jdk/test/javax/net/ssl/sanity/interop/ClientJSSEServerJSSE.java +++ b/jdk/test/javax/net/ssl/sanity/interop/ClientJSSEServerJSSE.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 @@ -33,13 +33,10 @@ import java.security.Security; public class ClientJSSEServerJSSE { public static void main(String[] args) throws Exception { - // reset the security property to make sure that the algorithms + // reset security properties to make sure that the algorithms // and keys used in this test are not disabled. Security.setProperty("jdk.tls.disabledAlgorithms", ""); - - // MD5 is used in this test case, don't disable MD5 algorithm. - Security.setProperty( - "jdk.certpath.disabledAlgorithms", "MD2, RSA keySize < 1024"); + Security.setProperty("jdk.certpath.disabledAlgorithms", ""); CipherTest.main(new JSSEFactory(), args); } diff --git a/jdk/test/sun/security/ec/TestEC.java b/jdk/test/sun/security/ec/TestEC.java index 62ffc076020..9418fb81da6 100644 --- a/jdk/test/sun/security/ec/TestEC.java +++ b/jdk/test/sun/security/ec/TestEC.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 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 @@ -60,13 +60,10 @@ import java.security.Security; public class TestEC { public static void main(String[] args) throws Exception { - // reset the security property to make sure that the algorithms + // reset security properties to make sure that the algorithms // and keys used in this test are not disabled. Security.setProperty("jdk.tls.disabledAlgorithms", ""); - - // MD5 is used in this test case, don't disable MD5 algorithm. - Security.setProperty( - "jdk.certpath.disabledAlgorithms", "MD2, RSA keySize < 1024"); + Security.setProperty("jdk.certpath.disabledAlgorithms", ""); ProvidersSnapshot snapshot = ProvidersSnapshot.create(); try { diff --git a/jdk/test/sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.java b/jdk/test/sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.java index 87e91e5b7a8..dea0495cd13 100644 --- a/jdk/test/sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.java +++ b/jdk/test/sun/security/pkcs11/sslecc/ClientJSSEServerJSSE.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 @@ -43,18 +43,15 @@ public class ClientJSSEServerJSSE extends PKCS11Test { private static String[] cmdArgs; public static void main(String[] args) throws Exception { - // reset the security property to make sure that the algorithms - // and keys used in this test are not disabled. - Security.setProperty("jdk.tls.disabledAlgorithms", ""); - cmdArgs = args; main(new ClientJSSEServerJSSE()); } public void main(Provider p) throws Exception { - // MD5 is used in this test case, don't disable MD5 algorithm. - Security.setProperty( - "jdk.certpath.disabledAlgorithms", "MD2, RSA keySize < 1024"); + // reset security properties to make sure that the algorithms + // and keys used in this test are not disabled. + Security.setProperty("jdk.tls.disabledAlgorithms", ""); + Security.setProperty("jdk.certpath.disabledAlgorithms", ""); if (p.getService("KeyFactory", "EC") == null) { System.out.println("Provider does not support EC, skipping"); diff --git a/jdk/test/sun/security/ssl/DHKeyExchange/DHEKeySizing.java b/jdk/test/sun/security/ssl/DHKeyExchange/DHEKeySizing.java index b94381ae629..4581d385c2c 100644 --- a/jdk/test/sun/security/ssl/DHKeyExchange/DHEKeySizing.java +++ b/jdk/test/sun/security/ssl/DHKeyExchange/DHEKeySizing.java @@ -377,9 +377,10 @@ public class DHEKeySizing { } public static void main(String args[]) throws Exception { - // reset the security property to make sure that the algorithms + // reset security properties to make sure that the algorithms // and keys used in this test are not disabled. Security.setProperty("jdk.tls.disabledAlgorithms", ""); + Security.setProperty("jdk.certpath.disabledAlgorithms", ""); if (args.length != 4) { System.out.println( diff --git a/jdk/test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java b/jdk/test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java index c546cfb7136..7196a2b017f 100644 --- a/jdk/test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java +++ b/jdk/test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 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 @@ -21,19 +21,22 @@ * questions. */ +// +// SunJSSE does not support dynamic system properties, no way to re-use +// system properties in samevm/agentvm mode. +// + /* * @test * @bug 4392475 * @summary Calling setWantClientAuth(true) disables anonymous suites * @run main/othervm/timeout=180 AnonCipherWithWantClientAuth - * - * SunJSSE does not support dynamic system properties, no way to re-use - * system properties in samevm/agentvm mode. */ import java.io.*; import java.net.*; import javax.net.ssl.*; +import java.security.Security; public class AnonCipherWithWantClientAuth { @@ -156,6 +159,11 @@ public class AnonCipherWithWantClientAuth { volatile Exception clientException = null; public static void main(String[] args) throws Exception { + // reset security properties to make sure that the algorithms + // and keys used in this test are not disabled. + Security.setProperty("jdk.tls.disabledAlgorithms", ""); + Security.setProperty("jdk.certpath.disabledAlgorithms", ""); + String keyFilename = System.getProperty("test.src", "./") + "/" + pathToStores + "/" + keyStoreFile;