From 58cc444e69adead98c25a5823860812ac328081d Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Mon, 31 Mar 2025 09:59:10 -0400 Subject: [PATCH 01/26] 8272875: Change the default key manager to PKIX --- .../share/classes/javax/net/ssl/KeyManagerFactory.java | 2 +- src/java.base/share/conf/security/java.security | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java index 652a20c837705..b591d5a30e4dc 100644 --- a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java +++ b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java @@ -64,7 +64,7 @@ public class KeyManagerFactory { public static final String getDefaultAlgorithm() { String type = Security.getProperty("ssl.KeyManagerFactory.algorithm"); if (type == null) { - type = "SunX509"; + type = "NewSunX509"; } return type; } diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 693d19438f6da..a5df0c4d2f785 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -320,7 +320,7 @@ security.overridePropertiesFile=true # Determines the default key and trust manager factory algorithms for # the javax.net.ssl package. # -ssl.KeyManagerFactory.algorithm=SunX509 +ssl.KeyManagerFactory.algorithm=NewSunX509 ssl.TrustManagerFactory.algorithm=PKIX # From c80fc6a8ab9e775b1ce456f12834e5b11ff5085d Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Thu, 17 Apr 2025 10:56:05 -0400 Subject: [PATCH 02/26] Use standard PKIX alias --- .../share/classes/javax/net/ssl/KeyManagerFactory.java | 2 +- src/java.base/share/conf/security/java.security | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java index b591d5a30e4dc..6c38e9577daf1 100644 --- a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java +++ b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java @@ -64,7 +64,7 @@ public class KeyManagerFactory { public static final String getDefaultAlgorithm() { String type = Security.getProperty("ssl.KeyManagerFactory.algorithm"); if (type == null) { - type = "NewSunX509"; + type = "PKIX"; } return type; } diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index e5826f549cfba..826e3b4a21d1d 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -320,7 +320,7 @@ security.overridePropertiesFile=true # Determines the default key and trust manager factory algorithms for # the javax.net.ssl package. # -ssl.KeyManagerFactory.algorithm=NewSunX509 +ssl.KeyManagerFactory.algorithm=PKIX ssl.TrustManagerFactory.algorithm=PKIX # From 19a2ad1d5501abbdda72dde0e833d1da8fc7856f Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 18 Apr 2025 12:56:29 -0400 Subject: [PATCH 03/26] Rework unit tests --- .../rmi/ssl/SSLSocketParametersTest.java | 34 ++-- test/jdk/javax/rmi/ssl/keystore | Bin 1364 -> 0 bytes test/jdk/javax/rmi/ssl/truststore | Bin 661 -> 0 bytes .../https/HttpsClient/ServerIdentityTest.java | 178 +++++++++++++++--- .../www/protocol/https/HttpsClient/dnsstore | Bin 1418 -> 0 bytes .../www/protocol/https/HttpsClient/ipstore | Bin 1413 -> 0 bytes .../sun/security/tools/keytool/PrintSSL.java | 6 +- .../test/lib/security/CertificateBuilder.java | 21 +++ 8 files changed, 186 insertions(+), 53 deletions(-) delete mode 100644 test/jdk/javax/rmi/ssl/keystore delete mode 100644 test/jdk/javax/rmi/ssl/truststore delete mode 100644 test/jdk/sun/net/www/protocol/https/HttpsClient/dnsstore delete mode 100644 test/jdk/sun/net/www/protocol/https/HttpsClient/ipstore diff --git a/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java b/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java index 6da3289458751..ead2473f8e361 100644 --- a/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java +++ b/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2004, 2025, 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 @@ -24,7 +24,8 @@ /* * @test * @bug 5016500 - * @library /test/lib/ + * @library /javax/net/ssl/templates + * /test/lib/ * @summary Test SslRmi[Client|Server]SocketFactory SSL socket parameters. * @run main/othervm SSLSocketParametersTest 1 * @run main/othervm SSLSocketParametersTest 2 @@ -36,8 +37,6 @@ */ import jdk.test.lib.Asserts; -import java.io.IOException; -import java.io.File; import java.io.Serializable; import java.lang.ref.Reference; import java.rmi.ConnectIOException; @@ -49,13 +48,18 @@ import javax.rmi.ssl.SslRMIClientSocketFactory; import javax.rmi.ssl.SslRMIServerSocketFactory; -public class SSLSocketParametersTest implements Serializable { +public class SSLSocketParametersTest extends SSLContextTemplate implements + Serializable { + + public SSLSocketParametersTest() throws Exception { + SSLContext.setDefault(createServerSSLContext()); + } public interface Hello extends Remote { String sayHello() throws RemoteException; } - public class HelloImpl implements Hello { + public static class HelloImpl implements Hello { public String sayHello() { return "Hello World!"; } @@ -134,23 +138,7 @@ public void runTest(int testNumber) throws Exception { } public static void main(String[] args) throws Exception { - // Set keystore properties (server-side) - // - final String keystore = System.getProperty("test.src") + - File.separator + "keystore"; - System.out.println("KeyStore = " + keystore); - System.setProperty("javax.net.ssl.keyStore", keystore); - System.setProperty("javax.net.ssl.keyStorePassword", "password"); - - // Set truststore properties (client-side) - // - final String truststore = System.getProperty("test.src") + - File.separator + "truststore"; - System.out.println("TrustStore = " + truststore); - System.setProperty("javax.net.ssl.trustStore", truststore); - System.setProperty("javax.net.ssl.trustStorePassword", "trustword"); - SSLSocketParametersTest test = new SSLSocketParametersTest(); test.runTest(Integer.parseInt(args[0])); } -} \ No newline at end of file +} diff --git a/test/jdk/javax/rmi/ssl/keystore b/test/jdk/javax/rmi/ssl/keystore deleted file mode 100644 index 05f535645827bae28d6168fe4ad38f55b4faa076..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1364 zcmezO_TO6u1_mY|W&~rFl+x@}AnVJnxrbPQ61xqWn6?}6v2kg$F|sgfF$pp@x>ho1^r&%56_HAo+kX-fS_qrLG%Yn5yK^;n~6$t&>v*xw~)X3eP!zSgL%KOtM?csA62-VTT4$i1_bl&AJ8J4Kf~ ziYq>~>~z7T=JwQ&`THIyDly((cmBuZsJWL9R9dwM*e?9V^W<2oX~rE?2@~@Z_DXF{XSl#`RkR;Szp|kbZYS*iaruy^P zST}Z@wO{qwyZOTFL-+qoSh8dGH)oMlb>{<@F6u5daCMvdGfKYnm+_yf)kmFuzP$0; zUO$K9wCIbL!f!l{|DreY@0nkf-PE&2@iS+ksfS&YmU@(7le43t zh=CADhFzFDI5DpzQNcMUu_)0{-ar;4$|WooT$-ogo0(jcUtC#SlA2qrqu`mBtY^q; zzztHsEzII&6zmG)G7Gb~0P~oEoH(z6v4M$^v7xbnk%2*!IIppRfu(^3luLgHHO@y4 z6JR!HZtP_+XzXNaY-HGfCcO30-Nk>C&t^D3kre0H7s|HG%zJ|HgZJC_FP+00a)3{8 z>dl}fM{>-U?K)u?^slWj`}isG>HM2}Hf@xwP3+FPCH2Eu?D39r?N9aJ&br)Lbi`95G5&Q? z%Y`i+Gi`!B4lzGva^Ygy+7q!NgeN#ov?O}MlGRgNvaYtgc(UBxPV3kQ^@DF}rhYtg zZtpz@_BOUjDzmggY+H9)n(a*vTbg~J;Y+sM;`SA@pBVj}A|Xg7Aa)+xIV>!y0mcPjKqZpe09g%$Myt zVHovdhjZj%K8IQPahLD-cyDL3^t-z{g1PXojrhM3%^3}^cKpjWE!;bG@5VE8^P1m# zwVl1{7ZbDj&;h%LjZ8wTCJ4`e`e`SF!W;o6r?AX#*@B;xnV1w2n|n5Gl&nqc&blS_!&&U{j&ki!_216A+*x$QQzJ3{by3TOEgds$f;|o~ zKV@>^V%pjhu_A;gI8L-Adcu;`Q(LmGw!C<<+}%#=*a!84Z)&D~JacaEJqPwSwn-|p zv_foKcUqe5O%7X{eV^e=w%p?O6|)c8LuwC@I@}FWYV3g%hzWZEXoRNOM3QpP0OFVGxC1= zZ?4UXUn&F;X;MA+(vzdSDpmd7_ASo&wqd$F=Uh(@v1!ja z8*h2e|NOg^Ntt8&?!D@X5!>ATzFa!ru}10R>bEmLW@#>1Tb7!-)6eyNNn`xPR#7|I z_Z(lo9XR{z!#+>0KT7$<%pH<$(LZOo?!Dap{~>qlrG+Y|+N)e&c2=C15{Wbyt-N(b zq2yHg6S;R2%U+j=NY=M~*}f-Sv1;GJCE|Oo#qV8v^4pJWr}JliwrXZPy8YJam~5p= zj52S|URSx@Vh+{2f*x#JARbbmI%DgadkN`pq9QvV#ZGs*#CBUjVc%;7yPErj0ta5m zg$DcGkDDSUbN8xl{;7*k`Sq`v-V@m{vCbz)cky^*L%$An_t(iDqbg$dgfj6Ew<)8f*;vm^=8eCKeY4A+z%WtT?3~{bu9e( z|EkR;!{jUJwx7OwUOFsk`^n*OuS{RxDhru=p%EXRJ$Zii%F}*H)1&O$WCV77AFSI4gbEXxh}hr5d9`LKxos=k5`gc zazG1U(Ju3HtIx^FZm9z7ewTkjG$X|+`I&VFxjie`<_Gc~XTCge_F zLT)u^VtluNnTe5!iN#vd#AN85K^Mn)D^27@X?ZUas>=1>+kVW!YvLtz6! z5QjsU%Q-P8GcCU;FEi0l$Up!j$S%z3Tw0J?RFavOZzyRX4ie)M76>lQQ}E49F3K;i zEG|jSEjAQ05CKVX3-fv7C^jfrD530@C%& zBb!4~pD=Bl=w|!ohPh&Fs!i`E|CvSgj#>|=zm7PmK9RjaYjbdAsoqVeo`{9pE!n+yoS2i5UtEIha9|WO16{29ZS{>W z-zHl9l&nm&bu)SL{_)~}9|VGTJxUKSUirHF@(HDdhtA%ZlfHpx^Ipzn_Ci`VZQX{^ zPFWJ2iPx52yK{Emr09;d_Z5Em)ZdX-Z?=hiUS+dULQCmxp;kdoV#BiCiQ6+Yu5McQ z!u#LSNbhZb6z1zbNn0zc=TYXeLVoY*u#LOlrcZA>FjG8oj=|jHdv5s}Hx*t60MwgF ATL1t6 diff --git a/test/jdk/sun/net/www/protocol/https/HttpsClient/ipstore b/test/jdk/sun/net/www/protocol/https/HttpsClient/ipstore deleted file mode 100644 index 04a9508e0a1c4472d60ffc5f2e3fcaa87812093f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1413 zcmezO_TO6u1_mY|W&~rl;?$zD)FL4J+1bf~>wr?b44Rm>8St@jX|pl1FlsRgGBUC< zurx6(5}mDfMEY-j<1~%bH$S*H`r-XsZ` z9+D8eY5i5Fx!@;v>8C9YJS`_ys=Qsv$h#u5SC+}k^R`1>@?tjCwbFMlOy%c!%Rg=D z<(X6bW8QImoc!Cn%0S}bjwrJiKjJt|ca~du3Y9GR&q9{=?AY^BzZi|%T(ZY0jGTHso*D9j~(Ev+h%{^!QV-b%6{Abm}cL1=F+#z zzN>gv-dSCF!gAf@UF)AldrtfP=G5iXHx;ZYHwq^1Uve|=cjy+s1IFF6kF4L9ANXl? zZ1?A|&ssr|mz}p~M67N}TN?7Nr%ox<>*=%ONhPU?tdFu@v(4w9kRX3bxp80k=YK+Z zjBiSHijPQY_8H0a$93mp^ZzJZ-V#*t=)vw8Ij`(C$!-n1 z?7(zi^2rB}DJIINRomsO&i|cz|Z3)qnZoOTaZ!9rOKkcco#?IYQMjuWr zEH+qm#=xt;`=k4J|Ep7tg{Dnf&VJHtq5K+aGd4s26vZn>PHRKDD>r9+lJu;ts+zv+ z!%CLxJ3hR*GEL)fcPi`0UZvyJbItxscdED@C`fu@syBaT>$lEb(`JHGG;4&OsevUh zA-4b%a)Ut=IJMe5+P?ELGP1BT7*rW@8*s8QhqABsgiezN4lR&yo4*+nZqh1wCbQ!cJ?a_RkEyDl)Oe zKXL}^70!giL5(3VUaUKO(LcZpNdJu diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 7cdc0a4577104..70c0fafac02ee 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2025, 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 @@ -53,8 +53,8 @@ public static void main(String[] args) throws Throwable { // make sure that "-printcert" works with weak algorithms OutputAnalyzer out = SecurityTools.keytool("-genkeypair " + "-keystore keystore -storepass passphrase " - + "-keypass passphrase -keyalg rsa -keysize 1024 " - + "-sigalg MD5withRSA -alias rsa_alias -dname CN=Server"); + + "-keypass passphrase -keyalg rsa -keysize 2048 " + + "-sigalg SHA256withRSA -alias rsa_alias -dname CN=Server"); System.out.println(out.getOutput()); out.shouldHaveExitValue(0); diff --git a/test/lib/jdk/test/lib/security/CertificateBuilder.java b/test/lib/jdk/test/lib/security/CertificateBuilder.java index 60358c9a4eabf..c86fe1049200c 100644 --- a/test/lib/jdk/test/lib/security/CertificateBuilder.java +++ b/test/lib/jdk/test/lib/security/CertificateBuilder.java @@ -43,6 +43,7 @@ import sun.security.x509.AlgorithmId; import sun.security.x509.AuthorityInfoAccessExtension; import sun.security.x509.AuthorityKeyIdentifierExtension; +import sun.security.x509.IPAddressName; import sun.security.x509.SubjectKeyIdentifierExtension; import sun.security.x509.BasicConstraintsExtension; import sun.security.x509.CertificateSerialNumber; @@ -233,6 +234,26 @@ public CertificateBuilder addSubjectAltNameDNSExt(List dnsNames) return this; } + /** + * Helper method to add IPAddress types for the SAN extension + * + * @param IPAddresses A {@code List} of names to add as IPAddress + * types + * @throws IOException if an encoding error occurs. + */ + public CertificateBuilder addSubjectAltNameIPExt(List IPAddresses) + throws IOException { + if (!IPAddresses.isEmpty()) { + GeneralNames gNames = new GeneralNames(); + for (String name : IPAddresses) { + gNames.add(new GeneralName(new IPAddressName(name))); + } + addExtension(new SubjectAlternativeNameExtension(false, + gNames)); + } + return this; + } + /** * Helper method to add one or more OCSP URIs to the Authority Info Access * certificate extension. Location strings can be in two forms: From e5e83514e3af4db23dde0143f109a111a090b7fd Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Thu, 24 Apr 2025 14:21:26 -0400 Subject: [PATCH 04/26] Skip explicit KeyPair initialization and let the provider default set it --- .../net/www/protocol/https/HttpsClient/ServerIdentityTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java b/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java index 5164acd66a5e2..5803da2510a6d 100644 --- a/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java +++ b/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java @@ -144,7 +144,6 @@ private static void initialize(String[] args) throws Exception { hostname = args[1]; KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); - kpg.initialize(2048); KeyPair caKeys = kpg.generateKeyPair(); KeyPair serverKeys = kpg.generateKeyPair(); KeyPair clientKeys = kpg.generateKeyPair(); From 2b0c5525262ca1f87a956d98ab5900d6ad137189 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Tue, 29 Apr 2025 17:47:45 -0400 Subject: [PATCH 05/26] Address review comments --- test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java | 3 +-- test/jdk/sun/security/tools/keytool/PrintSSL.java | 10 ++++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java b/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java index ead2473f8e361..3aa7a98c39403 100644 --- a/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java +++ b/test/jdk/javax/rmi/ssl/SSLSocketParametersTest.java @@ -48,8 +48,7 @@ import javax.rmi.ssl.SslRMIClientSocketFactory; import javax.rmi.ssl.SslRMIServerSocketFactory; -public class SSLSocketParametersTest extends SSLContextTemplate implements - Serializable { +public class SSLSocketParametersTest extends SSLContextTemplate { public SSLSocketParametersTest() throws Exception { SSLContext.setDefault(createServerSSLContext()); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 70c0fafac02ee..0a29b83159cc3 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -39,6 +39,7 @@ import java.net.ServerSocket; import java.nio.file.Files; import java.nio.file.Paths; +import java.security.Security; import java.util.concurrent.CountDownLatch; import javax.net.ssl.SSLServerSocketFactory; import javax.net.ssl.SSLSocket; @@ -48,13 +49,18 @@ public class PrintSSL { public static void main(String[] args) throws Throwable { + // Using "SunX509" KeyManager which doesn't check peer supported + // signature algorithms, so we can make keytool print certificate + // with weak MD5withRSA signature algorithm. + Security.setProperty("ssl.KeyManagerFactory.algorithm", "SunX509"); + Files.deleteIfExists(Paths.get("keystore")); // make sure that "-printcert" works with weak algorithms OutputAnalyzer out = SecurityTools.keytool("-genkeypair " + "-keystore keystore -storepass passphrase " - + "-keypass passphrase -keyalg rsa -keysize 2048 " - + "-sigalg SHA256withRSA -alias rsa_alias -dname CN=Server"); + + "-keypass passphrase -keyalg rsa -keysize 1024 " + + "-sigalg MD5withRSA -alias rsa_alias -dname CN=Server"); System.out.println(out.getOutput()); out.shouldHaveExitValue(0); From 211f0a606e5381876a9aaaca27c0e4cf8b10bfbf Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 2 May 2025 15:04:45 -0400 Subject: [PATCH 06/26] 8353113: Peer supported certificate signature algorithms are not being checked with default SunX509 key manager --- .../ssl/KeyManagerAlgorithmConstraints.java | 319 +++++++++++++ .../security/ssl/KeyManagerFactoryImpl.java | 24 +- .../classes/sun/security/ssl/SunJSSE.java | 2 + .../ssl/SunX509ConstraintsKeyManagerImpl.java | 448 ++++++++++++++++++ .../security/ssl/SunX509KeyManagerImpl.java | 411 +--------------- .../sun/security/ssl/X509KeyManagerImpl.java | 274 +---------- .../bench/java/security/SSLHandshake.java | 8 +- 7 files changed, 808 insertions(+), 678 deletions(-) create mode 100644 src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java create mode 100644 src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java diff --git a/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java new file mode 100644 index 0000000000000..16b6ed95ab093 --- /dev/null +++ b/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java @@ -0,0 +1,319 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package sun.security.ssl; + +import java.net.Socket; +import java.security.AlgorithmConstraints; +import java.security.cert.CertPathValidatorException; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Collections; +import java.util.Date; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import javax.net.ssl.ExtendedSSLSession; +import javax.net.ssl.SNIHostName; +import javax.net.ssl.SNIServerName; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.StandardConstants; +import sun.security.provider.certpath.AlgorithmChecker; +import sun.security.util.KnownOIDs; +import sun.security.validator.Validator; + +interface KeyManagerAlgorithmConstraints { + + // Gets algorithm constraints of the socket. + default AlgorithmConstraints getAlgorithmConstraints(Socket socket) { + if (socket != null && socket.isConnected() && + socket instanceof SSLSocket sslSocket) { + + SSLSession session = sslSocket.getHandshakeSession(); + + if (session != null) { + if (ProtocolVersion.useTLS12PlusSpec(session.getProtocol())) { + String[] peerSupportedSignAlgs = null; + + if (session instanceof ExtendedSSLSession extSession) { + peerSupportedSignAlgs = + extSession.getPeerSupportedSignatureAlgorithms(); + } + + return SSLAlgorithmConstraints.forSocket( + sslSocket, peerSupportedSignAlgs, true); + } + } + + return SSLAlgorithmConstraints.forSocket(sslSocket, true); + } + + return SSLAlgorithmConstraints.DEFAULT; + } + + // Gets algorithm constraints of the engine. + default AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { + if (engine != null) { + SSLSession session = engine.getHandshakeSession(); + if (session != null) { + if (ProtocolVersion.useTLS12PlusSpec(session.getProtocol())) { + String[] peerSupportedSignAlgs = null; + + if (session instanceof ExtendedSSLSession extSession) { + peerSupportedSignAlgs = + extSession.getPeerSupportedSignatureAlgorithms(); + } + + return SSLAlgorithmConstraints.forEngine( + engine, peerSupportedSignAlgs, true); + } + } + } + + return SSLAlgorithmConstraints.forEngine(engine, true); + } + + default boolean conformsToAlgorithmConstraints( + AlgorithmConstraints constraints, Certificate[] chain, + String variant) { + + AlgorithmChecker checker = new AlgorithmChecker(constraints, variant); + try { + checker.init(false); + } catch (CertPathValidatorException cpve) { + // unlikely to happen + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine( + "Cannot initialize algorithm constraints checker", + cpve); + } + + return false; + } + + // It is a forward checker, so we need to check from trust to target. + for (int i = chain.length - 1; i >= 0; i--) { + Certificate cert = chain[i]; + try { + // We don't care about the unresolved critical extensions. + checker.check(cert, Collections.emptySet()); + } catch (CertPathValidatorException cpve) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Certificate does not conform to " + + "algorithm constraints", cert, cpve); + } + + return false; + } + } + + return true; + } + + // enum for the result of the extension check + // NOTE: the order of the constants is important as they are used + // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH + enum CheckResult { + OK, // ok or not checked + INSENSITIVE, // server name indication insensitive + EXPIRED, // extensions valid but cert expired + EXTENSION_MISMATCH, // extensions invalid (expiration not checked) + } + + // enum for the type of certificate check we want to perform + // (client or server) + // also includes the check code itself + enum CheckType { + + // enum constant for "no check" (currently not used) + NONE(Collections.emptySet()), + + // enum constant for "tls client" check + // valid EKU for TLS client: any, tls_client + CLIENT(new HashSet<>(List.of( + KnownOIDs.anyExtendedKeyUsage.value(), + KnownOIDs.clientAuth.value() + ))), + + // enum constant for "tls server" check + // valid EKU for TLS server: any, tls_server, ns_sgc, ms_sgc + SERVER(new HashSet<>(List.of( + KnownOIDs.anyExtendedKeyUsage.value(), + KnownOIDs.serverAuth.value(), + KnownOIDs.NETSCAPE_ExportApproved.value(), + KnownOIDs.MICROSOFT_ExportApproved.value() + ))); + + // set of valid EKU values for this type + final Set validEku; + + CheckType(Set validEku) { + this.validEku = validEku; + } + + private static boolean getBit(boolean[] keyUsage, int bit) { + return (bit < keyUsage.length) && keyUsage[bit]; + } + + // Check if this certificate is appropriate for this type of use + // first check extensions, if they match, check expiration. + // + // Note: we may want to move this code into the sun.security.validator + // package + CheckResult check(X509Certificate cert, Date date, + List serverNames, String idAlgorithm) { + + if (this == NONE) { + return CheckResult.OK; + } + + // check extensions + try { + // check extended key usage + List certEku = cert.getExtendedKeyUsage(); + if ((certEku != null) && + Collections.disjoint(validEku, certEku)) { + // if extension is present and does not contain any of + // the valid EKU OIDs, return extension_mismatch + return CheckResult.EXTENSION_MISMATCH; + } + + // check key usage + boolean[] ku = cert.getKeyUsage(); + if (ku != null) { + String algorithm = cert.getPublicKey().getAlgorithm(); + boolean supportsDigitalSignature = getBit(ku, 0); + switch (algorithm) { + case "RSA": + // require either signature bit + // or if server also allow key encipherment bit + if (!supportsDigitalSignature) { + if (this == CLIENT || !getBit(ku, 2)) { + return CheckResult.EXTENSION_MISMATCH; + } + } + break; + case "RSASSA-PSS": + if (!supportsDigitalSignature && (this == SERVER)) { + return CheckResult.EXTENSION_MISMATCH; + } + break; + case "DSA": + // require signature bit + if (!supportsDigitalSignature) { + return CheckResult.EXTENSION_MISMATCH; + } + break; + case "DH": + // require keyagreement bit + if (!getBit(ku, 4)) { + return CheckResult.EXTENSION_MISMATCH; + } + break; + case "EC": + // require signature bit + if (!supportsDigitalSignature) { + return CheckResult.EXTENSION_MISMATCH; + } + // For servers, also require key agreement. + // This is not totally accurate as the keyAgreement + // bit is only necessary for static ECDH key + // exchange and not ephemeral ECDH. We leave it in + // for now until there are signs that this check + // causes problems for real world EC certificates. + if (this == SERVER && !getBit(ku, 4)) { + return CheckResult.EXTENSION_MISMATCH; + } + break; + } + } + } catch (CertificateException e) { + // extensions unparseable, return failure + return CheckResult.EXTENSION_MISMATCH; + } + + try { + cert.checkValidity(date); + } catch (CertificateException e) { + return CheckResult.EXPIRED; + } + + if (serverNames != null && !serverNames.isEmpty()) { + for (SNIServerName serverName : serverNames) { + if (serverName.getType() == + StandardConstants.SNI_HOST_NAME) { + if (!(serverName instanceof SNIHostName)) { + try { + serverName = + new SNIHostName(serverName.getEncoded()); + } catch (IllegalArgumentException iae) { + // unlikely to happen, just in case ... + if (SSLLogger.isOn && + SSLLogger.isOn("keymanager")) { + SSLLogger.fine( + "Illegal server name: " + serverName); + } + + return CheckResult.INSENSITIVE; + } + } + String hostname = + ((SNIHostName)serverName).getAsciiName(); + + try { + X509TrustManagerImpl.checkIdentity(hostname, + cert, idAlgorithm); + } catch (CertificateException e) { + if (SSLLogger.isOn && + SSLLogger.isOn("keymanager")) { + SSLLogger.fine( + "Certificate identity does not match " + + "Server Name Indication (SNI): " + + hostname); + } + return CheckResult.INSENSITIVE; + } + + break; + } + } + } + + return CheckResult.OK; + } + + public String getValidator() { + if (this == CLIENT) { + return Validator.VAR_TLS_CLIENT; + } else if (this == SERVER) { + return Validator.VAR_TLS_SERVER; + } + return Validator.VAR_GENERIC; + } + } +} diff --git a/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java b/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java index 95afc6cfe0c35..6c6d46d105b50 100644 --- a/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java @@ -54,7 +54,27 @@ protected KeyManager[] engineGetKeyManagers() { return new KeyManager[] { keyManager }; } - // Factory for the SunX509 keymanager + // Factory for the SunX509c key manager + public static final class SunX509c extends KeyManagerFactoryImpl { + + @Override + protected void engineInit(KeyStore ks, char[] password) throws + KeyStoreException, NoSuchAlgorithmException, + UnrecoverableKeyException { + keyManager = new SunX509ConstraintsKeyManagerImpl(ks, password); + isInitialized = true; + } + + @Override + protected void engineInit(ManagerFactoryParameters spec) throws + InvalidAlgorithmParameterException { + throw new InvalidAlgorithmParameterException( + "SunX509ConstraintsKeyManager does not use ManagerFactoryParameters"); + } + + } + + // Factory for the SunX509 key manager public static final class SunX509 extends KeyManagerFactoryImpl { @Override @@ -74,7 +94,7 @@ protected void engineInit(ManagerFactoryParameters spec) throws } - // Factory for the X509 keymanager + // Factory for the X509 key manager public static final class X509 extends KeyManagerFactoryImpl { @Override diff --git a/src/java.base/share/classes/sun/security/ssl/SunJSSE.java b/src/java.base/share/classes/sun/security/ssl/SunJSSE.java index 611a6588b4c28..543e799ab1e3d 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunJSSE.java +++ b/src/java.base/share/classes/sun/security/ssl/SunJSSE.java @@ -55,6 +55,8 @@ private void registerAlgorithms() { ps("Signature", "MD5andSHA1withRSA", "sun.security.ssl.RSASignature", null, null); + ps("KeyManagerFactory", "SunX509c", + "sun.security.ssl.KeyManagerFactoryImpl$SunX509c", null, null); ps("KeyManagerFactory", "SunX509", "sun.security.ssl.KeyManagerFactoryImpl$SunX509", null, null); ps("KeyManagerFactory", "NewSunX509", diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java new file mode 100644 index 0000000000000..f67fe45b61bcc --- /dev/null +++ b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java @@ -0,0 +1,448 @@ +/* + * Copyright (c) 1999, 2022, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package sun.security.ssl; + +import java.net.Socket; +import java.security.AlgorithmConstraints; +import java.security.Key; +import java.security.KeyStore; +import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; +import java.security.Principal; +import java.security.PrivateKey; +import java.security.UnrecoverableKeyException; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.X509ExtendedKeyManager; +import javax.security.auth.x500.X500Principal; + + +/** + * An implementation of X509KeyManager backed by a KeyStore. + * + * The backing KeyStore is inspected when this object is constructed. + * All key entries containing a PrivateKey and a non-empty chain of + * X509Certificate are then copied into an internal store. This means + * that subsequent modifications of the KeyStore have no effect on the + * X509KeyManagerImpl object. + * + * Note that this class assumes that all keys are protected by the same + * password. + * + * The JSSE handshake code currently calls into this class via + * chooseClientAlias() and chooseServerAlias() to find the certificates to + * use. As implemented here, both always return the first alias returned by + * getClientAliases() and getServerAliases(). In turn, these methods are + * implemented by calling getAliases(), which performs the actual lookup. + * + * Note that this class currently implements no checking of the local + * certificates. In particular, it is *not* guaranteed that: + * . the certificates are within their validity period and not revoked + * . the signatures verify + * . they form a PKIX compliant chain. + * . the certificate extensions allow the certificate to be used for + * the desired purpose. + * + * Chains that fail any of these criteria will probably be rejected by + * the remote peer. + * + */ +class SunX509ConstraintsKeyManagerImpl extends X509ExtendedKeyManager + implements KeyManagerAlgorithmConstraints { + + private static final String[] STRING0 = new String[0]; + + /* + * The credentials from the KeyStore as + * Map: String(alias) -> X509Credentials(credentials) + */ + private final Map credentialsMap; + + /* + * Cached server aliases for the case issuers == null. + * (in the current JSSE implementation, issuers are always null for + * server certs). See chooseServerAlias() for details. + * + * Map: String(keyType) -> String[](alias) + */ + private final Map serverAliasCache; + + /* + * Basic container for credentials implemented as an inner class. + */ + private static class X509Credentials { + final PrivateKey privateKey; + final X509Certificate[] certificates; + private final Set issuerX500Principals; + + X509Credentials(PrivateKey privateKey, X509Certificate[] certificates) { + // assert privateKey and certificates != null + this.privateKey = privateKey; + this.certificates = certificates; + this.issuerX500Principals = HashSet.newHashSet(certificates.length); + for (X509Certificate certificate : certificates) { + issuerX500Principals.add(certificate.getIssuerX500Principal()); + } + } + + Set getIssuerX500Principals() { + return issuerX500Principals; + } + } + + SunX509ConstraintsKeyManagerImpl(KeyStore ks, char[] password) + throws KeyStoreException, + NoSuchAlgorithmException, UnrecoverableKeyException { + + credentialsMap = new HashMap<>(); + serverAliasCache = Collections.synchronizedMap( + new HashMap<>()); + if (ks == null) { + return; + } + + for (Enumeration aliases = ks.aliases(); + aliases.hasMoreElements(); ) { + String alias = aliases.nextElement(); + if (!ks.isKeyEntry(alias)) { + continue; + } + Key key = ks.getKey(alias, password); + if (!(key instanceof PrivateKey)) { + continue; + } + Certificate[] certs = ks.getCertificateChain(alias); + if ((certs == null) || (certs.length == 0) || + !(certs[0] instanceof X509Certificate)) { + continue; + } + if (!(certs instanceof X509Certificate[])) { + Certificate[] tmp = new X509Certificate[certs.length]; + System.arraycopy(certs, 0, tmp, 0, certs.length); + certs = tmp; + } + + X509Credentials cred = new X509Credentials((PrivateKey)key, + (X509Certificate[])certs); + credentialsMap.put(alias, cred); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("found key for : " + alias, (Object[])certs); + } + } + } + + /* + * Returns the certificate chain associated with the given alias. + * + * @return the certificate chain (ordered with the user's certificate first + * and the root certificate authority last) + */ + @Override + public X509Certificate[] getCertificateChain(String alias) { + if (alias == null) { + return null; + } + X509Credentials cred = credentialsMap.get(alias); + if (cred == null) { + return null; + } else { + return cred.certificates.clone(); + } + } + + /* + * Returns the key associated with the given alias + */ + @Override + public PrivateKey getPrivateKey(String alias) { + if (alias == null) { + return null; + } + X509Credentials cred = credentialsMap.get(alias); + if (cred == null) { + return null; + } else { + return cred.privateKey; + } + } + + /* + * Choose an alias to authenticate the client side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String chooseClientAlias(String[] keyTypes, Principal[] issuers, + Socket socket) { + return chooseClientAlias(keyTypes, issuers, + getAlgorithmConstraints(socket)); + } + + /* + * Choose an alias to authenticate the client side of an + * SSLEngine connection given the public key type + * and the list of certificate issuer authorities recognized by + * the peer (if any). + * + * @since 1.5 + */ + @Override + public String chooseEngineClientAlias(String[] keyType, + Principal[] issuers, SSLEngine engine) { + return chooseClientAlias( + keyType, issuers, getAlgorithmConstraints(engine)); + } + + private String chooseClientAlias(String[] keyTypes, Principal[] issuers, + AlgorithmConstraints constraints) { + + if (keyTypes == null) { + return null; + } + + for (String keyType : keyTypes) { + String[] aliases = getAliases( + keyType, issuers, CheckType.CLIENT, constraints); + + if ((aliases != null) && (aliases.length > 0)) { + return aliases[0]; + } + } + + return null; + } + + /* + * Choose an alias to authenticate the server side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String chooseServerAlias(String keyType, + Principal[] issuers, Socket socket) { + return chooseServerAlias( + keyType, issuers, getAlgorithmConstraints(socket)); + } + + /* + * Choose an alias to authenticate the server side of an + * SSLEngine connection given the public key type + * and the list of certificate issuer authorities recognized by + * the peer (if any). + * + * @since 1.5 + */ + @Override + public String chooseEngineServerAlias(String keyType, + Principal[] issuers, SSLEngine engine) { + return chooseServerAlias( + keyType, issuers, getAlgorithmConstraints(engine)); + } + + private String chooseServerAlias(String keyType, + Principal[] issuers, AlgorithmConstraints constraints) { + + if (keyType == null) { + return null; + } + + String[] aliases; + + if (issuers == null || issuers.length == 0) { + aliases = serverAliasCache.get(keyType); + if (aliases == null) { + aliases = getAliases(keyType, issuers, + CheckType.SERVER, constraints); + // Cache the result (positive and negative lookups) + if (aliases == null) { + aliases = STRING0; + } + serverAliasCache.put(keyType, aliases); + } + } else { + aliases = getServerAliases(keyType, issuers); + } + + if ((aliases != null) && (aliases.length > 0)) { + return aliases[0]; + } + + return null; + } + + /* + * Get the matching aliases for authenticating the client side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String[] getClientAliases(String keyType, Principal[] issuers) { + return getAliases(keyType, issuers, CheckType.CLIENT, null); + } + + /* + * Get the matching aliases for authenticating the server side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String[] getServerAliases(String keyType, Principal[] issuers) { + return getAliases(keyType, issuers, CheckType.SERVER, null); + } + + /* + * Get the matching aliases for authenticating the either side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + * + * Issuers come to us in the form of X500Principal[]. + */ + private String[] getAliases(String keyType, Principal[] issuers, + CheckType checkType, AlgorithmConstraints constraints) { + if (keyType == null) { + return null; + } + if (issuers == null) { + issuers = new X500Principal[0]; + } + if (!(issuers instanceof X500Principal[])) { + // normally, this will never happen but try to recover if it does + issuers = convertPrincipals(issuers); + } + String sigType; + if (keyType.contains("_")) { + int k = keyType.indexOf('_'); + sigType = keyType.substring(k + 1); + keyType = keyType.substring(0, k); + } else { + sigType = null; + } + + X500Principal[] x500Issuers = (X500Principal[])issuers; + // the algorithm below does not produce duplicates, so avoid Set + List aliases = new ArrayList<>(); + + for (Map.Entry entry : + credentialsMap.entrySet()) { + + String alias = entry.getKey(); + X509Credentials credentials = entry.getValue(); + X509Certificate[] certs = credentials.certificates; + + if (!keyType.equals(certs[0].getPublicKey().getAlgorithm())) { + continue; + } + + if (sigType != null) { + if (certs.length > 1) { + // if possible, check the public key in the issuer cert + if (!sigType.equals( + certs[1].getPublicKey().getAlgorithm())) { + continue; + } + } else { + // Check the signature algorithm of the certificate itself. + // Look for the "withRSA" in "SHA1withRSA", etc. + String sigAlgName = + certs[0].getSigAlgName().toUpperCase(Locale.ENGLISH); + String pattern = "WITH" + + sigType.toUpperCase(Locale.ENGLISH); + if (!sigAlgName.contains(pattern)) { + continue; + } + } + + // check the algorithm constraints + if (constraints != null && + !conformsToAlgorithmConstraints(constraints, certs, + checkType.getValidator())) { + + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": certificate list does not conform to " + + "algorithm constraints"); + } + + continue; + } + } + + if (issuers.length == 0) { + // no issuer specified, match all + aliases.add(alias); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("matching alias: " + alias); + } + } else { + Set certIssuers = + credentials.getIssuerX500Principals(); + for (int i = 0; i < x500Issuers.length; i++) { + if (certIssuers.contains(issuers[i])) { + aliases.add(alias); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("matching alias: " + alias); + } + break; + } + } + } + } + + String[] aliasStrings = aliases.toArray(STRING0); + return ((aliasStrings.length == 0) ? null : aliasStrings); + } + + /* + * Convert an array of Principals to an array of X500Principals, if + * possible. Principals that cannot be converted are ignored. + */ + private static X500Principal[] convertPrincipals(Principal[] principals) { + List list = new ArrayList<>(principals.length); + for (int i = 0; i < principals.length; i++) { + Principal p = principals[i]; + if (p instanceof X500Principal) { + list.add((X500Principal)p); + } else { + try { + list.add(new X500Principal(p.getName())); + } catch (IllegalArgumentException e) { + // ignore + } + } + } + return list.toArray(new X500Principal[0]); + } +} diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index 80eb70984254a..fe3bb7b247532 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -1,429 +1,36 @@ -/* - * Copyright (c) 1999, 2022, 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - package sun.security.ssl; import java.net.Socket; -import java.security.Key; +import java.security.AlgorithmConstraints; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; -import java.security.Principal; -import java.security.PrivateKey; import java.security.UnrecoverableKeyException; import java.security.cert.Certificate; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; import javax.net.ssl.SSLEngine; -import javax.net.ssl.X509ExtendedKeyManager; -import javax.security.auth.x500.X500Principal; - - -/** - * An implementation of X509KeyManager backed by a KeyStore. - * - * The backing KeyStore is inspected when this object is constructed. - * All key entries containing a PrivateKey and a non-empty chain of - * X509Certificate are then copied into an internal store. This means - * that subsequent modifications of the KeyStore have no effect on the - * X509KeyManagerImpl object. - * - * Note that this class assumes that all keys are protected by the same - * password. - * - * The JSSE handshake code currently calls into this class via - * chooseClientAlias() and chooseServerAlias() to find the certificates to - * use. As implemented here, both always return the first alias returned by - * getClientAliases() and getServerAliases(). In turn, these methods are - * implemented by calling getAliases(), which performs the actual lookup. - * - * Note that this class currently implements no checking of the local - * certificates. In particular, it is *not* guaranteed that: - * . the certificates are within their validity period and not revoked - * . the signatures verify - * . they form a PKIX compliant chain. - * . the certificate extensions allow the certificate to be used for - * the desired purpose. - * - * Chains that fail any of these criteria will probably be rejected by - * the remote peer. - * - */ -final class SunX509KeyManagerImpl extends X509ExtendedKeyManager { - - private static final String[] STRING0 = new String[0]; - - /* - * The credentials from the KeyStore as - * Map: String(alias) -> X509Credentials(credentials) - */ - private final Map credentialsMap; - - /* - * Cached server aliases for the case issuers == null. - * (in the current JSSE implementation, issuers are always null for - * server certs). See chooseServerAlias() for details. - * - * Map: String(keyType) -> String[](alias) - */ - private final Map serverAliasCache; - /* - * Basic container for credentials implemented as an inner class. - */ - private static class X509Credentials { - final PrivateKey privateKey; - final X509Certificate[] certificates; - private final Set issuerX500Principals; - - X509Credentials(PrivateKey privateKey, X509Certificate[] certificates) { - // assert privateKey and certificates != null - this.privateKey = privateKey; - this.certificates = certificates; - this.issuerX500Principals = HashSet.newHashSet(certificates.length); - for (X509Certificate certificate : certificates) { - issuerX500Principals.add(certificate.getIssuerX500Principal()); - } - } - - Set getIssuerX500Principals() { - return issuerX500Principals; - } - } +public class SunX509KeyManagerImpl extends SunX509ConstraintsKeyManagerImpl { SunX509KeyManagerImpl(KeyStore ks, char[] password) throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { - - credentialsMap = new HashMap<>(); - serverAliasCache = Collections.synchronizedMap( - new HashMap<>()); - if (ks == null) { - return; - } - - for (Enumeration aliases = ks.aliases(); - aliases.hasMoreElements(); ) { - String alias = aliases.nextElement(); - if (!ks.isKeyEntry(alias)) { - continue; - } - Key key = ks.getKey(alias, password); - if (!(key instanceof PrivateKey)) { - continue; - } - Certificate[] certs = ks.getCertificateChain(alias); - if ((certs == null) || (certs.length == 0) || - !(certs[0] instanceof X509Certificate)) { - continue; - } - if (!(certs instanceof X509Certificate[])) { - Certificate[] tmp = new X509Certificate[certs.length]; - System.arraycopy(certs, 0, tmp, 0, certs.length); - certs = tmp; - } - - X509Credentials cred = new X509Credentials((PrivateKey)key, - (X509Certificate[])certs); - credentialsMap.put(alias, cred); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("found key for : " + alias, (Object[])certs); - } - } - } - - /* - * Returns the certificate chain associated with the given alias. - * - * @return the certificate chain (ordered with the user's certificate first - * and the root certificate authority last) - */ - @Override - public X509Certificate[] getCertificateChain(String alias) { - if (alias == null) { - return null; - } - X509Credentials cred = credentialsMap.get(alias); - if (cred == null) { - return null; - } else { - return cred.certificates.clone(); - } - } - - /* - * Returns the key associated with the given alias - */ - @Override - public PrivateKey getPrivateKey(String alias) { - if (alias == null) { - return null; - } - X509Credentials cred = credentialsMap.get(alias); - if (cred == null) { - return null; - } else { - return cred.privateKey; - } + super(ks, password); } - /* - * Choose an alias to authenticate the client side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ @Override - public String chooseClientAlias(String[] keyTypes, Principal[] issuers, - Socket socket) { - /* - * We currently don't do anything with socket, but - * someday we might. It might be a useful hint for - * selecting one of the aliases we get back from - * getClientAliases(). - */ - - if (keyTypes == null) { - return null; - } - - for (int i = 0; i < keyTypes.length; i++) { - String[] aliases = getClientAliases(keyTypes[i], issuers); - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } - } + public AlgorithmConstraints getAlgorithmConstraints(Socket socket) { return null; } - /* - * Choose an alias to authenticate the client side of an - * SSLEngine connection given the public key type - * and the list of certificate issuer authorities recognized by - * the peer (if any). - * - * @since 1.5 - */ @Override - public String chooseEngineClientAlias(String[] keyType, - Principal[] issuers, SSLEngine engine) { - /* - * If we ever start using socket as a selection criteria, - * we'll need to adjust this. - */ - return chooseClientAlias(keyType, issuers, null); - } - - /* - * Choose an alias to authenticate the server side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String chooseServerAlias(String keyType, - Principal[] issuers, Socket socket) { - /* - * We currently don't do anything with socket, but - * someday we might. It might be a useful hint for - * selecting one of the aliases we get back from - * getServerAliases(). - */ - if (keyType == null) { - return null; - } - - String[] aliases; - - if (issuers == null || issuers.length == 0) { - aliases = serverAliasCache.get(keyType); - if (aliases == null) { - aliases = getServerAliases(keyType, issuers); - // Cache the result (positive and negative lookups) - if (aliases == null) { - aliases = STRING0; - } - serverAliasCache.put(keyType, aliases); - } - } else { - aliases = getServerAliases(keyType, issuers); - } - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } + public AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { return null; } - /* - * Choose an alias to authenticate the server side of an - * SSLEngine connection given the public key type - * and the list of certificate issuer authorities recognized by - * the peer (if any). - * - * @since 1.5 - */ - @Override - public String chooseEngineServerAlias(String keyType, - Principal[] issuers, SSLEngine engine) { - /* - * If we ever start using socket as a selection criteria, - * we'll need to adjust this. - */ - return chooseServerAlias(keyType, issuers, null); - } - - /* - * Get the matching aliases for authenticating the client side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String[] getClientAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers); - } - - /* - * Get the matching aliases for authenticating the server side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ @Override - public String[] getServerAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers); - } - - /* - * Get the matching aliases for authenticating the either side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - * - * Issuers come to us in the form of X500Principal[]. - */ - private String[] getAliases(String keyType, Principal[] issuers) { - if (keyType == null) { - return null; - } - if (issuers == null) { - issuers = new X500Principal[0]; - } - if (!(issuers instanceof X500Principal[])) { - // normally, this will never happen but try to recover if it does - issuers = convertPrincipals(issuers); - } - String sigType; - if (keyType.contains("_")) { - int k = keyType.indexOf('_'); - sigType = keyType.substring(k + 1); - keyType = keyType.substring(0, k); - } else { - sigType = null; - } - - X500Principal[] x500Issuers = (X500Principal[])issuers; - // the algorithm below does not produce duplicates, so avoid Set - List aliases = new ArrayList<>(); - - for (Map.Entry entry : - credentialsMap.entrySet()) { - - String alias = entry.getKey(); - X509Credentials credentials = entry.getValue(); - X509Certificate[] certs = credentials.certificates; - - if (!keyType.equals(certs[0].getPublicKey().getAlgorithm())) { - continue; - } - if (sigType != null) { - if (certs.length > 1) { - // if possible, check the public key in the issuer cert - if (!sigType.equals( - certs[1].getPublicKey().getAlgorithm())) { - continue; - } - } else { - // Check the signature algorithm of the certificate itself. - // Look for the "withRSA" in "SHA1withRSA", etc. - String sigAlgName = - certs[0].getSigAlgName().toUpperCase(Locale.ENGLISH); - String pattern = "WITH" + - sigType.toUpperCase(Locale.ENGLISH); - if (!sigAlgName.contains(pattern)) { - continue; - } - } - } - - if (issuers.length == 0) { - // no issuer specified, match all - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - } else { - Set certIssuers = - credentials.getIssuerX500Principals(); - for (int i = 0; i < x500Issuers.length; i++) { - if (certIssuers.contains(issuers[i])) { - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - break; - } - } - } - } - - String[] aliasStrings = aliases.toArray(STRING0); - return ((aliasStrings.length == 0) ? null : aliasStrings); - } - - /* - * Convert an array of Principals to an array of X500Principals, if - * possible. Principals that cannot be converted are ignored. - */ - private static X500Principal[] convertPrincipals(Principal[] principals) { - List list = new ArrayList<>(principals.length); - for (int i = 0; i < principals.length; i++) { - Principal p = principals[i]; - if (p instanceof X500Principal) { - list.add((X500Principal)p); - } else { - try { - list.add(new X500Principal(p.getName())); - } catch (IllegalArgumentException e) { - // ignore - } - } - } - return list.toArray(new X500Principal[0]); + public boolean conformsToAlgorithmConstraints( + AlgorithmConstraints constraints, Certificate[] chain, + String variant) { + return true; } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index fffbddf7d121c..444846d1f7ad3 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -34,16 +34,11 @@ import java.security.KeyStore.PrivateKeyEntry; import java.security.Principal; import java.security.PrivateKey; -import java.security.cert.CertPathValidatorException; import java.security.cert.Certificate; -import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.*; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.*; -import sun.security.provider.certpath.AlgorithmChecker; -import sun.security.validator.Validator; -import sun.security.util.KnownOIDs; /** * The new X509 key manager implementation. The main differences to the @@ -63,7 +58,7 @@ * @author Andreas Sterbenz */ final class X509KeyManagerImpl extends X509ExtendedKeyManager - implements X509KeyManager { + implements KeyManagerAlgorithmConstraints { // for unit testing only, set via privileged reflection private static Date verificationDate; @@ -180,55 +175,6 @@ public String[] getServerAliases(String keyType, Principal[] issuers) { // implementation private methods // - // Gets algorithm constraints of the socket. - private AlgorithmConstraints getAlgorithmConstraints(Socket socket) { - if (socket != null && socket.isConnected() && - socket instanceof SSLSocket sslSocket) { - - SSLSession session = sslSocket.getHandshakeSession(); - - if (session != null) { - if (ProtocolVersion.useTLS12PlusSpec(session.getProtocol())) { - String[] peerSupportedSignAlgs = null; - - if (session instanceof ExtendedSSLSession extSession) { - peerSupportedSignAlgs = - extSession.getPeerSupportedSignatureAlgorithms(); - } - - return SSLAlgorithmConstraints.forSocket( - sslSocket, peerSupportedSignAlgs, true); - } - } - - return SSLAlgorithmConstraints.forSocket(sslSocket, true); - } - - return SSLAlgorithmConstraints.DEFAULT; - } - - // Gets algorithm constraints of the engine. - private AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { - if (engine != null) { - SSLSession session = engine.getHandshakeSession(); - if (session != null) { - if (ProtocolVersion.useTLS12PlusSpec(session.getProtocol())) { - String[] peerSupportedSignAlgs = null; - - if (session instanceof ExtendedSSLSession extSession) { - peerSupportedSignAlgs = - extSession.getPeerSupportedSignatureAlgorithms(); - } - - return SSLAlgorithmConstraints.forEngine( - engine, peerSupportedSignAlgs, true); - } - } - } - - return SSLAlgorithmConstraints.forEngine(engine, true); - } - // we construct the alias we return to JSSE as seen in the code below // a unique id is included to allow us to reliably cache entries // between the calls to getCertificateChain() and getPrivateKey() @@ -506,188 +452,6 @@ public String toString() { } } - // enum for the type of certificate check we want to perform - // (client or server) - // also includes the check code itself - private enum CheckType { - - // enum constant for "no check" (currently not used) - NONE(Collections.emptySet()), - - // enum constant for "tls client" check - // valid EKU for TLS client: any, tls_client - CLIENT(new HashSet<>(List.of( - KnownOIDs.anyExtendedKeyUsage.value(), - KnownOIDs.clientAuth.value() - ))), - - // enum constant for "tls server" check - // valid EKU for TLS server: any, tls_server, ns_sgc, ms_sgc - SERVER(new HashSet<>(List.of( - KnownOIDs.anyExtendedKeyUsage.value(), - KnownOIDs.serverAuth.value(), - KnownOIDs.NETSCAPE_ExportApproved.value(), - KnownOIDs.MICROSOFT_ExportApproved.value() - ))); - - // set of valid EKU values for this type - final Set validEku; - - CheckType(Set validEku) { - this.validEku = validEku; - } - - private static boolean getBit(boolean[] keyUsage, int bit) { - return (bit < keyUsage.length) && keyUsage[bit]; - } - - // Check if this certificate is appropriate for this type of use - // first check extensions, if they match, check expiration. - // - // Note: we may want to move this code into the sun.security.validator - // package - CheckResult check(X509Certificate cert, Date date, - List serverNames, String idAlgorithm) { - - if (this == NONE) { - return CheckResult.OK; - } - - // check extensions - try { - // check extended key usage - List certEku = cert.getExtendedKeyUsage(); - if ((certEku != null) && - Collections.disjoint(validEku, certEku)) { - // if extension is present and does not contain any of - // the valid EKU OIDs, return extension_mismatch - return CheckResult.EXTENSION_MISMATCH; - } - - // check key usage - boolean[] ku = cert.getKeyUsage(); - if (ku != null) { - String algorithm = cert.getPublicKey().getAlgorithm(); - boolean supportsDigitalSignature = getBit(ku, 0); - switch (algorithm) { - case "RSA": - // require either signature bit - // or if server also allow key encipherment bit - if (!supportsDigitalSignature) { - if (this == CLIENT || !getBit(ku, 2)) { - return CheckResult.EXTENSION_MISMATCH; - } - } - break; - case "RSASSA-PSS": - if (!supportsDigitalSignature && (this == SERVER)) { - return CheckResult.EXTENSION_MISMATCH; - } - break; - case "DSA": - // require signature bit - if (!supportsDigitalSignature) { - return CheckResult.EXTENSION_MISMATCH; - } - break; - case "DH": - // require keyagreement bit - if (!getBit(ku, 4)) { - return CheckResult.EXTENSION_MISMATCH; - } - break; - case "EC": - // require signature bit - if (!supportsDigitalSignature) { - return CheckResult.EXTENSION_MISMATCH; - } - // For servers, also require key agreement. - // This is not totally accurate as the keyAgreement - // bit is only necessary for static ECDH key - // exchange and not ephemeral ECDH. We leave it in - // for now until there are signs that this check - // causes problems for real world EC certificates. - if (this == SERVER && !getBit(ku, 4)) { - return CheckResult.EXTENSION_MISMATCH; - } - break; - } - } - } catch (CertificateException e) { - // extensions unparseable, return failure - return CheckResult.EXTENSION_MISMATCH; - } - - try { - cert.checkValidity(date); - } catch (CertificateException e) { - return CheckResult.EXPIRED; - } - - if (serverNames != null && !serverNames.isEmpty()) { - for (SNIServerName serverName : serverNames) { - if (serverName.getType() == - StandardConstants.SNI_HOST_NAME) { - if (!(serverName instanceof SNIHostName)) { - try { - serverName = - new SNIHostName(serverName.getEncoded()); - } catch (IllegalArgumentException iae) { - // unlikely to happen, just in case ... - if (SSLLogger.isOn && - SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Illegal server name: " + serverName); - } - - return CheckResult.INSENSITIVE; - } - } - String hostname = - ((SNIHostName)serverName).getAsciiName(); - - try { - X509TrustManagerImpl.checkIdentity(hostname, - cert, idAlgorithm); - } catch (CertificateException e) { - if (SSLLogger.isOn && - SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Certificate identity does not match " + - "Server Name Indication (SNI): " + - hostname); - } - return CheckResult.INSENSITIVE; - } - - break; - } - } - } - - return CheckResult.OK; - } - - public String getValidator() { - if (this == CLIENT) { - return Validator.VAR_TLS_CLIENT; - } else if (this == SERVER) { - return Validator.VAR_TLS_SERVER; - } - return Validator.VAR_GENERIC; - } - } - - // enum for the result of the extension check - // NOTE: the order of the constants is important as they are used - // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH - private enum CheckResult { - OK, // ok or not checked - INSENSITIVE, // server name indication insensitive - EXPIRED, // extensions valid but cert expired - EXTENSION_MISMATCH, // extensions invalid (expiration not checked) - } - /* * Return a List of all candidate matches in the specified builder * that fit the parameters. @@ -826,40 +590,4 @@ private List getAliases(int builderIndex, } return results; } - - private static boolean conformsToAlgorithmConstraints( - AlgorithmConstraints constraints, Certificate[] chain, - String variant) { - - AlgorithmChecker checker = new AlgorithmChecker(constraints, variant); - try { - checker.init(false); - } catch (CertPathValidatorException cpve) { - // unlikely to happen - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Cannot initialize algorithm constraints checker", cpve); - } - - return false; - } - - // It is a forward checker, so we need to check from trust to target. - for (int i = chain.length - 1; i >= 0; i--) { - Certificate cert = chain[i]; - try { - // We don't care about the unresolved critical extensions. - checker.check(cert, Collections.emptySet()); - } catch (CertPathValidatorException cpve) { - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Certificate does not conform to " + - "algorithm constraints", cert, cpve); - } - - return false; - } - } - - return true; - } } diff --git a/test/micro/org/openjdk/bench/java/security/SSLHandshake.java b/test/micro/org/openjdk/bench/java/security/SSLHandshake.java index d8773781b58cc..94cbf663ecc57 100644 --- a/test/micro/org/openjdk/bench/java/security/SSLHandshake.java +++ b/test/micro/org/openjdk/bench/java/security/SSLHandshake.java @@ -78,6 +78,9 @@ public class SSLHandshake { @Param({"TLSv1.2", "TLS"}) String tlsVersion; + @Param({"SunX509", "SunX509c", "PKIX"}) + String keyMgr; + private static SSLContext getServerContext() { try { KeyStore ks = TestCertificates.getKeyStore(); @@ -102,8 +105,11 @@ public void init() throws Exception { TrustManagerFactory.getDefaultAlgorithm()); tmf.init(ts); + KeyManagerFactory kmf = KeyManagerFactory.getInstance(keyMgr); + kmf.init(ts, new char[0]); + SSLContext sslCtx = SSLContext.getInstance(tlsVersion); - sslCtx.init(null, tmf.getTrustManagers(), null); + sslCtx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); sslClientCtx = sslCtx; } From 3af51b4f96c0e717200830db3c93e4a3d5185217 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 2 May 2025 20:24:39 -0400 Subject: [PATCH 07/26] Revert SSLHandshake.java. Set SunX509c as default. Update copyright. --- .../ssl/SunX509ConstraintsKeyManagerImpl.java | 37 +---------- .../security/ssl/SunX509KeyManagerImpl.java | 61 ++++++++++++++++++- .../sun/security/ssl/X509KeyManagerImpl.java | 1 + .../share/conf/security/java.security | 2 +- .../bench/java/security/SSLHandshake.java | 8 +-- 5 files changed, 64 insertions(+), 45 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java index f67fe45b61bcc..82901f15d67df 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1999, 2025, 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 @@ -49,37 +49,6 @@ import javax.net.ssl.X509ExtendedKeyManager; import javax.security.auth.x500.X500Principal; - -/** - * An implementation of X509KeyManager backed by a KeyStore. - * - * The backing KeyStore is inspected when this object is constructed. - * All key entries containing a PrivateKey and a non-empty chain of - * X509Certificate are then copied into an internal store. This means - * that subsequent modifications of the KeyStore have no effect on the - * X509KeyManagerImpl object. - * - * Note that this class assumes that all keys are protected by the same - * password. - * - * The JSSE handshake code currently calls into this class via - * chooseClientAlias() and chooseServerAlias() to find the certificates to - * use. As implemented here, both always return the first alias returned by - * getClientAliases() and getServerAliases(). In turn, these methods are - * implemented by calling getAliases(), which performs the actual lookup. - * - * Note that this class currently implements no checking of the local - * certificates. In particular, it is *not* guaranteed that: - * . the certificates are within their validity period and not revoked - * . the signatures verify - * . they form a PKIX compliant chain. - * . the certificate extensions allow the certificate to be used for - * the desired purpose. - * - * Chains that fail any of these criteria will probably be rejected by - * the remote peer. - * - */ class SunX509ConstraintsKeyManagerImpl extends X509ExtendedKeyManager implements KeyManagerAlgorithmConstraints { @@ -128,8 +97,8 @@ Set getIssuerX500Principals() { NoSuchAlgorithmException, UnrecoverableKeyException { credentialsMap = new HashMap<>(); - serverAliasCache = Collections.synchronizedMap( - new HashMap<>()); + serverAliasCache = Collections.synchronizedMap(new HashMap<>()); + if (ks == null) { return; } diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index fe3bb7b247532..fbc77704d3dbb 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -1,3 +1,28 @@ +/* + * Copyright (c) 1999, 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + package sun.security.ssl; import java.net.Socket; @@ -9,7 +34,37 @@ import java.security.cert.Certificate; import javax.net.ssl.SSLEngine; -public class SunX509KeyManagerImpl extends SunX509ConstraintsKeyManagerImpl { +/** + * An implementation of X509KeyManager backed by a KeyStore. + * + * The backing KeyStore is inspected when this object is constructed. + * All key entries containing a PrivateKey and a non-empty chain of + * X509Certificate are then copied into an internal store. This means + * that subsequent modifications of the KeyStore have no effect on the + * X509KeyManagerImpl object. + * + * Note that this class assumes that all keys are protected by the same + * password. + * + * The JSSE handshake code currently calls into this class via + * chooseClientAlias() and chooseServerAlias() to find the certificates to + * use. As implemented here, both always return the first alias returned by + * getClientAliases() and getServerAliases(). In turn, these methods are + * implemented by calling getAliases(), which performs the actual lookup. + * + * Note that this class currently implements no checking of the local + * certificates. In particular, it is *not* guaranteed that: + * . the certificates are within their validity period and not revoked + * . the signatures verify + * . they form a PKIX compliant chain. + * . the certificate extensions allow the certificate to be used for + * the desired purpose. + * + * Chains that fail any of these criteria will probably be rejected by + * the remote peer. + */ + +final class SunX509KeyManagerImpl extends SunX509ConstraintsKeyManagerImpl { SunX509KeyManagerImpl(KeyStore ks, char[] password) throws KeyStoreException, @@ -29,8 +84,8 @@ public AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { @Override public boolean conformsToAlgorithmConstraints( - AlgorithmConstraints constraints, Certificate[] chain, - String variant) { + AlgorithmConstraints constraints, + Certificate[] chain, String variant) { return true; } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 444846d1f7ad3..be540aaeb969f 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -57,6 +57,7 @@ * * @author Andreas Sterbenz */ + final class X509KeyManagerImpl extends X509ExtendedKeyManager implements KeyManagerAlgorithmConstraints { diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 826e3b4a21d1d..f485816576c0d 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -320,7 +320,7 @@ security.overridePropertiesFile=true # Determines the default key and trust manager factory algorithms for # the javax.net.ssl package. # -ssl.KeyManagerFactory.algorithm=PKIX +ssl.KeyManagerFactory.algorithm=SunX509c ssl.TrustManagerFactory.algorithm=PKIX # diff --git a/test/micro/org/openjdk/bench/java/security/SSLHandshake.java b/test/micro/org/openjdk/bench/java/security/SSLHandshake.java index 94cbf663ecc57..d8773781b58cc 100644 --- a/test/micro/org/openjdk/bench/java/security/SSLHandshake.java +++ b/test/micro/org/openjdk/bench/java/security/SSLHandshake.java @@ -78,9 +78,6 @@ public class SSLHandshake { @Param({"TLSv1.2", "TLS"}) String tlsVersion; - @Param({"SunX509", "SunX509c", "PKIX"}) - String keyMgr; - private static SSLContext getServerContext() { try { KeyStore ks = TestCertificates.getKeyStore(); @@ -105,11 +102,8 @@ public void init() throws Exception { TrustManagerFactory.getDefaultAlgorithm()); tmf.init(ts); - KeyManagerFactory kmf = KeyManagerFactory.getInstance(keyMgr); - kmf.init(ts, new char[0]); - SSLContext sslCtx = SSLContext.getInstance(tlsVersion); - sslCtx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); + sslCtx.init(null, tmf.getTrustManagers(), null); sslClientCtx = sslCtx; } From 79aa9fe0d4ce6a951d29e3870524b983d962660d Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 2 May 2025 20:59:40 -0400 Subject: [PATCH 08/26] Fix constraints check --- .../javax/net/ssl/KeyManagerFactory.java | 2 +- .../ssl/SunX509ConstraintsKeyManagerImpl.java | 22 +++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java index 6c38e9577daf1..f34ad90bced2d 100644 --- a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java +++ b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java @@ -64,7 +64,7 @@ public class KeyManagerFactory { public static final String getDefaultAlgorithm() { String type = Security.getProperty("ssl.KeyManagerFactory.algorithm"); if (type == null) { - type = "PKIX"; + type = "SunX509c"; } return type; } diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java index 82901f15d67df..30cd6e2a03675 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java @@ -353,20 +353,20 @@ private String[] getAliases(String keyType, Principal[] issuers, continue; } } + } - // check the algorithm constraints - if (constraints != null && - !conformsToAlgorithmConstraints(constraints, certs, - checkType.getValidator())) { - - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias + - ": certificate list does not conform to " + - "algorithm constraints"); - } + // check the algorithm constraints + if (constraints != null && + !conformsToAlgorithmConstraints(constraints, certs, + checkType.getValidator())) { - continue; + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": certificate list does not conform to " + + "algorithm constraints"); } + + continue; } if (issuers.length == 0) { From fd04c0ae79512478ec4d9290b8061f27fa460e6b Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 9 May 2025 16:26:27 -0400 Subject: [PATCH 09/26] Adding a system property to skip the constraints checking. Remove SunX509c. --- .../javax/net/ssl/KeyManagerFactory.java | 2 +- .../ssl/KeyManagerAlgorithmConstraints.java | 21 + .../security/ssl/KeyManagerFactoryImpl.java | 24 +- .../classes/sun/security/ssl/SunJSSE.java | 2 - .../ssl/SunX509ConstraintsKeyManagerImpl.java | 417 ------------------ .../security/ssl/SunX509KeyManagerImpl.java | 395 ++++++++++++++++- .../sun/security/ssl/X509KeyManagerImpl.java | 9 + .../share/conf/security/java.security | 2 +- ...NotAllowedInTLS13CertificateSignature.java | 9 +- 9 files changed, 422 insertions(+), 459 deletions(-) delete mode 100644 src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java diff --git a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java index f34ad90bced2d..652a20c837705 100644 --- a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java +++ b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactory.java @@ -64,7 +64,7 @@ public class KeyManagerFactory { public static final String getDefaultAlgorithm() { String type = Security.getProperty("ssl.KeyManagerFactory.algorithm"); if (type == null) { - type = "SunX509c"; + type = "SunX509"; } return type; } diff --git a/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java index 16b6ed95ab093..974facd6ae0b4 100644 --- a/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java @@ -51,6 +51,11 @@ interface KeyManagerAlgorithmConstraints { // Gets algorithm constraints of the socket. default AlgorithmConstraints getAlgorithmConstraints(Socket socket) { + + if (isConstraintsDisabled()) { + return null; + } + if (socket != null && socket.isConnected() && socket instanceof SSLSocket sslSocket) { @@ -78,6 +83,11 @@ default AlgorithmConstraints getAlgorithmConstraints(Socket socket) { // Gets algorithm constraints of the engine. default AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { + + if (isConstraintsDisabled()) { + return null; + } + if (engine != null) { SSLSession session = engine.getHandshakeSession(); if (session != null) { @@ -102,6 +112,10 @@ default boolean conformsToAlgorithmConstraints( AlgorithmConstraints constraints, Certificate[] chain, String variant) { + if (isConstraintsDisabled()) { + return true; + } + AlgorithmChecker checker = new AlgorithmChecker(constraints, variant); try { checker.init(false); @@ -135,6 +149,13 @@ default boolean conformsToAlgorithmConstraints( return true; } + default boolean isSystemConstraintsDisabled() { + return "true".equals(System.getProperty( + "jdk.tls.keymanager.disableConstraintsChecking")); + } + + boolean isConstraintsDisabled(); + // enum for the result of the extension check // NOTE: the order of the constants is important as they are used // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH diff --git a/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java b/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java index 6c6d46d105b50..95afc6cfe0c35 100644 --- a/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/KeyManagerFactoryImpl.java @@ -54,27 +54,7 @@ protected KeyManager[] engineGetKeyManagers() { return new KeyManager[] { keyManager }; } - // Factory for the SunX509c key manager - public static final class SunX509c extends KeyManagerFactoryImpl { - - @Override - protected void engineInit(KeyStore ks, char[] password) throws - KeyStoreException, NoSuchAlgorithmException, - UnrecoverableKeyException { - keyManager = new SunX509ConstraintsKeyManagerImpl(ks, password); - isInitialized = true; - } - - @Override - protected void engineInit(ManagerFactoryParameters spec) throws - InvalidAlgorithmParameterException { - throw new InvalidAlgorithmParameterException( - "SunX509ConstraintsKeyManager does not use ManagerFactoryParameters"); - } - - } - - // Factory for the SunX509 key manager + // Factory for the SunX509 keymanager public static final class SunX509 extends KeyManagerFactoryImpl { @Override @@ -94,7 +74,7 @@ protected void engineInit(ManagerFactoryParameters spec) throws } - // Factory for the X509 key manager + // Factory for the X509 keymanager public static final class X509 extends KeyManagerFactoryImpl { @Override diff --git a/src/java.base/share/classes/sun/security/ssl/SunJSSE.java b/src/java.base/share/classes/sun/security/ssl/SunJSSE.java index 543e799ab1e3d..611a6588b4c28 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunJSSE.java +++ b/src/java.base/share/classes/sun/security/ssl/SunJSSE.java @@ -55,8 +55,6 @@ private void registerAlgorithms() { ps("Signature", "MD5andSHA1withRSA", "sun.security.ssl.RSASignature", null, null); - ps("KeyManagerFactory", "SunX509c", - "sun.security.ssl.KeyManagerFactoryImpl$SunX509c", null, null); ps("KeyManagerFactory", "SunX509", "sun.security.ssl.KeyManagerFactoryImpl$SunX509", null, null); ps("KeyManagerFactory", "NewSunX509", diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java deleted file mode 100644 index 30cd6e2a03675..0000000000000 --- a/src/java.base/share/classes/sun/security/ssl/SunX509ConstraintsKeyManagerImpl.java +++ /dev/null @@ -1,417 +0,0 @@ -/* - * Copyright (c) 1999, 2025, 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package sun.security.ssl; - -import java.net.Socket; -import java.security.AlgorithmConstraints; -import java.security.Key; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.Principal; -import java.security.PrivateKey; -import java.security.UnrecoverableKeyException; -import java.security.cert.Certificate; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.X509ExtendedKeyManager; -import javax.security.auth.x500.X500Principal; - -class SunX509ConstraintsKeyManagerImpl extends X509ExtendedKeyManager - implements KeyManagerAlgorithmConstraints { - - private static final String[] STRING0 = new String[0]; - - /* - * The credentials from the KeyStore as - * Map: String(alias) -> X509Credentials(credentials) - */ - private final Map credentialsMap; - - /* - * Cached server aliases for the case issuers == null. - * (in the current JSSE implementation, issuers are always null for - * server certs). See chooseServerAlias() for details. - * - * Map: String(keyType) -> String[](alias) - */ - private final Map serverAliasCache; - - /* - * Basic container for credentials implemented as an inner class. - */ - private static class X509Credentials { - final PrivateKey privateKey; - final X509Certificate[] certificates; - private final Set issuerX500Principals; - - X509Credentials(PrivateKey privateKey, X509Certificate[] certificates) { - // assert privateKey and certificates != null - this.privateKey = privateKey; - this.certificates = certificates; - this.issuerX500Principals = HashSet.newHashSet(certificates.length); - for (X509Certificate certificate : certificates) { - issuerX500Principals.add(certificate.getIssuerX500Principal()); - } - } - - Set getIssuerX500Principals() { - return issuerX500Principals; - } - } - - SunX509ConstraintsKeyManagerImpl(KeyStore ks, char[] password) - throws KeyStoreException, - NoSuchAlgorithmException, UnrecoverableKeyException { - - credentialsMap = new HashMap<>(); - serverAliasCache = Collections.synchronizedMap(new HashMap<>()); - - if (ks == null) { - return; - } - - for (Enumeration aliases = ks.aliases(); - aliases.hasMoreElements(); ) { - String alias = aliases.nextElement(); - if (!ks.isKeyEntry(alias)) { - continue; - } - Key key = ks.getKey(alias, password); - if (!(key instanceof PrivateKey)) { - continue; - } - Certificate[] certs = ks.getCertificateChain(alias); - if ((certs == null) || (certs.length == 0) || - !(certs[0] instanceof X509Certificate)) { - continue; - } - if (!(certs instanceof X509Certificate[])) { - Certificate[] tmp = new X509Certificate[certs.length]; - System.arraycopy(certs, 0, tmp, 0, certs.length); - certs = tmp; - } - - X509Credentials cred = new X509Credentials((PrivateKey)key, - (X509Certificate[])certs); - credentialsMap.put(alias, cred); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("found key for : " + alias, (Object[])certs); - } - } - } - - /* - * Returns the certificate chain associated with the given alias. - * - * @return the certificate chain (ordered with the user's certificate first - * and the root certificate authority last) - */ - @Override - public X509Certificate[] getCertificateChain(String alias) { - if (alias == null) { - return null; - } - X509Credentials cred = credentialsMap.get(alias); - if (cred == null) { - return null; - } else { - return cred.certificates.clone(); - } - } - - /* - * Returns the key associated with the given alias - */ - @Override - public PrivateKey getPrivateKey(String alias) { - if (alias == null) { - return null; - } - X509Credentials cred = credentialsMap.get(alias); - if (cred == null) { - return null; - } else { - return cred.privateKey; - } - } - - /* - * Choose an alias to authenticate the client side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String chooseClientAlias(String[] keyTypes, Principal[] issuers, - Socket socket) { - return chooseClientAlias(keyTypes, issuers, - getAlgorithmConstraints(socket)); - } - - /* - * Choose an alias to authenticate the client side of an - * SSLEngine connection given the public key type - * and the list of certificate issuer authorities recognized by - * the peer (if any). - * - * @since 1.5 - */ - @Override - public String chooseEngineClientAlias(String[] keyType, - Principal[] issuers, SSLEngine engine) { - return chooseClientAlias( - keyType, issuers, getAlgorithmConstraints(engine)); - } - - private String chooseClientAlias(String[] keyTypes, Principal[] issuers, - AlgorithmConstraints constraints) { - - if (keyTypes == null) { - return null; - } - - for (String keyType : keyTypes) { - String[] aliases = getAliases( - keyType, issuers, CheckType.CLIENT, constraints); - - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } - } - - return null; - } - - /* - * Choose an alias to authenticate the server side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String chooseServerAlias(String keyType, - Principal[] issuers, Socket socket) { - return chooseServerAlias( - keyType, issuers, getAlgorithmConstraints(socket)); - } - - /* - * Choose an alias to authenticate the server side of an - * SSLEngine connection given the public key type - * and the list of certificate issuer authorities recognized by - * the peer (if any). - * - * @since 1.5 - */ - @Override - public String chooseEngineServerAlias(String keyType, - Principal[] issuers, SSLEngine engine) { - return chooseServerAlias( - keyType, issuers, getAlgorithmConstraints(engine)); - } - - private String chooseServerAlias(String keyType, - Principal[] issuers, AlgorithmConstraints constraints) { - - if (keyType == null) { - return null; - } - - String[] aliases; - - if (issuers == null || issuers.length == 0) { - aliases = serverAliasCache.get(keyType); - if (aliases == null) { - aliases = getAliases(keyType, issuers, - CheckType.SERVER, constraints); - // Cache the result (positive and negative lookups) - if (aliases == null) { - aliases = STRING0; - } - serverAliasCache.put(keyType, aliases); - } - } else { - aliases = getServerAliases(keyType, issuers); - } - - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } - - return null; - } - - /* - * Get the matching aliases for authenticating the client side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String[] getClientAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.CLIENT, null); - } - - /* - * Get the matching aliases for authenticating the server side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - */ - @Override - public String[] getServerAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.SERVER, null); - } - - /* - * Get the matching aliases for authenticating the either side of a secure - * socket given the public key type and the list of - * certificate issuer authorities recognized by the peer (if any). - * - * Issuers come to us in the form of X500Principal[]. - */ - private String[] getAliases(String keyType, Principal[] issuers, - CheckType checkType, AlgorithmConstraints constraints) { - if (keyType == null) { - return null; - } - if (issuers == null) { - issuers = new X500Principal[0]; - } - if (!(issuers instanceof X500Principal[])) { - // normally, this will never happen but try to recover if it does - issuers = convertPrincipals(issuers); - } - String sigType; - if (keyType.contains("_")) { - int k = keyType.indexOf('_'); - sigType = keyType.substring(k + 1); - keyType = keyType.substring(0, k); - } else { - sigType = null; - } - - X500Principal[] x500Issuers = (X500Principal[])issuers; - // the algorithm below does not produce duplicates, so avoid Set - List aliases = new ArrayList<>(); - - for (Map.Entry entry : - credentialsMap.entrySet()) { - - String alias = entry.getKey(); - X509Credentials credentials = entry.getValue(); - X509Certificate[] certs = credentials.certificates; - - if (!keyType.equals(certs[0].getPublicKey().getAlgorithm())) { - continue; - } - - if (sigType != null) { - if (certs.length > 1) { - // if possible, check the public key in the issuer cert - if (!sigType.equals( - certs[1].getPublicKey().getAlgorithm())) { - continue; - } - } else { - // Check the signature algorithm of the certificate itself. - // Look for the "withRSA" in "SHA1withRSA", etc. - String sigAlgName = - certs[0].getSigAlgName().toUpperCase(Locale.ENGLISH); - String pattern = "WITH" + - sigType.toUpperCase(Locale.ENGLISH); - if (!sigAlgName.contains(pattern)) { - continue; - } - } - } - - // check the algorithm constraints - if (constraints != null && - !conformsToAlgorithmConstraints(constraints, certs, - checkType.getValidator())) { - - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias + - ": certificate list does not conform to " + - "algorithm constraints"); - } - - continue; - } - - if (issuers.length == 0) { - // no issuer specified, match all - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - } else { - Set certIssuers = - credentials.getIssuerX500Principals(); - for (int i = 0; i < x500Issuers.length; i++) { - if (certIssuers.contains(issuers[i])) { - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - break; - } - } - } - } - - String[] aliasStrings = aliases.toArray(STRING0); - return ((aliasStrings.length == 0) ? null : aliasStrings); - } - - /* - * Convert an array of Principals to an array of X500Principals, if - * possible. Principals that cannot be converted are ignored. - */ - private static X500Principal[] convertPrincipals(Principal[] principals) { - List list = new ArrayList<>(principals.length); - for (int i = 0; i < principals.length; i++) { - Principal p = principals[i]; - if (p instanceof X500Principal) { - list.add((X500Principal)p); - } else { - try { - list.add(new X500Principal(p.getName())); - } catch (IllegalArgumentException e) { - // ignore - } - } - } - return list.toArray(new X500Principal[0]); - } -} diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index fbc77704d3dbb..9621168c70931 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -27,12 +27,29 @@ import java.net.Socket; import java.security.AlgorithmConstraints; +import java.security.Key; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.security.Principal; +import java.security.PrivateKey; import java.security.UnrecoverableKeyException; import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; import javax.net.ssl.SSLEngine; +import javax.net.ssl.X509ExtendedKeyManager; +import javax.net.ssl.X509KeyManager; +import javax.security.auth.x500.X500Principal; + /** * An implementation of X509KeyManager backed by a KeyStore. @@ -54,38 +71,390 @@ * * Note that this class currently implements no checking of the local * certificates. In particular, it is *not* guaranteed that: - * . the certificates are within their validity period and not revoked - * . the signatures verify - * . they form a PKIX compliant chain. - * . the certificate extensions allow the certificate to be used for - * the desired purpose. + * . the certificates are within their validity period and not revoked + * . the signatures verify + * . they form a PKIX compliant chain. + * . the certificate extensions allow the certificate to be used for + * the desired purpose. * * Chains that fail any of these criteria will probably be rejected by * the remote peer. */ -final class SunX509KeyManagerImpl extends SunX509ConstraintsKeyManagerImpl { +final class SunX509KeyManagerImpl extends X509ExtendedKeyManager + implements KeyManagerAlgorithmConstraints { + + private static final String[] STRING0 = new String[0]; + + /* + * The credentials from the KeyStore as + * Map: String(alias) -> X509Credentials(credentials) + */ + private final Map credentialsMap; + + /* + * Cached server aliases for the case issuers == null. + * (in the current JSSE implementation, issuers are always null for + * server certs). See chooseServerAlias() for details. + * + * Map: String(keyType) -> String[](alias) + */ + private final Map serverAliasCache; + + // Indicates whether we should skip the constraints check. + private final boolean constraintsDisabled; + + /* + * Basic container for credentials implemented as an inner class. + */ + private static class X509Credentials { + + final PrivateKey privateKey; + final X509Certificate[] certificates; + private final Set issuerX500Principals; + + X509Credentials(PrivateKey privateKey, X509Certificate[] certificates) { + // assert privateKey and certificates != null + this.privateKey = privateKey; + this.certificates = certificates; + this.issuerX500Principals = HashSet.newHashSet(certificates.length); + for (X509Certificate certificate : certificates) { + issuerX500Principals.add(certificate.getIssuerX500Principal()); + } + } + + Set getIssuerX500Principals() { + return issuerX500Principals; + } + } SunX509KeyManagerImpl(KeyStore ks, char[] password) throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { - super(ks, password); + + constraintsDisabled = isSystemConstraintsDisabled(); + credentialsMap = new HashMap<>(); + serverAliasCache = Collections.synchronizedMap( + new HashMap<>()); + if (ks == null) { + return; + } + + for (Enumeration aliases = ks.aliases(); + aliases.hasMoreElements(); ) { + String alias = aliases.nextElement(); + if (!ks.isKeyEntry(alias)) { + continue; + } + Key key = ks.getKey(alias, password); + if (!(key instanceof PrivateKey)) { + continue; + } + Certificate[] certs = ks.getCertificateChain(alias); + if ((certs == null) || (certs.length == 0) || + !(certs[0] instanceof X509Certificate)) { + continue; + } + if (!(certs instanceof X509Certificate[])) { + Certificate[] tmp = new X509Certificate[certs.length]; + System.arraycopy(certs, 0, tmp, 0, certs.length); + certs = tmp; + } + + X509Credentials cred = new X509Credentials((PrivateKey) key, + (X509Certificate[]) certs); + credentialsMap.put(alias, cred); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("found key for : " + alias, (Object[]) certs); + } + } } @Override - public AlgorithmConstraints getAlgorithmConstraints(Socket socket) { + public boolean isConstraintsDisabled() { + return constraintsDisabled; + } + + /* + * Returns the certificate chain associated with the given alias. + * + * @return the certificate chain (ordered with the user's certificate first + * and the root certificate authority last) + */ + @Override + public X509Certificate[] getCertificateChain(String alias) { + if (alias == null) { + return null; + } + X509Credentials cred = credentialsMap.get(alias); + if (cred == null) { + return null; + } else { + return cred.certificates.clone(); + } + } + + /* + * Returns the key associated with the given alias + */ + @Override + public PrivateKey getPrivateKey(String alias) { + if (alias == null) { + return null; + } + X509Credentials cred = credentialsMap.get(alias); + if (cred == null) { + return null; + } else { + return cred.privateKey; + } + } + + /* + * Choose an alias to authenticate the client side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String chooseClientAlias(String[] keyTypes, Principal[] issuers, + Socket socket) { + return chooseClientAlias(keyTypes, issuers, + getAlgorithmConstraints(socket)); + } + + /* + * Choose an alias to authenticate the client side of an + * SSLEngine connection given the public key type + * and the list of certificate issuer authorities recognized by + * the peer (if any). + * + * @since 1.5 + */ + @Override + public String chooseEngineClientAlias(String[] keyType, + Principal[] issuers, SSLEngine engine) { + return chooseClientAlias( + keyType, issuers, getAlgorithmConstraints(engine)); + } + + private String chooseClientAlias(String[] keyTypes, Principal[] issuers, + AlgorithmConstraints constraints) { + + if (keyTypes == null) { + return null; + } + + for (String keyType : keyTypes) { + String[] aliases = getAliases( + keyType, issuers, CheckType.CLIENT, constraints); + + if ((aliases != null) && (aliases.length > 0)) { + return aliases[0]; + } + } + return null; } + /* + * Choose an alias to authenticate the server side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String chooseServerAlias(String keyType, + Principal[] issuers, Socket socket) { + return chooseServerAlias( + keyType, issuers, getAlgorithmConstraints(socket)); + } + + /* + * Choose an alias to authenticate the server side of an + * SSLEngine connection given the public key type + * and the list of certificate issuer authorities recognized by + * the peer (if any). + * + * @since 1.5 + */ @Override - public AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { + public String chooseEngineServerAlias(String keyType, + Principal[] issuers, SSLEngine engine) { + return chooseServerAlias( + keyType, issuers, getAlgorithmConstraints(engine)); + } + + private String chooseServerAlias(String keyType, + Principal[] issuers, AlgorithmConstraints constraints) { + + if (keyType == null) { + return null; + } + + String[] aliases; + + if (issuers == null || issuers.length == 0) { + aliases = serverAliasCache.get(keyType); + if (aliases == null) { + aliases = getAliases(keyType, issuers, + CheckType.SERVER, constraints); + // Cache the result (positive and negative lookups) + if (aliases == null) { + aliases = STRING0; + } + serverAliasCache.put(keyType, aliases); + } + } else { + aliases = getServerAliases(keyType, issuers); + } + + if ((aliases != null) && (aliases.length > 0)) { + return aliases[0]; + } + return null; } + /* + * Get the matching aliases for authenticating the client side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ + @Override + public String[] getClientAliases(String keyType, Principal[] issuers) { + return getAliases(keyType, issuers, CheckType.CLIENT, null); + } + + /* + * Get the matching aliases for authenticating the server side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + */ @Override - public boolean conformsToAlgorithmConstraints( - AlgorithmConstraints constraints, - Certificate[] chain, String variant) { - return true; + public String[] getServerAliases(String keyType, Principal[] issuers) { + return getAliases(keyType, issuers, CheckType.SERVER, null); + } + + /* + * Get the matching aliases for authenticating the either side of a secure + * socket given the public key type and the list of + * certificate issuer authorities recognized by the peer (if any). + * + * Issuers come to us in the form of X500Principal[]. + */ + private String[] getAliases(String keyType, Principal[] issuers, + CheckType checkType, AlgorithmConstraints constraints) { + if (keyType == null) { + return null; + } + if (issuers == null) { + issuers = new X500Principal[0]; + } + if (!(issuers instanceof X500Principal[])) { + // normally, this will never happen but try to recover if it does + issuers = convertPrincipals(issuers); + } + String sigType; + if (keyType.contains("_")) { + int k = keyType.indexOf('_'); + sigType = keyType.substring(k + 1); + keyType = keyType.substring(0, k); + } else { + sigType = null; + } + + X500Principal[] x500Issuers = (X500Principal[]) issuers; + // the algorithm below does not produce duplicates, so avoid Set + List aliases = new ArrayList<>(); + + for (Map.Entry entry : + credentialsMap.entrySet()) { + + String alias = entry.getKey(); + X509Credentials credentials = entry.getValue(); + X509Certificate[] certs = credentials.certificates; + + if (!keyType.equals(certs[0].getPublicKey().getAlgorithm())) { + continue; + } + + if (sigType != null) { + if (certs.length > 1) { + // if possible, check the public key in the issuer cert + if (!sigType.equals( + certs[1].getPublicKey().getAlgorithm())) { + continue; + } + } else { + // Check the signature algorithm of the certificate itself. + // Look for the "withRSA" in "SHA1withRSA", etc. + String sigAlgName = + certs[0].getSigAlgName() + .toUpperCase(Locale.ENGLISH); + String pattern = "WITH" + + sigType.toUpperCase(Locale.ENGLISH); + if (!sigAlgName.contains(pattern)) { + continue; + } + } + } + + // check the algorithm constraints + if (constraints != null && + !conformsToAlgorithmConstraints(constraints, certs, + checkType.getValidator())) { + + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": certificate list does not conform to " + + "algorithm constraints"); + } + + continue; + } + + if (issuers.length == 0) { + // no issuer specified, match all + aliases.add(alias); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("matching alias: " + alias); + } + } else { + Set certIssuers = + credentials.getIssuerX500Principals(); + for (int i = 0; i < x500Issuers.length; i++) { + if (certIssuers.contains(issuers[i])) { + aliases.add(alias); + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("matching alias: " + alias); + } + break; + } + } + } + } + + String[] aliasStrings = aliases.toArray(STRING0); + return ((aliasStrings.length == 0) ? null : aliasStrings); + } + + /* + * Convert an array of Principals to an array of X500Principals, if + * possible. Principals that cannot be converted are ignored. + */ + private static X500Principal[] convertPrincipals(Principal[] principals) { + List list = new ArrayList<>(principals.length); + for (int i = 0; i < principals.length; i++) { + Principal p = principals[i]; + if (p instanceof X500Principal) { + list.add((X500Principal) p); + } else { + try { + list.add(new X500Principal(p.getName())); + } catch (IllegalArgumentException e) { + // ignore + } + } + } + return list.toArray(new X500Principal[0]); } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index be540aaeb969f..4e258350a1785 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -73,11 +73,15 @@ final class X509KeyManagerImpl extends X509ExtendedKeyManager // cached entries private final Map> entryCacheMap; + // Indicates whether we should skip the constraints check. + private final boolean constraintsDisabled; + X509KeyManagerImpl(Builder builder) { this(Collections.singletonList(builder)); } X509KeyManagerImpl(List builders) { + constraintsDisabled = isSystemConstraintsDisabled(); this.builders = builders; uidCounter = new AtomicLong(); entryCacheMap = Collections.synchronizedMap @@ -99,6 +103,11 @@ private static class SizedMap extends LinkedHashMap { // public methods // + @Override + public boolean isConstraintsDisabled() { + return constraintsDisabled; + } + @Override public X509Certificate[] getCertificateChain(String alias) { PrivateKeyEntry entry = getEntry(alias); diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index f485816576c0d..b115d47983848 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -320,7 +320,7 @@ security.overridePropertiesFile=true # Determines the default key and trust manager factory algorithms for # the javax.net.ssl package. # -ssl.KeyManagerFactory.algorithm=SunX509c +ssl.KeyManagerFactory.algorithm=SunX509 ssl.TrustManagerFactory.algorithm=PKIX # diff --git a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java index ed61aace3f476..261db85518b5e 100644 --- a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java +++ b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java @@ -150,10 +150,13 @@ private static SSLContext getSSLContext( // create SSL context SSLContext ctx = SSLContext.getInstance(protocol); - // Using "SunX509" which doesn't check peer supported signature - // algorithms, so we check against local supported signature + // Disable KeyManager's algorithm constraints checking, + // so we check against local supported signature // algorithms which constitutes the fix being tested. - KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509"); + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", "true"); + KeyManagerFactory kmf = KeyManagerFactory.getInstance( + KeyManagerFactory.getDefaultAlgorithm()); kmf.init(ks, passphrase); ctx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); From 80488f410084c2427024af5e68450b7315abad8d Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 9 May 2025 18:18:20 -0400 Subject: [PATCH 10/26] Fix open unit tests --- .../share/classes/sun/security/ssl/X509KeyManagerImpl.java | 2 +- test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java | 4 ++++ .../security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java | 3 +++ test/jdk/sun/security/tools/keytool/PrintSSL.java | 7 +++---- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 4e258350a1785..36ca816211f12 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2004, 2025, 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 diff --git a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java index 26180a8fcdcba..3a8da980108b1 100644 --- a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java +++ b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java @@ -233,6 +233,10 @@ private void checkKeySize(KeyStore ks) throws Exception { private static String clientCiperSuite = null; public static void main(String[] args) throws Exception { + // Disable KeyManager's algorithm constraints checking. + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", "true"); + if (debug) { System.setProperty("javax.net.debug", "all"); } diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index e2e5e066f9c6a..749ce6f2e362f 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -1118,6 +1118,9 @@ public static void main(String args[]) throws Exception { throw new Exception("Incorrect number of arguments"); } Test test = tests[Integer.parseInt(args[0])]; + // Disable KeyManager's algorithm constraints checking. + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", "true"); Security.setProperty("jdk.tls.disabledAlgorithms", test.tlsDisAlgs); Security.setProperty("jdk.certpath.disabledAlgorithms", test.certPathDisAlgs); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 0a29b83159cc3..452e8db426faf 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -49,10 +49,9 @@ public class PrintSSL { public static void main(String[] args) throws Throwable { - // Using "SunX509" KeyManager which doesn't check peer supported - // signature algorithms, so we can make keytool print certificate - // with weak MD5withRSA signature algorithm. - Security.setProperty("ssl.KeyManagerFactory.algorithm", "SunX509"); + // Disable KeyManager's algorithm constraints checking. + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", "true"); Files.deleteIfExists(Paths.get("keystore")); From d0cfbdeff60a3151abc2f81195774334a31d36f0 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 9 May 2025 18:35:40 -0400 Subject: [PATCH 11/26] Code refactoring --- .../security/ssl/SunX509KeyManagerImpl.java | 15 +---------- ...ts.java => X509KeyManagerConstraints.java} | 26 ++++++++++++------- .../sun/security/ssl/X509KeyManagerImpl.java | 12 +-------- 3 files changed, 18 insertions(+), 35 deletions(-) rename src/java.base/share/classes/sun/security/ssl/{KeyManagerAlgorithmConstraints.java => X509KeyManagerConstraints.java} (94%) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index 9621168c70931..f234bc67e0094 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -46,8 +46,6 @@ import java.util.Map; import java.util.Set; import javax.net.ssl.SSLEngine; -import javax.net.ssl.X509ExtendedKeyManager; -import javax.net.ssl.X509KeyManager; import javax.security.auth.x500.X500Principal; @@ -81,8 +79,7 @@ * the remote peer. */ -final class SunX509KeyManagerImpl extends X509ExtendedKeyManager - implements KeyManagerAlgorithmConstraints { +final class SunX509KeyManagerImpl extends X509KeyManagerConstraints { private static final String[] STRING0 = new String[0]; @@ -101,9 +98,6 @@ final class SunX509KeyManagerImpl extends X509ExtendedKeyManager */ private final Map serverAliasCache; - // Indicates whether we should skip the constraints check. - private final boolean constraintsDisabled; - /* * Basic container for credentials implemented as an inner class. */ @@ -131,8 +125,6 @@ Set getIssuerX500Principals() { SunX509KeyManagerImpl(KeyStore ks, char[] password) throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { - - constraintsDisabled = isSystemConstraintsDisabled(); credentialsMap = new HashMap<>(); serverAliasCache = Collections.synchronizedMap( new HashMap<>()); @@ -170,11 +162,6 @@ Set getIssuerX500Principals() { } } - @Override - public boolean isConstraintsDisabled() { - return constraintsDisabled; - } - /* * Returns the certificate chain associated with the given alias. * diff --git a/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java similarity index 94% rename from src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java rename to src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java index 974facd6ae0b4..a4f39aab5cdaa 100644 --- a/src/java.base/share/classes/sun/security/ssl/KeyManagerAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java @@ -43,16 +43,24 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; import javax.net.ssl.StandardConstants; +import javax.net.ssl.X509ExtendedKeyManager; import sun.security.provider.certpath.AlgorithmChecker; import sun.security.util.KnownOIDs; import sun.security.validator.Validator; -interface KeyManagerAlgorithmConstraints { +abstract class X509KeyManagerConstraints extends X509ExtendedKeyManager { + + // Indicates whether we should skip the constraints check. + private final boolean constraintsDisabled; + + protected X509KeyManagerConstraints() { + constraintsDisabled = isConstraintsDisabled(); + } // Gets algorithm constraints of the socket. - default AlgorithmConstraints getAlgorithmConstraints(Socket socket) { + protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { - if (isConstraintsDisabled()) { + if (constraintsDisabled) { return null; } @@ -82,9 +90,9 @@ default AlgorithmConstraints getAlgorithmConstraints(Socket socket) { } // Gets algorithm constraints of the engine. - default AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { + protected AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { - if (isConstraintsDisabled()) { + if (constraintsDisabled) { return null; } @@ -108,11 +116,11 @@ default AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { return SSLAlgorithmConstraints.forEngine(engine, true); } - default boolean conformsToAlgorithmConstraints( + protected boolean conformsToAlgorithmConstraints( AlgorithmConstraints constraints, Certificate[] chain, String variant) { - if (isConstraintsDisabled()) { + if (constraintsDisabled) { return true; } @@ -149,13 +157,11 @@ default boolean conformsToAlgorithmConstraints( return true; } - default boolean isSystemConstraintsDisabled() { + protected boolean isConstraintsDisabled() { return "true".equals(System.getProperty( "jdk.tls.keymanager.disableConstraintsChecking")); } - boolean isConstraintsDisabled(); - // enum for the result of the extension check // NOTE: the order of the constants is important as they are used // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 36ca816211f12..0909d1ceee4ac 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -58,8 +58,7 @@ * @author Andreas Sterbenz */ -final class X509KeyManagerImpl extends X509ExtendedKeyManager - implements KeyManagerAlgorithmConstraints { +final class X509KeyManagerImpl extends X509KeyManagerConstraints { // for unit testing only, set via privileged reflection private static Date verificationDate; @@ -73,15 +72,11 @@ final class X509KeyManagerImpl extends X509ExtendedKeyManager // cached entries private final Map> entryCacheMap; - // Indicates whether we should skip the constraints check. - private final boolean constraintsDisabled; - X509KeyManagerImpl(Builder builder) { this(Collections.singletonList(builder)); } X509KeyManagerImpl(List builders) { - constraintsDisabled = isSystemConstraintsDisabled(); this.builders = builders; uidCounter = new AtomicLong(); entryCacheMap = Collections.synchronizedMap @@ -103,11 +98,6 @@ private static class SizedMap extends LinkedHashMap { // public methods // - @Override - public boolean isConstraintsDisabled() { - return constraintsDisabled; - } - @Override public X509Certificate[] getCertificateChain(String alias) { PrivateKeyEntry entry = getEntry(alias); From 75528109f3bcf4f7ab81a92d74fed35f0d5747b3 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Mon, 12 May 2025 12:05:16 -0400 Subject: [PATCH 12/26] Add unit test --- .../ssl/X509KeyManagerConstraints.java | 12 +- .../AlgorithmConstraintsCheck.java | 186 ++++++++++++++++++ .../X509TrustManagerImpl/PKIXExtendedTM.java | 10 +- 3 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java index a4f39aab5cdaa..62cca5f184ef9 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java @@ -48,8 +48,18 @@ import sun.security.util.KnownOIDs; import sun.security.validator.Validator; +/** + * Layer that adds algorithm constraints functionality and certificate + * key usage check to a key manager. Algorithm constraints check can be + * disabled by setting "jdk.tls.keymanager.disableConstraintsChecking" + * system property to "true". + */ + abstract class X509KeyManagerConstraints extends X509ExtendedKeyManager { + private static final String PROPERTY_KEYMANAGER_DISABLE_CONSTRAINTS = + "jdk.tls.keymanager.disableConstraintsChecking"; + // Indicates whether we should skip the constraints check. private final boolean constraintsDisabled; @@ -159,7 +169,7 @@ protected boolean conformsToAlgorithmConstraints( protected boolean isConstraintsDisabled() { return "true".equals(System.getProperty( - "jdk.tls.keymanager.disableConstraintsChecking")); + PROPERTY_KEYMANAGER_DISABLE_CONSTRAINTS)); } // enum for the result of the extension check diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java new file mode 100644 index 0000000000000..c8a728b57bb02 --- /dev/null +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -0,0 +1,186 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import static jdk.test.lib.Asserts.assertEquals; +import static jdk.test.lib.Asserts.assertNull; + +import java.io.IOException; +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.KeyStore; +import java.security.PublicKey; +import java.security.SecureRandom; +import java.security.Security; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Date; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.X509KeyManager; +import jdk.test.lib.security.CertificateBuilder; +import sun.security.x509.AuthorityKeyIdentifierExtension; +import sun.security.x509.GeneralName; +import sun.security.x509.GeneralNames; +import sun.security.x509.KeyIdentifier; +import sun.security.x509.SerialNumber; +import sun.security.x509.X500Name; + +/* + * @test + * @bug 8353113 + * @summary Peer supported certificate signature algorithms are not being + * checked with default SunX509 key manager + * @modules java.base/sun.security.x509 + * java.base/sun.security.util + * @library /test/lib + * @run main/othervm AlgorithmConstraintsCheck false SunX509 SHA256withRSA + * @run main/othervm AlgorithmConstraintsCheck true SunX509 SHA256withRSA + * @run main/othervm AlgorithmConstraintsCheck false PKIX SHA256withRSA + * @run main/othervm AlgorithmConstraintsCheck true PKIX SHA256withRSA + */ + +public class AlgorithmConstraintsCheck { + + private static final String CERT_ALIAS = "testalias"; + private static final String KEY_TYPE = "RSA"; + + public static void main(String[] args) throws Exception { + if (args.length != 3) { + throw new RuntimeException("Wrong number of arguments"); + } + + String disabled = args[0]; + String kmAlg = args[1]; + String certSignatureAlg = args[2]; + + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", disabled); + Security.setProperty("jdk.tls.disabledAlgorithms", + certSignatureAlg); + + X509KeyManager km = getKeyManager(kmAlg, certSignatureAlg); + String serverAlias = km.chooseServerAlias(KEY_TYPE, null, null); + String clientAlias = km.chooseClientAlias( + new String[]{KEY_TYPE}, null, null); + + // PKIX KeyManager adds a cache prefix to an alias. + String serverAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "1.0." : ""; + String clientAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "2.0." : ""; + + if ("false".equals(disabled)) { + assertNull(serverAlias); + assertNull(clientAlias); + } else { + assertEquals(serverAliasPrefix + CERT_ALIAS, serverAlias); + assertEquals(clientAliasPrefix + CERT_ALIAS, clientAlias); + } + } + + private static X509KeyManager getKeyManager(String kmAlg, + String certSignatureAlg) + throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance(KEY_TYPE); + KeyPair caKeys = kpg.generateKeyPair(); + KeyPair endpointKeys = kpg.generateKeyPair(); + + X509Certificate trustedCert = createTrustedCert(caKeys, + certSignatureAlg); + + X509Certificate endpointCert = customCertificateBuilder( + "O=Some-Org, L=Some-City, ST=Some-State, C=US", + endpointKeys.getPublic(), caKeys.getPublic()) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), certSignatureAlg); + + // create a key store + KeyStore ks = KeyStore.getInstance("PKCS12"); + ks.load(null, null); + + // import the trusted cert + ks.setCertificateEntry("TLS Signer", trustedCert); + + // generate certificate chain + Certificate[] chain = new Certificate[2]; + chain[0] = endpointCert; + chain[1] = trustedCert; + + // import the key entry. + final char[] passphrase = "passphrase".toCharArray(); + ks.setKeyEntry(CERT_ALIAS, endpointKeys.getPrivate(), passphrase, + chain); + + KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmAlg); + kmf.init(ks, passphrase); + + return (X509KeyManager) kmf.getKeyManagers()[0]; + } + + // Certificate-building helper methods. + + private static X509Certificate createTrustedCert(KeyPair caKeys, + String certSignatureAlg) + throws Exception { + SecureRandom random = new SecureRandom(); + + KeyIdentifier kid = new KeyIdentifier(caKeys.getPublic()); + GeneralNames gns = new GeneralNames(); + GeneralName name = new GeneralName(new X500Name( + "O=Some-Org, L=Some-City, ST=Some-State, C=US")); + gns.add(name); + BigInteger serialNumber = BigInteger.valueOf( + random.nextLong(1000000) + 1); + return customCertificateBuilder( + "O=Some-Org, L=Some-City, ST=Some-State, C=US", + caKeys.getPublic(), caKeys.getPublic()) + .setSerialNumber(serialNumber) + .addExtension(new AuthorityKeyIdentifierExtension(kid, gns, + new SerialNumber(serialNumber))) + .addBasicConstraintsExt(true, true, -1) + .build(null, caKeys.getPrivate(), certSignatureAlg); + } + + private static CertificateBuilder customCertificateBuilder( + String subjectName, PublicKey publicKey, PublicKey caKey) + throws CertificateException, IOException { + SecureRandom random = new SecureRandom(); + + CertificateBuilder builder = new CertificateBuilder() + .setSubjectName(subjectName) + .setPublicKey(publicKey) + .setNotAfter( + Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) + .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) + .setSerialNumber( + BigInteger.valueOf(random.nextLong(1000000) + 1)) + .addSubjectKeyIdExt(publicKey) + .addAuthorityKeyIdExt(caKey); + builder.addKeyUsageExt( + new boolean[]{true, true, true, true, true, true}); + + return builder; + } + +} diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index 749ce6f2e362f..4af384bb474be 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, 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 @@ -1114,13 +1114,15 @@ static class Test { }; public static void main(String args[]) throws Exception { + // Disable KeyManager's algorithm constraints checking as this test + // is about TrustManager's constraints check. + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", "true"); + if (args.length != 1) { throw new Exception("Incorrect number of arguments"); } Test test = tests[Integer.parseInt(args[0])]; - // Disable KeyManager's algorithm constraints checking. - System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", "true"); Security.setProperty("jdk.tls.disabledAlgorithms", test.tlsDisAlgs); Security.setProperty("jdk.certpath.disabledAlgorithms", test.certPathDisAlgs); From 88cd4016fa9a1657c5ceb4ba7b8d948ea6d3b457 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Mon, 12 May 2025 14:41:41 -0400 Subject: [PATCH 13/26] Add PeerConstraintsCheck unit test --- ...NotAllowedInTLS13CertificateSignature.java | 1 - .../AlgorithmConstraintsCheck.java | 13 +- .../X509KeyManager/PeerConstraintsCheck.java | 272 ++++++++++++++++++ 3 files changed, 279 insertions(+), 7 deletions(-) create mode 100644 test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java diff --git a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java index 261db85518b5e..8040f401b5312 100644 --- a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java +++ b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java @@ -169,7 +169,6 @@ private static SSLContext getSSLContext( private void setupCertificates() throws Exception { KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); - kpg.initialize(1024); KeyPair caKeys = kpg.generateKeyPair(); this.serverKeys = kpg.generateKeyPair(); this.clientKeys = kpg.generateKeyPair(); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index c8a728b57bb02..95a6558f0e0cf 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -41,6 +41,7 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.X509KeyManager; import jdk.test.lib.security.CertificateBuilder; +import jdk.test.lib.security.SecurityUtils; import sun.security.x509.AuthorityKeyIdentifierExtension; import sun.security.x509.GeneralName; import sun.security.x509.GeneralNames; @@ -78,8 +79,7 @@ public static void main(String[] args) throws Exception { System.setProperty( "jdk.tls.keymanager.disableConstraintsChecking", disabled); - Security.setProperty("jdk.tls.disabledAlgorithms", - certSignatureAlg); + SecurityUtils.addToDisabledTlsAlgs(certSignatureAlg); X509KeyManager km = getKeyManager(kmAlg, certSignatureAlg); String serverAlias = km.chooseServerAlias(KEY_TYPE, null, null); @@ -90,12 +90,13 @@ public static void main(String[] args) throws Exception { String serverAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "1.0." : ""; String clientAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "2.0." : ""; - if ("false".equals(disabled)) { - assertNull(serverAlias); - assertNull(clientAlias); - } else { + if ("true".equals(disabled)) { assertEquals(serverAliasPrefix + CERT_ALIAS, serverAlias); assertEquals(clientAliasPrefix + CERT_ALIAS, clientAlias); + } else { + assertNull(serverAlias); + assertNull(clientAlias); + } } diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java new file mode 100644 index 0000000000000..db208bc8a7116 --- /dev/null +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -0,0 +1,272 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import static jdk.test.lib.Asserts.assertEquals; +import static jdk.test.lib.Asserts.assertTrue; +import static jdk.test.lib.Utils.runAndCheckException; + +import java.io.IOException; +import java.math.BigInteger; +import java.net.Socket; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.KeyStore; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.SecureRandom; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Date; +import java.util.List; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509TrustManager; +import jdk.test.lib.security.CertificateBuilder; +import jdk.test.lib.security.SecurityUtils; +import sun.security.x509.AuthorityKeyIdentifierExtension; +import sun.security.x509.GeneralName; +import sun.security.x509.GeneralNames; +import sun.security.x509.KeyIdentifier; +import sun.security.x509.SerialNumber; +import sun.security.x509.X500Name; + +/* + * @test + * @bug 8353113 + * @summary Peer supported certificate signature algorithms are not being + * checked with default SunX509 key manager + * @modules java.base/sun.security.x509 + * java.base/sun.security.util + * @library /javax/net/ssl/templates + * /test/lib + * @run main/othervm PeerConstraintsCheck false SunX509 + * @run main/othervm PeerConstraintsCheck true SunX509 + * @run main/othervm PeerConstraintsCheck false PKIX + * @run main/othervm PeerConstraintsCheck true PKIX + */ + +public class PeerConstraintsCheck extends SSLSocketTemplate { + + private static final String CLIENT_CERT_SIG_ALG = "SHA384withRSA"; + private static final String SERVER_CERT_SIG_ALG = "SHA256withRSA"; + private static final String TRUSTED_CERT_SIG_ALG = "SHA512withRSA"; + + private final String kmAlg; + private X509Certificate trustedCert; + private X509Certificate serverCert; + private X509Certificate clientCert; + private KeyPair serverKeys; + private KeyPair clientKeys; + + protected PeerConstraintsCheck(String kmAlg) throws Exception { + super(); + this.kmAlg = kmAlg; + setupCertificates(); + } + + public static void main(String[] args) throws Exception { + // Make sure only server's certificate signature algorithm is disabled. + SecurityUtils.removeFromDisabledAlgs("jdk.certpath.disabledAlgorithms", + List.of(CLIENT_CERT_SIG_ALG, TRUSTED_CERT_SIG_ALG)); + + SecurityUtils.removeFromDisabledAlgs("jdk.tls.disabledAlgorithms", + List.of(CLIENT_CERT_SIG_ALG, TRUSTED_CERT_SIG_ALG)); + + SecurityUtils.addToDisabledTlsAlgs(SERVER_CERT_SIG_ALG); + + String disabled = args[0]; + String kmAlg = args[1]; + + System.setProperty( + "jdk.tls.keymanager.disableConstraintsChecking", disabled); + + if ("true".equals(disabled)) { + new PeerConstraintsCheck(kmAlg).run(); + } else { + runAndCheckException( + () -> new PeerConstraintsCheck(kmAlg).run(), + ex -> { + assertTrue(ex instanceof javax.net.ssl.SSLHandshakeException); + assertEquals(ex.getMessage(), "(handshake_failure) " + + "No available authentication scheme"); + } + ); + } + } + + @Override + public SSLContext createServerSSLContext() throws Exception { + return getSSLContext( + trustedCert, serverCert, serverKeys.getPrivate(), kmAlg); + } + + @Override + public SSLContext createClientSSLContext() throws Exception { + return getSSLContext( + trustedCert, clientCert, clientKeys.getPrivate(), kmAlg); + } + + private static SSLContext getSSLContext( + X509Certificate trustedCertificate, X509Certificate keyCertificate, + PrivateKey privateKey, String kmAlg) + throws Exception { + + // create a key store + KeyStore ks = KeyStore.getInstance("PKCS12"); + ks.load(null, null); + + // import the trusted cert + ks.setCertificateEntry("TLS Signer", trustedCertificate); + + // generate certificate chain + Certificate[] chain = new Certificate[2]; + chain[0] = keyCertificate; + chain[1] = trustedCertificate; + + // import the key entry. + final char[] passphrase = "passphrase".toCharArray(); + ks.setKeyEntry("Whatever", privateKey, passphrase, chain); + + // create SSL context + SSLContext ctx = SSLContext.getInstance("TLS"); + + KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmAlg); + kmf.init(ks, passphrase); + + // Use custom trust-all TrustManager so we perform only KeyManager's + // constraints check. + X509ExtendedTrustManager[] trustAll = new X509ExtendedTrustManager[]{ + new X509ExtendedTrustManager() { + @Override + public void checkClientTrusted(X509Certificate[] chain, + String authType, Socket socket) + throws CertificateException { + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, + String authType, Socket socket) + throws CertificateException { + } + + @Override + public void checkClientTrusted(X509Certificate[] chain, + String authType, SSLEngine engine) + throws CertificateException { + } + + @Override + public void checkServerTrusted(X509Certificate[] chain, + String authType, SSLEngine engine) + throws CertificateException { + } + + public void checkClientTrusted(X509Certificate[] chain, + String authType) throws CertificateException { + } + + public void checkServerTrusted(X509Certificate[] chain, + String authType) throws CertificateException { + } + + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } + }; + + ctx.init(kmf.getKeyManagers(), trustAll, null); + + return ctx; + } + + // Certificate-building helper methods. + + private void setupCertificates() throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + KeyPair caKeys = kpg.generateKeyPair(); + this.serverKeys = kpg.generateKeyPair(); + this.clientKeys = kpg.generateKeyPair(); + + this.trustedCert = createTrustedCert(caKeys); + + this.serverCert = customCertificateBuilder( + "O=Some-Org, L=Some-City, ST=Some-State, C=US", + serverKeys.getPublic(), caKeys.getPublic()) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), SERVER_CERT_SIG_ALG); + + this.clientCert = customCertificateBuilder( + "CN=localhost, OU=SSL-Client, O=Some-Org, L=Some-City, ST=Some-State, C=US", + clientKeys.getPublic(), caKeys.getPublic()) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), CLIENT_CERT_SIG_ALG); + } + + private static X509Certificate createTrustedCert(KeyPair caKeys) + throws Exception { + SecureRandom random = new SecureRandom(); + + KeyIdentifier kid = new KeyIdentifier(caKeys.getPublic()); + GeneralNames gns = new GeneralNames(); + GeneralName name = new GeneralName(new X500Name( + "O=Some-Org, L=Some-City, ST=Some-State, C=US")); + gns.add(name); + BigInteger serialNumber = BigInteger.valueOf( + random.nextLong(1000000) + 1); + return customCertificateBuilder( + "O=Some-Org, L=Some-City, ST=Some-State, C=US", + caKeys.getPublic(), caKeys.getPublic()) + .setSerialNumber(serialNumber) + .addExtension(new AuthorityKeyIdentifierExtension(kid, gns, + new SerialNumber(serialNumber))) + .addBasicConstraintsExt(true, true, -1) + .build(null, caKeys.getPrivate(), TRUSTED_CERT_SIG_ALG); + } + + private static CertificateBuilder customCertificateBuilder( + String subjectName, PublicKey publicKey, PublicKey caKey) + throws CertificateException, IOException { + SecureRandom random = new SecureRandom(); + + CertificateBuilder builder = new CertificateBuilder() + .setSubjectName(subjectName) + .setPublicKey(publicKey) + .setNotAfter( + Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) + .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) + .setSerialNumber( + BigInteger.valueOf(random.nextLong(1000000) + 1)) + .addSubjectKeyIdExt(publicKey) + .addAuthorityKeyIdExt(caKey); + builder.addKeyUsageExt( + new boolean[]{true, true, true, true, true, true}); + + return builder; + } +} From 5c137c143947f82e0b305f7069e252ca9a09524e Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Mon, 12 May 2025 15:59:35 -0400 Subject: [PATCH 14/26] Make sure the exception happens during KeyManager's algorithm check --- .../X509KeyManager/PeerConstraintsCheck.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index db208bc8a7116..b78edc49d1867 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -40,14 +40,11 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Date; -import java.util.List; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.X509ExtendedTrustManager; -import javax.net.ssl.X509TrustManager; import jdk.test.lib.security.CertificateBuilder; -import jdk.test.lib.security.SecurityUtils; import sun.security.x509.AuthorityKeyIdentifierExtension; import sun.security.x509.GeneralName; import sun.security.x509.GeneralNames; @@ -90,14 +87,12 @@ protected PeerConstraintsCheck(String kmAlg) throws Exception { } public static void main(String[] args) throws Exception { - // Make sure only server's certificate signature algorithm is disabled. - SecurityUtils.removeFromDisabledAlgs("jdk.certpath.disabledAlgorithms", - List.of(CLIENT_CERT_SIG_ALG, TRUSTED_CERT_SIG_ALG)); - - SecurityUtils.removeFromDisabledAlgs("jdk.tls.disabledAlgorithms", - List.of(CLIENT_CERT_SIG_ALG, TRUSTED_CERT_SIG_ALG)); - - SecurityUtils.addToDisabledTlsAlgs(SERVER_CERT_SIG_ALG); + // Make sure both client and server support client's signature scheme, + // so the exception happens later during KeyManager's algorithm check. + System.setProperty( + "jdk.tls.client.SignatureSchemes", "rsa_pkcs1_sha384"); + System.setProperty( + "jdk.tls.server.SignatureSchemes", "rsa_pkcs1_sha384"); String disabled = args[0]; String kmAlg = args[1]; @@ -111,9 +106,10 @@ public static void main(String[] args) throws Exception { runAndCheckException( () -> new PeerConstraintsCheck(kmAlg).run(), ex -> { - assertTrue(ex instanceof javax.net.ssl.SSLHandshakeException); + assertTrue( + ex instanceof javax.net.ssl.SSLHandshakeException); assertEquals(ex.getMessage(), "(handshake_failure) " - + "No available authentication scheme"); + + "no cipher suites in common"); } ); } @@ -152,8 +148,9 @@ private static SSLContext getSSLContext( final char[] passphrase = "passphrase".toCharArray(); ks.setKeyEntry("Whatever", privateKey, passphrase, chain); - // create SSL context - SSLContext ctx = SSLContext.getInstance("TLS"); + // Use TLSv1.2 which supports SHA*withRSA algorithms for both handshake + // and certificate signatures. + SSLContext ctx = SSLContext.getInstance("TLSv1.2"); KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmAlg); kmf.init(ks, passphrase); From 451e1efd75df15c191d950e91d431261e06e99fe Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 14 May 2025 14:13:20 -0400 Subject: [PATCH 15/26] Make the test run on TLSv1.3 --- .../AlgorithmConstraintsCheck.java | 4 +-- .../X509KeyManager/PeerConstraintsCheck.java | 34 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index 95a6558f0e0cf..7ed156ae2a5d3 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -96,13 +96,11 @@ public static void main(String[] args) throws Exception { } else { assertNull(serverAlias); assertNull(clientAlias); - } } private static X509KeyManager getKeyManager(String kmAlg, - String certSignatureAlg) - throws Exception { + String certSignatureAlg) throws Exception { KeyPairGenerator kpg = KeyPairGenerator.getInstance(KEY_TYPE); KeyPair caKeys = kpg.generateKeyPair(); KeyPair endpointKeys = kpg.generateKeyPair(); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index b78edc49d1867..621f183fa85f5 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -43,6 +43,7 @@ import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.X509ExtendedTrustManager; import jdk.test.lib.security.CertificateBuilder; import sun.security.x509.AuthorityKeyIdentifierExtension; @@ -69,9 +70,12 @@ public class PeerConstraintsCheck extends SSLSocketTemplate { - private static final String CLIENT_CERT_SIG_ALG = "SHA384withRSA"; - private static final String SERVER_CERT_SIG_ALG = "SHA256withRSA"; - private static final String TRUSTED_CERT_SIG_ALG = "SHA512withRSA"; + private static final String KEY_ALGORITHM = "EC"; + private static final String CLIENT_CERT_SIG_SCHEME = + "ecdsa_secp384r1_sha384"; + private static final String CLIENT_CERT_SIG_ALG = "SHA384withECDSA"; + private static final String SERVER_CERT_SIG_ALG = "SHA256withECDSA"; + private static final String TRUSTED_CERT_SIG_ALG = "SHA512withECDSA"; private final String kmAlg; private X509Certificate trustedCert; @@ -90,9 +94,9 @@ public static void main(String[] args) throws Exception { // Make sure both client and server support client's signature scheme, // so the exception happens later during KeyManager's algorithm check. System.setProperty( - "jdk.tls.client.SignatureSchemes", "rsa_pkcs1_sha384"); + "jdk.tls.client.SignatureSchemes", CLIENT_CERT_SIG_SCHEME); System.setProperty( - "jdk.tls.server.SignatureSchemes", "rsa_pkcs1_sha384"); + "jdk.tls.server.SignatureSchemes", CLIENT_CERT_SIG_SCHEME); String disabled = args[0]; String kmAlg = args[1]; @@ -106,10 +110,9 @@ public static void main(String[] args) throws Exception { runAndCheckException( () -> new PeerConstraintsCheck(kmAlg).run(), ex -> { - assertTrue( - ex instanceof javax.net.ssl.SSLHandshakeException); + assertTrue(ex instanceof SSLHandshakeException); assertEquals(ex.getMessage(), "(handshake_failure) " - + "no cipher suites in common"); + + "No available authentication scheme"); } ); } @@ -127,9 +130,8 @@ public SSLContext createClientSSLContext() throws Exception { trustedCert, clientCert, clientKeys.getPrivate(), kmAlg); } - private static SSLContext getSSLContext( - X509Certificate trustedCertificate, X509Certificate keyCertificate, - PrivateKey privateKey, String kmAlg) + private static SSLContext getSSLContext(X509Certificate trustedCertificate, + X509Certificate keyCertificate, PrivateKey privateKey, String kmAlg) throws Exception { // create a key store @@ -148,9 +150,7 @@ private static SSLContext getSSLContext( final char[] passphrase = "passphrase".toCharArray(); ks.setKeyEntry("Whatever", privateKey, passphrase, chain); - // Use TLSv1.2 which supports SHA*withRSA algorithms for both handshake - // and certificate signatures. - SSLContext ctx = SSLContext.getInstance("TLSv1.2"); + SSLContext ctx = SSLContext.getInstance("TLS"); KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmAlg); kmf.init(ks, passphrase); @@ -205,7 +205,7 @@ public X509Certificate[] getAcceptedIssuers() { // Certificate-building helper methods. private void setupCertificates() throws Exception { - KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + KeyPairGenerator kpg = KeyPairGenerator.getInstance(KEY_ALGORITHM); KeyPair caKeys = kpg.generateKeyPair(); this.serverKeys = kpg.generateKeyPair(); this.clientKeys = kpg.generateKeyPair(); @@ -219,7 +219,8 @@ private void setupCertificates() throws Exception { .build(trustedCert, caKeys.getPrivate(), SERVER_CERT_SIG_ALG); this.clientCert = customCertificateBuilder( - "CN=localhost, OU=SSL-Client, O=Some-Org, L=Some-City, ST=Some-State, C=US", + "CN=localhost, OU=SSL-Client, O=Some-Org, L=Some-City," + + " ST=Some-State, C=US", clientKeys.getPublic(), caKeys.getPublic()) .addBasicConstraintsExt(false, false, -1) .build(trustedCert, caKeys.getPrivate(), CLIENT_CERT_SIG_ALG); @@ -228,7 +229,6 @@ private void setupCertificates() throws Exception { private static X509Certificate createTrustedCert(KeyPair caKeys) throws Exception { SecureRandom random = new SecureRandom(); - KeyIdentifier kid = new KeyIdentifier(caKeys.getPublic()); GeneralNames gns = new GeneralNames(); GeneralName name = new GeneralName(new X500Name( From 5fe3285d2b552a6ec27eff1b3a032916aa4fbe6c Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Sun, 8 Jun 2025 21:18:36 -0400 Subject: [PATCH 16/26] Address some review comments --- .../sun/security/ssl/SunX509KeyManagerImpl.java | 6 +----- .../sun/security/ssl/X509KeyManagerConstraints.java | 11 ++--------- .../classes/sun/security/ssl/X509KeyManagerImpl.java | 2 +- .../ssl/X509KeyManager/AlgorithmConstraintsCheck.java | 6 ++---- .../ssl/X509KeyManager/PeerConstraintsCheck.java | 5 +++++ .../lib/jdk/test/lib/security/CertificateBuilder.java | 8 ++++---- 6 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index f234bc67e0094..c696a0a7e5207 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -214,8 +214,6 @@ public String chooseClientAlias(String[] keyTypes, Principal[] issuers, * SSLEngine connection given the public key type * and the list of certificate issuer authorities recognized by * the peer (if any). - * - * @since 1.5 */ @Override public String chooseEngineClientAlias(String[] keyType, @@ -260,8 +258,6 @@ public String chooseServerAlias(String keyType, * SSLEngine connection given the public key type * and the list of certificate issuer authorities recognized by * the peer (if any). - * - * @since 1.5 */ @Override public String chooseEngineServerAlias(String keyType, @@ -392,7 +388,7 @@ private String[] getAliases(String keyType, Principal[] issuers, if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { SSLLogger.fine("Ignore alias " + alias + - ": certificate list does not conform to " + + ": certificate chain does not conform to " + "algorithm constraints"); } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java index 62cca5f184ef9..c661a5e5f5b15 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java @@ -57,14 +57,12 @@ abstract class X509KeyManagerConstraints extends X509ExtendedKeyManager { - private static final String PROPERTY_KEYMANAGER_DISABLE_CONSTRAINTS = - "jdk.tls.keymanager.disableConstraintsChecking"; - // Indicates whether we should skip the constraints check. private final boolean constraintsDisabled; protected X509KeyManagerConstraints() { - constraintsDisabled = isConstraintsDisabled(); + constraintsDisabled = "true".equals(System.getProperty( + "jdk.tls.keymanager.disableConstraintsChecking")); } // Gets algorithm constraints of the socket. @@ -167,11 +165,6 @@ protected boolean conformsToAlgorithmConstraints( return true; } - protected boolean isConstraintsDisabled() { - return "true".equals(System.getProperty( - PROPERTY_KEYMANAGER_DISABLE_CONSTRAINTS)); - } - // enum for the result of the extension check // NOTE: the order of the constants is important as they are used // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 0909d1ceee4ac..9f0e1d40905c4 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -559,7 +559,7 @@ private List getAliases(int builderIndex, if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { SSLLogger.fine("Ignore alias " + alias + - ": certificate list does not conform to " + + ": certificate chain does not conform to " + "algorithm constraints"); } continue; diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index 7ed156ae2a5d3..24c174bc17fb2 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -31,7 +31,6 @@ import java.security.KeyStore; import java.security.PublicKey; import java.security.SecureRandom; -import java.security.Security; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -51,9 +50,8 @@ /* * @test - * @bug 8353113 - * @summary Peer supported certificate signature algorithms are not being - * checked with default SunX509 key manager + * @bug 8170706 + * @summary Support algorithm constraints in SunX509 key manager * @modules java.base/sun.security.x509 * java.base/sun.security.util * @library /test/lib diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index 621f183fa85f5..8e4cfde3ce536 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -68,6 +68,11 @@ * @run main/othervm PeerConstraintsCheck true PKIX */ +/** + * This class tests against the peer supported certificate signatures sent in + * "signature_algorithms_cert" extension. + */ + public class PeerConstraintsCheck extends SSLSocketTemplate { private static final String KEY_ALGORITHM = "EC"; diff --git a/test/lib/jdk/test/lib/security/CertificateBuilder.java b/test/lib/jdk/test/lib/security/CertificateBuilder.java index c86fe1049200c..857d585f02973 100644 --- a/test/lib/jdk/test/lib/security/CertificateBuilder.java +++ b/test/lib/jdk/test/lib/security/CertificateBuilder.java @@ -237,15 +237,15 @@ public CertificateBuilder addSubjectAltNameDNSExt(List dnsNames) /** * Helper method to add IPAddress types for the SAN extension * - * @param IPAddresses A {@code List} of names to add as IPAddress + * @param ipAddresses A {@code List} of names to add as IPAddress * types * @throws IOException if an encoding error occurs. */ - public CertificateBuilder addSubjectAltNameIPExt(List IPAddresses) + public CertificateBuilder addSubjectAltNameIPExt(List ipAddresses) throws IOException { - if (!IPAddresses.isEmpty()) { + if (!ipAddresses.isEmpty()) { GeneralNames gNames = new GeneralNames(); - for (String name : IPAddresses) { + for (String name : ipAddresses) { gNames.add(new GeneralName(new IPAddressName(name))); } addExtension(new SubjectAlternativeNameExtension(false, From 448442e9a1dbb859224555676b103a50da87a4d9 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 13 Jun 2025 10:41:40 -0400 Subject: [PATCH 17/26] Support certificate checks in SunX509 key manager --- .../security/ssl/SunX509KeyManagerImpl.java | 236 +++-------- ...s.java => X509KeyManagerCertChecking.java} | 256 +++++++++-- .../sun/security/ssl/X509KeyManagerImpl.java | 226 ++-------- .../https/HttpsClient/ServerIdentityTest.java | 2 +- .../security/mscapi/ShortRSAKeyWithinTLS.java | 2 +- ...NotAllowedInTLS13CertificateSignature.java | 4 +- .../AlgorithmConstraintsCheck.java | 5 +- .../ssl/X509KeyManager/CertChecking.java | 396 ++++++++++++++++++ .../X509KeyManager/PeerConstraintsCheck.java | 6 +- .../X509TrustManagerImpl/PKIXExtendedTM.java | 5 +- .../sun/security/tools/keytool/PrintSSL.java | 2 +- 11 files changed, 724 insertions(+), 416 deletions(-) rename src/java.base/share/classes/sun/security/ssl/{X509KeyManagerConstraints.java => X509KeyManagerCertChecking.java} (59%) create mode 100644 test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index c696a0a7e5207..c0c9fc67b4292 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -40,11 +40,10 @@ import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; -import java.util.HashSet; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Set; +import javax.net.ssl.SNIServerName; import javax.net.ssl.SSLEngine; import javax.security.auth.x500.X500Principal; @@ -61,27 +60,9 @@ * Note that this class assumes that all keys are protected by the same * password. * - * The JSSE handshake code currently calls into this class via - * chooseClientAlias() and chooseServerAlias() to find the certificates to - * use. As implemented here, both always return the first alias returned by - * getClientAliases() and getServerAliases(). In turn, these methods are - * implemented by calling getAliases(), which performs the actual lookup. - * - * Note that this class currently implements no checking of the local - * certificates. In particular, it is *not* guaranteed that: - * . the certificates are within their validity period and not revoked - * . the signatures verify - * . they form a PKIX compliant chain. - * . the certificate extensions allow the certificate to be used for - * the desired purpose. - * - * Chains that fail any of these criteria will probably be rejected by - * the remote peer. */ -final class SunX509KeyManagerImpl extends X509KeyManagerConstraints { - - private static final String[] STRING0 = new String[0]; +final class SunX509KeyManagerImpl extends X509KeyManagerCertChecking { /* * The credentials from the KeyStore as @@ -89,15 +70,6 @@ final class SunX509KeyManagerImpl extends X509KeyManagerConstraints { */ private final Map credentialsMap; - /* - * Cached server aliases for the case issuers == null. - * (in the current JSSE implementation, issuers are always null for - * server certs). See chooseServerAlias() for details. - * - * Map: String(keyType) -> String[](alias) - */ - private final Map serverAliasCache; - /* * Basic container for credentials implemented as an inner class. */ @@ -105,29 +77,20 @@ private static class X509Credentials { final PrivateKey privateKey; final X509Certificate[] certificates; - private final Set issuerX500Principals; X509Credentials(PrivateKey privateKey, X509Certificate[] certificates) { // assert privateKey and certificates != null this.privateKey = privateKey; this.certificates = certificates; - this.issuerX500Principals = HashSet.newHashSet(certificates.length); - for (X509Certificate certificate : certificates) { - issuerX500Principals.add(certificate.getIssuerX500Principal()); - } - } - - Set getIssuerX500Principals() { - return issuerX500Principals; } } SunX509KeyManagerImpl(KeyStore ks, char[] password) throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { + credentialsMap = new HashMap<>(); - serverAliasCache = Collections.synchronizedMap( - new HashMap<>()); + if (ks == null) { return; } @@ -205,8 +168,8 @@ public PrivateKey getPrivateKey(String alias) { @Override public String chooseClientAlias(String[] keyTypes, Principal[] issuers, Socket socket) { - return chooseClientAlias(keyTypes, issuers, - getAlgorithmConstraints(socket)); + return chooseAlias(getKeyTypes(keyTypes), issuers, CheckType.CLIENT, + getAlgorithmConstraints(socket), null, null); } /* @@ -216,29 +179,10 @@ public String chooseClientAlias(String[] keyTypes, Principal[] issuers, * the peer (if any). */ @Override - public String chooseEngineClientAlias(String[] keyType, + public String chooseEngineClientAlias(String[] keyTypes, Principal[] issuers, SSLEngine engine) { - return chooseClientAlias( - keyType, issuers, getAlgorithmConstraints(engine)); - } - - private String chooseClientAlias(String[] keyTypes, Principal[] issuers, - AlgorithmConstraints constraints) { - - if (keyTypes == null) { - return null; - } - - for (String keyType : keyTypes) { - String[] aliases = getAliases( - keyType, issuers, CheckType.CLIENT, constraints); - - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } - } - - return null; + return chooseAlias(getKeyTypes(keyTypes), issuers, CheckType.CLIENT, + getAlgorithmConstraints(engine), null, null); } /* @@ -249,8 +193,9 @@ private String chooseClientAlias(String[] keyTypes, Principal[] issuers, @Override public String chooseServerAlias(String keyType, Principal[] issuers, Socket socket) { - return chooseServerAlias( - keyType, issuers, getAlgorithmConstraints(socket)); + return chooseAlias(getKeyTypes(keyType), issuers, CheckType.SERVER, + getAlgorithmConstraints(socket), + X509TrustManagerImpl.getRequestedServerNames(socket), "HTTPS"); } /* @@ -262,39 +207,9 @@ public String chooseServerAlias(String keyType, @Override public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) { - return chooseServerAlias( - keyType, issuers, getAlgorithmConstraints(engine)); - } - - private String chooseServerAlias(String keyType, - Principal[] issuers, AlgorithmConstraints constraints) { - - if (keyType == null) { - return null; - } - - String[] aliases; - - if (issuers == null || issuers.length == 0) { - aliases = serverAliasCache.get(keyType); - if (aliases == null) { - aliases = getAliases(keyType, issuers, - CheckType.SERVER, constraints); - // Cache the result (positive and negative lookups) - if (aliases == null) { - aliases = STRING0; - } - serverAliasCache.put(keyType, aliases); - } - } else { - aliases = getServerAliases(keyType, issuers); - } - - if ((aliases != null) && (aliases.length > 0)) { - return aliases[0]; - } - - return null; + return chooseAlias(getKeyTypes(keyType), issuers, CheckType.SERVER, + getAlgorithmConstraints(engine), + X509TrustManagerImpl.getRequestedServerNames(engine), "HTTPS"); } /* @@ -304,7 +219,8 @@ private String chooseServerAlias(String keyType, */ @Override public String[] getClientAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.CLIENT, null); + return getAliases(getKeyTypes(keyType), issuers, CheckType.CLIENT, + null, null, null); } /* @@ -314,7 +230,27 @@ public String[] getClientAliases(String keyType, Principal[] issuers) { */ @Override public String[] getServerAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.SERVER, null); + return getAliases(getKeyTypes(keyType), issuers, CheckType.SERVER, + null, null, null); + } + + private String chooseAlias(List keyTypes, Principal[] issuers, + CheckType checkType, AlgorithmConstraints constraints, + List requestedServerNames, String idAlgorithm) { + + if (keyTypes == null) { + return null; + } + + String[] aliases = getAliases( + keyTypes, issuers, checkType, + constraints, requestedServerNames, idAlgorithm); + + if (aliases != null && aliases.length > 0) { + return aliases[0]; + } + + return null; } /* @@ -324,100 +260,56 @@ public String[] getServerAliases(String keyType, Principal[] issuers) { * * Issuers come to us in the form of X500Principal[]. */ - private String[] getAliases(String keyType, Principal[] issuers, - CheckType checkType, AlgorithmConstraints constraints) { - if (keyType == null) { + private String[] getAliases(List keyTypes, Principal[] issuers, + CheckType checkType, AlgorithmConstraints constraints, + List requestedServerNames, + String idAlgorithm) { + + if (keyTypes == null || keyTypes.isEmpty()) { return null; } + if (issuers == null) { issuers = new X500Principal[0]; } + if (!(issuers instanceof X500Principal[])) { // normally, this will never happen but try to recover if it does issuers = convertPrincipals(issuers); } - String sigType; - if (keyType.contains("_")) { - int k = keyType.indexOf('_'); - sigType = keyType.substring(k + 1); - keyType = keyType.substring(0, k); - } else { - sigType = null; - } - X500Principal[] x500Issuers = (X500Principal[]) issuers; - // the algorithm below does not produce duplicates, so avoid Set - List aliases = new ArrayList<>(); + Set issuerSet = getIssuerSet(issuers); + List results = null; for (Map.Entry entry : credentialsMap.entrySet()) { - String alias = entry.getKey(); - X509Credentials credentials = entry.getValue(); - X509Certificate[] certs = credentials.certificates; + EntryStatus status = checkAlias(0, entry.getKey(), + entry.getValue().certificates, + null, keyTypes, issuerSet, checkType, + constraints, requestedServerNames, idAlgorithm); - if (!keyType.equals(certs[0].getPublicKey().getAlgorithm())) { + if (status == null) { continue; } - if (sigType != null) { - if (certs.length > 1) { - // if possible, check the public key in the issuer cert - if (!sigType.equals( - certs[1].getPublicKey().getAlgorithm())) { - continue; - } - } else { - // Check the signature algorithm of the certificate itself. - // Look for the "withRSA" in "SHA1withRSA", etc. - String sigAlgName = - certs[0].getSigAlgName() - .toUpperCase(Locale.ENGLISH); - String pattern = "WITH" + - sigType.toUpperCase(Locale.ENGLISH); - if (!sigAlgName.contains(pattern)) { - continue; - } - } + if (results == null) { + results = new ArrayList<>(); } - // check the algorithm constraints - if (constraints != null && - !conformsToAlgorithmConstraints(constraints, certs, - checkType.getValidator())) { - - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias + - ": certificate chain does not conform to " + - "algorithm constraints"); - } - - continue; - } + results.add(status); + } - if (issuers.length == 0) { - // no issuer specified, match all - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - } else { - Set certIssuers = - credentials.getIssuerX500Principals(); - for (int i = 0; i < x500Issuers.length; i++) { - if (certIssuers.contains(issuers[i])) { - aliases.add(alias); - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("matching alias: " + alias); - } - break; - } - } + if (results == null) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("KeyMgr: no matching key found"); } + return null; } - String[] aliasStrings = aliases.toArray(STRING0); - return ((aliasStrings.length == 0) ? null : aliasStrings); + // Sort results in order of alias preference. + Collections.sort(results); + return results.stream().map(r -> r.alias).toArray(String[]::new); } /* diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java similarity index 59% rename from src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java rename to src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index c661a5e5f5b15..4e7d75a3360a1 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerConstraints.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -27,14 +27,18 @@ import java.net.Socket; import java.security.AlgorithmConstraints; +import java.security.Principal; import java.security.cert.CertPathValidatorException; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Set; import javax.net.ssl.ExtendedSSLSession; import javax.net.ssl.SNIHostName; @@ -48,27 +52,27 @@ import sun.security.util.KnownOIDs; import sun.security.validator.Validator; -/** - * Layer that adds algorithm constraints functionality and certificate - * key usage check to a key manager. Algorithm constraints check can be - * disabled by setting "jdk.tls.keymanager.disableConstraintsChecking" - * system property to "true". +/* + * Layer that adds algorithm constraints and certificate + * checking to a key manager. Checks can be disabled by setting + * "jdk.tls.keymanager.disableCertChecking" system property to "true" + * before creating a KeyManager object. */ -abstract class X509KeyManagerConstraints extends X509ExtendedKeyManager { +abstract class X509KeyManagerCertChecking extends X509ExtendedKeyManager { - // Indicates whether we should skip the constraints check. - private final boolean constraintsDisabled; + // Indicates whether we should skip the certificate checks. + private final boolean checksDisabled; - protected X509KeyManagerConstraints() { - constraintsDisabled = "true".equals(System.getProperty( - "jdk.tls.keymanager.disableConstraintsChecking")); + protected X509KeyManagerCertChecking() { + checksDisabled = "true".equals(System.getProperty( + "jdk.tls.keymanager.disableCertChecking")); } // Gets algorithm constraints of the socket. protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { - if (constraintsDisabled) { + if (checksDisabled) { return null; } @@ -100,7 +104,7 @@ protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { // Gets algorithm constraints of the engine. protected AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { - if (constraintsDisabled) { + if (checksDisabled) { return null; } @@ -124,11 +128,12 @@ protected AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { return SSLAlgorithmConstraints.forEngine(engine, true); } + // Algorithm constraints check. protected boolean conformsToAlgorithmConstraints( AlgorithmConstraints constraints, Certificate[] chain, String variant) { - if (constraintsDisabled) { + if (checksDisabled) { return true; } @@ -165,6 +170,13 @@ protected boolean conformsToAlgorithmConstraints( return true; } + protected CheckResult certificateCheck( + CheckType checkType, X509Certificate cert, Date date, + List serverNames, String idAlgorithm) { + return checksDisabled ? CheckResult.OK + : checkType.check(cert, date, serverNames, idAlgorithm); + } + // enum for the result of the extension check // NOTE: the order of the constants is important as they are used // for sorting, i.e. OK is best, followed by EXPIRED and EXTENSION_MISMATCH @@ -212,9 +224,8 @@ private static boolean getBit(boolean[] keyUsage, int bit) { // Check if this certificate is appropriate for this type of use // first check extensions, if they match, check expiration. - // - // Note: we may want to move this code into the sun.security.validator - // package + // NOTE: `conformsToAlgorithmConstraints` call above also does some + // basic keyUsage checks. CheckResult check(X509Certificate cert, Date date, List serverNames, String idAlgorithm) { @@ -260,7 +271,7 @@ CheckResult check(X509Certificate cert, Date date, } break; case "DH": - // require keyagreement bit + // require key agreement bit if (!getBit(ku, 4)) { return CheckResult.EXTENSION_MISMATCH; } @@ -299,21 +310,21 @@ CheckResult check(X509Certificate cert, Date date, StandardConstants.SNI_HOST_NAME) { if (!(serverName instanceof SNIHostName)) { try { - serverName = - new SNIHostName(serverName.getEncoded()); + serverName = new SNIHostName( + serverName.getEncoded()); } catch (IllegalArgumentException iae) { // unlikely to happen, just in case ... if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Illegal server name: " + serverName); + SSLLogger.fine("Illegal server name: " + + serverName); } return CheckResult.INSENSITIVE; } } String hostname = - ((SNIHostName)serverName).getAsciiName(); + ((SNIHostName) serverName).getAsciiName(); try { X509TrustManagerImpl.checkIdentity(hostname, @@ -322,9 +333,9 @@ CheckResult check(X509Certificate cert, Date date, if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { SSLLogger.fine( - "Certificate identity does not match " + - "Server Name Indication (SNI): " + - hostname); + "Certificate identity does not match " + + "Server Name Indication (SNI): " + + hostname); } return CheckResult.INSENSITIVE; } @@ -337,7 +348,7 @@ CheckResult check(X509Certificate cert, Date date, return CheckResult.OK; } - public String getValidator() { + String getValidator() { if (this == CLIENT) { return Validator.VAR_TLS_CLIENT; } else if (this == SERVER) { @@ -346,4 +357,195 @@ public String getValidator() { return Validator.VAR_GENERIC; } } + + // A candidate match + // identifies the entry by builder and alias + // and includes the result of the certificate check + protected static class EntryStatus implements Comparable { + + final int keyStoreIndex; + final int keyIndex; + final String alias; + final CheckResult checkResult; + + EntryStatus(int keyStoreIndex, int keyIndex, String alias, + Certificate[] chain, CheckResult checkResult) { + this.keyStoreIndex = keyStoreIndex; + this.keyIndex = keyIndex; + this.alias = alias; + this.checkResult = checkResult; + } + + @Override + public int compareTo(EntryStatus other) { + int result = this.checkResult.compareTo(other.checkResult); + return (result == 0) ? (this.keyIndex - other.keyIndex) : result; + } + + @Override + public String toString() { + String s = alias + " (verified: " + checkResult + ")"; + if (keyStoreIndex == 0) { + return s; + } else { + return "KeyStore #" + keyStoreIndex + ", alias: " + s; + } + } + } + + // Class to help verify that the public key algorithm (and optionally + // the signature algorithm) of a certificate matches what we need. + protected static class KeyType { + + final String keyAlgorithm; + + // In TLS 1.2, the signature algorithm has been obsoleted by the + // supported_signature_algorithms, and the certificate type no longer + // restricts the algorithm used to sign the certificate. + // + // However, because we don't support certificate type checking other + // than rsa_sign, dss_sign and ecdsa_sign, we don't have to check the + // protocol version here. + final String sigKeyAlgorithm; + + KeyType(String algorithm) { + int k = algorithm.indexOf('_'); + if (k == -1) { + keyAlgorithm = algorithm; + sigKeyAlgorithm = null; + } else { + keyAlgorithm = algorithm.substring(0, k); + sigKeyAlgorithm = algorithm.substring(k + 1); + } + } + + boolean matches(Certificate[] chain) { + if (!chain[0].getPublicKey().getAlgorithm().equals(keyAlgorithm)) { + return false; + } + if (sigKeyAlgorithm == null) { + return true; + } + if (chain.length > 1) { + // if possible, check the public key in the issuer cert + return sigKeyAlgorithm.equals( + chain[1].getPublicKey().getAlgorithm()); + } else { + // Check the signature algorithm of the certificate itself. + // Look for the "withRSA" in "SHA1withRSA", etc. + X509Certificate issuer = (X509Certificate) chain[0]; + String sigAlgName = + issuer.getSigAlgName().toUpperCase(Locale.ENGLISH); + String pattern = + "WITH" + sigKeyAlgorithm.toUpperCase(Locale.ENGLISH); + return sigAlgName.contains(pattern); + } + } + } + + protected static List getKeyTypes(String... keyTypes) { + if ((keyTypes == null) || + (keyTypes.length == 0) || (keyTypes[0] == null)) { + return null; + } + List list = new ArrayList<>(keyTypes.length); + for (String keyType : keyTypes) { + list.add(new KeyType(keyType)); + } + return list; + } + + // Make a Set out of the array + protected Set getIssuerSet(Principal[] issuers) { + if ((issuers != null) && (issuers.length != 0)) { + return new HashSet<>(Arrays.asList(issuers)); + } else { + return null; + } + } + + protected EntryStatus checkAlias(int keyStoreIndex, String alias, + Certificate[] chain, Date verificationDate, List keyTypes, + Set issuerSet, CheckType checkType, + AlgorithmConstraints constraints, + List requestedServerNames, String idAlgorithm) { + + // --- Mandatory checks --- + + if ((chain == null) || (chain.length == 0)) { + // Must be secret key entry, ignore + return null; + } + + for (Certificate cert : chain) { + if (!(cert instanceof X509Certificate)) { + // Not an X509Certificate, ignore this alias + return null; + } + } + + // Check keytype + int keyIndex = -1; + int j = 0; + + for (KeyType keyType : keyTypes) { + if (keyType.matches(chain)) { + keyIndex = j; + break; + } + j++; + } + + if (keyIndex == -1) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": key algorithm does not match"); + } + return null; + } + + // Check issuers + if (issuerSet != null) { + boolean found = false; + for (Certificate cert : chain) { + X509Certificate xcert = (X509Certificate) cert; + if (issuerSet.contains(xcert.getIssuerX500Principal())) { + found = true; + break; + } + } + if (!found) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine( + "Ignore alias " + alias + + ": issuers do not match"); + } + return null; + } + } + + // --- Optional checks, depending on "checksDisabled" toggle --- + + // Check the algorithm constraints + if (constraints != null && + !conformsToAlgorithmConstraints(constraints, chain, + checkType.getValidator())) { + + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": certificate chain does not conform to " + + "algorithm constraints"); + } + return null; + } + + // Endpoint certificate check + CheckResult checkResult = certificateCheck(checkType, + (X509Certificate) chain[0], + verificationDate == null ? new Date() : verificationDate, + requestedServerNames, idAlgorithm); + + return new EntryStatus( + keyStoreIndex, keyIndex, alias, chain, checkResult); + } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 9f0e1d40905c4..cf1429bf185a5 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -34,9 +34,9 @@ import java.security.KeyStore.PrivateKeyEntry; import java.security.Principal; import java.security.PrivateKey; -import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.*; @@ -58,7 +58,7 @@ * @author Andreas Sterbenz */ -final class X509KeyManagerImpl extends X509KeyManagerConstraints { +final class X509KeyManagerImpl extends X509KeyManagerCertChecking { // for unit testing only, set via privileged reflection private static Date verificationDate; @@ -79,8 +79,7 @@ final class X509KeyManagerImpl extends X509KeyManagerConstraints { X509KeyManagerImpl(List builders) { this.builders = builders; uidCounter = new AtomicLong(); - entryCacheMap = Collections.synchronizedMap - (new SizedMap<>()); + entryCacheMap = new ConcurrentHashMap<>(); } // LinkedHashMap with a max size of 10 @@ -115,14 +114,14 @@ public PrivateKey getPrivateKey(String alias) { public String chooseClientAlias(String[] keyTypes, Principal[] issuers, Socket socket) { return chooseAlias(getKeyTypes(keyTypes), issuers, CheckType.CLIENT, - getAlgorithmConstraints(socket)); + getAlgorithmConstraints(socket), null, null); } @Override public String chooseEngineClientAlias(String[] keyTypes, Principal[] issuers, SSLEngine engine) { return chooseAlias(getKeyTypes(keyTypes), issuers, CheckType.CLIENT, - getAlgorithmConstraints(engine)); + getAlgorithmConstraints(engine), null, null); } @Override @@ -163,12 +162,12 @@ public String chooseEngineServerAlias(String keyType, @Override public String[] getClientAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.CLIENT, null); + return getAliases(keyType, issuers, CheckType.CLIENT); } @Override public String[] getServerAliases(String keyType, Principal[] issuers) { - return getAliases(keyType, issuers, CheckType.SERVER, null); + return getAliases(keyType, issuers, CheckType.SERVER); } // @@ -180,7 +179,7 @@ public String[] getServerAliases(String keyType, Principal[] issuers) { // between the calls to getCertificateChain() and getPrivateKey() // even if tokens are inserted or removed private String makeAlias(EntryStatus entry) { - return uidCounter.incrementAndGet() + "." + entry.builderIndex + "." + return uidCounter.incrementAndGet() + "." + entry.keyStoreIndex + "." + entry.alias; } @@ -225,68 +224,6 @@ private PrivateKeyEntry getEntry(String alias) { } } - // Class to help verify that the public key algorithm (and optionally - // the signature algorithm) of a certificate matches what we need. - private static class KeyType { - - final String keyAlgorithm; - - // In TLS 1.2, the signature algorithm has been obsoleted by the - // supported_signature_algorithms, and the certificate type no longer - // restricts the algorithm used to sign the certificate. - // - // However, because we don't support certificate type checking other - // than rsa_sign, dss_sign and ecdsa_sign, we don't have to check the - // protocol version here. - final String sigKeyAlgorithm; - - KeyType(String algorithm) { - int k = algorithm.indexOf('_'); - if (k == -1) { - keyAlgorithm = algorithm; - sigKeyAlgorithm = null; - } else { - keyAlgorithm = algorithm.substring(0, k); - sigKeyAlgorithm = algorithm.substring(k + 1); - } - } - - boolean matches(Certificate[] chain) { - if (!chain[0].getPublicKey().getAlgorithm().equals(keyAlgorithm)) { - return false; - } - if (sigKeyAlgorithm == null) { - return true; - } - if (chain.length > 1) { - // if possible, check the public key in the issuer cert - return sigKeyAlgorithm.equals( - chain[1].getPublicKey().getAlgorithm()); - } else { - // Check the signature algorithm of the certificate itself. - // Look for the "withRSA" in "SHA1withRSA", etc. - X509Certificate issuer = (X509Certificate)chain[0]; - String sigAlgName = - issuer.getSigAlgName().toUpperCase(Locale.ENGLISH); - String pattern = - "WITH" + sigKeyAlgorithm.toUpperCase(Locale.ENGLISH); - return sigAlgName.contains(pattern); - } - } - } - - private static List getKeyTypes(String ... keyTypes) { - if ((keyTypes == null) || - (keyTypes.length == 0) || (keyTypes[0] == null)) { - return null; - } - List list = new ArrayList<>(keyTypes.length); - for (String keyType : keyTypes) { - list.add(new KeyType(keyType)); - } - return list; - } - /* * Return the best alias that fits the given parameters. * The algorithm we use is: @@ -300,13 +237,6 @@ private static List getKeyTypes(String ... keyTypes) { * with appropriate key usage to certs with the wrong key usage. * return the first one of them. */ - private String chooseAlias(List keyTypeList, Principal[] issuers, - CheckType checkType, AlgorithmConstraints constraints) { - - return chooseAlias(keyTypeList, issuers, - checkType, constraints, null, null); - } - private String chooseAlias(List keyTypeList, Principal[] issuers, CheckType checkType, AlgorithmConstraints constraints, List requestedServerNames, String idAlgorithm) { @@ -361,8 +291,8 @@ private String chooseAlias(List keyTypeList, Principal[] issuers, * and certificates with the wrong extensions). * The perfect matches will be first in the array. */ - public String[] getAliases(String keyType, Principal[] issuers, - CheckType checkType, AlgorithmConstraints constraints) { + private String[] getAliases( + String keyType, Principal[] issuers, CheckType checkType) { if (keyType == null) { return null; } @@ -373,8 +303,7 @@ public String[] getAliases(String keyType, Principal[] issuers, for (int i = 0, n = builders.size(); i < n; i++) { try { List results = getAliases(i, keyTypeList, - issuerSet, true, checkType, constraints, - null, null); + issuerSet, true, checkType, null, null, null); if (results != null) { if (allResults == null) { allResults = new ArrayList<>(); @@ -408,50 +337,6 @@ private String[] toAliases(List results) { return s; } - // make a Set out of the array - private Set getIssuerSet(Principal[] issuers) { - if ((issuers != null) && (issuers.length != 0)) { - return new HashSet<>(Arrays.asList(issuers)); - } else { - return null; - } - } - - // a candidate match - // identifies the entry by builder and alias - // and includes the result of the certificate check - private static class EntryStatus implements Comparable { - - final int builderIndex; - final int keyIndex; - final String alias; - final CheckResult checkResult; - - EntryStatus(int builderIndex, int keyIndex, String alias, - Certificate[] chain, CheckResult checkResult) { - this.builderIndex = builderIndex; - this.keyIndex = keyIndex; - this.alias = alias; - this.checkResult = checkResult; - } - - @Override - public int compareTo(EntryStatus other) { - int result = this.checkResult.compareTo(other.checkResult); - return (result == 0) ? (this.keyIndex - other.keyIndex) : result; - } - - @Override - public String toString() { - String s = alias + " (verified: " + checkResult + ")"; - if (builderIndex == 0) { - return s; - } else { - return "Builder #" + builderIndex + ", alias: " + s; - } - } - } - /* * Return a List of all candidate matches in the specified builder * that fit the parameters. @@ -488,97 +373,33 @@ private List getAliases(int builderIndex, Builder builder = builders.get(builderIndex); KeyStore ks = builder.getKeyStore(); List results = null; - Date date = verificationDate; boolean preferred = false; + for (Enumeration e = ks.aliases(); e.hasMoreElements(); ) { + String alias = e.nextElement(); + // check if it is a key entry (private key or secret key) if (!ks.isKeyEntry(alias)) { continue; } - Certificate[] chain = ks.getCertificateChain(alias); - if ((chain == null) || (chain.length == 0)) { - // must be secret key entry, ignore - continue; - } + EntryStatus status = checkAlias(builderIndex, alias, + ks.getCertificateChain(alias), + verificationDate, keyTypes, issuerSet, checkType, + constraints, requestedServerNames, idAlgorithm); - boolean incompatible = false; - for (Certificate cert : chain) { - if (!(cert instanceof X509Certificate)) { - // not an X509Certificate, ignore this alias - incompatible = true; - break; - } - } - if (incompatible) { + if (status == null) { continue; } - // check keytype - int keyIndex = -1; - int j = 0; - for (KeyType keyType : keyTypes) { - if (keyType.matches(chain)) { - keyIndex = j; - break; - } - j++; - } - if (keyIndex == -1) { - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias - + ": key algorithm does not match"); - } - continue; - } - // check issuers - if (issuerSet != null) { - boolean found = false; - for (Certificate cert : chain) { - X509Certificate xcert = (X509Certificate)cert; - if (issuerSet.contains(xcert.getIssuerX500Principal())) { - found = true; - break; - } - } - if (!found) { - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Ignore alias " + alias - + ": issuers do not match"); - } - continue; - } - } - - // check the algorithm constraints - if (constraints != null && - !conformsToAlgorithmConstraints(constraints, chain, - checkType.getValidator())) { - - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias + - ": certificate chain does not conform to " + - "algorithm constraints"); - } - continue; - } - - if (date == null) { - date = new Date(); - } - CheckResult checkResult = - checkType.check((X509Certificate)chain[0], date, - requestedServerNames, idAlgorithm); - EntryStatus status = - new EntryStatus(builderIndex, keyIndex, - alias, chain, checkResult); - if (!preferred && checkResult == CheckResult.OK && keyIndex == 0) { + if (!preferred && status.checkResult == CheckResult.OK + && status.keyIndex == 0) { preferred = true; } + if (preferred && !findAll) { - // if we have a good match and do not need all matches, + // If we have a good match and do not need all matches, // return immediately return Collections.singletonList(status); } else { @@ -588,6 +409,7 @@ private List getAliases(int builderIndex, results.add(status); } } + return results; } } diff --git a/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java b/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java index 5803da2510a6d..1e0afd0e586a4 100644 --- a/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java +++ b/test/jdk/sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java @@ -246,7 +246,7 @@ private static CertificateBuilder customCertificateBuilder( CertificateBuilder builder = new CertificateBuilder() .setSubjectName(subjectName) .setPublicKey(publicKey) - .setNotAfter( + .setNotBefore( Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) .setSerialNumber( diff --git a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java index 3a8da980108b1..d690bd970d390 100644 --- a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java +++ b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java @@ -235,7 +235,7 @@ private void checkKeySize(KeyStore ks) throws Exception { public static void main(String[] args) throws Exception { // Disable KeyManager's algorithm constraints checking. System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", "true"); + "jdk.tls.keymanager.disableCertChecking", "true"); if (debug) { System.setProperty("javax.net.debug", "all"); diff --git a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java index 8040f401b5312..7d8abdb7833b0 100644 --- a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java +++ b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java @@ -154,7 +154,7 @@ private static SSLContext getSSLContext( // so we check against local supported signature // algorithms which constitutes the fix being tested. System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", "true"); + "jdk.tls.keymanager.disableCertChecking", "true"); KeyManagerFactory kmf = KeyManagerFactory.getInstance( KeyManagerFactory.getDefaultAlgorithm()); kmf.init(ks, passphrase); @@ -217,7 +217,7 @@ private static CertificateBuilder customCertificateBuilder( CertificateBuilder builder = new CertificateBuilder() .setSubjectName(subjectName) .setPublicKey(publicKey) - .setNotAfter( + .setNotBefore( Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) .setSerialNumber( diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index 24c174bc17fb2..fb410f33dd144 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -76,7 +76,7 @@ public static void main(String[] args) throws Exception { String certSignatureAlg = args[2]; System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", disabled); + "jdk.tls.keymanager.disableCertChecking", disabled); SecurityUtils.addToDisabledTlsAlgs(certSignatureAlg); X509KeyManager km = getKeyManager(kmAlg, certSignatureAlg); @@ -167,7 +167,7 @@ private static CertificateBuilder customCertificateBuilder( CertificateBuilder builder = new CertificateBuilder() .setSubjectName(subjectName) .setPublicKey(publicKey) - .setNotAfter( + .setNotBefore( Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) .setSerialNumber( @@ -179,5 +179,4 @@ private static CertificateBuilder customCertificateBuilder( return builder; } - } diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java new file mode 100644 index 0000000000000..6a47dd3329fd5 --- /dev/null +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -0,0 +1,396 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import static jdk.test.lib.Asserts.assertEquals; +import static jdk.test.lib.Asserts.assertNull; + +import java.io.IOException; +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.KeyStore; +import java.security.Principal; +import java.security.PublicKey; +import java.security.SecureRandom; +import java.security.cert.Certificate; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Date; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.X509KeyManager; +import javax.security.auth.x500.X500Principal; +import jdk.test.lib.security.CertificateBuilder; +import sun.security.x509.AuthorityKeyIdentifierExtension; +import sun.security.x509.GeneralName; +import sun.security.x509.GeneralNames; +import sun.security.x509.KeyIdentifier; +import sun.security.x509.SerialNumber; +import sun.security.x509.X500Name; + +/* + * @test + * @bug 8359069 + * @summary Support certificate checks in SunX509 key manager + * @modules java.base/sun.security.x509 + * java.base/sun.security.util + * @library /test/lib + * @run main/othervm CertChecking false SunX509 + * @run main/othervm CertChecking true SunX509 + * @run main/othervm CertChecking false PKIX + * @run main/othervm CertChecking true PKIX + */ + +/* + * This class tests against the certificate's expiration, key usage and issuers. + */ + +public class CertChecking { + + private static final String PREFERRED_ALIAS = "preferred-alias"; + private static final String EXPIRED_ALIAS = "expired-alias"; + private static final String USAGE_MISMATCH_ALIAS = "usage-mismatch-alias"; + private static final String CA_KEY_TYPE = "RSA"; + private static final String CERT_SIG_ALG = "SHA256withRSA"; + private static final String CA_ISSUER_STRING = + "O=TrustedCert, L=Some-City, ST=Some-State, C=US"; + private static final String EE_ISSUER_STRING = + "O=EndpointCert, L=Some-City, ST=Some-State, C=US"; + private static final String UNKNOWN_ISSUER_STRING = + "O=UnknownCert, L=Some-City, ST=Some-State, C=US"; + + /* + * Certificate KeyUsage reference: + * + * digitalSignature (0), + * nonRepudiation (1), + * keyEncipherment (2), + * dataEncipherment (3), + * keyAgreement (4), + * keyCertSign (5), + * cRLSign (6), + * encipherOnly (7), + * decipherOnly (8) + */ + + private static final boolean[] DEFAULT_KEY_USAGES = + new boolean[]{true, true, true, true, true, true}; + private static final boolean[] NONE_KEY_USAGES = + new boolean[]{false, false, false, false, false, false}; + private static final boolean[] NO_DG_USAGE = + new boolean[]{false, true, true, true, true, true}; + private static final boolean[] NO_DG_NO_KE_USAGE = + new boolean[]{false, true, false, true, true, true}; + private static final boolean[] NO_KA_USAGE = + new boolean[]{true, true, true, true, false, true}; + + + public static void main(String[] args) throws Exception { + if (args.length != 2) { + throw new RuntimeException("Wrong number of arguments"); + } + + String disabled = args[0]; + String kmAlg = args[1]; + + System.setProperty( + "jdk.tls.keymanager.disableCertChecking", disabled); + + // --- Usage and expired test cases -- + + // Both should fail with no usages at all + usageTestCase(disabled, kmAlg, "RSA", NONE_KEY_USAGES, true, true); + + // Only client should fail with RSA algorithm and + // no digital signature bit set + usageTestCase(disabled, kmAlg, "RSA", NO_DG_USAGE, false, true); + + // Only server should fail with RSA algorithm and + // no digital signature bit set + usageTestCase(disabled, kmAlg, "RSASSA-PSS", NO_DG_USAGE, true, false); + + // Both should fail with DSA algorithm and no digital signature bit set + usageTestCase(disabled, kmAlg, "DSA", NO_DG_USAGE, true, true); + + // Both should fail with EC algorithm and no digital signature bit set + usageTestCase(disabled, kmAlg, "EC", NO_DG_USAGE, true, true); + + // Both should fail with RSA algorithm and missing digital signature and + // key encipherment bits. + usageTestCase(disabled, kmAlg, "RSA", NO_DG_NO_KE_USAGE, true, true); + + // Both should fail with DH algorithm and no key agreement bit set. + usageTestCase(disabled, kmAlg, "DH", NO_KA_USAGE, true, true); + + // Only server should fail with EC algorithm and + // no digital signature bit set + usageTestCase(disabled, kmAlg, "EC", NO_KA_USAGE, true, false); + + // --- Issuer match test cases --- + + // Check CA issuer match + issuerTestCase(disabled, kmAlg, "RSA", + new Principal[]{new X500Principal(CA_ISSUER_STRING)}, true); + + // Issuer that is not in the chain should not match. + issuerTestCase(disabled, kmAlg, "RSA", + new Principal[]{new X500Principal(UNKNOWN_ISSUER_STRING)}, false); + } + + private static void usageTestCase(String disabled, String kmAlg, + String keyAlg, boolean[] certKeyUsages, boolean checkServer, + boolean checkClient) throws Exception { + + X509KeyManager km = getKeyManager( + kmAlg, keyAlg, certKeyUsages); + + String chosenServerAlias = km.chooseServerAlias(keyAlg, null, null); + String chosenClientAlias = km.chooseClientAlias( + new String[]{keyAlg}, null, null); + + String[] allServerAliases = km.getServerAliases(keyAlg, null); + String[] allClientAliases = km.getClientAliases(keyAlg, null); + + if ("true".equals(disabled)) { + // Initial order alias returned + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(chosenServerAlias)); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(chosenClientAlias)); + + // Assert the initial order of all aliases. + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allServerAliases[0])); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allClientAliases[0])); + assertEquals(PREFERRED_ALIAS, normalizeAlias(allServerAliases[1])); + assertEquals(PREFERRED_ALIAS, normalizeAlias(allClientAliases[1])); + assertEquals(EXPIRED_ALIAS, normalizeAlias(allServerAliases[2])); + assertEquals(EXPIRED_ALIAS, normalizeAlias(allClientAliases[2])); + + } else { + if (checkServer) { + // Preferred alias returned + assertEquals(PREFERRED_ALIAS, + normalizeAlias(chosenServerAlias)); + + // Assert the correct sorted order of all aliases. + assertEquals(PREFERRED_ALIAS, + normalizeAlias(allServerAliases[0])); + assertEquals(EXPIRED_ALIAS, + normalizeAlias(allServerAliases[1])); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allServerAliases[2])); + } + + if (checkClient) { + // Preferred alias returned + assertEquals(PREFERRED_ALIAS, + normalizeAlias(chosenClientAlias)); + + // Assert the correct sorted order of all aliases. + assertEquals(PREFERRED_ALIAS, + normalizeAlias(allClientAliases[0])); + assertEquals(EXPIRED_ALIAS, + normalizeAlias(allClientAliases[1])); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allClientAliases[2])); + } + } + } + + private static void issuerTestCase(String disabled, String kmAlg, + String keyAlg, Principal[] issuers, boolean found) throws Exception { + + X509KeyManager km = getKeyManager( + kmAlg, keyAlg, NONE_KEY_USAGES); + + String chosenServerAlias = km.chooseServerAlias(keyAlg, issuers, null); + String chosenClientAlias = km.chooseClientAlias( + new String[]{keyAlg}, issuers, null); + + String[] allServerAliases = km.getServerAliases(keyAlg, issuers); + String[] allClientAliases = km.getClientAliases(keyAlg, issuers); + + if (found) { + if ("true".equals(disabled)) { + assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenServerAlias)); + assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenClientAlias)); + + // Assert the initial order of all aliases. + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allServerAliases[0])); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allClientAliases[0])); + assertEquals(PREFERRED_ALIAS, normalizeAlias(allServerAliases[1])); + assertEquals(PREFERRED_ALIAS, normalizeAlias(allClientAliases[1])); + assertEquals(EXPIRED_ALIAS, normalizeAlias(allServerAliases[2])); + assertEquals(EXPIRED_ALIAS, normalizeAlias(allClientAliases[2])); + } else { + assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenServerAlias)); + assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenClientAlias)); + + // Assert the correct sorted order of all aliases. + assertEquals(PREFERRED_ALIAS, + normalizeAlias(allServerAliases[0])); + assertEquals(EXPIRED_ALIAS, + normalizeAlias(allServerAliases[1])); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(allServerAliases[2])); + } + } else { + assertNull(chosenServerAlias); + assertNull(chosenClientAlias); + assertNull(allServerAliases); + assertNull(allClientAliases); + } + } + + + // PKIX KeyManager adds a cache prefix to an alias. + private static String normalizeAlias(String alias) { + return alias.substring(alias.lastIndexOf(".") + 1); + + } + + private static X509KeyManager getKeyManager(String kmAlg, + String keyAlg, boolean[] certKeyUsages) + throws Exception { + + // Create a key store. + KeyStore ks = KeyStore.getInstance("PKCS12"); + ks.load(null, null); + + // Generate and set the trusted cert. + KeyPair caKeys = KeyPairGenerator.getInstance(CA_KEY_TYPE) + .generateKeyPair(); + X509Certificate trustedCert = createTrustedCert(caKeys); + ks.setCertificateEntry("CA entry", trustedCert); + + // Generate valid certificate chain. + KeyPairGenerator kpg = KeyPairGenerator.getInstance(keyAlg); + KeyPair validEndpointKeys = kpg.generateKeyPair(); + + X509Certificate validEndpointCert = customCertificateBuilder( + EE_ISSUER_STRING, + validEndpointKeys.getPublic(), caKeys.getPublic(), + Instant.now(), DEFAULT_KEY_USAGES) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), CERT_SIG_ALG); + + Certificate[] validChain = new Certificate[2]; + validChain[0] = validEndpointCert; + validChain[1] = trustedCert; + + // Generate expired certificate chain. + KeyPair expiredEndpointKeys = kpg.generateKeyPair(); + + X509Certificate expiredEndpointCert = customCertificateBuilder( + EE_ISSUER_STRING, + expiredEndpointKeys.getPublic(), caKeys.getPublic(), + Instant.now().minus(1, ChronoUnit.DAYS), DEFAULT_KEY_USAGES) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), CERT_SIG_ALG); + + Certificate[] expiredChain = new Certificate[2]; + expiredChain[0] = expiredEndpointCert; + expiredChain[1] = trustedCert; + + // Generate usage mismatch certificate chain. + KeyPair usageMismatchEndpointKeys = kpg.generateKeyPair(); + + X509Certificate usageMismatchEndpointCert = customCertificateBuilder( + EE_ISSUER_STRING, + usageMismatchEndpointKeys.getPublic(), caKeys.getPublic(), + Instant.now(), certKeyUsages) + .addBasicConstraintsExt(false, false, -1) + .build(trustedCert, caKeys.getPrivate(), CERT_SIG_ALG); + + Certificate[] usageMismatchChain = new Certificate[2]; + usageMismatchChain[0] = usageMismatchEndpointCert; + usageMismatchChain[1] = trustedCert; + + // Import the key entries, order matters. + final char[] passphrase = "passphrase".toCharArray(); + ks.setKeyEntry(USAGE_MISMATCH_ALIAS, + usageMismatchEndpointKeys.getPrivate(), passphrase, + usageMismatchChain); + ks.setKeyEntry(PREFERRED_ALIAS, validEndpointKeys.getPrivate(), + passphrase, + validChain); + ks.setKeyEntry(EXPIRED_ALIAS, expiredEndpointKeys.getPrivate(), + passphrase, + expiredChain); + + KeyManagerFactory kmf = KeyManagerFactory.getInstance(kmAlg); + kmf.init(ks, passphrase); + + return (X509KeyManager) kmf.getKeyManagers()[0]; + } + + // Certificate-building helper methods. + + private static X509Certificate createTrustedCert(KeyPair caKeys) + throws Exception { + SecureRandom random = new SecureRandom(); + + KeyIdentifier kid = new KeyIdentifier(caKeys.getPublic()); + GeneralNames gns = new GeneralNames(); + GeneralName name = new GeneralName(new X500Name( + "O=Some-Org, L=Some-City, ST=Some-State, C=US")); + gns.add(name); + BigInteger serialNumber = BigInteger.valueOf( + random.nextLong(1000000) + 1); + return customCertificateBuilder( + CA_ISSUER_STRING, + caKeys.getPublic(), caKeys.getPublic(), Instant.now(), + DEFAULT_KEY_USAGES) + .setSerialNumber(serialNumber) + .addExtension(new AuthorityKeyIdentifierExtension(kid, gns, + new SerialNumber(serialNumber))) + .addBasicConstraintsExt(true, true, -1) + .build(null, caKeys.getPrivate(), CERT_SIG_ALG); + } + + private static CertificateBuilder customCertificateBuilder( + String subjectName, PublicKey publicKey, PublicKey caKey, + Instant certDate, boolean[] certKeyUsages) + throws CertificateException, IOException { + SecureRandom random = new SecureRandom(); + + CertificateBuilder builder = new CertificateBuilder() + .setSubjectName(subjectName) + .setPublicKey(publicKey) + .setNotBefore( + Date.from(certDate.minus(1, ChronoUnit.HOURS))) + .setNotAfter(Date.from(certDate.plus(1, ChronoUnit.HOURS))) + .setSerialNumber( + BigInteger.valueOf(random.nextLong(1000000) + 1)) + .addSubjectKeyIdExt(publicKey) + .addAuthorityKeyIdExt(caKey); + builder.addKeyUsageExt(certKeyUsages); + + return builder; + } +} diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index 8e4cfde3ce536..c72b75ad89f94 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -68,7 +68,7 @@ * @run main/othervm PeerConstraintsCheck true PKIX */ -/** +/* * This class tests against the peer supported certificate signatures sent in * "signature_algorithms_cert" extension. */ @@ -107,7 +107,7 @@ public static void main(String[] args) throws Exception { String kmAlg = args[1]; System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", disabled); + "jdk.tls.keymanager.disableCertChecking", disabled); if ("true".equals(disabled)) { new PeerConstraintsCheck(kmAlg).run(); @@ -259,7 +259,7 @@ private static CertificateBuilder customCertificateBuilder( CertificateBuilder builder = new CertificateBuilder() .setSubjectName(subjectName) .setPublicKey(publicKey) - .setNotAfter( + .setNotBefore( Date.from(Instant.now().minus(1, ChronoUnit.HOURS))) .setNotAfter(Date.from(Instant.now().plus(1, ChronoUnit.HOURS))) .setSerialNumber( diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index 4af384bb474be..c3f9403604ac4 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -30,10 +30,7 @@ * @test * @bug 6916074 8170131 * @summary Add support for TLS 1.2 - * @run main/othervm PKIXExtendedTM 0 - * @run main/othervm PKIXExtendedTM 1 * @run main/othervm PKIXExtendedTM 2 - * @run main/othervm PKIXExtendedTM 3 */ import java.net.*; @@ -1117,7 +1114,7 @@ public static void main(String args[]) throws Exception { // Disable KeyManager's algorithm constraints checking as this test // is about TrustManager's constraints check. System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", "true"); + "jdk.tls.keymanager.disableCertChecking", "true"); if (args.length != 1) { throw new Exception("Incorrect number of arguments"); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 452e8db426faf..0f7714dac46db 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -51,7 +51,7 @@ public class PrintSSL { public static void main(String[] args) throws Throwable { // Disable KeyManager's algorithm constraints checking. System.setProperty( - "jdk.tls.keymanager.disableConstraintsChecking", "true"); + "jdk.tls.keymanager.disableCertChecking", "true"); Files.deleteIfExists(Paths.get("keystore")); From eaf343ad002880bf9d8e60fbcf99762006d4033e Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 18 Jun 2025 11:52:09 -0400 Subject: [PATCH 18/26] Add more unit tests. Some code refactoring and adjustments. --- .../security/ssl/SunX509KeyManagerImpl.java | 36 +------- .../ssl/X509KeyManagerCertChecking.java | 35 ++++++-- .../sun/security/ssl/X509KeyManagerImpl.java | 18 ++-- .../AlgorithmConstraintsCheck.java | 22 ++++- .../ssl/X509KeyManager/CertChecking.java | 89 ++++++++++++++++--- 5 files changed, 135 insertions(+), 65 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index c0c9fc67b4292..968da1f21bcdb 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -238,10 +238,6 @@ private String chooseAlias(List keyTypes, Principal[] issuers, CheckType checkType, AlgorithmConstraints constraints, List requestedServerNames, String idAlgorithm) { - if (keyTypes == null) { - return null; - } - String[] aliases = getAliases( keyTypes, issuers, checkType, constraints, requestedServerNames, idAlgorithm); @@ -269,16 +265,7 @@ private String[] getAliases(List keyTypes, Principal[] issuers, return null; } - if (issuers == null) { - issuers = new X500Principal[0]; - } - - if (!(issuers instanceof X500Principal[])) { - // normally, this will never happen but try to recover if it does - issuers = convertPrincipals(issuers); - } - - Set issuerSet = getIssuerSet(issuers); + Set issuerSet = getIssuerSet(issuers); List results = null; for (Map.Entry entry : @@ -311,25 +298,4 @@ private String[] getAliases(List keyTypes, Principal[] issuers, Collections.sort(results); return results.stream().map(r -> r.alias).toArray(String[]::new); } - - /* - * Convert an array of Principals to an array of X500Principals, if - * possible. Principals that cannot be converted are ignored. - */ - private static X500Principal[] convertPrincipals(Principal[] principals) { - List list = new ArrayList<>(principals.length); - for (int i = 0; i < principals.length; i++) { - Principal p = principals[i]; - if (p instanceof X500Principal) { - list.add((X500Principal) p); - } else { - try { - list.add(new X500Principal(p.getName())); - } catch (IllegalArgumentException e) { - // ignore - } - } - } - return list.toArray(new X500Principal[0]); - } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index 4e7d75a3360a1..bc264a956de51 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -33,7 +33,6 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashSet; @@ -48,6 +47,7 @@ import javax.net.ssl.SSLSocket; import javax.net.ssl.StandardConstants; import javax.net.ssl.X509ExtendedKeyManager; +import javax.security.auth.x500.X500Principal; import sun.security.provider.certpath.AlgorithmChecker; import sun.security.util.KnownOIDs; import sun.security.validator.Validator; @@ -129,7 +129,7 @@ protected AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { } // Algorithm constraints check. - protected boolean conformsToAlgorithmConstraints( + private boolean conformsToAlgorithmConstraints( AlgorithmConstraints constraints, Certificate[] chain, String variant) { @@ -170,7 +170,7 @@ protected boolean conformsToAlgorithmConstraints( return true; } - protected CheckResult certificateCheck( + private CheckResult certificateCheck( CheckType checkType, X509Certificate cert, Date date, List serverNames, String idAlgorithm) { return checksDisabled ? CheckResult.OK @@ -455,18 +455,35 @@ protected static List getKeyTypes(String... keyTypes) { return list; } - // Make a Set out of the array - protected Set getIssuerSet(Principal[] issuers) { - if ((issuers != null) && (issuers.length != 0)) { - return new HashSet<>(Arrays.asList(issuers)); + // Make a Set out of the array. + protected static Set getIssuerSet(Principal[] issuers) { + + if (issuers != null && issuers.length != 0) { + Set ret = new HashSet<>(issuers.length); + + for (Principal p : issuers) { + if (p instanceof X500Principal) { + ret.add((X500Principal) p); + } else { + // Normally, this will never happen but try to recover if + // it does. + try { + ret.add(new X500Principal(p.getName())); + } catch (Exception e) { + // ignore + } + } + } + return ret.isEmpty() ? null : ret; } else { return null; } } + // Entry point to do all certificate checks. protected EntryStatus checkAlias(int keyStoreIndex, String alias, Certificate[] chain, Date verificationDate, List keyTypes, - Set issuerSet, CheckType checkType, + Set issuerSet, CheckType checkType, AlgorithmConstraints constraints, List requestedServerNames, String idAlgorithm) { @@ -505,7 +522,7 @@ protected EntryStatus checkAlias(int keyStoreIndex, String alias, } // Check issuers - if (issuerSet != null) { + if (issuerSet != null && !issuerSet.isEmpty()) { boolean found = false; for (Certificate cert : chain) { X509Certificate xcert = (X509Certificate) cert; diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index cf1429bf185a5..4482ae59e178a 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -32,13 +32,16 @@ import java.security.KeyStore.Builder; import java.security.KeyStore.Entry; import java.security.KeyStore.PrivateKeyEntry; +import java.security.KeyStoreException; import java.security.Principal; import java.security.PrivateKey; +import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.*; +import javax.security.auth.x500.X500Principal; /** * The new X509 key manager implementation. The main differences to the @@ -245,8 +248,9 @@ private String chooseAlias(List keyTypeList, Principal[] issuers, return null; } - Set issuerSet = getIssuerSet(issuers); + Set issuerSet = getIssuerSet(issuers); List allResults = null; + for (int i = 0, n = builders.size(); i < n; i++) { try { List results = getAliases(i, keyTypeList, @@ -266,7 +270,7 @@ private String chooseAlias(List keyTypeList, Principal[] issuers, } allResults.addAll(results); } - } catch (Exception e) { + } catch (KeyStoreException e) { // ignore } } @@ -293,13 +297,15 @@ private String chooseAlias(List keyTypeList, Principal[] issuers, */ private String[] getAliases( String keyType, Principal[] issuers, CheckType checkType) { + if (keyType == null) { return null; } - Set issuerSet = getIssuerSet(issuers); + Set issuerSet = getIssuerSet(issuers); List keyTypeList = getKeyTypes(keyType); List allResults = null; + for (int i = 0, n = builders.size(); i < n; i++) { try { List results = getAliases(i, keyTypeList, @@ -310,7 +316,7 @@ private String[] getAliases( } allResults.addAll(results); } - } catch (Exception e) { + } catch (KeyStoreException e) { // ignore } } @@ -364,11 +370,11 @@ private String[] toAliases(List results) { * matches */ private List getAliases(int builderIndex, - List keyTypes, Set issuerSet, + List keyTypes, Set issuerSet, boolean findAll, CheckType checkType, AlgorithmConstraints constraints, List requestedServerNames, - String idAlgorithm) throws Exception { + String idAlgorithm) throws KeyStoreException { Builder builder = builders.get(builderIndex); KeyStore ks = builder.getKeyStore(); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index fb410f33dd144..c394d69d9d779 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -38,6 +38,7 @@ import java.time.temporal.ChronoUnit; import java.util.Date; import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509KeyManager; import jdk.test.lib.security.CertificateBuilder; import jdk.test.lib.security.SecurityUtils; @@ -79,24 +80,39 @@ public static void main(String[] args) throws Exception { "jdk.tls.keymanager.disableCertChecking", disabled); SecurityUtils.addToDisabledTlsAlgs(certSignatureAlg); - X509KeyManager km = getKeyManager(kmAlg, certSignatureAlg); + X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( + kmAlg, certSignatureAlg); String serverAlias = km.chooseServerAlias(KEY_TYPE, null, null); + String engineServerAlias = km.chooseEngineServerAlias( + KEY_TYPE, null, null); String clientAlias = km.chooseClientAlias( new String[]{KEY_TYPE}, null, null); + String engineClientAlias = km.chooseEngineClientAlias( + new String[]{KEY_TYPE}, null, null); // PKIX KeyManager adds a cache prefix to an alias. String serverAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "1.0." : ""; String clientAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "2.0." : ""; if ("true".equals(disabled)) { - assertEquals(serverAliasPrefix + CERT_ALIAS, serverAlias); - assertEquals(clientAliasPrefix + CERT_ALIAS, clientAlias); + assertEquals(CERT_ALIAS, normalizeAlias(serverAlias)); + assertEquals(CERT_ALIAS, normalizeAlias(engineServerAlias)); + assertEquals(CERT_ALIAS, normalizeAlias(clientAlias)); + assertEquals(CERT_ALIAS, normalizeAlias(engineClientAlias)); } else { assertNull(serverAlias); + assertNull(engineServerAlias); assertNull(clientAlias); + assertNull(engineClientAlias); } } + // PKIX KeyManager adds a cache prefix to an alias. + private static String normalizeAlias(String alias) { + return alias.substring(alias.lastIndexOf(".") + 1); + + } + private static X509KeyManager getKeyManager(String kmAlg, String certSignatureAlg) throws Exception { KeyPairGenerator kpg = KeyPairGenerator.getInstance(KEY_TYPE); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java index 6a47dd3329fd5..f82e15fa80d68 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -24,6 +24,7 @@ import static jdk.test.lib.Asserts.assertEquals; import static jdk.test.lib.Asserts.assertNull; +import com.sun.security.auth.UserPrincipal; import java.io.IOException; import java.math.BigInteger; import java.security.KeyPair; @@ -37,10 +38,14 @@ import java.security.cert.X509Certificate; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; import java.util.Date; +import java.util.List; import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.X509ExtendedKeyManager; import javax.net.ssl.X509KeyManager; import javax.security.auth.x500.X500Principal; +import jdk.test.lib.Asserts; import jdk.test.lib.security.CertificateBuilder; import sun.security.x509.AuthorityKeyIdentifierExtension; import sun.security.x509.GeneralName; @@ -153,21 +158,47 @@ public static void main(String[] args) throws Exception { issuerTestCase(disabled, kmAlg, "RSA", new Principal[]{new X500Principal(CA_ISSUER_STRING)}, true); + // Check CA issuer match with non-X500 principal + issuerTestCase(disabled, kmAlg, "RSA", + new Principal[]{new UserPrincipal(CA_ISSUER_STRING)}, true); + + // Non-convertable principal should match all + issuerTestCase(disabled, kmAlg, "RSA", + new Principal[]{new InvalidPrincipal()}, true); + + // Empty issuer array should match all + issuerTestCase(disabled, kmAlg, "RSA", new Principal[]{}, true); + + // Null issuer array should match all + issuerTestCase(disabled, kmAlg, "RSA", null, true); + // Issuer that is not in the chain should not match. issuerTestCase(disabled, kmAlg, "RSA", new Principal[]{new X500Principal(UNKNOWN_ISSUER_STRING)}, false); + + // --- Alias not found for given KeyType test cases --- + + // Null KeyType + aliasNotFoundTestCase(kmAlg, "RSA", null); + + // Wrong KeyType + aliasNotFoundTestCase(kmAlg, "RSA", "EC"); } private static void usageTestCase(String disabled, String kmAlg, String keyAlg, boolean[] certKeyUsages, boolean checkServer, boolean checkClient) throws Exception { - X509KeyManager km = getKeyManager( + X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( kmAlg, keyAlg, certKeyUsages); String chosenServerAlias = km.chooseServerAlias(keyAlg, null, null); + String chosenEngineServerAlias = km.chooseEngineServerAlias( + keyAlg, null, null); String chosenClientAlias = km.chooseClientAlias( new String[]{keyAlg}, null, null); + String chosenEngineClientAlias = km.chooseEngineClientAlias( + new String[]{keyAlg}, null, null); String[] allServerAliases = km.getServerAliases(keyAlg, null); String[] allClientAliases = km.getClientAliases(keyAlg, null); @@ -178,6 +209,10 @@ private static void usageTestCase(String disabled, String kmAlg, normalizeAlias(chosenServerAlias)); assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenClientAlias)); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(chosenEngineServerAlias)); + assertEquals(USAGE_MISMATCH_ALIAS, + normalizeAlias(chosenEngineClientAlias)); // Assert the initial order of all aliases. assertEquals(USAGE_MISMATCH_ALIAS, @@ -194,6 +229,8 @@ private static void usageTestCase(String disabled, String kmAlg, // Preferred alias returned assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenServerAlias)); + assertEquals(PREFERRED_ALIAS, + normalizeAlias(chosenEngineServerAlias)); // Assert the correct sorted order of all aliases. assertEquals(PREFERRED_ALIAS, @@ -208,6 +245,8 @@ private static void usageTestCase(String disabled, String kmAlg, // Preferred alias returned assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenClientAlias)); + assertEquals(PREFERRED_ALIAS, + normalizeAlias(chosenEngineClientAlias)); // Assert the correct sorted order of all aliases. assertEquals(PREFERRED_ALIAS, @@ -223,20 +262,25 @@ private static void usageTestCase(String disabled, String kmAlg, private static void issuerTestCase(String disabled, String kmAlg, String keyAlg, Principal[] issuers, boolean found) throws Exception { - X509KeyManager km = getKeyManager( + X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( kmAlg, keyAlg, NONE_KEY_USAGES); - String chosenServerAlias = km.chooseServerAlias(keyAlg, issuers, null); - String chosenClientAlias = km.chooseClientAlias( - new String[]{keyAlg}, issuers, null); + List chosenAliases = new ArrayList<>(4); + + chosenAliases.add(km.chooseServerAlias(keyAlg, issuers, null)); + chosenAliases.add(km.chooseEngineServerAlias(keyAlg, issuers, null)); + chosenAliases.add( + km.chooseClientAlias(new String[]{keyAlg}, issuers, null)); + chosenAliases.add(km.chooseEngineClientAlias( + new String[]{keyAlg}, issuers, null)); String[] allServerAliases = km.getServerAliases(keyAlg, issuers); String[] allClientAliases = km.getClientAliases(keyAlg, issuers); if (found) { if ("true".equals(disabled)) { - assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenServerAlias)); - assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenClientAlias)); + chosenAliases.forEach(a -> + assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(a))); // Assert the initial order of all aliases. assertEquals(USAGE_MISMATCH_ALIAS, @@ -248,8 +292,8 @@ private static void issuerTestCase(String disabled, String kmAlg, assertEquals(EXPIRED_ALIAS, normalizeAlias(allServerAliases[2])); assertEquals(EXPIRED_ALIAS, normalizeAlias(allClientAliases[2])); } else { - assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenServerAlias)); - assertEquals(PREFERRED_ALIAS, normalizeAlias(chosenClientAlias)); + chosenAliases.forEach(a -> + assertEquals(PREFERRED_ALIAS, normalizeAlias(a))); // Assert the correct sorted order of all aliases. assertEquals(PREFERRED_ALIAS, @@ -260,20 +304,41 @@ private static void issuerTestCase(String disabled, String kmAlg, normalizeAlias(allServerAliases[2])); } } else { - assertNull(chosenServerAlias); - assertNull(chosenClientAlias); + chosenAliases.forEach(Asserts::assertNull); assertNull(allServerAliases); assertNull(allClientAliases); } } + private static void aliasNotFoundTestCase( + String kmAlg, String keyAlg, String keyType) throws Exception { + + X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( + kmAlg, keyAlg, DEFAULT_KEY_USAGES); - // PKIX KeyManager adds a cache prefix to an alias. + assertNull(km.chooseServerAlias(keyType, null, null)); + assertNull(km.chooseEngineServerAlias(keyType, null, null)); + assertNull(km.chooseClientAlias(new String[]{keyType}, null, null)); + assertNull( + km.chooseEngineClientAlias(new String[]{keyType}, null, null)); + assertNull(km.getServerAliases(keyType, null)); + assertNull(km.getClientAliases(keyType, null)); + } + + // PKIX KeyManager adds a cache prefix to an alias. private static String normalizeAlias(String alias) { return alias.substring(alias.lastIndexOf(".") + 1); } + private static class InvalidPrincipal implements Principal { + + @Override + public String getName() { + return null; + } + } + private static X509KeyManager getKeyManager(String kmAlg, String keyAlg, boolean[] certKeyUsages) throws Exception { From 45a4a48e2400eb5cbf3223e575db34a405c5fb7c Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 18 Jun 2025 16:50:13 -0400 Subject: [PATCH 19/26] Make certificate checking toggle specific to SunX509. Update issue number. --- .../security/ssl/SunX509KeyManagerImpl.java | 13 +++++- .../ssl/X509KeyManagerCertChecking.java | 11 +++-- .../sun/security/ssl/X509KeyManagerImpl.java | 6 ++- .../security/mscapi/ShortRSAKeyWithinTLS.java | 2 +- .../AlgorithmConstraintsCheck.java | 11 ++--- .../ssl/X509KeyManager/CertChecking.java | 45 ++++++++++--------- .../X509KeyManager/PeerConstraintsCheck.java | 12 ++--- .../X509TrustManagerImpl/PKIXExtendedTM.java | 4 +- .../sun/security/tools/keytool/PrintSSL.java | 2 +- 9 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index 968da1f21bcdb..89e2fa1091281 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -50,15 +50,18 @@ /** * An implementation of X509KeyManager backed by a KeyStore. - * + *

* The backing KeyStore is inspected when this object is constructed. * All key entries containing a PrivateKey and a non-empty chain of * X509Certificate are then copied into an internal store. This means * that subsequent modifications of the KeyStore have no effect on the * X509KeyManagerImpl object. - * + *

* Note that this class assumes that all keys are protected by the same * password. + *

+ * Algorithm constraints and certificate checks can be disabled by setting + * "jdk.tls.SunX509keymanager.certSelectionChecking" system property to "false". * */ @@ -70,6 +73,12 @@ final class SunX509KeyManagerImpl extends X509KeyManagerCertChecking { */ private final Map credentialsMap; + @Override + boolean isCheckingDisabled() { + return "false".equalsIgnoreCase(System.getProperty( + "jdk.tls.SunX509keymanager.certSelectionChecking", "true")); + } + /* * Basic container for credentials implemented as an inner class. */ diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index bc264a956de51..dc9d9692f8248 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -53,10 +53,8 @@ import sun.security.validator.Validator; /* - * Layer that adds algorithm constraints and certificate - * checking to a key manager. Checks can be disabled by setting - * "jdk.tls.keymanager.disableCertChecking" system property to "true" - * before creating a KeyManager object. + * Layer that adds algorithm constraints and certificate checking to a key + * manager. */ abstract class X509KeyManagerCertChecking extends X509ExtendedKeyManager { @@ -65,10 +63,11 @@ abstract class X509KeyManagerCertChecking extends X509ExtendedKeyManager { private final boolean checksDisabled; protected X509KeyManagerCertChecking() { - checksDisabled = "true".equals(System.getProperty( - "jdk.tls.keymanager.disableCertChecking")); + checksDisabled = isCheckingDisabled(); } + abstract boolean isCheckingDisabled(); + // Gets algorithm constraints of the socket. protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 4482ae59e178a..13651cf9c8bc3 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -35,7 +35,6 @@ import java.security.KeyStoreException; import java.security.Principal; import java.security.PrivateKey; -import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -85,6 +84,11 @@ final class X509KeyManagerImpl extends X509KeyManagerCertChecking { entryCacheMap = new ConcurrentHashMap<>(); } + @Override + boolean isCheckingDisabled() { + return false; + } + // LinkedHashMap with a max size of 10 // see LinkedHashMap JavaDocs private static class SizedMap extends LinkedHashMap { diff --git a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java index d690bd970d390..cc9cff0ff4fed 100644 --- a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java +++ b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java @@ -235,7 +235,7 @@ private void checkKeySize(KeyStore ks) throws Exception { public static void main(String[] args) throws Exception { // Disable KeyManager's algorithm constraints checking. System.setProperty( - "jdk.tls.keymanager.disableCertChecking", "true"); + "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); if (debug) { System.setProperty("javax.net.debug", "all"); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index c394d69d9d779..b8c8fd235ee7c 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -51,8 +51,9 @@ /* * @test - * @bug 8170706 - * @summary Support algorithm constraints in SunX509 key manager + * @bug 8359956 + * @summary Support algorithm constraints and certificate checks in SunX509 + * key manager * @modules java.base/sun.security.x509 * java.base/sun.security.util * @library /test/lib @@ -72,12 +73,12 @@ public static void main(String[] args) throws Exception { throw new RuntimeException("Wrong number of arguments"); } - String disabled = args[0]; + String enabled = args[0]; String kmAlg = args[1]; String certSignatureAlg = args[2]; System.setProperty( - "jdk.tls.keymanager.disableCertChecking", disabled); + "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); SecurityUtils.addToDisabledTlsAlgs(certSignatureAlg); X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( @@ -94,7 +95,7 @@ public static void main(String[] args) throws Exception { String serverAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "1.0." : ""; String clientAliasPrefix = kmAlg.equalsIgnoreCase("PKIX") ? "2.0." : ""; - if ("true".equals(disabled)) { + if ("false".equals(enabled) && kmAlg.equals("SunX509")) { assertEquals(CERT_ALIAS, normalizeAlias(serverAlias)); assertEquals(CERT_ALIAS, normalizeAlias(engineServerAlias)); assertEquals(CERT_ALIAS, normalizeAlias(clientAlias)); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java index f82e15fa80d68..b96035a676d8a 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -56,8 +56,9 @@ /* * @test - * @bug 8359069 - * @summary Support certificate checks in SunX509 key manager + * @bug 8359956 + * @summary Support algorithm constraints and certificate checks in SunX509 + * key manager * @modules java.base/sun.security.x509 * java.base/sun.security.util * @library /test/lib @@ -116,64 +117,64 @@ public static void main(String[] args) throws Exception { throw new RuntimeException("Wrong number of arguments"); } - String disabled = args[0]; + String enabled = args[0]; String kmAlg = args[1]; System.setProperty( - "jdk.tls.keymanager.disableCertChecking", disabled); + "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); // --- Usage and expired test cases -- // Both should fail with no usages at all - usageTestCase(disabled, kmAlg, "RSA", NONE_KEY_USAGES, true, true); + usageTestCase(enabled, kmAlg, "RSA", NONE_KEY_USAGES, true, true); // Only client should fail with RSA algorithm and // no digital signature bit set - usageTestCase(disabled, kmAlg, "RSA", NO_DG_USAGE, false, true); + usageTestCase(enabled, kmAlg, "RSA", NO_DG_USAGE, false, true); // Only server should fail with RSA algorithm and // no digital signature bit set - usageTestCase(disabled, kmAlg, "RSASSA-PSS", NO_DG_USAGE, true, false); + usageTestCase(enabled, kmAlg, "RSASSA-PSS", NO_DG_USAGE, true, false); // Both should fail with DSA algorithm and no digital signature bit set - usageTestCase(disabled, kmAlg, "DSA", NO_DG_USAGE, true, true); + usageTestCase(enabled, kmAlg, "DSA", NO_DG_USAGE, true, true); // Both should fail with EC algorithm and no digital signature bit set - usageTestCase(disabled, kmAlg, "EC", NO_DG_USAGE, true, true); + usageTestCase(enabled, kmAlg, "EC", NO_DG_USAGE, true, true); // Both should fail with RSA algorithm and missing digital signature and // key encipherment bits. - usageTestCase(disabled, kmAlg, "RSA", NO_DG_NO_KE_USAGE, true, true); + usageTestCase(enabled, kmAlg, "RSA", NO_DG_NO_KE_USAGE, true, true); // Both should fail with DH algorithm and no key agreement bit set. - usageTestCase(disabled, kmAlg, "DH", NO_KA_USAGE, true, true); + usageTestCase(enabled, kmAlg, "DH", NO_KA_USAGE, true, true); // Only server should fail with EC algorithm and // no digital signature bit set - usageTestCase(disabled, kmAlg, "EC", NO_KA_USAGE, true, false); + usageTestCase(enabled, kmAlg, "EC", NO_KA_USAGE, true, false); // --- Issuer match test cases --- // Check CA issuer match - issuerTestCase(disabled, kmAlg, "RSA", + issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{new X500Principal(CA_ISSUER_STRING)}, true); // Check CA issuer match with non-X500 principal - issuerTestCase(disabled, kmAlg, "RSA", + issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{new UserPrincipal(CA_ISSUER_STRING)}, true); // Non-convertable principal should match all - issuerTestCase(disabled, kmAlg, "RSA", + issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{new InvalidPrincipal()}, true); // Empty issuer array should match all - issuerTestCase(disabled, kmAlg, "RSA", new Principal[]{}, true); + issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{}, true); // Null issuer array should match all - issuerTestCase(disabled, kmAlg, "RSA", null, true); + issuerTestCase(enabled, kmAlg, "RSA", null, true); // Issuer that is not in the chain should not match. - issuerTestCase(disabled, kmAlg, "RSA", + issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{new X500Principal(UNKNOWN_ISSUER_STRING)}, false); // --- Alias not found for given KeyType test cases --- @@ -185,7 +186,7 @@ public static void main(String[] args) throws Exception { aliasNotFoundTestCase(kmAlg, "RSA", "EC"); } - private static void usageTestCase(String disabled, String kmAlg, + private static void usageTestCase(String enabled, String kmAlg, String keyAlg, boolean[] certKeyUsages, boolean checkServer, boolean checkClient) throws Exception { @@ -203,7 +204,7 @@ private static void usageTestCase(String disabled, String kmAlg, String[] allServerAliases = km.getServerAliases(keyAlg, null); String[] allClientAliases = km.getClientAliases(keyAlg, null); - if ("true".equals(disabled)) { + if ("false".equals(enabled) && kmAlg.equals("SunX509")) { // Initial order alias returned assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(chosenServerAlias)); @@ -259,7 +260,7 @@ private static void usageTestCase(String disabled, String kmAlg, } } - private static void issuerTestCase(String disabled, String kmAlg, + private static void issuerTestCase(String enabled, String kmAlg, String keyAlg, Principal[] issuers, boolean found) throws Exception { X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( @@ -278,7 +279,7 @@ private static void issuerTestCase(String disabled, String kmAlg, String[] allClientAliases = km.getClientAliases(keyAlg, issuers); if (found) { - if ("true".equals(disabled)) { + if ("false".equals(enabled) && kmAlg.equals("SunX509")) { chosenAliases.forEach(a -> assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(a))); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index c72b75ad89f94..148ab78115cc9 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -55,9 +55,9 @@ /* * @test - * @bug 8353113 - * @summary Peer supported certificate signature algorithms are not being - * checked with default SunX509 key manager + * @bug 8359956 + * @summary Support algorithm constraints and certificate checks in SunX509 + * key manager * @modules java.base/sun.security.x509 * java.base/sun.security.util * @library /javax/net/ssl/templates @@ -103,13 +103,13 @@ public static void main(String[] args) throws Exception { System.setProperty( "jdk.tls.server.SignatureSchemes", CLIENT_CERT_SIG_SCHEME); - String disabled = args[0]; + String enabled = args[0]; String kmAlg = args[1]; System.setProperty( - "jdk.tls.keymanager.disableCertChecking", disabled); + "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); - if ("true".equals(disabled)) { + if ("false".equals(enabled) && kmAlg.equals("SunX509")) { new PeerConstraintsCheck(kmAlg).run(); } else { runAndCheckException( diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index c3f9403604ac4..843e776744e37 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -33,8 +33,6 @@ * @run main/othervm PKIXExtendedTM 2 */ -import java.net.*; -import java.util.*; import java.io.*; import javax.net.ssl.*; import java.security.Security; @@ -1114,7 +1112,7 @@ public static void main(String args[]) throws Exception { // Disable KeyManager's algorithm constraints checking as this test // is about TrustManager's constraints check. System.setProperty( - "jdk.tls.keymanager.disableCertChecking", "true"); + "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); if (args.length != 1) { throw new Exception("Incorrect number of arguments"); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 0f7714dac46db..41b4c5a371de6 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -51,7 +51,7 @@ public class PrintSSL { public static void main(String[] args) throws Throwable { // Disable KeyManager's algorithm constraints checking. System.setProperty( - "jdk.tls.keymanager.disableCertChecking", "true"); + "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); Files.deleteIfExists(Paths.get("keystore")); From bcef73464e00c7b71d34a8c4cc3404077ac98f2c Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 18 Jun 2025 17:09:48 -0400 Subject: [PATCH 20/26] Remove a couple of empty lines --- .../security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java | 1 - .../sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java | 1 - 2 files changed, 2 deletions(-) diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index b8c8fd235ee7c..d43ff73ccdabd 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -111,7 +111,6 @@ public static void main(String[] args) throws Exception { // PKIX KeyManager adds a cache prefix to an alias. private static String normalizeAlias(String alias) { return alias.substring(alias.lastIndexOf(".") + 1); - } private static X509KeyManager getKeyManager(String kmAlg, diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index 148ab78115cc9..ec20ac566a5f9 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -203,7 +203,6 @@ public X509Certificate[] getAcceptedIssuers() { }; ctx.init(kmf.getKeyManagers(), trustAll, null); - return ctx; } From a83ea8bde483e29ec670f2f3ed39c2b6856c3a1c Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 18 Jun 2025 17:32:46 -0400 Subject: [PATCH 21/26] Update system property name in one more test --- .../MD5NotAllowedInTLS13CertificateSignature.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java index 7d8abdb7833b0..2808b7bab7a54 100644 --- a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java +++ b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java @@ -154,9 +154,8 @@ private static SSLContext getSSLContext( // so we check against local supported signature // algorithms which constitutes the fix being tested. System.setProperty( - "jdk.tls.keymanager.disableCertChecking", "true"); - KeyManagerFactory kmf = KeyManagerFactory.getInstance( - KeyManagerFactory.getDefaultAlgorithm()); + "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); + KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509"); kmf.init(ks, passphrase); ctx.init(kmf.getKeyManagers(), tmf.getTrustManagers(), null); From c5e381ec1d12f1c019a78700b1a30c3614a971c5 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Fri, 20 Jun 2025 14:01:17 -0400 Subject: [PATCH 22/26] Update unit tests. Some code refactoring. --- .../security/ssl/SunX509KeyManagerImpl.java | 5 +- .../ssl/X509KeyManagerCertChecking.java | 171 +++++++++--------- .../sun/security/ssl/X509KeyManagerImpl.java | 2 +- .../security/mscapi/ShortRSAKeyWithinTLS.java | 8 +- .../sun/security/tools/keytool/PrintSSL.java | 5 +- 5 files changed, 97 insertions(+), 94 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index 89e2fa1091281..a414a99fb8f8f 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -61,7 +61,8 @@ * password. *

* Algorithm constraints and certificate checks can be disabled by setting - * "jdk.tls.SunX509keymanager.certSelectionChecking" system property to "false". + * "jdk.tls.SunX509keymanager.certSelectionChecking" system property to "false" + * before calling a class constructor. * */ @@ -74,7 +75,7 @@ final class SunX509KeyManagerImpl extends X509KeyManagerCertChecking { private final Map credentialsMap; @Override - boolean isCheckingDisabled() { + protected boolean isCheckingDisabled() { return "false".equalsIgnoreCase(System.getProperty( "jdk.tls.SunX509keymanager.certSelectionChecking", "true")); } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index dc9d9692f8248..9633b65428a7a 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -68,6 +68,91 @@ protected X509KeyManagerCertChecking() { abstract boolean isCheckingDisabled(); + // Entry point to do all certificate checks. + protected EntryStatus checkAlias(int keyStoreIndex, String alias, + Certificate[] chain, Date verificationDate, List keyTypes, + Set issuerSet, CheckType checkType, + AlgorithmConstraints constraints, + List requestedServerNames, String idAlgorithm) { + + // --- Mandatory checks --- + + if ((chain == null) || (chain.length == 0)) { + return null; + } + + for (Certificate cert : chain) { + if (!(cert instanceof X509Certificate)) { + // Not an X509Certificate, ignore this alias + return null; + } + } + + // Check key type, get key type index. + int keyIndex = -1; + int j = 0; + + for (KeyType keyType : keyTypes) { + if (keyType.matches(chain)) { + keyIndex = j; + break; + } + j++; + } + + if (keyIndex == -1) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": key algorithm does not match"); + } + return null; + } + + // Check issuers + if (issuerSet != null && !issuerSet.isEmpty()) { + boolean found = false; + for (Certificate cert : chain) { + X509Certificate xcert = (X509Certificate) cert; + if (issuerSet.contains(xcert.getIssuerX500Principal())) { + found = true; + break; + } + } + if (!found) { + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine( + "Ignore alias " + alias + + ": issuers do not match"); + } + return null; + } + } + + // --- Optional checks, depending on "checksDisabled" toggle --- + + // Check the algorithm constraints + if (constraints != null && + !conformsToAlgorithmConstraints(constraints, chain, + checkType.getValidator())) { + + if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { + SSLLogger.fine("Ignore alias " + alias + + ": certificate chain does not conform to " + + "algorithm constraints"); + } + return null; + } + + // Endpoint certificate check + CheckResult checkResult = certificateCheck(checkType, + (X509Certificate) chain[0], + verificationDate == null ? new Date() : verificationDate, + requestedServerNames, idAlgorithm); + + return new EntryStatus( + keyStoreIndex, keyIndex, alias, chain, checkResult); + } + // Gets algorithm constraints of the socket. protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { @@ -478,90 +563,4 @@ protected static Set getIssuerSet(Principal[] issuers) { return null; } } - - // Entry point to do all certificate checks. - protected EntryStatus checkAlias(int keyStoreIndex, String alias, - Certificate[] chain, Date verificationDate, List keyTypes, - Set issuerSet, CheckType checkType, - AlgorithmConstraints constraints, - List requestedServerNames, String idAlgorithm) { - - // --- Mandatory checks --- - - if ((chain == null) || (chain.length == 0)) { - // Must be secret key entry, ignore - return null; - } - - for (Certificate cert : chain) { - if (!(cert instanceof X509Certificate)) { - // Not an X509Certificate, ignore this alias - return null; - } - } - - // Check keytype - int keyIndex = -1; - int j = 0; - - for (KeyType keyType : keyTypes) { - if (keyType.matches(chain)) { - keyIndex = j; - break; - } - j++; - } - - if (keyIndex == -1) { - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias - + ": key algorithm does not match"); - } - return null; - } - - // Check issuers - if (issuerSet != null && !issuerSet.isEmpty()) { - boolean found = false; - for (Certificate cert : chain) { - X509Certificate xcert = (X509Certificate) cert; - if (issuerSet.contains(xcert.getIssuerX500Principal())) { - found = true; - break; - } - } - if (!found) { - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine( - "Ignore alias " + alias - + ": issuers do not match"); - } - return null; - } - } - - // --- Optional checks, depending on "checksDisabled" toggle --- - - // Check the algorithm constraints - if (constraints != null && - !conformsToAlgorithmConstraints(constraints, chain, - checkType.getValidator())) { - - if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { - SSLLogger.fine("Ignore alias " + alias + - ": certificate chain does not conform to " + - "algorithm constraints"); - } - return null; - } - - // Endpoint certificate check - CheckResult checkResult = certificateCheck(checkType, - (X509Certificate) chain[0], - verificationDate == null ? new Date() : verificationDate, - requestedServerNames, idAlgorithm); - - return new EntryStatus( - keyStoreIndex, keyIndex, alias, chain, checkResult); - } } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 13651cf9c8bc3..63adf897e1217 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -85,7 +85,7 @@ final class X509KeyManagerImpl extends X509KeyManagerCertChecking { } @Override - boolean isCheckingDisabled() { + protected boolean isCheckingDisabled() { return false; } diff --git a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java index cc9cff0ff4fed..ca5a6226cc305 100644 --- a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java +++ b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java @@ -29,6 +29,7 @@ * @modules java.base/sun.security.util * java.base/sun.security.tools.keytool * java.base/sun.security.x509 + * @library /test/lib * @run main ShortRSAKeyWithinTLS 1024 * @run main ShortRSAKeyWithinTLS 768 * @run main ShortRSAKeyWithinTLS 512 @@ -42,6 +43,7 @@ import javax.net.*; import javax.net.ssl.*; +import jdk.test.lib.security.SecurityUtils; import sun.security.tools.keytool.CertAndKeyGen; import sun.security.util.KeyUtil; import sun.security.x509.X500Name; @@ -233,9 +235,9 @@ private void checkKeySize(KeyStore ks) throws Exception { private static String clientCiperSuite = null; public static void main(String[] args) throws Exception { - // Disable KeyManager's algorithm constraints checking. - System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); + // Make sure we don't block the key on algorithm constraints check. + SecurityUtils.removeFromDisabledAlgs("jdk.certpath.disabledAlgorithms", + List.of("RSA keySize < 1024")); if (debug) { System.setProperty("javax.net.debug", "all"); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index 41b4c5a371de6..afcc27910b568 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -39,7 +39,6 @@ import java.net.ServerSocket; import java.nio.file.Files; import java.nio.file.Paths; -import java.security.Security; import java.util.concurrent.CountDownLatch; import javax.net.ssl.SSLServerSocketFactory; import javax.net.ssl.SSLSocket; @@ -49,7 +48,9 @@ public class PrintSSL { public static void main(String[] args) throws Throwable { - // Disable KeyManager's algorithm constraints checking. + // Disable KeyManager's algorithm constraints checking, + // so we can make keytool print certificate with weak + // MD5withRSA signature algorithm. System.setProperty( "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); From 3edca307af0aa9f11976104830f87d0d20c11dee Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Mon, 23 Jun 2025 15:47:07 -0400 Subject: [PATCH 23/26] Add more unit test cases. Some code tweaks. --- .../ssl/X509KeyManagerCertChecking.java | 16 ++-- .../ssl/X509KeyManager/CertChecking.java | 88 ++++++++++--------- 2 files changed, 57 insertions(+), 47 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index 9633b65428a7a..b34b7c0cb8dab 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -254,6 +254,7 @@ private boolean conformsToAlgorithmConstraints( return true; } + // Certificate check. private CheckResult certificateCheck( CheckType checkType, X509Certificate cert, Date date, List serverNames, String idAlgorithm) { @@ -306,8 +307,8 @@ private static boolean getBit(boolean[] keyUsage, int bit) { return (bit < keyUsage.length) && keyUsage[bit]; } - // Check if this certificate is appropriate for this type of use - // first check extensions, if they match, check expiration. + // Check if this certificate is appropriate for this type of use. + // First check extensions, if they match then check expiration. // NOTE: `conformsToAlgorithmConstraints` call above also does some // basic keyUsage checks. CheckResult check(X509Certificate cert, Date date, @@ -442,9 +443,9 @@ String getValidator() { } } - // A candidate match - // identifies the entry by builder and alias - // and includes the result of the certificate check + // A candidate match. + // Identifies the entry by key store index and alias + // and includes the result of the certificate check. protected static class EntryStatus implements Comparable { final int keyStoreIndex; @@ -522,11 +523,12 @@ boolean matches(Certificate[] chain) { issuer.getSigAlgName().toUpperCase(Locale.ENGLISH); String pattern = "WITH" + sigKeyAlgorithm.toUpperCase(Locale.ENGLISH); - return sigAlgName.contains(pattern); + return sigAlgName.endsWith(pattern); } } } + // Make a list of key types. protected static List getKeyTypes(String... keyTypes) { if ((keyTypes == null) || (keyTypes.length == 0) || (keyTypes[0] == null)) { @@ -539,7 +541,7 @@ protected static List getKeyTypes(String... keyTypes) { return list; } - // Make a Set out of the array. + // Make a set out of the array. protected static Set getIssuerSet(Principal[] issuers) { if (issuers != null && issuers.length != 0) { diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java index b96035a676d8a..29a653b7db076 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -69,7 +69,8 @@ */ /* - * This class tests against the certificate's expiration, key usage and issuers. + * This class tests against the certificate's expiration, key usage, key type + * and issuers. */ public class CertChecking { @@ -156,34 +157,51 @@ public static void main(String[] args) throws Exception { // --- Issuer match test cases --- // Check CA issuer match - issuerTestCase(enabled, kmAlg, "RSA", + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", new Principal[]{new X500Principal(CA_ISSUER_STRING)}, true); // Check CA issuer match with non-X500 principal - issuerTestCase(enabled, kmAlg, "RSA", + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", new Principal[]{new UserPrincipal(CA_ISSUER_STRING)}, true); // Non-convertable principal should match all - issuerTestCase(enabled, kmAlg, "RSA", + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", new Principal[]{new InvalidPrincipal()}, true); // Empty issuer array should match all - issuerTestCase(enabled, kmAlg, "RSA", new Principal[]{}, true); + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", + new Principal[]{}, true); // Null issuer array should match all - issuerTestCase(enabled, kmAlg, "RSA", null, true); + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", null, true); // Issuer that is not in the chain should not match. - issuerTestCase(enabled, kmAlg, "RSA", - new Principal[]{new X500Principal(UNKNOWN_ISSUER_STRING)}, false); + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA", + new Principal[]{new X500Principal(UNKNOWN_ISSUER_STRING)}, + false); - // --- Alias not found for given KeyType test cases --- + // --- Key Type match test cases --- - // Null KeyType - aliasNotFoundTestCase(kmAlg, "RSA", null); + // Exact key type match. + issuerAndKeyTypeTestCase(enabled, kmAlg, "EC", "EC", null, true); - // Wrong KeyType - aliasNotFoundTestCase(kmAlg, "RSA", "EC"); + // Key type with a signature algorithm match. + issuerAndKeyTypeTestCase( + enabled, kmAlg, "EC", "EC_" + CA_KEY_TYPE, null, true); + + // Null KeyType should not match. + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", null, null, false); + + // Wrong KeyType should not match. + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "EC", null, false); + + // Wrong signature algorithm should not match. + issuerAndKeyTypeTestCase(enabled, kmAlg, "RSA", "RSA_EC", null, false); + + // Correct signature algorithm but incorrect key algorithm + // should not match. + issuerAndKeyTypeTestCase( + enabled, kmAlg, "RSA", "EC_" + CA_KEY_TYPE, null, false); } private static void usageTestCase(String enabled, String kmAlg, @@ -260,23 +278,24 @@ private static void usageTestCase(String enabled, String kmAlg, } } - private static void issuerTestCase(String enabled, String kmAlg, - String keyAlg, Principal[] issuers, boolean found) throws Exception { + private static void issuerAndKeyTypeTestCase(String enabled, String kmAlg, + String keyAlg, String keyType, Principal[] issuers, boolean found) + throws Exception { X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( kmAlg, keyAlg, NONE_KEY_USAGES); List chosenAliases = new ArrayList<>(4); - chosenAliases.add(km.chooseServerAlias(keyAlg, issuers, null)); - chosenAliases.add(km.chooseEngineServerAlias(keyAlg, issuers, null)); + chosenAliases.add(km.chooseServerAlias(keyType, issuers, null)); + chosenAliases.add(km.chooseEngineServerAlias(keyType, issuers, null)); chosenAliases.add( - km.chooseClientAlias(new String[]{keyAlg}, issuers, null)); + km.chooseClientAlias(new String[]{keyType}, issuers, null)); chosenAliases.add(km.chooseEngineClientAlias( - new String[]{keyAlg}, issuers, null)); + new String[]{keyType}, issuers, null)); - String[] allServerAliases = km.getServerAliases(keyAlg, issuers); - String[] allClientAliases = km.getClientAliases(keyAlg, issuers); + String[] allServerAliases = km.getServerAliases(keyType, issuers); + String[] allClientAliases = km.getClientAliases(keyType, issuers); if (found) { if ("false".equals(enabled) && kmAlg.equals("SunX509")) { @@ -288,10 +307,14 @@ private static void issuerTestCase(String enabled, String kmAlg, normalizeAlias(allServerAliases[0])); assertEquals(USAGE_MISMATCH_ALIAS, normalizeAlias(allClientAliases[0])); - assertEquals(PREFERRED_ALIAS, normalizeAlias(allServerAliases[1])); - assertEquals(PREFERRED_ALIAS, normalizeAlias(allClientAliases[1])); - assertEquals(EXPIRED_ALIAS, normalizeAlias(allServerAliases[2])); - assertEquals(EXPIRED_ALIAS, normalizeAlias(allClientAliases[2])); + assertEquals(PREFERRED_ALIAS, + normalizeAlias(allServerAliases[1])); + assertEquals(PREFERRED_ALIAS, + normalizeAlias(allClientAliases[1])); + assertEquals(EXPIRED_ALIAS, + normalizeAlias(allServerAliases[2])); + assertEquals(EXPIRED_ALIAS, + normalizeAlias(allClientAliases[2])); } else { chosenAliases.forEach(a -> assertEquals(PREFERRED_ALIAS, normalizeAlias(a))); @@ -311,21 +334,6 @@ private static void issuerTestCase(String enabled, String kmAlg, } } - private static void aliasNotFoundTestCase( - String kmAlg, String keyAlg, String keyType) throws Exception { - - X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( - kmAlg, keyAlg, DEFAULT_KEY_USAGES); - - assertNull(km.chooseServerAlias(keyType, null, null)); - assertNull(km.chooseEngineServerAlias(keyType, null, null)); - assertNull(km.chooseClientAlias(new String[]{keyType}, null, null)); - assertNull( - km.chooseEngineClientAlias(new String[]{keyType}, null, null)); - assertNull(km.getServerAliases(keyType, null)); - assertNull(km.getClientAliases(keyType, null)); - } - // PKIX KeyManager adds a cache prefix to an alias. private static String normalizeAlias(String alias) { return alias.substring(alias.lastIndexOf(".") + 1); From eb972544f7a69e66790883e9921dc9b083204c67 Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Wed, 23 Jul 2025 17:17:35 -0400 Subject: [PATCH 24/26] Update system property name --- .../share/classes/sun/security/ssl/SunX509KeyManagerImpl.java | 4 ++-- src/java.base/share/conf/security/java.security | 1 + .../MD5NotAllowedInTLS13CertificateSignature.java | 2 +- .../ssl/X509KeyManager/AlgorithmConstraintsCheck.java | 3 +-- test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java | 3 +-- .../sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java | 3 +-- .../sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java | 3 +-- test/jdk/sun/security/tools/keytool/PrintSSL.java | 2 +- 8 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java index a414a99fb8f8f..2441ad91fde19 100644 --- a/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java @@ -61,7 +61,7 @@ * password. *

* Algorithm constraints and certificate checks can be disabled by setting - * "jdk.tls.SunX509keymanager.certSelectionChecking" system property to "false" + * "jdk.tls.SunX509KeyManager.certChecking" system property to "false" * before calling a class constructor. * */ @@ -77,7 +77,7 @@ final class SunX509KeyManagerImpl extends X509KeyManagerCertChecking { @Override protected boolean isCheckingDisabled() { return "false".equalsIgnoreCase(System.getProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", "true")); + "jdk.tls.SunX509KeyManager.certChecking", "true")); } /* diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 5d96d74539e1c..793eef01bcdff 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -26,6 +26,7 @@ # If this properties file fails to load, the JDK implementation will throw # an unspecified error when initializing the java.security.Security class. # Properties in this file are typically parsed only once. If any of the +# Properties in this file are typically parsed only once. If any of the # properties are modified, applications should be restarted to ensure the # changes are properly reflected. # diff --git a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java index 2808b7bab7a54..2fa046e1dc2e1 100644 --- a/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java +++ b/test/jdk/sun/security/ssl/SignatureScheme/MD5NotAllowedInTLS13CertificateSignature.java @@ -154,7 +154,7 @@ private static SSLContext getSSLContext( // so we check against local supported signature // algorithms which constitutes the fix being tested. System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); + "jdk.tls.SunX509KeyManager.certChecking", "false"); KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509"); kmf.init(ks, passphrase); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java index d43ff73ccdabd..997fde5a07ade 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/AlgorithmConstraintsCheck.java @@ -77,8 +77,7 @@ public static void main(String[] args) throws Exception { String kmAlg = args[1]; String certSignatureAlg = args[2]; - System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); + System.setProperty("jdk.tls.SunX509KeyManager.certChecking", enabled); SecurityUtils.addToDisabledTlsAlgs(certSignatureAlg); X509ExtendedKeyManager km = (X509ExtendedKeyManager) getKeyManager( diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java index 29a653b7db076..ebd0b4c4a5452 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -121,8 +121,7 @@ public static void main(String[] args) throws Exception { String enabled = args[0]; String kmAlg = args[1]; - System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); + System.setProperty("jdk.tls.SunX509KeyManager.certChecking", enabled); // --- Usage and expired test cases -- diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index ec20ac566a5f9..0baf3253cf115 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -106,8 +106,7 @@ public static void main(String[] args) throws Exception { String enabled = args[0]; String kmAlg = args[1]; - System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", enabled); + System.setProperty("jdk.tls.SunX509KeyManager.certChecking", enabled); if ("false".equals(enabled) && kmAlg.equals("SunX509")) { new PeerConstraintsCheck(kmAlg).run(); diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index 843e776744e37..4e6ca1c0fe339 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -1111,8 +1111,7 @@ static class Test { public static void main(String args[]) throws Exception { // Disable KeyManager's algorithm constraints checking as this test // is about TrustManager's constraints check. - System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); + System.setProperty("jdk.tls.SunX509KeyManager.certChecking", "false"); if (args.length != 1) { throw new Exception("Incorrect number of arguments"); diff --git a/test/jdk/sun/security/tools/keytool/PrintSSL.java b/test/jdk/sun/security/tools/keytool/PrintSSL.java index afcc27910b568..9403ae62d924a 100644 --- a/test/jdk/sun/security/tools/keytool/PrintSSL.java +++ b/test/jdk/sun/security/tools/keytool/PrintSSL.java @@ -52,7 +52,7 @@ public static void main(String[] args) throws Throwable { // so we can make keytool print certificate with weak // MD5withRSA signature algorithm. System.setProperty( - "jdk.tls.SunX509keymanager.certSelectionChecking", "false"); + "jdk.tls.SunX509KeyManager.certChecking", "false"); Files.deleteIfExists(Paths.get("keystore")); From df1a1fac6df995d260395c364ca30f695b8db6be Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Tue, 29 Jul 2025 11:07:52 -0400 Subject: [PATCH 25/26] Address review comments --- src/java.base/share/conf/security/java.security | 1 - test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java | 2 +- .../sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java | 3 +++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 793eef01bcdff..5d96d74539e1c 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -26,7 +26,6 @@ # If this properties file fails to load, the JDK implementation will throw # an unspecified error when initializing the java.security.Security class. # Properties in this file are typically parsed only once. If any of the -# Properties in this file are typically parsed only once. If any of the # properties are modified, applications should be restarted to ensure the # changes are properly reflected. # diff --git a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java index ca5a6226cc305..5ecbb60227358 100644 --- a/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java +++ b/test/jdk/sun/security/mscapi/ShortRSAKeyWithinTLS.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2025, 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 diff --git a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java index 4e6ca1c0fe339..fcc7cbf73f090 100644 --- a/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java +++ b/test/jdk/sun/security/ssl/X509TrustManagerImpl/PKIXExtendedTM.java @@ -30,7 +30,10 @@ * @test * @bug 6916074 8170131 * @summary Add support for TLS 1.2 + * @run main/othervm PKIXExtendedTM 0 + * @run main/othervm PKIXExtendedTM 1 * @run main/othervm PKIXExtendedTM 2 + * @run main/othervm PKIXExtendedTM 3 */ import java.io.*; From c8c4008ad8f063f5420701a99f8c86d0d518cdec Mon Sep 17 00:00:00 2001 From: Artur Barashev Date: Tue, 29 Jul 2025 16:57:30 -0400 Subject: [PATCH 26/26] Review fixes --- .../ssl/X509KeyManagerCertChecking.java | 15 ++++++-- .../sun/security/ssl/X509KeyManagerImpl.java | 6 ++-- .../ssl/X509KeyManager/CertChecking.java | 35 ++++++++++--------- .../X509KeyManager/PeerConstraintsCheck.java | 8 +++++ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java index b34b7c0cb8dab..00a7ae8435239 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerCertChecking.java @@ -53,8 +53,15 @@ import sun.security.validator.Validator; /* - * Layer that adds algorithm constraints and certificate checking to a key - * manager. + * Layer that adds algorithm constraints and certificate checking functionality + * to a key manager: + * 1) Check against peer supported certificate signature algorithms (sent with + * "signature_algorithms_cert" TLS extension). + * 2) Check against local TLS algorithm constraints ("java.security" config + * file). + * 3) Mark alias results based on validity period and certificate extensions, + * so results can be sorted to find the best match. See "CheckResult" and + * "EntryStatus" for details. */ abstract class X509KeyManagerCertChecking extends X509ExtendedKeyManager { @@ -170,6 +177,8 @@ protected AlgorithmConstraints getAlgorithmConstraints(Socket socket) { String[] peerSupportedSignAlgs = null; if (session instanceof ExtendedSSLSession extSession) { + // Peer supported certificate signature algorithms + // sent with "signature_algorithms_cert" TLS extension. peerSupportedSignAlgs = extSession.getPeerSupportedSignatureAlgorithms(); } @@ -199,6 +208,8 @@ protected AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) { String[] peerSupportedSignAlgs = null; if (session instanceof ExtendedSSLSession extSession) { + // Peer supported certificate signature algorithms + // sent with "signature_algorithms_cert" TLS extension. peerSupportedSignAlgs = extSession.getPeerSupportedSignatureAlgorithms(); } diff --git a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java index 63adf897e1217..df6ecaf7a4241 100644 --- a/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java +++ b/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java @@ -37,7 +37,6 @@ import java.security.PrivateKey; import java.security.cert.X509Certificate; import java.util.*; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.*; import javax.security.auth.x500.X500Principal; @@ -81,7 +80,7 @@ final class X509KeyManagerImpl extends X509KeyManagerCertChecking { X509KeyManagerImpl(List builders) { this.builders = builders; uidCounter = new AtomicLong(); - entryCacheMap = new ConcurrentHashMap<>(); + entryCacheMap = Collections.synchronizedMap(new SizedMap<>()); } @Override @@ -89,8 +88,7 @@ protected boolean isCheckingDisabled() { return false; } - // LinkedHashMap with a max size of 10 - // see LinkedHashMap JavaDocs + // LinkedHashMap with a max size of 10, see LinkedHashMap JavaDocs private static class SizedMap extends LinkedHashMap { @java.io.Serial private static final long serialVersionUID = -8211222668790986062L; diff --git a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java index ebd0b4c4a5452..81e16b925fcb8 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/CertChecking.java @@ -105,9 +105,9 @@ public class CertChecking { new boolean[]{true, true, true, true, true, true}; private static final boolean[] NONE_KEY_USAGES = new boolean[]{false, false, false, false, false, false}; - private static final boolean[] NO_DG_USAGE = + private static final boolean[] NO_DS_USAGE = new boolean[]{false, true, true, true, true, true}; - private static final boolean[] NO_DG_NO_KE_USAGE = + private static final boolean[] NO_DS_NO_KE_USAGE = new boolean[]{false, true, false, true, true, true}; private static final boolean[] NO_KA_USAGE = new boolean[]{true, true, true, true, false, true}; @@ -125,31 +125,34 @@ public static void main(String[] args) throws Exception { // --- Usage and expired test cases -- - // Both should fail with no usages at all + // Both client and server should be checked with no usages at all usageTestCase(enabled, kmAlg, "RSA", NONE_KEY_USAGES, true, true); - // Only client should fail with RSA algorithm and + // Only client should be checked with RSA algorithm and // no digital signature bit set - usageTestCase(enabled, kmAlg, "RSA", NO_DG_USAGE, false, true); + usageTestCase(enabled, kmAlg, "RSA", NO_DS_USAGE, false, true); - // Only server should fail with RSA algorithm and + // Only server should be checked with RSA algorithm and // no digital signature bit set - usageTestCase(enabled, kmAlg, "RSASSA-PSS", NO_DG_USAGE, true, false); + usageTestCase(enabled, kmAlg, "RSASSA-PSS", NO_DS_USAGE, true, false); - // Both should fail with DSA algorithm and no digital signature bit set - usageTestCase(enabled, kmAlg, "DSA", NO_DG_USAGE, true, true); + // Both client and server should be checked with DSA algorithm and no + // digital signature bit set + usageTestCase(enabled, kmAlg, "DSA", NO_DS_USAGE, true, true); - // Both should fail with EC algorithm and no digital signature bit set - usageTestCase(enabled, kmAlg, "EC", NO_DG_USAGE, true, true); + // Both client and server should be checked with EC algorithm and no + // digital signature bit set + usageTestCase(enabled, kmAlg, "EC", NO_DS_USAGE, true, true); - // Both should fail with RSA algorithm and missing digital signature and - // key encipherment bits. - usageTestCase(enabled, kmAlg, "RSA", NO_DG_NO_KE_USAGE, true, true); + // Both client and server should be checked with RSA algorithm and + // missing digital signature and key encipherment bits. + usageTestCase(enabled, kmAlg, "RSA", NO_DS_NO_KE_USAGE, true, true); - // Both should fail with DH algorithm and no key agreement bit set. + // Both client and server should be checked with DH algorithm and no + // key agreement bit set. usageTestCase(enabled, kmAlg, "DH", NO_KA_USAGE, true, true); - // Only server should fail with EC algorithm and + // Only server should be checked with EC algorithm and // no digital signature bit set usageTestCase(enabled, kmAlg, "EC", NO_KA_USAGE, true, false); diff --git a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java index 0baf3253cf115..bbfbfe0b46e8a 100644 --- a/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java +++ b/test/jdk/sun/security/ssl/X509KeyManager/PeerConstraintsCheck.java @@ -111,6 +111,14 @@ public static void main(String[] args) throws Exception { if ("false".equals(enabled) && kmAlg.equals("SunX509")) { new PeerConstraintsCheck(kmAlg).run(); } else { + // "jdk.tls.client.SignatureSchemes" and + // "jdk.tls.server.SignatureSchemes" system properties set + // signature schemes for both "signature_algorithms" and + // "signature_algorithms_cert" extensions. Then we fail because + // server's certificate is signed with "SHA256withECDSA" while + // "signature_algorithms_cert" extension only contains an + // "ecdsa_secp384r1_sha384" signature scheme corresponding to + // "SHA384withECDSA" certificate signature. runAndCheckException( () -> new PeerConstraintsCheck(kmAlg).run(), ex -> {