Skip to content

Commit 6f0efc4

Browse files
GoeLinGoeLin
committed
8292297: Fix up loading of override java.security properties file
Reviewed-by: mbaesken Backport-of: 4be52ee572e4fd65f2ac66d5e78c711c8eb6a61e
1 parent edbe574 commit 6f0efc4

File tree

3 files changed

+96
-97
lines changed

3 files changed

+96
-97
lines changed

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

Lines changed: 51 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -84,113 +84,83 @@ public Void run() {
8484

8585
private static void initialize() {
8686
props = new Properties();
87-
boolean loadedProps = false;
8887
boolean overrideAll = false;
8988

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

12497
if ("true".equalsIgnoreCase(props.getProperty
12598
("security.overridePropertiesFile"))) {
12699

127100
String extraPropFile = System.getProperty
128-
("java.security.properties");
101+
("java.security.properties");
129102
if (extraPropFile != null && extraPropFile.startsWith("=")) {
130103
overrideAll = true;
131104
extraPropFile = extraPropFile.substring(1);
132105
}
106+
loadProps(null, extraPropFile, overrideAll);
107+
}
108+
}
133109

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

126+
is = propURL.openStream();
127+
if (overrideAll) {
128+
props = new Properties();
161129
if (sdebug != null) {
162-
sdebug.println("reading security properties file: " +
163-
propURL);
164-
if (overrideAll) {
165-
sdebug.println
130+
sdebug.println
166131
("overriding other security properties files!");
167-
}
168132
}
169-
} catch (Exception e) {
133+
}
134+
} else {
135+
// unexpected
136+
return false;
137+
}
138+
props.load(is);
139+
if (sdebug != null) {
140+
// ExceptionInInitializerError if masterFile.getName() is
141+
// called here (NPE!). Leave as is (and few lines down)
142+
sdebug.println("reading security properties file: " +
143+
masterFile == null ? extraPropFile : "java.security");
144+
}
145+
return true;
146+
} catch (IOException | PropertyExpander.ExpandException e) {
147+
if (sdebug != null) {
148+
sdebug.println("unable to load security properties from " +
149+
masterFile == null ? extraPropFile : "java.security");
150+
e.printStackTrace();
151+
}
152+
return false;
153+
} finally {
154+
if (is != null) {
155+
try {
156+
is.close();
157+
} catch (IOException ioe) {
170158
if (sdebug != null) {
171-
sdebug.println
172-
("unable to load security properties from " +
173-
extraPropFile);
174-
e.printStackTrace();
175-
}
176-
} finally {
177-
if (bis != null) {
178-
try {
179-
bis.close();
180-
} catch (IOException ioe) {
181-
if (sdebug != null) {
182-
sdebug.println("unable to close input stream");
183-
}
184-
}
159+
sdebug.println("unable to close input stream");
185160
}
186161
}
187162
}
188163
}
189-
190-
if (!loadedProps) {
191-
throw new InternalError("java.security file missing");
192-
}
193-
194164
}
195165

196166
/**

‎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: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +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.3=sun.security.ssl.SunJSSE
3+
security.provider.2=SunRsaSign
4+
security.provider.3=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
6+
security.provider.5=SunJGSS
7+
security.provider.6=SunSASL

0 commit comments

Comments
 (0)