RH2090378: Revert to disabling system security properties and FIPS mode support together (#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

Author: gnu-andrew

.github/workflows/submit.yml

@@ -384,6 +384,167 @@ jobs:
           path: ~/linux-x64${{ matrix.artifact }}_testsupport_${{ env.logsuffix }}.zip
         continue-on-error: true
 
+  linux_x64_test_fips:
+    name: Linux x64 + FIPS
+    runs-on: "ubuntu-20.04"
+    needs:
+      - prerequisites
+      - linux_x64_build
+
+    strategy:
+      fail-fast: false
+      matrix:
+        test:
+          - jdk/tier1 part 1
+          - jdk/tier1 part 2
+          - jdk/tier1 part 3
+        include:
+          - test: jdk/tier1 part 1
+            suites: test/jdk/:tier1_part1
+          - test: jdk/tier1 part 2
+            suites: test/jdk/:tier1_part2
+          - test: jdk/tier1 part 3
+            suites: test/jdk/:tier1_part3
+
+    env:
+      JDK_VERSION: "${{ fromJson(needs.prerequisites.outputs.dependencies).DEFAULT_VERSION_FEATURE }}.${{ fromJson(needs.prerequisites.outputs.dependencies).DEFAULT_VERSION_INTERIM }}.${{ fromJson(needs.prerequisites.outputs.dependencies).DEFAULT_VERSION_UPDATE }}"
+      BOOT_JDK_VERSION: "${{ fromJson(needs.prerequisites.outputs.dependencies).BOOT_JDK_VERSION }}"
+      BOOT_JDK_FILENAME: "${{ fromJson(needs.prerequisites.outputs.dependencies).LINUX_X64_BOOT_JDK_FILENAME }}"
+      BOOT_JDK_URL: "${{ fromJson(needs.prerequisites.outputs.dependencies).LINUX_X64_BOOT_JDK_URL }}"
+      BOOT_JDK_SHA256: "${{ fromJson(needs.prerequisites.outputs.dependencies).LINUX_X64_BOOT_JDK_SHA256 }}"
+
+    steps:
+      - name: Checkout the source
+        uses: actions/checkout@v2
+
+      - name: Restore boot JDK from cache
+        id: bootjdk
+        uses: actions/cache@v2
+        with:
+          path: ~/bootjdk/${{ env.BOOT_JDK_VERSION }}
+          key: bootjdk-${{ runner.os }}-${{ env.BOOT_JDK_VERSION }}-${{ env.BOOT_JDK_SHA256 }}-v1
+
+      - name: Download boot JDK
+        run: |
+          mkdir -p "${HOME}/bootjdk/${BOOT_JDK_VERSION}"
+          wget -O "${HOME}/bootjdk/${BOOT_JDK_FILENAME}" "${BOOT_JDK_URL}"
+          echo "${BOOT_JDK_SHA256} ${HOME}/bootjdk/${BOOT_JDK_FILENAME}" | sha256sum -c >/dev/null -
+          tar -xf "${HOME}/bootjdk/${BOOT_JDK_FILENAME}" -C "${HOME}/bootjdk/${BOOT_JDK_VERSION}"
+          mv "${HOME}/bootjdk/${BOOT_JDK_VERSION}/"*/* "${HOME}/bootjdk/${BOOT_JDK_VERSION}/"
+        if: steps.bootjdk.outputs.cache-hit != 'true'
+
+      - name: Restore jtreg artifact
+        id: jtreg_restore
+        uses: actions/download-artifact@v2
+        with:
+          name: transient_jtreg_${{ needs.prerequisites.outputs.bundle_id }}
+          path: ~/jtreg/
+        continue-on-error: true
+
+      - name: Restore jtreg artifact (retry)
+        uses: actions/download-artifact@v2
+        with:
+          name: transient_jtreg_${{ needs.prerequisites.outputs.bundle_id }}
+          path: ~/jtreg/
+        if: steps.jtreg_restore.outcome == 'failure'
+
+      - name: Restore build artifacts
+        id: build_restore
+        uses: actions/download-artifact@v2
+        with:
+          name: transient_jdk-linux-x64${{ matrix.artifact }}_${{ needs.prerequisites.outputs.bundle_id }}
+          path: ~/jdk-linux-x64${{ matrix.artifact }}
+        continue-on-error: true
+
+      - name: Restore build artifacts (retry)
+        uses: actions/download-artifact@v2
+        with:
+          name: transient_jdk-linux-x64${{ matrix.artifact }}_${{ needs.prerequisites.outputs.bundle_id }}
+          path: ~/jdk-linux-x64${{ matrix.artifact }}
+        if: steps.build_restore.outcome == 'failure'
+
+      - name: Unpack jdk
+        run: |
+          mkdir -p "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin${{ matrix.artifact }}"
+          tar -xf "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin${{ matrix.artifact }}.tar.gz" -C "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin${{ matrix.artifact }}"
+
+      - name: Unpack tests
+        run: |
+          mkdir -p "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin-tests${{ matrix.artifact }}"
+          tar -xf "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin-tests${{ matrix.artifact }}.tar.gz" -C "${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin-tests${{ matrix.artifact }}"
+
+      - name: Find root of jdk image dir
+        run: |
+          imageroot=`find ${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin${{ matrix.artifact }} -name release -type f`
+          echo "imageroot=`dirname ${imageroot}`" >> $GITHUB_ENV
+
+      - name: Turn on system security properties and FIPS mode support
+        run: |
+          sed -i -e "s:^security.useSystemPropertiesFile=.*:security.useSystemPropertiesFile=true:" ${{ env.imageroot }}/conf/security/java.security
+
+      - name: Run tests
+        run: >
+          JDK_IMAGE_DIR=${{ env.imageroot }}
+          TEST_IMAGE_DIR=${HOME}/jdk-linux-x64${{ matrix.artifact }}/jdk-${{ env.JDK_VERSION }}-internal+0_linux-x64_bin-tests${{ matrix.artifact }}
+          BOOT_JDK=${HOME}/bootjdk/${BOOT_JDK_VERSION}
+          JT_HOME=${HOME}/jtreg
+          make run-test-prebuilt
+          CONF_NAME=run-test-prebuilt
+          LOG_CMDLINES=true
+          JTREG_VERBOSE=fail,error,time
+          TEST="${{ matrix.suites }}"
+          TEST_OPTS_JAVA_OPTIONS=
+          JTREG_KEYWORDS="!headful"
+          JTREG="JAVA_OPTIONS='-Djava.security.debug=properties -XX:-CreateCoredumpOnCrash'"
+
+      - name: Check that all tests executed successfully
+        if: always()
+        run: >
+          if ! grep --include=test-summary.txt -lqr build/*/test-results -e "TEST SUCCESS" ; then
+            cat build/*/test-results/*/text/newfailures.txt ;
+            cat build/*/test-results/*/text/other_errors.txt ;
+            exit 1 ;
+          fi
+
+      - name: Create suitable test log artifact name
+        if: always()
+        run: echo "logsuffix=`echo ${{ matrix.test }} | sed -e 's!/!_!'g -e 's! !_!'g`" >> $GITHUB_ENV
+
+      - name: Package test results
+        if: always()
+        working-directory: build/run-test-prebuilt/test-results/
+        run: >
+          zip -r9
+          "$HOME/linux-x64${{ matrix.artifact }}-fips_testresults_${{ env.logsuffix }}.zip"
+          .
+        continue-on-error: true
+
+      - name: Package test support
+        if: always()
+        working-directory: build/run-test-prebuilt/test-support/
+        run: >
+          zip -r9
+          "$HOME/linux-x64${{ matrix.artifact }}-fips_testsupport_${{ env.logsuffix }}.zip"
+          .
+          -i *.jtr
+          -i */hs_err*.log
+          -i */replay*.log
+        continue-on-error: true
+
+      - name: Persist test results
+        if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          path: ~/linux-x64${{ matrix.artifact }}-fips_testresults_${{ env.logsuffix }}.zip
+        continue-on-error: true
+
+      - name: Persist test outputs
+        if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          path: ~/linux-x64${{ matrix.artifact }}-fips_testsupport_${{ env.logsuffix }}.zip
+        continue-on-error: true
+
   linux_additional_build:
     name: Linux additional
     runs-on: "ubuntu-20.04"
@@ -1464,6 +1625,7 @@ jobs:
       - prerequisites
       - linux_additional_build
       - linux_x64_test
+      - linux_x64_test_fips
       - linux_x86_test
       - windows_x64_test
       - macos_x64_test

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

@@ -57,6 +57,11 @@
 
 public final class Security {
 
+    private static final String SYS_PROP_SWITCH =
+        "java.security.disableSystemPropertiesFile";
+    private static final String SEC_PROP_SWITCH =
+        "security.useSystemPropertiesFile";
+
     /* Are we debugging? -- for developers */
     private static final Debug sdebug =
                         Debug.getInstance("properties");
@@ -100,6 +105,7 @@ private static void initialize() {
         props = new Properties();
         boolean loadedProps = false;
         boolean overrideAll = false;
+        boolean systemSecPropsEnabled = false;
 
         // first load the system properties file
         // to determine the value of security.overridePropertiesFile
@@ -210,27 +216,60 @@ private static void initialize() {
             }
         }
 
-        String disableSystemProps = System.getProperty("java.security.disableSystemPropertiesFile");
-        if ((disableSystemProps == null || "false".equalsIgnoreCase(disableSystemProps)) &&
-            "true".equalsIgnoreCase(props.getProperty("security.useSystemPropertiesFile"))) {
-            if (!SystemConfigurator.configureSysProps(props)) {
+        boolean sysUseProps = Boolean.valueOf(System.getProperty(SYS_PROP_SWITCH, "false"));
+        boolean secUseProps = Boolean.valueOf(props.getProperty(SEC_PROP_SWITCH));
+        if (sdebug != null) {
+            sdebug.println(SYS_PROP_SWITCH + "=" + sysUseProps);
+            sdebug.println(SEC_PROP_SWITCH + "=" + secUseProps);
+        }
+        if (!sysUseProps && secUseProps) {
+            systemSecPropsEnabled = SystemConfigurator.configureSysProps(props);
+            if (!systemSecPropsEnabled) {
                 if (sdebug != null) {
-                    sdebug.println("WARNING: System properties could not be loaded.");
+                    sdebug.println("WARNING: System security properties could not be loaded.");
                 }
             }
+        } else {
+            if (sdebug != null) {
+                sdebug.println("System security property support disabled by user.");
+            }
         }
 
         // FIPS support depends on the contents of java.security so
         // ensure it has loaded first
-        if (loadedProps) {
-            boolean fipsEnabled = SystemConfigurator.configureFIPS(props);
-            if (sdebug != null) {
-                if (fipsEnabled) {
-                    sdebug.println("FIPS support enabled.");
-                } else {
-                    sdebug.println("FIPS support disabled.");
+        if (loadedProps && systemSecPropsEnabled) {
+            boolean shouldEnable;
+            String sysProp = System.getProperty("com.redhat.fips");
+            if (sysProp == null) {
+                shouldEnable = true;
+                if (sdebug != null) {
+                    sdebug.println("com.redhat.fips unset, using default value of true");
+                }
+            } else {
+                shouldEnable = Boolean.valueOf(sysProp);
+                if (sdebug != null) {
+                    sdebug.println("com.redhat.fips set, using its value " + shouldEnable);
                 }
             }
+            if (shouldEnable) {
+                boolean fipsEnabled = SystemConfigurator.configureFIPS(props);
+                if (sdebug != null) {
+                    if (fipsEnabled) {
+                        sdebug.println("FIPS mode support configured and enabled.");
+                    } else {
+                        sdebug.println("FIPS mode support disabled.");
+                    }
+                }
+            } else {
+                if (sdebug != null ) {
+                    sdebug.println("FIPS mode support disabled by user.");
+                }
+            }
+        } else {
+            if (sdebug != null) {
+                sdebug.println("WARNING: FIPS mode support can not be enabled without " +
+                               "system security properties being enabled.");
+            }
         }
     }
 

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

@@ -77,13 +77,13 @@ public Void run() {
      * security.useSystemPropertiesFile is true.
      */
     static boolean configureSysProps(Properties props) {
-        boolean loadedProps = false;
+        boolean systemSecPropsLoaded = false;
 
         try (BufferedInputStream bis =
                 new BufferedInputStream(
                         new FileInputStream(CRYPTO_POLICIES_JAVA_CONFIG))) {
             props.load(bis);
-            loadedProps = true;
+            systemSecPropsLoaded = true;
             if (sdebug != null) {
                 sdebug.println("reading system security properties file " +
                         CRYPTO_POLICIES_JAVA_CONFIG);
@@ -96,7 +96,7 @@ static boolean configureSysProps(Properties props) {
                 e.printStackTrace();
             }
         }
-        return loadedProps;
+        return systemSecPropsLoaded;
     }
 
     /*
@@ -168,6 +168,8 @@ static boolean configureFIPS(Properties props) {
                         sdebug.println("FIPS support enabled without plain key support");
                     }
                 }
+            } else {
+                if (sdebug != null) { sdebug.println("FIPS mode not detected"); }
             }
         } catch (Exception e) {
             if (sdebug != null) {
@@ -208,37 +210,39 @@ static boolean isPlainKeySupportEnabled() {
         return plainKeySupportEnabled;
     }
 
-    /*
-     * OpenJDK FIPS mode will be enabled only if the com.redhat.fips
-     * system property is true (default) and the system is in FIPS mode.
+    /**
+     * Determines whether FIPS mode should be enabled.
+     *
+     * OpenJDK FIPS mode will be enabled only if the system is in
+     * FIPS mode.
+     *
+     * Calls to this method only occur if the system property
+     * com.redhat.fips is not set to false.
      *
      * There are 2 possible ways in which OpenJDK detects that the system
      * is in FIPS mode: 1) if the NSS SECMOD_GetSystemFIPSEnabled API is
      * available at OpenJDK's built-time, it is called; 2) otherwise, the
      * /proc/sys/crypto/fips_enabled file is read.
+     *
+     * @return true if the system is in FIPS mode
      */
     private static boolean enableFips() throws Exception {
-        boolean shouldEnable = Boolean.valueOf(System.getProperty("com.redhat.fips", "true"));
-        if (shouldEnable) {
+        if (sdebug != null) {
+            sdebug.println("Calling getSystemFIPSEnabled (libsystemconf)...");
+        }
+        try {
+            boolean fipsEnabled = getSystemFIPSEnabled();
             if (sdebug != null) {
-                sdebug.println("Calling getSystemFIPSEnabled (libsystemconf)...");
+                sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) returned: "
+                               + fipsEnabled);
             }
-            try {
-                shouldEnable = getSystemFIPSEnabled();
-                if (sdebug != null) {
-                    sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) returned: "
-                            + shouldEnable);
-                }
-                return shouldEnable;
-            } catch (IOException e) {
-                if (sdebug != null) {
-                    sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) failed:");
-                    sdebug.println(e.getMessage());
-                }
-                throw e;
+            return fipsEnabled;
+        } catch (IOException e) {
+            if (sdebug != null) {
+                sdebug.println("Call to getSystemFIPSEnabled (libsystemconf) failed:");
+                sdebug.println(e.getMessage());
             }
-        } else {
-            return false;
+            throw e;
         }
     }
 }

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

@@ -86,7 +86,7 @@ security.provider.tbd=SunPKCS11
 #endif
 
 #
-# Security providers used when global crypto-policies are set to FIPS.
+# Security providers used when FIPS mode support is active
 #
 fips.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg
 fips.provider.2=SUN
@@ -353,7 +353,7 @@ security.overridePropertiesFile=true
 # using the system properties file stored at
 # /etc/crypto-policies/back-ends/java.config
 #
-security.useSystemPropertiesFile=true
+security.useSystemPropertiesFile=false
 
 #
 # Determines the default key and trust manager factory algorithms for