Skip to content

Commit 1f9ff41

Browse files
coffeyscoffeys
committed
8292297: Fix up loading of override java.security properties file
Reviewed-by: xuelei
1 parent 64b96e5 commit 1f9ff41

File tree

3 files changed

+95
-96
lines changed

3 files changed

+95
-96
lines changed

‎src/java.base/share/classes/java/security/Security.java

Lines changed: 51 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -83,112 +83,83 @@ private static class ProviderProperty {
8383

8484
private static void initialize() {
8585
props = new Properties();
86-
boolean loadedProps = false;
8786
boolean overrideAll = false;
8887

8988
// first load the system properties file
9089
// to determine the value of security.overridePropertiesFile
9190
File propFile = securityPropFile("java.security");
92-
if (propFile.exists()) {
93-
InputStream is = null;
94-
try {
95-
is = new FileInputStream(propFile);
96-
props.load(is);
97-
loadedProps = true;
98-
99-
if (sdebug != null) {
100-
sdebug.println("reading security properties file: " +
101-
propFile);
102-
}
103-
} catch (IOException e) {
104-
if (sdebug != null) {
105-
sdebug.println("unable to load security properties from " +
106-
propFile);
107-
e.printStackTrace();
108-
}
109-
} finally {
110-
if (is != null) {
111-
try {
112-
is.close();
113-
} catch (IOException ioe) {
114-
if (sdebug != null) {
115-
sdebug.println("unable to close input stream");
116-
}
117-
}
118-
}
119-
}
91+
boolean success = loadProps(propFile, null, false);
92+
if (!success) {
93+
throw new InternalError("Error loading java.security file");
12094
}
12195

12296
if ("true".equalsIgnoreCase(props.getProperty
12397
("security.overridePropertiesFile"))) {
12498

12599
String extraPropFile = System.getProperty
126-
("java.security.properties");
100+
("java.security.properties");
127101
if (extraPropFile != null && extraPropFile.startsWith("=")) {
128102
overrideAll = true;
129103
extraPropFile = extraPropFile.substring(1);
130104
}
105+
loadProps(null, extraPropFile, overrideAll);
106+
}
107+
}
131108

132-
if (overrideAll) {
133-
props = new Properties();
134-
if (sdebug != null) {
135-
sdebug.println
136-
("overriding other security properties files!");
109+
private static boolean loadProps(File masterFile, String extraPropFile, boolean overrideAll) {
110+
InputStream is = null;
111+
try {
112+
if (masterFile != null && masterFile.exists()) {
113+
is = new FileInputStream(masterFile);
114+
} else if (extraPropFile != null) {
115+
extraPropFile = PropertyExpander.expand(extraPropFile);
116+
File propFile = new File(extraPropFile);
117+
URL propURL;
118+
if (propFile.exists()) {
119+
propURL = new URL
120+
("file:" + propFile.getCanonicalPath());
121+
} else {
122+
propURL = new URL(extraPropFile);
137123
}
138-
}
139-
140-
// now load the user-specified file so its values
141-
// will win if they conflict with the earlier values
142-
if (extraPropFile != null) {
143-
InputStream is = null;
144-
try {
145-
URL propURL;
146-
147-
extraPropFile = PropertyExpander.expand(extraPropFile);
148-
propFile = new File(extraPropFile);
149-
if (propFile.exists()) {
150-
propURL = new URL
151-
("file:" + propFile.getCanonicalPath());
152-
} else {
153-
propURL = new URL(extraPropFile);
154-
}
155-
is = propURL.openStream();
156-
props.load(is);
157-
loadedProps = true;
158124

125+
is = propURL.openStream();
126+
if (overrideAll) {
127+
props = new Properties();
159128
if (sdebug != null) {
160-
sdebug.println("reading security properties file: " +
161-
propURL);
162-
if (overrideAll) {
163-
sdebug.println
129+
sdebug.println
164130
("overriding other security properties files!");
165-
}
166131
}
167-
} catch (Exception e) {
132+
}
133+
} else {
134+
// unexpected
135+
return false;
136+
}
137+
props.load(is);
138+
if (sdebug != null) {
139+
// ExceptionInInitializerError if masterFile.getName() is
140+
// called here (NPE!). Leave as is (and few lines down)
141+
sdebug.println("reading security properties file: " +
142+
masterFile == null ? extraPropFile : "java.security");
143+
}
144+
return true;
145+
} catch (IOException | PropertyExpander.ExpandException e) {
146+
if (sdebug != null) {
147+
sdebug.println("unable to load security properties from " +
148+
masterFile == null ? extraPropFile : "java.security");
149+
e.printStackTrace();
150+
}
151+
return false;
152+
} finally {
153+
if (is != null) {
154+
try {
155+
is.close();
156+
} catch (IOException ioe) {
168157
if (sdebug != null) {
169-
sdebug.println
170-
("unable to load security properties from " +
171-
extraPropFile);
172-
e.printStackTrace();
173-
}
174-
} finally {
175-
if (is != null) {
176-
try {
177-
is.close();
178-
} catch (IOException ioe) {
179-
if (sdebug != null) {
180-
sdebug.println("unable to close input stream");
181-
}
182-
}
158+
sdebug.println("unable to close input stream");
183159
}
184160
}
185161
}
186162
}
187-
188-
if (!loadedProps) {
189-
throw new InternalError("java.security file missing");
190-
}
191-
192163
}
193164

194165
/**

‎test/jdk/java/security/Security/ConfigFileTest.java

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@
2828
import java.io.UncheckedIOException;
2929
import java.nio.file.*;
3030

31+
import java.security.Provider;
3132
import java.security.Security;
3233
import java.util.Arrays;
3334
import java.util.Optional;
3435

3536
/*
3637
* @test
3738
* @summary Throw error if default java.security file is missing
38-
* @bug 8155246
39+
* @bug 8155246 8292297
3940
* @library /test/lib
4041
* @run main ConfigFileTest
4142
*/
@@ -50,39 +51,66 @@ public static void main(String[] args) throws Exception {
5051

5152
if (args.length == 1) {
5253
// set up is complete. Run code to exercise loading of java.security
53-
System.out.println(Arrays.toString(Security.getProviders()));
54+
Provider[] provs = Security.getProviders();
55+
System.out.println(Arrays.toString(provs) + "NumProviders: " + provs.length);
5456
} else {
5557
Files.createDirectory(copyJdkDir);
5658
Path jdkTestDir = Path.of(Optional.of(System.getProperty("test.jdk"))
5759
.orElseThrow(() -> new RuntimeException("Couldn't load JDK Test Dir"))
5860
);
5961

60-
copyJDKMinusJavaSecurity(jdkTestDir, copyJdkDir);
62+
copyJDK(jdkTestDir, copyJdkDir);
6163
String extraPropsFile = Path.of(System.getProperty("test.src"), "override.props").toString();
6264

6365
// exercise some debug flags while we're here
64-
// launch JDK without java.security file being present or specified
65-
exerciseSecurity(copiedJava.toString(), "-cp", System.getProperty("test.classes"),
66+
// regular JDK install - should expect success
67+
exerciseSecurity(0, "java",
68+
copiedJava.toString(), "-cp", System.getProperty("test.classes"),
6669
"-Djava.security.debug=all", "-Djavax.net.debug=all", "ConfigFileTest", "runner");
6770

71+
// given an overriding security conf file that doesn't exist, we shouldn't
72+
// overwrite the properties from original/master security conf file
73+
exerciseSecurity(0, "SUN version",
74+
copiedJava.toString(), "-cp", System.getProperty("test.classes"),
75+
"-Djava.security.debug=all", "-Djavax.net.debug=all",
76+
"-Djava.security.properties==file:///" + extraPropsFile + "badFileName",
77+
"ConfigFileTest", "runner");
78+
79+
// test JDK launch with customized properties file
80+
exerciseSecurity(0, "NumProviders: 6",
81+
copiedJava.toString(), "-cp", System.getProperty("test.classes"),
82+
"-Djava.security.debug=all", "-Djavax.net.debug=all",
83+
"-Djava.security.properties==file:///" + extraPropsFile,
84+
"ConfigFileTest", "runner");
85+
86+
// delete the master conf file
87+
Files.delete(Path.of(copyJdkDir.toString(), "conf",
88+
"security","java.security"));
89+
90+
// launch JDK without java.security file being present or specified
91+
exerciseSecurity(1, "Error loading java.security file",
92+
copiedJava.toString(), "-cp", System.getProperty("test.classes"),
93+
"-Djava.security.debug=all", "-Djavax.net.debug=all",
94+
"ConfigFileTest", "runner");
95+
6896
// test the override functionality also. Should not be allowed since
6997
// "security.overridePropertiesFile=true" Security property is missing.
70-
exerciseSecurity(copiedJava.toString(), "-cp", System.getProperty("test.classes"),
98+
exerciseSecurity(1, "Error loading java.security file",
99+
copiedJava.toString(), "-cp", System.getProperty("test.classes"),
71100
"-Djava.security.debug=all", "-Djavax.net.debug=all",
72-
"-Djava.security.properties==file://" + extraPropsFile, "ConfigFileTest", "runner");
101+
"-Djava.security.properties==file:///" + extraPropsFile, "ConfigFileTest", "runner");
73102
}
74103
}
75104

76-
private static void exerciseSecurity(String... args) throws Exception {
105+
private static void exerciseSecurity(int exitCode, String output, String... args) throws Exception {
77106
ProcessBuilder process = new ProcessBuilder(args);
78107
OutputAnalyzer oa = ProcessTools.executeProcess(process);
79-
oa.shouldHaveExitValue(1).shouldContain("java.security file missing");
108+
oa.shouldHaveExitValue(exitCode).shouldContain(output);
80109
}
81110

82-
private static void copyJDKMinusJavaSecurity(Path src, Path dst) throws Exception {
111+
private static void copyJDK(Path src, Path dst) throws Exception {
83112
Files.walk(src)
84113
.skip(1)
85-
.filter(p -> !p.toString().endsWith("java.security"))
86114
.forEach(file -> {
87115
try {
88116
Files.copy(file, dst.resolve(src.relativize(file)), StandardCopyOption.COPY_ATTRIBUTES);
@@ -91,4 +119,4 @@ private static void copyJDKMinusJavaSecurity(Path src, Path dst) throws Exceptio
91119
}
92120
});
93121
}
94-
}
122+
}

‎test/jdk/java/security/Security/override.props

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
# exercise ServiceLoader and legacy (class load) approach
12
security.provider.1=sun.security.provider.Sun
2-
security.provider.2=sun.security.rsa.SunRsaSign
3+
security.provider.2=SunRsaSign
34
security.provider.3=sun.security.ssl.SunJSSE
45
security.provider.4=com.sun.crypto.provider.SunJCE
5-
security.provider.5=sun.security.jgss.SunProvider
6-
security.provider.6=com.sun.security.sasl.Provider
7-
6+
security.provider.5=SunJGSS
7+
security.provider.6=SunSASL

0 commit comments

Comments
 (0)