From 2879030caf2866f2fa19887e662a284f771a81ff Mon Sep 17 00:00:00 2001 From: Andrew John Hughes Date: Wed, 22 Jun 2022 20:17:41 +0100 Subject: [PATCH] Update FIPS support to bring in latest changes * RH2036462: sun.security.pkcs11.wrapper.PKCS11.getInstance breakage * RH2090378: Revert to disabling system security properties and FIPS mode support together Rebase RH1648249 nss.cfg patch so it applies after the FIPS patch Enable system security properties in the RPM (now disabled by default in the FIPS repo) Improve security properties test to check both enabled and disabled behaviour Run security properties test with property debugging on --- TestSecurityProperties.java | 34 +++- ...85b13d.patch => fips-17u-f8142a23d0a.patch | 167 +++++++++++++----- java-17-openjdk.spec | 30 +++- ...ut_nss_cfg_provider_to_java_security.patch | 4 +- 4 files changed, 175 insertions(+), 60 deletions(-) rename fips-17u-3625385b13d.patch => fips-17u-f8142a23d0a.patch (96%) diff --git a/TestSecurityProperties.java b/TestSecurityProperties.java index 06a0b07..552bd0f 100644 --- a/TestSecurityProperties.java +++ b/TestSecurityProperties.java @@ -9,35 +9,59 @@ public class TestSecurityProperties { // JDK 8 private static final String JDK_PROPS_FILE_JDK_8 = System.getProperty("java.home") + "/lib/security/java.security"; + private static final String POLICY_FILE = "/etc/crypto-policies/back-ends/java.config"; + + private static final String MSG_PREFIX = "DEBUG: "; + public static void main(String[] args) { + if (args.length == 0) { + System.err.println("TestSecurityProperties "); + System.err.println("Invoke with 'true' if system security properties should be enabled."); + System.err.println("Invoke with 'false' if system security properties should be disabled."); + System.exit(1); + } + boolean enabled = Boolean.valueOf(args[0]); + System.out.println(MSG_PREFIX + "System security properties enabled: " + enabled); Properties jdkProps = new Properties(); loadProperties(jdkProps); + if (enabled) { + loadPolicy(jdkProps); + } for (Object key: jdkProps.keySet()) { String sKey = (String)key; String securityVal = Security.getProperty(sKey); String jdkSecVal = jdkProps.getProperty(sKey); if (!securityVal.equals(jdkSecVal)) { - String msg = "Expected value '" + jdkSecVal + "' for key '" + + String msg = "Expected value '" + jdkSecVal + "' for key '" + sKey + "'" + " but got value '" + securityVal + "'"; throw new RuntimeException("Test failed! " + msg); } else { - System.out.println("DEBUG: " + sKey + " = " + jdkSecVal + " as expected."); + System.out.println(MSG_PREFIX + sKey + " = " + jdkSecVal + " as expected."); } } System.out.println("TestSecurityProperties PASSED!"); } - + private static void loadProperties(Properties props) { String javaVersion = System.getProperty("java.version"); - System.out.println("Debug: Java version is " + javaVersion); + System.out.println(MSG_PREFIX + "Java version is " + javaVersion); String propsFile = JDK_PROPS_FILE_JDK_11; if (javaVersion.startsWith("1.8.0")) { propsFile = JDK_PROPS_FILE_JDK_8; } - try (FileInputStream fin = new FileInputStream(new File(propsFile))) { + try (FileInputStream fin = new FileInputStream(propsFile)) { props.load(fin); } catch (Exception e) { throw new RuntimeException("Test failed!", e); } } + + private static void loadPolicy(Properties props) { + try (FileInputStream fin = new FileInputStream(POLICY_FILE)) { + props.load(fin); + } catch (Exception e) { + throw new RuntimeException("Test failed!", e); + } + } + } diff --git a/fips-17u-3625385b13d.patch b/fips-17u-f8142a23d0a.patch similarity index 96% rename from fips-17u-3625385b13d.patch rename to fips-17u-f8142a23d0a.patch index eecef3b..c07a4bf 100644 --- a/fips-17u-3625385b13d.patch +++ b/fips-17u-f8142a23d0a.patch @@ -1398,7 +1398,7 @@ index a020e1c15d8..6d459fdec01 100644 // Return the instance of this class or create one if needed. diff --git a/src/java.base/share/classes/java/security/Security.java b/src/java.base/share/classes/java/security/Security.java -index ff2bc942c03..d303ae5c8f3 100644 +index ff2bc942c03..96a3ba4040c 100644 --- a/src/java.base/share/classes/java/security/Security.java +++ b/src/java.base/share/classes/java/security/Security.java @@ -32,6 +32,7 @@ import java.net.URL; @@ -1409,7 +1409,7 @@ index ff2bc942c03..d303ae5c8f3 100644 import jdk.internal.access.SharedSecrets; import jdk.internal.util.StaticProperty; import sun.security.util.Debug; -@@ -47,6 +48,9 @@ import sun.security.jca.*; +@@ -47,12 +48,20 @@ import sun.security.jca.*; * implementation-specific location, which is typically the properties file * {@code conf/security/java.security} in the Java installation directory. * @@ -1419,7 +1419,18 @@ index ff2bc942c03..d303ae5c8f3 100644 * @author Benjamin Renaud * @since 1.1 */ -@@ -67,6 +71,19 @@ public final class Security { + + 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"); +@@ -67,6 +76,19 @@ public final class Security { } static { @@ -1439,7 +1450,15 @@ index ff2bc942c03..d303ae5c8f3 100644 // doPrivileged here because there are multiple // things in initialize that might require privs. // (the FileInputStream call and the File.exists call, -@@ -99,6 +116,7 @@ public final class Security { +@@ -84,6 +106,7 @@ public final class Security { + 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 +@@ -99,6 +122,7 @@ public final class Security { if (sdebug != null) { sdebug.println("reading security properties file: " + propFile); @@ -1447,30 +1466,63 @@ index ff2bc942c03..d303ae5c8f3 100644 } } catch (IOException e) { if (sdebug != null) { -@@ -193,6 +211,28 @@ public final class Security { +@@ -193,6 +217,61 @@ public final class Security { } } -+ 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."); + } + } } @@ -1478,10 +1530,10 @@ index ff2bc942c03..d303ae5c8f3 100644 /* diff --git a/src/java.base/share/classes/java/security/SystemConfigurator.java b/src/java.base/share/classes/java/security/SystemConfigurator.java new file mode 100644 -index 00000000000..da2af5defda +index 00000000000..98ffced455b --- /dev/null +++ b/src/java.base/share/classes/java/security/SystemConfigurator.java -@@ -0,0 +1,245 @@ +@@ -0,0 +1,249 @@ +/* + * Copyright (c) 2019, 2021, Red Hat, Inc. + * @@ -1562,13 +1614,13 @@ index 00000000000..da2af5defda + * 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); @@ -1581,7 +1633,7 @@ index 00000000000..da2af5defda + e.printStackTrace(); + } + } -+ return loadedProps; ++ return systemSecPropsLoaded; + } + + /* @@ -1653,6 +1705,8 @@ index 00000000000..da2af5defda + 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) { @@ -1693,37 +1747,39 @@ index 00000000000..da2af5defda + 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; + } + } +} @@ -2352,7 +2408,7 @@ index 894e26dfad8..8b16378b96b 100644 "sun.security.ssl.SSLContextImpl$TLSContext", List.of("SSL"), null); diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security -index 6d91e3f8e4e..5a355e70cae 100644 +index 6d91e3f8e4e..adfaf57d29e 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -79,6 +79,16 @@ security.provider.tbd=Apple @@ -2360,7 +2416,7 @@ index 6d91e3f8e4e..5a355e70cae 100644 security.provider.tbd=SunPKCS11 +# -+# 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 @@ -2393,7 +2449,7 @@ index 6d91e3f8e4e..5a355e70cae 100644 +# 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 @@ -3074,7 +3130,7 @@ index 112b639aa96..5549cd9ed4e 100644 if (config.getHandleStartupErrors() == Config.ERR_IGNORE_ALL) { throw new UnsupportedOperationException diff --git a/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java b/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java -index 5c0aacd1a67..372a50dd587 100644 +index 5c0aacd1a67..1e98ce2e280 100644 --- a/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java +++ b/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java @@ -49,6 +49,9 @@ package sun.security.pkcs11.wrapper; @@ -3087,8 +3143,21 @@ index 5c0aacd1a67..372a50dd587 100644 import java.util.*; import java.security.AccessController; -@@ -152,16 +155,30 @@ public class PKCS11 { +@@ -150,18 +153,43 @@ public class PKCS11 { + this.pkcs11ModulePath = pkcs11ModulePath; + } ++ /* ++ * Compatibility wrapper to allow this method to work as before ++ * when FIPS mode support is not active. ++ */ ++ public static synchronized PKCS11 getInstance(String pkcs11ModulePath, ++ String functionList, CK_C_INITIALIZE_ARGS pInitArgs, ++ boolean omitInitialize) throws IOException, PKCS11Exception { ++ return getInstance(pkcs11ModulePath, functionList, ++ pInitArgs, omitInitialize, null, null); ++ } ++ public static synchronized PKCS11 getInstance(String pkcs11ModulePath, String functionList, CK_C_INITIALIZE_ARGS pInitArgs, - boolean omitInitialize) throws IOException, PKCS11Exception { @@ -3121,7 +3190,7 @@ index 5c0aacd1a67..372a50dd587 100644 } if (omitInitialize == false) { try { -@@ -1911,4 +1928,194 @@ static class SynchronizedPKCS11 extends PKCS11 { +@@ -1911,4 +1939,194 @@ static class SynchronizedPKCS11 extends PKCS11 { super.C_GenerateRandom(hSession, randomData); } } diff --git a/java-17-openjdk.spec b/java-17-openjdk.spec index 057f7ad..b9b18b5 100644 --- a/java-17-openjdk.spec +++ b/java-17-openjdk.spec @@ -328,7 +328,7 @@ # Define IcedTea version used for SystemTap tapsets and desktop file %global icedteaver 6.0.0pre00-c848b93a8598 # Define current Git revision for the FIPS support patches -%global fipsver 3625385b13d +%global fipsver f8142a23d0a # Standard JPackage naming and versioning defines %global origin openjdk @@ -336,7 +336,7 @@ %global top_level_dir_name %{origin} %global top_level_dir_name_backup %{top_level_dir_name}-backup %global buildver 7 -%global rpmrelease 2 +%global rpmrelease 3 # Priority must be 8 digits in total; up to openjdk 1.8, we were using 18..... so when we moved to 11, we had to add another digit %if %is_system_jdk # Using 10 digits may overflow the int used for priority, so we combine the patch and build versions @@ -1327,6 +1327,8 @@ Patch6: rh1684077-openjdk_should_depend_on_pcsc-lite-libs_instead_of_pcsc-lite-d # RH2052070: Enable AlgorithmParameters and AlgorithmParameterGenerator services in FIPS mode # RH2023467: Enable FIPS keys export # RH2094027: SunEC runtime permission for FIPS +# RH2036462: sun.security.pkcs11.wrapper.PKCS11.getInstance breakage +# RH2090378: Revert to disabling system security properties and FIPS mode support together Patch1001: fips-17u-%{fipsver}.patch ############################################# @@ -2035,6 +2037,12 @@ top_dir_abs_staticlibs_build_path=$(pwd)/%{buildoutputdir -- ${suffix}%{staticli export JAVA_HOME=${top_dir_abs_main_build_path}/images/%{jdkimage} +# Pre-test setup + +# Turn on system security properties +sed -i -e "s:^security.useSystemPropertiesFile=.*:security.useSystemPropertiesFile=true:" \ + ${JAVA_HOME}/conf/security/java.security + #check Shenandoah is enabled %if %{use_shenandoah_hotspot} $JAVA_HOME//bin/java -XX:+UnlockExperimentalVMOptions -XX:+UseShenandoahGC -version @@ -2048,9 +2056,14 @@ $JAVA_HOME/bin/java --add-opens java.base/javax.crypto=ALL-UNNAMED TestCryptoLev $JAVA_HOME/bin/javac -d . %{SOURCE14} $JAVA_HOME/bin/java $(echo $(basename %{SOURCE14})|sed "s|\.java||") -# Check system crypto (policy) can be disabled +# Check system crypto (policy) is active and can be disabled +# Test takes a single argument - true or false - to state whether system +# security properties are enabled or not. $JAVA_HOME/bin/javac -d . %{SOURCE15} -$JAVA_HOME/bin/java -Djava.security.disableSystemPropertiesFile=true $(echo $(basename %{SOURCE15})|sed "s|\.java||") +export PROG=$(echo $(basename %{SOURCE15})|sed "s|\.java||") +export SEC_DEBUG="-Djava.security.debug=properties" +$JAVA_HOME/bin/java ${SEC_DEBUG} ${PROG} true +$JAVA_HOME/bin/java ${SEC_DEBUG} -Djava.security.disableSystemPropertiesFile=true ${PROG} false # Check java launcher has no SSB mitigation if ! nm $JAVA_HOME/bin/java | grep set_speculation ; then true ; else false; fi @@ -2517,6 +2530,15 @@ cjc.mainProgram(args) %endif %changelog +* Wed Jun 22 2022 Andrew Hughes - 1:17.0.3.0.7-3 +- Update FIPS support to bring in latest changes +- * RH2036462: sun.security.pkcs11.wrapper.PKCS11.getInstance breakage +- * RH2090378: Revert to disabling system security properties and FIPS mode support together +- Rebase RH1648249 nss.cfg patch so it applies after the FIPS patch +- Enable system security properties in the RPM (now disabled by default in the FIPS repo) +- Improve security properties test to check both enabled and disabled behaviour +- Run security properties test with property debugging on + * Sun Jun 12 2022 Andrew Hughes - 1:17.0.3.0.7-2 - Rebase FIPS patches from fips-17u branch and simplify by using a single patch from that repository - Rebase RH1648249 nss.cfg patch so it applies after the FIPS patch diff --git a/rh1648249-add_commented_out_nss_cfg_provider_to_java_security.patch b/rh1648249-add_commented_out_nss_cfg_provider_to_java_security.patch index b552b99..6d2342a 100644 --- a/rh1648249-add_commented_out_nss_cfg_provider_to_java_security.patch +++ b/rh1648249-add_commented_out_nss_cfg_provider_to_java_security.patch @@ -1,5 +1,5 @@ diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security -index 5a355e70cae..c730ea26ea2 100644 +index adfaf57d29e..abf89bbf327 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -78,6 +78,7 @@ security.provider.tbd=SunMSCAPI @@ -9,4 +9,4 @@ index 5a355e70cae..c730ea26ea2 100644 +#security.provider.tbd=SunPKCS11 ${java.home}/lib/security/nss.cfg # - # Security providers used when global crypto-policies are set to FIPS. + # Security providers used when FIPS mode support is active