From 77c715c69984c6823ad416267f932017b2d1674d Mon Sep 17 00:00:00 2001 From: valeriep Date: Tue, 7 Jul 2020 16:55:29 +0000 Subject: [PATCH] 8248505: Unexpected NoSuchAlgorithmException when using secure random impl from BCFIPS provider Summary: Use getService(...) call for Provider.getDefaultSecureRandomService() Reviewed-by: weijun, coffeys, mullan --- .../share/classes/java/security/Provider.java | 33 ++++++----- .../security/SecureRandom/DefaultAlgo.java | 55 ++++++++++++++++--- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/java.base/share/classes/java/security/Provider.java b/src/java.base/share/classes/java/security/Provider.java index 579b1c422b..e210b8b98f 100644 --- a/src/java.base/share/classes/java/security/Provider.java +++ b/src/java.base/share/classes/java/security/Provider.java @@ -867,7 +867,7 @@ public abstract class Provider extends Properties { // For backward compatibility, the registration ordering of // SecureRandom (RNG) algorithms needs to be preserved for // "new SecureRandom()" calls when this provider is used - private transient Set prngServices; + private transient Set prngAlgos; // Map // used for services added via legacy methods, init on demand @@ -1087,7 +1087,7 @@ public abstract class Provider extends Properties { legacyChanged = false; servicesChanged = false; serviceSet = null; - prngServices = null; + prngAlgos = null; super.clear(); putId(); } @@ -1219,7 +1219,7 @@ public abstract class Provider extends Properties { s.className = className; if (type.equals("SecureRandom")) { - updateSecureRandomEntries(true, s); + updateSecureRandomEntries(true, s.algorithm); } } else { // attribute // e.g. put("MessageDigest.SHA-1 ImplementedIn", "Software"); @@ -1381,25 +1381,25 @@ public abstract class Provider extends Properties { synchronized (this) { putPropertyStrings(s); if (type.equals("SecureRandom")) { - updateSecureRandomEntries(true, s); + updateSecureRandomEntries(true, s.algorithm); } } } - private void updateSecureRandomEntries(boolean doAdd, Service s) { + // keep tracks of the registered secure random algos and store them in order + private void updateSecureRandomEntries(boolean doAdd, String s) { Objects.requireNonNull(s); if (doAdd) { - if (prngServices == null) { - prngServices = new LinkedHashSet(); + if (prngAlgos == null) { + prngAlgos = new LinkedHashSet(); } - prngServices.add(s); + prngAlgos.add(s); } else { - prngServices.remove(s); + prngAlgos.remove(s); } if (debug != null) { - debug.println((doAdd? "Add":"Remove") + " SecureRandom algo " + - s.getAlgorithm()); + debug.println((doAdd? "Add":"Remove") + " SecureRandom algo " + s); } } @@ -1409,12 +1409,15 @@ public abstract class Provider extends Properties { checkInitialized(); if (legacyChanged) { - prngServices = null; + prngAlgos = null; ensureLegacyParsed(); } - if (prngServices != null && !prngServices.isEmpty()) { - return prngServices.iterator().next(); + if (prngAlgos != null && !prngAlgos.isEmpty()) { + // IMPORTANT: use the Service obj returned by getService(...) call + // as providers may override putService(...)/getService(...) and + // return their own Service objects + return getService("SecureRandom", prngAlgos.iterator().next()); } return null; @@ -1514,7 +1517,7 @@ public abstract class Provider extends Properties { synchronized (this) { removePropertyStrings(s); if (type.equals("SecureRandom")) { - updateSecureRandomEntries(false, s); + updateSecureRandomEntries(false, s.algorithm); } } } diff --git a/test/jdk/java/security/SecureRandom/DefaultAlgo.java b/test/jdk/java/security/SecureRandom/DefaultAlgo.java index d85d73b2cb..06027f7162 100644 --- a/test/jdk/java/security/SecureRandom/DefaultAlgo.java +++ b/test/jdk/java/security/SecureRandom/DefaultAlgo.java @@ -26,13 +26,12 @@ import java.security.Provider; import java.security.Security; import java.security.SecureRandom; import java.security.Provider.Service; -import java.util.Objects; import java.util.Arrays; import sun.security.provider.SunEntries; /** * @test - * @bug 8228613 8246613 + * @bug 8228613 8246613 8248505 * @summary Ensure that the default SecureRandom algo used is based * on the registration ordering, and falls to next provider * if none are found @@ -40,6 +39,9 @@ import sun.security.provider.SunEntries; */ public class DefaultAlgo { + private static final String SR_IMPLCLASS = + "sun.security.provider.SecureRandom"; + public static void main(String[] args) throws Exception { String[] algos = { "A", "B", "C" }; test3rdParty(algos); @@ -51,7 +53,8 @@ public class DefaultAlgo { private static void test3rdParty(String[] algos) { Provider[] provs = { new SampleLegacyProvider(algos), - new SampleServiceProvider(algos) + new SampleServiceProvider(algos), + new CustomProvider(algos) }; for (Provider p : provs) { checkDefault(p, algos); @@ -72,9 +75,10 @@ public class DefaultAlgo { } private static void checkDefault(Provider p, String ... algos) { - out.println(p.getName() + " with " + Arrays.toString(algos)); - int pos = Security.insertProviderAt(p, 1); String pName = p.getName(); + out.println(pName + " with " + Arrays.toString(algos)); + int pos = Security.insertProviderAt(p, 1); + boolean isLegacy = pName.equals("SampleLegacy"); try { if (isLegacy) { @@ -91,7 +95,13 @@ public class DefaultAlgo { out.println("=> Test Passed"); } finally { if (pos != -1) { - Security.removeProvider(p.getName()); + Security.removeProvider(pName); + } + if (isLegacy) { + // add back the removed algos + for (String s : algos) { + p.put("SecureRandom." + s, SR_IMPLCLASS); + } } } } @@ -100,7 +110,7 @@ public class DefaultAlgo { SampleLegacyProvider(String[] listOfSupportedRNGs) { super("SampleLegacy", "1.0", "test provider using legacy put"); for (String s : listOfSupportedRNGs) { - put("SecureRandom." + s, "sun.security.provider.SecureRandom"); + put("SecureRandom." + s, SR_IMPLCLASS); } } } @@ -110,8 +120,37 @@ public class DefaultAlgo { super("SampleService", "1.0", "test provider using putService"); for (String s : listOfSupportedRNGs) { putService(new Provider.Service(this, "SecureRandom", s, - "sun.security.provider.SecureRandom", null, null)); + SR_IMPLCLASS, null, null)); + } + } + } + + // custom provider which overrides putService(...)/getService() and uses + // its own Service impl + private static class CustomProvider extends Provider { + private static class CustomService extends Provider.Service { + CustomService(Provider p, String type, String algo, String cName) { + super(p, type, algo, cName, null, null); } } + + CustomProvider(String[] listOfSupportedRNGs) { + super("Custom", "1.0", "test provider overrides putService with " + + " custom service with legacy registration"); + for (String s : listOfSupportedRNGs) { + putService(new CustomService(this, "SecureRandom", s , + SR_IMPLCLASS)); + } + } + @Override + protected void putService(Provider.Service s) { + // convert to legacy puts + put(s.getType() + "." + s.getAlgorithm(), s.getClassName()); + put(s.getType() + ":" + s.getAlgorithm(), s); + } + @Override + public Provider.Service getService(String type, String algo) { + return (Provider.Service) get(type + ":" + algo); + } } } -- GitLab