Updates according to code review

closed/adds/jdk/src/share/classes/openj9/internal/security/RestrictedSecurity.java

@@ -586,22 +586,9 @@ private void initProviders() {
  // Provider with argument (provider name + optional argument).
  providers.add(pNum - 1, providerName);
 
- // Remove the provider's optional arguments if there are.
- pos = providerName.indexOf(' ');
- providerName = (pos < 0) ? providerName.trim() : providerName.substring(0, pos).trim();
- // Remove the provider's class package names if there are.
- pos = providerName.lastIndexOf('.');
- providerName = (pos < 0) ? providerName : providerName.substring(pos + 1, providerName.length());
- // Provider without arguments and package names.
- if (providerName.equals("Provider")) {
- // In JDK 8, the main class for the SunJSSE provider is com.sun.net.ssl.internal.ssl.Provider
- providersSimpleName.add(pNum - 1, "SunJSSE");
- } else if (providerName.equals("Sun")) {
- // In JDK 8, the main class for the SUN provider is sun.security.provider.Sun
- providersSimpleName.add(pNum - 1, "SUN");
- } else {
- providersSimpleName.add(pNum - 1, providerName);
- }
+ // Provider name defined in provider construction method.
+ providerName = getProvidersSimpleName(providerName);
+ providersSimpleName.add(pNum - 1, providerName);
 
  if (debug != null) {
  debug.println(
@@ -768,17 +755,9 @@ boolean isRestrictedServiceAllowed(Service service) {
 
  if (constraints == null) {
  // Disallow unknown providers.
- if (debug != null) {
- debug.println("Security constraints check."
- + " Disallow unknown provider: " + providerName);
- }
  return false;
  } else if (constraints.length == 0) {
  // Allow this provider with no constraints.
- if (debug != null) {
- debug.println("Security constraints check."
- + " Allow this provider with no constraints: " + providerName);
- }
  return true;
  }
 
@@ -795,7 +774,7 @@ boolean isRestrictedServiceAllowed(Service service) {
  // The constraint doesn't apply to the service type.
  continue;
  }
- if (!isAsterisk(cAlgorithm) && !algorithm.equalsIgnoreCase(cAlgorithm)) {
+ if (!isAsterisk(cAlgorithm) && !algorithm.equals(cAlgorithm)) {
  // The constraint doesn't apply to the service algorith.
  continue;
  }
@@ -866,18 +845,7 @@ boolean isRestrictedProviderAllowed(String providerName) {
  debug.println("Checking the provider " + providerName + " in restricted security mode.");
  }
 
- // Remove argument, e.g. -NSS-FIPS, if there is.
- int pos = providerName.indexOf('-');
- providerName = (pos < 0) ? providerName : providerName.substring(0, pos);
-
- // Remove the provider class package name if there is.
- pos = providerName.lastIndexOf('.');
- providerName = (pos < 0) ? providerName : providerName.substring(pos + 1, providerName.length());
-
- // In JDK 8, the main class for the SunJSSE provider is com.sun.net.ssl.internal.ssl.Provider.
- // And, the main class for the SUN provider is sun.security.provider.Sun.
- providerName = providerName.equals("Sun") ? "SUN" : providerName;
- providerName = providerName.equals("Provider") ? "SunJSSE" : providerName;
+ providerName = getProvidersSimpleName(providerName);
 
  // Check if the provider is in restricted security provider list.
  // If not, the provider won't be registered.
@@ -902,6 +870,36 @@ boolean isRestrictedProviderAllowed(String providerName) {
  return false;
  }
 
+ /**
+ *
+ * @param providerName Provider name or provider with packages or arguments.
+ * @return Provider name defined in provider construction method
+ */
+ private String getProvidersSimpleName(String providerName) {
+ // Remove the provider's optional arguments if there are.
+ int pos = providerName.indexOf(' ');
+ providerName = (pos < 0) ? providerName.trim() : providerName.substring(0, pos).trim();
+
+ // Remove argument, e.g. -NSS-FIPS, if there is.
+ pos = providerName.indexOf('-');
+ providerName = (pos < 0) ? providerName : providerName.substring(0, pos);
+
+ if (providerName.equals("com.sun.net.ssl.internal.ssl.Provider")) {
+ // In JDK 8, the main class for the SunJSSE provider is
+ // com.sun.net.ssl.internal.ssl.Provider
+ return "SunJSSE";
+ } else if (providerName.equals("sun.security.provider.Sun")) {
+ // In JDK 8, the main class for the SUN provider is sun.security.provider.Sun
+ return "SUN";
+ } else {
+ // Remove the provider's class package names if there are.
+ pos = providerName.lastIndexOf('.');
+ providerName = (pos < 0) ? providerName : providerName.substring(pos + 1, providerName.length());
+ // Provider without arguments and package names.
+ return providerName;
+ }
+ }
+
  /**
  * List audit info if userSecurityAudit is true, default as false.
  */

jdk/src/share/classes/java/security/Provider.java

@@ -928,7 +928,7 @@ private void removeInvalidServices(Map<ServiceKey,Service> map) {
  for (Iterator<Map.Entry<ServiceKey, Service>> t =
  map.entrySet().iterator(); t.hasNext(); ) {
  Service s = t.next().getValue();
- if (s.isValid() == false) {
+ if ((s.isValid() == false) || !RestrictedSecurity.isServiceAllowed(s)) {
  t.remove();
  }
  }
@@ -970,9 +970,6 @@ private void parseLegacyPut(String name, String value) {
  s = new Service(this);
  s.type = type;
  s.algorithm = stdAlg;
- if (!RestrictedSecurity.isServiceAllowed(s)) {
- return;
- }
  legacyMap.put(key, s);
  }
  legacyMap.put(new ServiceKey(type, aliasAlg, true), s);
@@ -994,9 +991,6 @@ private void parseLegacyPut(String name, String value) {
  s = new Service(this);
  s.type = type;
  s.algorithm = stdAlg;
- if (!RestrictedSecurity.isServiceAllowed(s)) {
- return;
- }
  legacyMap.put(key, s);
  }
  s.className = className;
@@ -1021,9 +1015,6 @@ private void parseLegacyPut(String name, String value) {
  legacyMap.put(key, s);
  }
  s.addAttribute(attributeName, attributeValue);
- if (!RestrictedSecurity.isServiceAllowed(s)) {
- legacyMap.remove(key);
- }
  }
  }
  }

jdk/src/share/classes/java/security/SecureRandom.java

@@ -201,7 +201,7 @@ public SecureRandom(byte seed[]) {
  private void getDefaultPRNG(boolean setSeed, byte[] seed) {
  String prng;
 
- // If in FIPS mode, use the SecureRandom from the FIPS provider.
+ // In restricted security mode, use the SecureRandom from restricted security provider.
  if (RestrictedSecurity.isEnabled()) {
  prng = RestrictedSecurity.getRandomAlgorithm();
  } else {

jdk/src/share/classes/java/util/ServiceLoader.java

@@ -369,8 +369,8 @@ private boolean hasNextService() {
  ArrayList<String> provNames = new ArrayList<>();
  while (pending.hasNext()) {
  String className = pending.next();
- Class<?> clazz = null;
  try {
+ Class<?> clazz = null;
  clazz = Class.forName(className, false, loader);
  if (RestrictedSecurity.isProviderAllowed(clazz)) {
  provNames.add(className);

jdk/src/share/lib/security/java.security-linux

@@ -107,14 +107,17 @@ RestrictedSecurity1.tls.legacyAlgorithms =
 RestrictedSecurity1.jce.certpath.disabledAlgorithms =
 RestrictedSecurity1.jce.legacyAlgorithms =
 RestrictedSecurity1.jce.provider.1 = sun.security.pkcs11.SunPKCS11 ${java.home}/lib/security/nss.fips.cfg
-RestrictedSecurity1.jce.provider.2 = sun.security.provider.Sun [{CertificateFactory, X.509, *}, \
- {CertStore, Collection, *}, \
- {CertStore, com.sun.security.IndexedCollection, *}, \
- {Policy, JavaPolicy, *}, {Configuration, JavaLoginConfig, *}, \
- {CertPathBuilder, PKIX, *}, \
- {CertPathValidator, PKIX, *}]
-RestrictedSecurity1.jce.provider.3 = sun.security.ec.SunEC [{KeyFactory, EC, *}, \
- {AlgorithmParameters, EC, *}]
+RestrictedSecurity1.jce.provider.2 = sun.security.provider.Sun \
+ [{CertificateFactory, X.509, ImplementedIn=Software}, \
+ {CertPathBuilder, PKIX, ValidationAlgorithm=RFC5280:ImplementedIn=Software}, \
+ {CertPathValidator, PKIX, ValidationAlgorithm=RFC5280:ImplementedIn=Software}, \
+ {CertStore, Collection, ImplementedIn=Software}, \
+ {CertStore, com.sun.security.IndexedCollection, ImplementedIn=Software}, \
+ {Configuration, JavaLoginConfig, *}, \
+ {Policy, JavaPolicy, *}]
+RestrictedSecurity1.jce.provider.3 = sun.security.ec.SunEC \
+ [{AlgorithmParameters, EC, *}, \
+ {KeyFactory, EC, ImplementedIn=Software}]
 RestrictedSecurity1.jce.provider.4 = com.sun.net.ssl.internal.ssl.Provider
 
 RestrictedSecurity1.keystore.type = PKCS11