From c869d478e8f71cd2bc4461e5c637ba78ca84e108 Mon Sep 17 00:00:00 2001 From: Matthew Donovan Date: Tue, 2 Sep 2025 10:37:06 -0400 Subject: [PATCH 1/2] 8366182: Some PKCS11Tests are being skipped when they shouldn't --- .../security/pkcs11/Cipher/TestKATForGCM.java | 7 +- .../pkcs11/KeyStore/SecretKeysBasic.java | 17 +++-- test/jdk/sun/security/pkcs11/PKCS11Test.java | 75 +++++++++---------- .../pkcs11/Secmod/AddTrustedCert.java | 12 +-- .../pkcs11/Signature/TestDSAKeyLength.java | 21 ++++-- test/jdk/sun/security/pkcs11/ec/TestECDH.java | 1 + 6 files changed, 70 insertions(+), 63 deletions(-) diff --git a/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java b/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java index 9844e8ecfd28f..9177e859ebb8e 100644 --- a/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java +++ b/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java @@ -324,9 +324,12 @@ public void main(Provider p) throws Exception { System.out.println("Exception occured using " + p.getName() + " version " + p.getVersionStr()); if (isNSS(p)) { - double ver = getNSSInfo("nss"); + String ver = getNSSInfo("nss"); String osName = System.getProperty("os.name"); - if (ver > 3.139 && ver < 3.15 && osName.equals("Linux")) { + int idx = ver.indexOf("."); + double major = Double.parseDouble(ver.substring(0, idx)); + double minor = Double.parseDouble(ver.substring(idx+1)); + if (major == 3 && minor > 13.9 && minor < 15 && osName.equals("Linux")) { // warn about buggy behaviour on Linux with nss 3.14 System.out.println("Warning: old NSS " + ver + " might be problematic, consider upgrading it"); } diff --git a/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java b/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java index 4d876604c0182..96e10b976cf31 100644 --- a/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java +++ b/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2023, 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 @@ -116,11 +116,14 @@ private static boolean checkSecretKeyEntry(String alias, // A bug in NSS 3.12 (Mozilla bug 471665) causes AES key lengths // to be read incorrectly. Checking for improper 16 byte length // in key string. - if (isNSS(provider) && expected.getAlgorithm().equals("AES") && - (getNSSVersion() >= 3.12 && getNSSVersion() <= 3.122)) { - System.out.println("NSS 3.12 bug returns incorrect AES key "+ - "length breaking key storage. Aborting..."); - return true; + if (isNSS(provider) && expected.getAlgorithm().equals("AES")) { + String version = getNSSVersion(); + if (version.equals("3.12") || version.equals("3.12.1") + || version.equals("3.12.2")) { + System.out.println("NSS 3.12 bug returns incorrect AES key " + + "length breaking key storage. Aborting..."); + return true; + } } if (saveBeforeCheck) { @@ -168,7 +171,7 @@ private static void dumpKey(String info, SecretKey key) { private static void doTest() throws Exception { // Make sure both NSS libraries are the same version. if (isNSS(provider) && - (getLibsoftokn3Version() != getLibnss3Version())) { + (!getLibsoftokn3Version().equals(getLibnss3Version()))) { System.out.println("libsoftokn3 and libnss3 versions do not match. Aborting test..."); return; } diff --git a/test/jdk/sun/security/pkcs11/PKCS11Test.java b/test/jdk/sun/security/pkcs11/PKCS11Test.java index 0f1da62efcc79..07017ea7e3aa3 100644 --- a/test/jdk/sun/security/pkcs11/PKCS11Test.java +++ b/test/jdk/sun/security/pkcs11/PKCS11Test.java @@ -52,6 +52,8 @@ import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -83,7 +85,7 @@ public abstract class PKCS11Test { private static final String NSS_BUNDLE_VERSION = "3.111"; private static final String NSSLIB = "jpg.tests.jdk.nsslib"; - static double nss_version = -1; + static String nss_version = null; static ECCState nss_ecc_status = ECCState.Basic; // The NSS library we need to search for in getNSSLibDir() @@ -93,8 +95,8 @@ public abstract class PKCS11Test { // NSS versions of each library. It is simpler to keep nss_version // for quick checking for generic testing than many if-else statements. - static double softoken3_version = -1; - static double nss3_version = -1; + static String softoken3_version = null; + static String nss3_version = null; static Provider pkcs11 = newPKCS11Provider(); private static String PKCS11_BASE; private static Map osMap; @@ -258,13 +260,23 @@ private static String getOsId() { } static boolean isBadNSSVersion(Provider p) { - double nssVersion = getNSSVersion(); - if (isNSS(p) && nssVersion >= 3.11 && nssVersion < 3.12) { - System.out.println("NSS 3.11 has a DER issue that recent " + - "version do not, skipping"); - return true; + String nssVersion = getNSSVersion(); + if (isNSS(p)) { + // bad version is just between [3.11,3.12) + // strings may include a trailing dash e.g., 3.111-debug + Pattern pattern = Pattern.compile("^(\\d+)(?:\\.(.*?))?(?:-.*)?$"); + Matcher m = pattern.matcher(nssVersion); + if (m.matches()) { + int major = Integer.parseInt(m.group(1)); + String tmp = m.group(2); + double minor = tmp == null ? 0 : Double.parseDouble(tmp); + return major == 3 && (11 <= minor && minor < 12); + } else { + throw new RuntimeException("Unexpected version format: " + nssVersion); + } + } else { + return false; } - return false; } protected static void safeReload(String lib) { @@ -293,26 +305,26 @@ public static boolean isNSS(Provider p) { return p.getName().equalsIgnoreCase("SUNPKCS11-NSS"); } - static double getNSSVersion() { - if (nss_version == -1) + static String getNSSVersion() { + if (nss_version == null) getNSSInfo(); return nss_version; } static ECCState getNSSECC() { - if (nss_version == -1) + if (nss_version == null) getNSSInfo(); return nss_ecc_status; } - public static double getLibsoftokn3Version() { - if (softoken3_version == -1) + public static String getLibsoftokn3Version() { + if (softoken3_version == null) return getNSSInfo("softokn3"); return softoken3_version; } - public static double getLibnss3Version() { - if (nss3_version == -1) + public static String getLibnss3Version() { + if (nss3_version == null) return getNSSInfo("nss3"); return nss3_version; } @@ -327,7 +339,7 @@ static void getNSSInfo() { // $Header: NSS // Version: NSS // Here, stands for NSS version. - static double getNSSInfo(String library) { + static String getNSSInfo(String library) { // look for two types of headers in NSS libraries String nssHeader1 = "$Header: NSS"; String nssHeader2 = "Version: NSS"; @@ -336,15 +348,15 @@ static double getNSSInfo(String library) { int i = 0; Path libfile = null; - if (library.compareTo("softokn3") == 0 && softoken3_version > -1) + if (library.compareTo("softokn3") == 0 && softoken3_version != null) return softoken3_version; - if (library.compareTo("nss3") == 0 && nss3_version > -1) + if (library.compareTo("nss3") == 0 && nss3_version != null) return nss3_version; try { libfile = getNSSLibPath(); if (libfile == null) { - return 0.0; + return "0.0"; } try (InputStream is = Files.newInputStream(libfile)) { byte[] data = new byte[1000]; @@ -380,7 +392,7 @@ static double getNSSInfo(String library) { if (!found) { System.out.println("lib" + library + " version not found, set to 0.0: " + libfile); - nss_version = 0.0; + nss_version = "0.0"; return nss_version; } @@ -393,26 +405,7 @@ static double getNSSInfo(String library) { version.append(c); } - // If a "dot dot" release, strip the extra dots for double parsing - String[] dot = version.toString().split("\\."); - if (dot.length > 2) { - version = new StringBuilder(dot[0] + "." + dot[1]); - for (int j = 2; dot.length > j; j++) { - version.append(dot[j]); - } - } - - // Convert to double for easier version value checking - try { - nss_version = Double.parseDouble(version.toString()); - } catch (NumberFormatException e) { - System.out.println("===== Content start ====="); - System.out.println(s); - System.out.println("===== Content end ====="); - System.out.println("Failed to parse lib" + library + - " version. Set to 0.0"); - e.printStackTrace(); - } + nss_version = version.toString(); System.out.print("library: " + library + ", version: " + version + ". "); diff --git a/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java b/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java index 880adc954ea50..741bf08f4450f 100644 --- a/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java +++ b/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -121,10 +121,12 @@ public static void main(String[] args) throws Exception { } private static boolean improperNSSVersion(Provider p) { - double nssVersion = getNSSVersion(); - if (p.getName().equalsIgnoreCase("SunPKCS11-NSSKeyStore") - && nssVersion >= 3.28 && nssVersion < 3.35) { - return true; + String nssVersion = getNSSVersion(); + if (p.getName().equalsIgnoreCase("SunPKCS11-NSSKeyStore")) { + int idx = nssVersion.indexOf("."); + double major = Double.parseDouble(nssVersion.substring(0, idx)); + double minor = Double.parseDouble(nssVersion.substring(idx+1)); + return major == 3 && (minor >= 28 && minor < 35); } return false; diff --git a/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java b/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java index a9b43a647a9ad..5c302970e95e4 100644 --- a/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java +++ b/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 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 @@ -47,13 +47,18 @@ public static void main(String[] args) throws Exception { @Override protected boolean skipTest(Provider provider) { - double version = getNSSVersion(); - String[] versionStrs = Double.toString(version).split("\\."); - int major = Integer.parseInt(versionStrs[0]); - int minor = Integer.parseInt(versionStrs[1]); - if (isNSS(provider) && (version == 0.0 || (major >= 3 && minor >= 14))) { - System.out.println("Skip testing NSS " + version); - return true; + if (isNSS(provider)) { + String version = getNSSVersion(); + if (version == null) { + return true; + } + int idx = version.indexOf("."); + double major = Double.parseDouble(version.substring(0, idx)); + double minor = Double.parseDouble(version.substring(idx+1)); + if (major >= 3 && minor >= 14){ + System.out.println("Skip testing NSS " + version); + return true; + } } return false; diff --git a/test/jdk/sun/security/pkcs11/ec/TestECDH.java b/test/jdk/sun/security/pkcs11/ec/TestECDH.java index b6821b8837212..2900656f6261e 100644 --- a/test/jdk/sun/security/pkcs11/ec/TestECDH.java +++ b/test/jdk/sun/security/pkcs11/ec/TestECDH.java @@ -111,6 +111,7 @@ protected boolean skipTest(Provider p) { * PKCS11Test.main will remove this provider if needed */ Providers.setAt(p, 1); + System.out.println("Testing provider " + p.getName()); if (false) { KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC", p); From cd250fab8049bb1671fa892a22d908e436774a6f Mon Sep 17 00:00:00 2001 From: Matthew Donovan Date: Fri, 5 Sep 2025 09:40:28 -0400 Subject: [PATCH 2/2] refactored to encapsulate the version as a record instead of doing repeated string parsing --- .../security/pkcs11/Cipher/TestKATForGCM.java | 16 +++--- .../pkcs11/KeyStore/SecretKeysBasic.java | 6 +-- test/jdk/sun/security/pkcs11/PKCS11Test.java | 52 ++++++++++--------- .../pkcs11/Secmod/AddTrustedCert.java | 8 ++- .../pkcs11/Signature/TestDSAKeyLength.java | 7 +-- 5 files changed, 45 insertions(+), 44 deletions(-) diff --git a/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java b/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java index 9177e859ebb8e..e5e8284e6f4ac 100644 --- a/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java +++ b/test/jdk/sun/security/pkcs11/Cipher/TestKATForGCM.java @@ -321,17 +321,19 @@ public void main(Provider p) throws Exception { System.out.println("Test Passed!"); } } catch (Exception e) { - System.out.println("Exception occured using " + p.getName() + " version " + p.getVersionStr()); + System.out.println("Exception occured using " + p.getName() + + " version " + p.getVersionStr()); if (isNSS(p)) { - String ver = getNSSInfo("nss"); + Version ver = getNSSInfo("nss"); String osName = System.getProperty("os.name"); - int idx = ver.indexOf("."); - double major = Double.parseDouble(ver.substring(0, idx)); - double minor = Double.parseDouble(ver.substring(idx+1)); - if (major == 3 && minor > 13.9 && minor < 15 && osName.equals("Linux")) { + + if (osName.equals("Linux") && + ver.major() == 3 && ver.minor() < 15 + && (ver.minor() > 13 && ver.patch() >= 9)) { // warn about buggy behaviour on Linux with nss 3.14 - System.out.println("Warning: old NSS " + ver + " might be problematic, consider upgrading it"); + System.out.println("Warning: old NSS " + ver + + " might be problematic, consider upgrading it"); } } throw e; diff --git a/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java b/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java index 96e10b976cf31..1ff80fcaf07f9 100644 --- a/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java +++ b/test/jdk/sun/security/pkcs11/KeyStore/SecretKeysBasic.java @@ -117,9 +117,9 @@ private static boolean checkSecretKeyEntry(String alias, // to be read incorrectly. Checking for improper 16 byte length // in key string. if (isNSS(provider) && expected.getAlgorithm().equals("AES")) { - String version = getNSSVersion(); - if (version.equals("3.12") || version.equals("3.12.1") - || version.equals("3.12.2")) { + Version version = getNSSVersion(); + if (version.major() == 3 && version.minor() == 12 + && version.patch() <= 2) { System.out.println("NSS 3.12 bug returns incorrect AES key " + "length breaking key storage. Aborting..."); return true; diff --git a/test/jdk/sun/security/pkcs11/PKCS11Test.java b/test/jdk/sun/security/pkcs11/PKCS11Test.java index 07017ea7e3aa3..7aaafdad1b6f2 100644 --- a/test/jdk/sun/security/pkcs11/PKCS11Test.java +++ b/test/jdk/sun/security/pkcs11/PKCS11Test.java @@ -52,8 +52,6 @@ import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -85,7 +83,7 @@ public abstract class PKCS11Test { private static final String NSS_BUNDLE_VERSION = "3.111"; private static final String NSSLIB = "jpg.tests.jdk.nsslib"; - static String nss_version = null; + static Version nss_version = null; static ECCState nss_ecc_status = ECCState.Basic; // The NSS library we need to search for in getNSSLibDir() @@ -95,8 +93,8 @@ public abstract class PKCS11Test { // NSS versions of each library. It is simpler to keep nss_version // for quick checking for generic testing than many if-else statements. - static String softoken3_version = null; - static String nss3_version = null; + static Version softoken3_version = null; + static Version nss3_version = null; static Provider pkcs11 = newPKCS11Provider(); private static String PKCS11_BASE; private static Map osMap; @@ -260,25 +258,31 @@ private static String getOsId() { } static boolean isBadNSSVersion(Provider p) { - String nssVersion = getNSSVersion(); + Version nssVersion = getNSSVersion(); if (isNSS(p)) { // bad version is just between [3.11,3.12) - // strings may include a trailing dash e.g., 3.111-debug - Pattern pattern = Pattern.compile("^(\\d+)(?:\\.(.*?))?(?:-.*)?$"); - Matcher m = pattern.matcher(nssVersion); - if (m.matches()) { - int major = Integer.parseInt(m.group(1)); - String tmp = m.group(2); - double minor = tmp == null ? 0 : Double.parseDouble(tmp); - return major == 3 && (11 <= minor && minor < 12); - } else { - throw new RuntimeException("Unexpected version format: " + nssVersion); - } + return nssVersion.major == 3 && 11 == nssVersion.minor; } else { return false; } } + public record Version(int major, int minor, int patch) {} + + protected static Version parseVersionString(String version) { + String [] parts = version.split("\\."); + int major = Integer.parseInt(parts[0]); + int minor = 0; + int patch = 0; + if (parts.length >= 2) { + minor = Integer.parseInt(parts[1]); + } + if (parts.length >= 3) { + patch = Integer.parseInt(parts[2]); + } + return new Version(major, minor, patch); + } + protected static void safeReload(String lib) { try { System.load(lib); @@ -305,7 +309,7 @@ public static boolean isNSS(Provider p) { return p.getName().equalsIgnoreCase("SUNPKCS11-NSS"); } - static String getNSSVersion() { + static Version getNSSVersion() { if (nss_version == null) getNSSInfo(); return nss_version; @@ -317,13 +321,13 @@ static ECCState getNSSECC() { return nss_ecc_status; } - public static String getLibsoftokn3Version() { + public static Version getLibsoftokn3Version() { if (softoken3_version == null) return getNSSInfo("softokn3"); return softoken3_version; } - public static String getLibnss3Version() { + public static Version getLibnss3Version() { if (nss3_version == null) return getNSSInfo("nss3"); return nss3_version; @@ -339,7 +343,7 @@ static void getNSSInfo() { // $Header: NSS // Version: NSS // Here, stands for NSS version. - static String getNSSInfo(String library) { + static Version getNSSInfo(String library) { // look for two types of headers in NSS libraries String nssHeader1 = "$Header: NSS"; String nssHeader2 = "Version: NSS"; @@ -356,7 +360,7 @@ static String getNSSInfo(String library) { try { libfile = getNSSLibPath(); if (libfile == null) { - return "0.0"; + return parseVersionString("0.0"); } try (InputStream is = Files.newInputStream(libfile)) { byte[] data = new byte[1000]; @@ -392,7 +396,7 @@ static String getNSSInfo(String library) { if (!found) { System.out.println("lib" + library + " version not found, set to 0.0: " + libfile); - nss_version = "0.0"; + nss_version = parseVersionString("0.0"); return nss_version; } @@ -405,7 +409,7 @@ static String getNSSInfo(String library) { version.append(c); } - nss_version = version.toString(); + nss_version = parseVersionString(version.toString()); System.out.print("library: " + library + ", version: " + version + ". "); diff --git a/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java b/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java index 741bf08f4450f..7b4a5075da867 100644 --- a/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java +++ b/test/jdk/sun/security/pkcs11/Secmod/AddTrustedCert.java @@ -121,12 +121,10 @@ public static void main(String[] args) throws Exception { } private static boolean improperNSSVersion(Provider p) { - String nssVersion = getNSSVersion(); + Version nssVersion = getNSSVersion(); if (p.getName().equalsIgnoreCase("SunPKCS11-NSSKeyStore")) { - int idx = nssVersion.indexOf("."); - double major = Double.parseDouble(nssVersion.substring(0, idx)); - double minor = Double.parseDouble(nssVersion.substring(idx+1)); - return major == 3 && (minor >= 28 && minor < 35); + return nssVersion.major() == 3 && + (nssVersion.minor() >= 28 && nssVersion.minor() < 35); } return false; diff --git a/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java b/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java index 5c302970e95e4..ffd7b9e3ee060 100644 --- a/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java +++ b/test/jdk/sun/security/pkcs11/Signature/TestDSAKeyLength.java @@ -48,14 +48,11 @@ public static void main(String[] args) throws Exception { @Override protected boolean skipTest(Provider provider) { if (isNSS(provider)) { - String version = getNSSVersion(); + Version version = getNSSVersion(); if (version == null) { return true; } - int idx = version.indexOf("."); - double major = Double.parseDouble(version.substring(0, idx)); - double minor = Double.parseDouble(version.substring(idx+1)); - if (major >= 3 && minor >= 14){ + if (version.major() >= 3 && version.minor() >= 14){ System.out.println("Skip testing NSS " + version); return true; }