Skip to content

Commit 0685301

Browse files
gnu-andrewgnu-andrew
committed
RH2090378: Revert to disabling system security properties and FIPS mode support together (openjdk#4)
- Improve debug output of all properties for FIPS mode and system security property support. - Run JDK tests in GHA with system security properties both disabled and enabled in java.security - General code cleanup Reviewed-by: @martinuy Reviewed-by: @franferrax
1 parent c7a932d commit 0685301

File tree

5 files changed

+268
-33
lines changed

5 files changed

+268
-33
lines changed

‎.github/workflows/main.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,16 @@ jobs:
309309
bootjdk-platform: linux-x64
310310
runs-on: ubuntu-22.04
311311

312+
test-linux-x64-fips:
313+
name: linux-x64-fips
314+
needs:
315+
- build-linux-x64
316+
uses: ./.github/workflows/test-fips.yml
317+
with:
318+
platform: linux-x64
319+
bootjdk-platform: linux-x64
320+
runs-on: ubuntu-22.04
321+
312322
test-linux-x86:
313323
name: linux-x86
314324
needs:

‎.github/workflows/test-fips.yml‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
#
2+
# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
3+
# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
#
5+
# This code is free software; you can redistribute it and/or modify it
6+
# under the terms of the GNU General Public License version 2 only, as
7+
# published by the Free Software Foundation. Oracle designates this
8+
# particular file as subject to the "Classpath" exception as provided
9+
# by Oracle in the LICENSE file that accompanied this code.
10+
#
11+
# This code is distributed in the hope that it will be useful, but WITHOUT
12+
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
# version 2 for more details (a copy is included in the LICENSE file that
15+
# accompanied this code).
16+
#
17+
# You should have received a copy of the GNU General Public License version
18+
# 2 along with this work; if not, write to the Free Software Foundation,
19+
# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
#
21+
# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
# or visit www.oracle.com if you need additional information or have any
23+
# questions.
24+
#
25+
26+
name: 'Run FIPS tests'
27+
28+
on:
29+
workflow_call:
30+
inputs:
31+
platform:
32+
required: true
33+
type: string
34+
bootjdk-platform:
35+
required: true
36+
type: string
37+
runs-on:
38+
required: true
39+
type: string
40+
41+
env:
42+
# These are needed to make the MSYS2 bash work properly
43+
MSYS2_PATH_TYPE: minimal
44+
CHERE_INVOKING: 1
45+
46+
jobs:
47+
test:
48+
name: test
49+
runs-on: ${{ inputs.runs-on }}
50+
defaults:
51+
run:
52+
shell: bash
53+
54+
strategy:
55+
fail-fast: false
56+
matrix:
57+
test-name:
58+
- 'jdk/tier1 part 1'
59+
- 'jdk/tier1 part 2'
60+
- 'jdk/tier1 part 3'
61+
62+
include:
63+
- test-name: 'jdk/tier1 part 1'
64+
test-suite: 'test/jdk/:tier1_part1'
65+
66+
- test-name: 'jdk/tier1 part 2'
67+
test-suite: 'test/jdk/:tier1_part2'
68+
69+
- test-name: 'jdk/tier1 part 3'
70+
test-suite: 'test/jdk/:tier1_part3'
71+
72+
steps:
73+
- name: 'Checkout the JDK source'
74+
uses: actions/checkout@v3
75+
76+
- name: 'Get MSYS2'
77+
uses: ./.github/actions/get-msys2
78+
if: runner.os == 'Windows'
79+
80+
- name: 'Get the BootJDK'
81+
id: bootjdk
82+
uses: ./.github/actions/get-bootjdk
83+
with:
84+
platform: ${{ inputs.bootjdk-platform }}
85+
86+
- name: 'Get JTReg'
87+
id: jtreg
88+
uses: ./.github/actions/get-jtreg
89+
90+
- name: 'Get bundles'
91+
id: bundles
92+
uses: ./.github/actions/get-bundles
93+
with:
94+
platform: ${{ inputs.platform }}
95+
debug-suffix: ${{ matrix.debug-suffix }}
96+
97+
- name: 'Install dependencies'
98+
run: |
99+
# On macOS we need to install some dependencies for testing
100+
brew install make
101+
sudo xcode-select --switch /Applications/Xcode_11.7.app/Contents/Developer
102+
# This will make GNU make available as 'make' and not only as 'gmake'
103+
echo '/usr/local/opt/make/libexec/gnubin' >> $GITHUB_PATH
104+
if: runner.os == 'macOS'
105+
106+
- name: 'Set PATH'
107+
id: path
108+
run: |
109+
# We need a minimal PATH on Windows
110+
# Set PATH to "", so just GITHUB_PATH is included
111+
if [[ '${{ runner.os }}' == 'Windows' ]]; then
112+
echo "value=" >> $GITHUB_OUTPUT
113+
else
114+
echo "value=$PATH" >> $GITHUB_OUTPUT
115+
fi
116+
117+
- name: Turn on system security properties and FIPS mode support
118+
run: |
119+
sed -i -e "s:^security.useSystemPropertiesFile=.*:security.useSystemPropertiesFile=true:" ${{ steps.bundles.outputs.jdk-path }}/conf/security/java.security
120+
121+
- name: 'Run tests'
122+
id: run-tests
123+
run: >
124+
make test-prebuilt
125+
TEST='${{ matrix.test-suite }}'
126+
BOOT_JDK=${{ steps.bootjdk.outputs.path }}
127+
JT_HOME=${{ steps.jtreg.outputs.path }}
128+
JDK_IMAGE_DIR=${{ steps.bundles.outputs.jdk-path }}
129+
SYMBOLS_IMAGE_DIR=${{ steps.bundles.outputs.symbols-path }}
130+
TEST_IMAGE_DIR=${{ steps.bundles.outputs.tests-path }}
131+
JTREG='JAVA_OPTIONS=-XX:-CreateCoredumpOnCrash;VERBOSE=fail,error,time;KEYWORDS=!headful'
132+
&& bash ./.github/scripts/gen-test-summary.sh "$GITHUB_STEP_SUMMARY" "$GITHUB_OUTPUT"
133+
env:
134+
PATH: ${{ steps.path.outputs.value }}
135+
136+
# This is a separate step, since if the markdown from a step gets bigger than
137+
# 1024 kB it is skipped, but then the short summary above is still generated
138+
- name: 'Generate test report'
139+
run: bash ./.github/scripts/gen-test-results.sh "$GITHUB_STEP_SUMMARY"
140+
if: always()
141+
142+
- name: 'Package test results'
143+
id: package
144+
run: |
145+
# Package test-results and relevant parts of test-support
146+
mkdir results
147+
148+
if [[ -d build/run-test-prebuilt/test-results ]]; then
149+
cd build/run-test-prebuilt/test-results/
150+
zip -r -9 "$GITHUB_WORKSPACE/results/fips-test-results.zip" .
151+
cd $GITHUB_WORKSPACE
152+
else
153+
echo '::warning ::Missing test-results directory'
154+
fi
155+
156+
if [[ -d build/run-test-prebuilt/test-support ]]; then
157+
cd build/run-test-prebuilt/test-support/
158+
zip -r -9 "$GITHUB_WORKSPACE/results/fips-test-support.zip" . -i *.jtr -i */hs_err*.log -i */replay*.log
159+
cd $GITHUB_WORKSPACE
160+
else
161+
echo '::warning ::Missing test-support directory'
162+
fi
163+
164+
artifact_name="results-${{ inputs.platform }}-$(echo ${{ matrix.test-name }} | tr '/ ' '__')"
165+
echo "artifact-name=$artifact_name" >> $GITHUB_OUTPUT
166+
if: always()
167+
168+
- name: 'Upload test results'
169+
uses: actions/upload-artifact@v3
170+
with:
171+
path: results
172+
name: ${{ steps.package.outputs.artifact-name }}
173+
if: always()
174+
175+
# This is the best way I found to abort the job with an error message
176+
- name: 'Notify about test failures'
177+
uses: actions/github-script@v6
178+
with:
179+
script: core.setFailed('${{ steps.run-tests.outputs.error-message }}')
180+
if: steps.run-tests.outputs.failure == 'true'

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

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@
5959

6060
public final class Security {
6161

62+
private static final String SYS_PROP_SWITCH =
63+
"java.security.disableSystemPropertiesFile";
64+
private static final String SEC_PROP_SWITCH =
65+
"security.useSystemPropertiesFile";
66+
6267
/* Are we debugging? -- for developers */
6368
private static final Debug sdebug =
6469
Debug.getInstance("properties");
@@ -110,6 +115,7 @@ public Properties getInitialProperties() {
110115
private static void initialize() {
111116
props = new Properties();
112117
boolean overrideAll = false;
118+
boolean systemSecPropsEnabled = false;
113119

114120
// first load the system properties file
115121
// to determine the value of security.overridePropertiesFile
@@ -131,22 +137,57 @@ private static void initialize() {
131137
loadProps(null, extraPropFile, overrideAll);
132138
}
133139

134-
String disableSystemProps = System.getProperty("java.security.disableSystemPropertiesFile");
135-
if ((disableSystemProps == null || "false".equalsIgnoreCase(disableSystemProps)) &&
136-
"true".equalsIgnoreCase(props.getProperty("security.useSystemPropertiesFile"))) {
137-
if (!SystemConfigurator.configureSysProps(props)) {
140+
boolean sysUseProps = Boolean.valueOf(System.getProperty(SYS_PROP_SWITCH, "false"));
141+
boolean secUseProps = Boolean.valueOf(props.getProperty(SEC_PROP_SWITCH));
142+
if (sdebug != null) {
143+
sdebug.println(SYS_PROP_SWITCH + "=" + sysUseProps);
144+
sdebug.println(SEC_PROP_SWITCH + "=" + secUseProps);
145+
}
146+
if (!sysUseProps && secUseProps) {
147+
systemSecPropsEnabled = SystemConfigurator.configureSysProps(props);
148+
if (!systemSecPropsEnabled) {
138149
if (sdebug != null) {
139-
sdebug.println("WARNING: System properties could not be loaded.");
150+
sdebug.println("WARNING: System security properties could not be loaded.");
140151
}
141152
}
153+
} else {
154+
if (sdebug != null) {
155+
sdebug.println("System security property support disabled by user.");
156+
}
142157
}
143158

144-
boolean fipsEnabled = SystemConfigurator.configureFIPS(props);
145-
if (sdebug != null) {
146-
if (fipsEnabled) {
147-
sdebug.println("FIPS support enabled.");
159+
if (systemSecPropsEnabled) {
160+
boolean shouldEnable;
161+
String sysProp = System.getProperty("com.redhat.fips");
162+
if (sysProp == null) {
163+
shouldEnable = true;
164+
if (sdebug != null) {
165+
sdebug.println("com.redhat.fips unset, using default value of true");
166+
}
167+
} else {
168+
shouldEnable = Boolean.valueOf(sysProp);
169+
if (sdebug != null) {
170+
sdebug.println("com.redhat.fips set, using its value " + shouldEnable);
171+
}
172+
}
173+
if (shouldEnable) {
174+
boolean fipsEnabled = SystemConfigurator.configureFIPS(props);
175+
if (sdebug != null) {
176+
if (fipsEnabled) {
177+
sdebug.println("FIPS mode support configured and enabled.");
178+
} else {
179+
sdebug.println("FIPS mode support disabled.");
180+
}
181+
}
148182
} else {
149-
sdebug.println("FIPS support disabled.");
183+
if (sdebug != null ) {
184+
sdebug.println("FIPS mode support disabled by user.");
185+
}
186+
}
187+
} else {
188+
if (sdebug != null) {
189+
sdebug.println("WARNING: FIPS mode support can not be enabled without " +
190+
"system security properties being enabled.");
150191
}
151192
}
152193

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

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ static boolean configureFIPS(Properties props) {
152152
sdebug.println("FIPS support enabled without plain key support");
153153
}
154154
}
155+
} else {
156+
if (sdebug != null) { sdebug.println("FIPS mode not detected"); }
155157
}
156158
} catch (Exception e) {
157159
if (sdebug != null) {
@@ -192,37 +194,39 @@ static boolean isPlainKeySupportEnabled() {
192194
return plainKeySupportEnabled;
193195
}
194196

195-
/*
196-
* OpenJDK FIPS mode will be enabled only if the com.redhat.fips
197-
* system property is true (default) and the system is in FIPS mode.
197+
/**
198+
* Determines whether FIPS mode should be enabled.
199+
*
200+
* OpenJDK FIPS mode will be enabled only if the system is in
201+
* FIPS mode.
202+
*
203+
* Calls to this method only occur if the system property
204+
* com.redhat.fips is not set to false.
198205
*
199206
* There are 2 possible ways in which OpenJDK detects that the system
200207
* is in FIPS mode: 1) if the NSS SECMOD_GetSystemFIPSEnabled API is
201208
* available at OpenJDK's built-time, it is called; 2) otherwise, the
202209
* /proc/sys/crypto/fips_enabled file is read.
210+
*
211+
* @return true if the system is in FIPS mode
203212
*/
204213
private static boolean enableFips() throws Exception {
205-
boolean shouldEnable = Boolean.valueOf(System.getProperty("com.redhat.fips", "true"));
206-
if (shouldEnable) {
214+
if (sdebug != null) {
215+
sdebug.println("Calling getSystemFIPSEnabled (libsystemconf)...");
216+
}
217+
try {
218+
boolean fipsEnabled = getSystemFIPSEnabled();
207219
if (sdebug != null) {
208-
sdebug.println("Calling getSystemFIPSEnabled (libsystemconf)...");
220+
sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) returned: "
221+
+ fipsEnabled);
209222
}
210-
try {
211-
shouldEnable = getSystemFIPSEnabled();
212-
if (sdebug != null) {
213-
sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) returned: "
214-
+ shouldEnable);
215-
}
216-
return shouldEnable;
217-
} catch (IOException e) {
218-
if (sdebug != null) {
219-
sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) failed:");
220-
sdebug.println(e.getMessage());
221-
}
222-
throw e;
223+
return fipsEnabled;
224+
} catch (IOException e) {
225+
if (sdebug != null) {
226+
sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) failed:");
227+
sdebug.println(e.getMessage());
223228
}
224-
} else {
225-
return false;
229+
throw e;
226230
}
227231
}
228232
}

‎src/java.base/share/conf/security/java.security‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ security.provider.tbd=Apple
8686
security.provider.tbd=SunPKCS11
8787

8888
#
89-
# Security providers used when global crypto-policies are set to FIPS.
89+
# Security providers used when FIPS mode support is active
9090
#
9191
fips.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg
9292
fips.provider.2=SUN
@@ -352,7 +352,7 @@ security.overridePropertiesFile=true
352352
# using the system properties file stored at
353353
# /etc/crypto-policies/back-ends/java.config
354354
#
355-
security.useSystemPropertiesFile=true
355+
security.useSystemPropertiesFile=false
356356

357357
#
358358
# Determines the default key and trust manager factory algorithms for

0 commit comments

Comments
 (0)