Skip to content

Commit e2e14f7

Browse files
KostasTsiounisKostasTsiounis
committed
Address review comments
1 parent f42c9c3 commit e2e14f7

File tree

3 files changed

+95
-74
lines changed

3 files changed

+95
-74
lines changed

‎closed/src/java.base/share/classes/openj9/internal/security/RestrictedSecurity.java

+70-46
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*/
2424
package openj9.internal.security;
2525

26+
import java.lang.StringBuilder;
2627
import java.nio.charset.StandardCharsets;
2728
import java.security.MessageDigest;
2829
import java.security.NoSuchAlgorithmException;
@@ -239,7 +240,7 @@ public static boolean isFIPSEnabled() {
239240
*/
240241
public static boolean isServiceAllowed(Service service) {
241242
if (securityEnabled) {
242-
return restricts.isRestrictedServiceAllowed(service, false);
243+
return restricts.isRestrictedServiceAllowed(service, true);
243244
}
244245
return true;
245246
}
@@ -252,7 +253,7 @@ public static boolean isServiceAllowed(Service service) {
252253
*/
253254
public static boolean canServiceBeRegistered(Service service) {
254255
if (securityEnabled) {
255-
return restricts.isRestrictedServiceAllowed(service, true);
256+
return restricts.isRestrictedServiceAllowed(service, false);
256257
}
257258
return true;
258259
}
@@ -753,10 +754,11 @@ private RestrictedSecurityProperties(String profileID, ProfileParser parser) {
753754
/**
754755
* Check if the Service is allowed in restricted security mode.
755756
*
756-
* @param service the Service to check
757+
* @param service the Service to check
758+
* @param checkUse should its attempted use be checked against the accepted
757759
* @return true if the Service is allowed
758760
*/
759-
boolean isRestrictedServiceAllowed(Service service, boolean isServiceAdded) {
761+
boolean isRestrictedServiceAllowed(Service service, boolean checkUse) {
760762
Provider provider = service.getProvider();
761763
String providerClassName = provider.getClass().getName();
762764

@@ -854,12 +856,8 @@ boolean isRestrictedServiceAllowed(Service service, boolean isServiceAdded) {
854856

855857
// See if accepted uses have been specified and apply
856858
// them to the call stack.
857-
if (!isServiceAdded && !isNullOrBlank(cAcceptedUses)) {
859+
if (checkUse && !isNullOrBlank(cAcceptedUses)) {
858860
String[] optionAndValue = cAcceptedUses.split(":");
859-
if (optionAndValue.length != 2) {
860-
printStackTraceAndExit("Incorrect specification of accepted uses in constraint: '"
861-
+ constraint + "'. Couldn't find option and value separated by ':'");
862-
}
863861
String option = optionAndValue[0];
864862
String value = optionAndValue[1];
865863
StackTraceElement[] stackElements = Thread.currentThread().getStackTrace();
@@ -870,43 +868,33 @@ boolean isRestrictedServiceAllowed(Service service, boolean isServiceAdded) {
870868
}
871869
String stackElemModule = stackElement.getModuleName();
872870
String stackElemFullClassName = stackElement.getClassName();
873-
int stackElemEnd = stackElemFullClassName.lastIndexOf(".");
871+
int stackElemEnd = stackElemFullClassName.lastIndexOf('.');
874872
String stackElemPackage = null;
875873
if (stackElemEnd != -1) {
876874
stackElemPackage = stackElemFullClassName.substring(0, stackElemEnd);
877875
}
878876
String module;
879877
switch (option) {
880-
case "ModuleAndFullClassName":
881-
String[] moduleAndFullClassName = value.split("/");
882-
if (moduleAndFullClassName.length != 2) {
883-
printStackTraceAndExit("Incorrect specification of accepted uses in constraint: '"
884-
+ constraint + "'. Couldn't find module and classname separated by '/'");
885-
}
886-
module = moduleAndFullClassName[0];
887-
String fullClassName = moduleAndFullClassName[1];
888-
found = (stackElemModule != null) && stackElemModule.equals(module)
889-
&& stackElemFullClassName.equals(fullClassName);
890-
break;
891-
case "ModuleAndPackage":
892-
String[] moduleAndPackage = value.split("/");
893-
if (moduleAndPackage.length != 2) {
894-
printStackTraceAndExit("Incorrect specification of accepted uses in constraint: '"
895-
+ constraint + "'. Couldn't find module and classname separated by '/'");
896-
}
897-
module = moduleAndPackage[0];
898-
String packageValue = moduleAndPackage[1];
899-
found = (stackElemModule != null) && stackElemModule.equals(module)
900-
&& (stackElemPackage != null) && stackElemPackage.equals(packageValue);
901-
break;
902-
case "FullClassName":
903-
found = stackElemFullClassName.equals(value);
904-
break;
905-
case "Package":
906-
found = (stackElemPackage != null) && stackElemPackage.equals(value);
907-
break;
908-
default:
909-
printStackTraceAndExit("Incorrect option to match in constraint: " + constraint);
878+
case "ModuleAndFullClassName":
879+
String[] moduleAndFullClassName = value.split("/");
880+
module = moduleAndFullClassName[0];
881+
String fullClassName = moduleAndFullClassName[1];
882+
found = module.equals(stackElemModule) && stackElemFullClassName.equals(fullClassName);
883+
break;
884+
case "ModuleAndPackage":
885+
String[] moduleAndPackage = value.split("/");
886+
module = moduleAndPackage[0];
887+
String packageValue = moduleAndPackage[1];
888+
found = module.equals(stackElemModule) && packageValue.equals(stackElemPackage);
889+
break;
890+
case "FullClassName":
891+
found = stackElemFullClassName.equals(value);
892+
break;
893+
case "Package":
894+
found = value.equals(stackElemPackage);
895+
break;
896+
default:
897+
printStackTraceAndExit("Incorrect option to match in constraint: " + constraint);
910898
}
911899

912900
if (found) {
@@ -1549,10 +1537,40 @@ private void setConstraints(String providerName, String providerInfo, boolean pr
15491537
String inAttributes = m.group(3);
15501538
String inAcceptedUses = m.group(4);
15511539

1552-
if (isNullOrBlank(inAcceptedUses)) {
1553-
inAcceptedUses = null;
1554-
} else {
1540+
if (inAcceptedUses != null) {
15551541
inAcceptedUses = inAcceptedUses.substring(1);
1542+
boolean isSpecIncorrect = false;
1543+
String[] optionAndValue = inAcceptedUses.split(":");
1544+
if (optionAndValue.length != 2) {
1545+
isSpecIncorrect = true;
1546+
}
1547+
String option = optionAndValue[0];
1548+
String value = optionAndValue[1];
1549+
switch (option) {
1550+
case "ModuleAndFullClassName":
1551+
String[] moduleAndFullClassName = value.split("/");
1552+
if (moduleAndFullClassName.length != 2) {
1553+
isSpecIncorrect = true;
1554+
}
1555+
break;
1556+
case "ModuleAndPackage":
1557+
String[] moduleAndPackage = value.split("/");
1558+
if (moduleAndPackage.length != 2) {
1559+
isSpecIncorrect = true;
1560+
}
1561+
break;
1562+
case "FullClassName":
1563+
case "Package":
1564+
// Nothing further to check in those options.
1565+
break;
1566+
default:
1567+
isSpecIncorrect = true;
1568+
break;
1569+
}
1570+
if (isSpecIncorrect) {
1571+
printStackTraceAndExit("Incorrect specification of accepted uses in constraint for "
1572+
+ inType + ", " + inAlgorithm + ": " + inAcceptedUses);
1573+
}
15561574
}
15571575

15581576
// Each attribute must includes 2 fields (key and value) or *.
@@ -1913,9 +1931,15 @@ private static final class Constraint {
19131931

19141932
@Override
19151933
public String toString() {
1916-
String constraintInfo = type + ", " + algorithm + ", " + attributes;
1917-
constraintInfo = (acceptedUses != null) ? constraintInfo + acceptedUses : constraintInfo;
1918-
return "{" + constraintInfo + "}";
1934+
StringBuilder buffer = new StringBuilder();
1935+
buffer.append("{").append(type);
1936+
buffer.append(", ").append(algorithm);
1937+
buffer.append(", ").append(attributes);
1938+
if (acceptedUses != null) {
1939+
buffer.append(", ").append(acceptedUses);
1940+
}
1941+
buffer.append("}");
1942+
return buffer.toString();
19191943
}
19201944

19211945
@Override

‎closed/test/jdk/openj9/internal/security/TestConstraintsFailure.java

+24-24
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@
3636
import java.security.KeyStore;
3737
import java.security.KeyStoreException;
3838
import java.security.MessageDigest;
39-
import java.security.cert.CertificateException;
4039
import java.security.NoSuchAlgorithmException;
4140
import java.security.Signature;
41+
import java.security.cert.CertificateException;
4242
import java.security.cert.CertPathValidator;
4343
import java.security.cert.CertStore;
4444
import java.security.cert.CertificateFactory;
@@ -61,110 +61,110 @@ public class TestConstraintsFailure {
6161
private static void getInstances() throws Exception {
6262
try {
6363
CertificateFactory.getInstance("X.509");
64-
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
65-
} catch(CertificateException ce) {
64+
throw new RuntimeException("A CertificateException should have been thrown");
65+
} catch (CertificateException ce) {
6666
// Do nothing. This is expected.
6767
}
6868
try {
6969
CertPathValidator.getInstance("PKIX");
7070
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
71-
} catch(NoSuchAlgorithmException nsae) {
71+
} catch (NoSuchAlgorithmException nsae) {
7272
// Do nothing. This is expected.
7373
}
7474
try {
7575
MessageDigest.getInstance("SHA-512");
7676
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
77-
} catch(NoSuchAlgorithmException nsae) {
77+
} catch (NoSuchAlgorithmException nsae) {
7878
// Do nothing. This is expected.
7979
}
8080
try {
8181
KeyStore.getInstance("PKCS12");
82-
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
83-
} catch(KeyStoreException ke) {
82+
throw new RuntimeException("A KeyStoreException should have been thrown");
83+
} catch (KeyStoreException ke) {
8484
// Do nothing. This is expected.
8585
}
86-
8786
try {
8887
Signature.getInstance("SHA256withECDSA");
8988
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
90-
} catch(NoSuchAlgorithmException nsae) {
89+
} catch (NoSuchAlgorithmException nsae) {
9190
// Do nothing. This is expected.
9291
}
9392
try {
9493
KeyPairGenerator.getInstance("EC");
9594
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
96-
} catch(NoSuchAlgorithmException nsae) {
95+
} catch (NoSuchAlgorithmException nsae) {
9796
// Do nothing. This is expected.
9897
}
9998
try {
10099
KeyAgreement.getInstance("ECDH");
101100
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
102-
} catch(NoSuchAlgorithmException nsae) {
101+
} catch (NoSuchAlgorithmException nsae) {
103102
// Do nothing. This is expected.
104103
}
105104
try {
106105
KeyFactory.getInstance("EC");
107106
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
108-
} catch(NoSuchAlgorithmException nsae) {
107+
} catch (NoSuchAlgorithmException nsae) {
109108
// Do nothing. This is expected.
110109
}
111-
112110
try {
113111
Cipher.getInstance("RSA");
114112
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
115-
} catch(NoSuchAlgorithmException nsae) {
113+
} catch (NoSuchAlgorithmException nsae) {
116114
// Do nothing. This is expected.
117115
}
118116
try {
119117
KeyGenerator.getInstance("AES");
120118
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
121-
} catch(NoSuchAlgorithmException nsae) {
119+
} catch (NoSuchAlgorithmException nsae) {
122120
// Do nothing. This is expected.
123121
}
124122
try {
125123
AlgorithmParameterGenerator.getInstance("DiffieHellman");
126124
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
127-
} catch(NoSuchAlgorithmException nsae) {
125+
} catch (NoSuchAlgorithmException nsae) {
128126
// Do nothing. This is expected.
129127
}
130128

131129
// Still a preview, but can be enabled in future versions.
132-
/*try {
130+
/*
131+
try {
133132
KDF.getInstance("HKDF-SHA256");
134133
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
135-
} catch(NoSuchAlgorithmException nsae) {
134+
} catch (NoSuchAlgorithmException nsae) {
136135
// Do nothing. This is expected.
137-
}*/
136+
}
137+
*/
138138

139139
try {
140140
SecretKeyFactory.getInstance("PBEWithMD5AndDES");
141141
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
142-
} catch(NoSuchAlgorithmException nsae) {
142+
} catch (NoSuchAlgorithmException nsae) {
143143
// Do nothing. This is expected.
144144
}
145145
try {
146146
Mac.getInstance("HmacSHA256");
147147
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
148-
} catch(NoSuchAlgorithmException nsae) {
148+
} catch (NoSuchAlgorithmException nsae) {
149149
// Do nothing. This is expected.
150150
}
151151

152152
try {
153153
KeyManagerFactory.getInstance("SunX509");
154154
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
155-
} catch(NoSuchAlgorithmException nsae) {
155+
} catch (NoSuchAlgorithmException nsae) {
156156
// Do nothing. This is expected.
157157
}
158158
try {
159159
TrustManagerFactory.getInstance("SunX509");
160160
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
161-
} catch(NoSuchAlgorithmException nsae) {
161+
} catch (NoSuchAlgorithmException nsae) {
162162
// Do nothing. This is expected.
163163
}
164164
try {
165165
SSLContext.getInstance("TLSv1.3");
166166
throw new RuntimeException("A NoSuchAlgorithmException should have been thrown");
167-
} catch(NoSuchAlgorithmException nsae) {
167+
} catch (NoSuchAlgorithmException nsae) {
168168
// Do nothing. This is expected.
169169
}
170170
}

‎closed/test/jdk/openj9/internal/security/TestConstraintsSuccess.java

+1-4
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@
3737
import java.security.MessageDigest;
3838
import java.security.NoSuchAlgorithmException;
3939
import java.security.Signature;
40+
import java.security.cert.CertificateFactory;
4041
import java.security.cert.CertPathValidator;
4142
import java.security.cert.CertStore;
42-
import java.security.cert.CertificateFactory;
4343

4444
import javax.crypto.Cipher;
4545
//import javax.crypto.KDF;
@@ -61,12 +61,10 @@ private static void getInstances() throws Exception {
6161
CertPathValidator.getInstance("PKIX");
6262
MessageDigest.getInstance("SHA-512");
6363
KeyStore.getInstance("PKCS12");
64-
6564
Signature.getInstance("SHA256withECDSA");
6665
KeyPairGenerator.getInstance("EC");
6766
KeyAgreement.getInstance("ECDH");
6867
KeyFactory.getInstance("EC");
69-
7068
Cipher.getInstance("RSA");
7169
KeyGenerator.getInstance("AES");
7270
AlgorithmParameterGenerator.getInstance("DiffieHellman");
@@ -76,7 +74,6 @@ private static void getInstances() throws Exception {
7674

7775
SecretKeyFactory.getInstance("PBEWithMD5AndDES");
7876
Mac.getInstance("HmacSHA256");
79-
8077
KeyManagerFactory.getInstance("SunX509");
8178
TrustManagerFactory.getInstance("SunX509");
8279
SSLContext.getInstance("TLSv1.3");

0 commit comments

Comments
 (0)