Merge pull request #448 from KostasTsiounis/constraints_stack_strict

keithc-ca · web-flow · commit 04e4f00e8fad · 2025-03-04T13:19:31.000-05:00
Allow multiple constraints for each algorithm
diff --git a/closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java b/closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java
@@ -805,6 +805,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
             if (debug != null) {
                 debug.println("Security constraints check of provider.");
             }
+            constraints:
             for (Constraint constraint : constraints) {
                 String cType = constraint.type;
                 String cAlgorithm = constraint.algorithm;
@@ -823,14 +824,14 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
                     if (debug != null) {
                         debug.println("The constraint doesn't apply to the service type.");
                     }
-                    continue;
+                    continue constraints;
                 }
                 if (!isAsterisk(cAlgorithm) && !algorithm.equalsIgnoreCase(cAlgorithm)) {
                     // The constraint doesn't apply to the service algorithm.
                     if (debug != null) {
                         debug.println("The constraint doesn't apply to the service algorithm.");
                     }
-                    continue;
+                    continue constraints;
                 }
 
                 // For type and algorithm match, and attribute is not *.
@@ -852,7 +853,8 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
                                     + "\nagainst the service attribute value: " + sValue);
                         }
                         if ((sValue == null) || !cValue.equalsIgnoreCase(sValue)) {
-                            // If any attribute doesn't match, return service is not allowed.
+                            // If any of the attributes don't match,
+                            // then this constraint doesn't match so move on.
                             if (debug != null) {
                                 debug.println("Attributes don't match!");
                                 debug.println("The following service:"
@@ -861,7 +863,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
                                             + "\n\tAttribute: " + cAttribute
                                             + "\nis NOT allowed in provider: " + providerClassName);
                             }
-                            return false;
+                            continue constraints;
                         }
                         if (debug != null) {
                             debug.println("Attributes match!");
@@ -919,7 +921,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
                     }
 
                     // If nothing matching the accepted uses is found in the call stack,
-                    // this service is not allowed.
+                    // then this constraint doesn't match so move on.
                     if (!found) {
                         if (debug != null) {
                             debug.println("Classes in call stack are not part of accepted uses!");
@@ -930,7 +932,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
                                         + "\n\tAccepted uses: " + cAcceptedUses
                                         + "\nis NOT allowed in provider: " + providerClassName);
                         }
-                        return false;
+                        continue constraints;
                     }
                 }
 
diff --git a/closed/test/jdk/openj9/internal/security/TestConstraintsSuccess.java b/closed/test/jdk/openj9/internal/security/TestConstraintsSuccess.java
@@ -72,6 +72,16 @@ private static void getInstances() throws Exception {
         KeyManagerFactory.getInstance("SunX509");
         TrustManagerFactory.getInstance("SunX509");
         SSLContext.getInstance("TLSv1.3");
+
+        // Since there are three constraints for MD5, with only the middle one
+        // allowing for use by this class, successfully getting the algorithm
+        // verifies that all constraints are checked.
+        MessageDigest.getInstance("MD5");
+
+        // Since there are three constraints for SHA512withECDSA, with only the
+        // middle one having the correct attributes, successfully getting the
+        // algorithm verifies that all constraints are checked.
+        Signature.getInstance("SHA512withECDSA");
     }
 
     @Test
diff --git a/closed/test/jdk/openj9/internal/security/constraints-java.security b/closed/test/jdk/openj9/internal/security/constraints-java.security
@@ -21,7 +21,7 @@
 RestrictedSecurity.TestConstraints.Version.desc.name = Test Base Profile
 RestrictedSecurity.TestConstraints.Version.desc.default = false
 RestrictedSecurity.TestConstraints.Version.desc.fips = false
-RestrictedSecurity.TestConstraints.Version.desc.hash = SHA256:3162e55fbeed3c2453ebdacd854243eb8b9af3769a84bf66bb12614f9076ea64
+RestrictedSecurity.TestConstraints.Version.desc.hash = SHA256:235727d782ff9e04d875627c694d509b758ed7c037eaf5aed8dcd014f2602af2
 RestrictedSecurity.TestConstraints.Version.desc.number = Certificate #XXX
 RestrictedSecurity.TestConstraints.Version.desc.policy =
 RestrictedSecurity.TestConstraints.Version.fips.mode = test
@@ -33,12 +33,24 @@ RestrictedSecurity.TestConstraints.Version.jce.provider.1 = sun.security.provide
     {CertPathBuilder, PKIX, *, FullClassName:TestConstraintsSuccess}, \
     {CertPathValidator, PKIX, *, FullClassName:TestConstraintsSuccess}, \
     {SecureRandom, SHA1PRNG, *, FullClassName:TestConstraintsSuccess}, \
+    {MessageDigest, MD5, *, FullClassName:NonExistingClass}, \
+    {MessageDigest, MD5, *, FullClassName:TestConstraintsSuccess}, \
+    {MessageDigest, MD5, *, FullClassName:AnotherNonExistingClass}, \
     {MessageDigest, SHA-256, *}, \
     {MessageDigest, SHA-512, *, FullClassName:TestConstraintsSuccess}, \
     {KeyStore, PKCS12, *, FullClassName:TestConstraintsSuccess}]
 RestrictedSecurity.TestConstraints.Version.jce.provider.2 = sun.security.ec.SunEC [ \
     {AlgorithmParameters, EC, *, ModuleAndFullClassName:java.base/java.security.KeyPairGenerator}, \
     {Signature, SHA256withECDSA, *, FullClassName:TestConstraintsSuccess}, \
+    {Signature, SHA512withECDSA, ImplementedIn=Software: \
+        SupportedKeyClasses=java.security.interfaces.ECPublicKey|java.security.interfaces.ECPrivateKey: \
+        KeySize=255, FullClassName:TestConstraintsSuccess}, \
+    {Signature, SHA512withECDSA, ImplementedIn=Software: \
+        SupportedKeyClasses=java.security.interfaces.ECPublicKey|java.security.interfaces.ECPrivateKey: \
+        KeySize=256, FullClassName:TestConstraintsSuccess}, \
+    {Signature, SHA512withECDSA, ImplementedIn=Software: \
+        SupportedKeyClasses=java.security.interfaces.ECPublicKey|java.security.interfaces.ECPrivateKey: \
+        KeySize=257, FullClassName:TestConstraintsSuccess}, \
     {KeyPairGenerator, EC, *, FullClassName:TestConstraintsSuccess}, \
     {KeyAgreement, ECDH, *, FullClassName:TestConstraintsSuccess}, \
     {KeyFactory, EC, *, FullClassName:TestConstraintsSuccess}]

Original file line number	Diff line number	Diff line change
`@@ -805,6 +805,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`805`	`805`	`if (debug != null) {`
`806`	`806`	`debug.println("Security constraints check of provider.");`
`807`	`807`	`}`
	`808`	`+ constraints:`
`808`	`809`	`for (Constraint constraint : constraints) {`
`809`	`810`	`String cType = constraint.type;`
`810`	`811`	`String cAlgorithm = constraint.algorithm;`
`@@ -823,14 +824,14 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`823`	`824`	`if (debug != null) {`
`824`	`825`	`debug.println("The constraint doesn't apply to the service type.");`
`825`	`826`	`}`
`826`		`- continue;`
	`827`	`+ continue constraints;`
`827`	`828`	`}`
`828`	`829`	`if (!isAsterisk(cAlgorithm) && !algorithm.equalsIgnoreCase(cAlgorithm)) {`
`829`	`830`	`// The constraint doesn't apply to the service algorithm.`
`830`	`831`	`if (debug != null) {`
`831`	`832`	`debug.println("The constraint doesn't apply to the service algorithm.");`
`832`	`833`	`}`
`833`		`- continue;`
	`834`	`+ continue constraints;`
`834`	`835`	`}`
`835`	`836`
`836`	`837`	`// For type and algorithm match, and attribute is not *.`
`@@ -852,7 +853,8 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`852`	`853`	`+ "\nagainst the service attribute value: " + sValue);`
`853`	`854`	`}`
`854`	`855`	`if ((sValue == null) \|\| !cValue.equalsIgnoreCase(sValue)) {`
`855`		`- // If any attribute doesn't match, return service is not allowed.`
	`856`	`+ // If any of the attributes don't match,`
	`857`	`+ // then this constraint doesn't match so move on.`
`856`	`858`	`if (debug != null) {`
`857`	`859`	`debug.println("Attributes don't match!");`
`858`	`860`	`debug.println("The following service:"`
`@@ -861,7 +863,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`861`	`863`	`+ "\n\tAttribute: " + cAttribute`
`862`	`864`	`+ "\nis NOT allowed in provider: " + providerClassName);`
`863`	`865`	`}`
`864`		`- return false;`
	`866`	`+ continue constraints;`
`865`	`867`	`}`
`866`	`868`	`if (debug != null) {`
`867`	`869`	`debug.println("Attributes match!");`
`@@ -919,7 +921,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`919`	`921`	`}`
`920`	`922`
`921`	`923`	`// If nothing matching the accepted uses is found in the call stack,`
`922`		`- // this service is not allowed.`
	`924`	`+ // then this constraint doesn't match so move on.`
`923`	`925`	`if (!found) {`
`924`	`926`	`if (debug != null) {`
`925`	`927`	`debug.println("Classes in call stack are not part of accepted uses!");`
`@@ -930,7 +932,7 @@ boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {`
`930`	`932`	`+ "\n\tAccepted uses: " + cAcceptedUses`
`931`	`933`	`+ "\nis NOT allowed in provider: " + providerClassName);`
`932`	`934`	`}`
`933`		`- return false;`
	`935`	`+ continue constraints;`
`934`	`936`	`}`
`935`	`937`	`}`
`936`	`938`