RH2048582: Support PKCS#12 keystores #2
Conversation
|
|
Yes. I believe this is a side-effect of the JDK repositories being setup to operate via the Skara bots. I can fix this as we did for IcedTea here: icedtea-git/icedtea8@bf40770 |
|
This one is going to take a while to review. Most of these changes affect upstream code, even in the non-FIPS case. Is there a bug or bugs for getting this into upstream OpenJDK? Do we have a hook to turn off these changes? |
There was a problem hiding this comment.
@franferrax Doesn't this only run the test with -Dcom.redhat.fips=false (i.e. in FIPS disabled mode)? I'd have expected this test to at least have:
@run main/othervm/timeout=30 -Dcom.redhat.fips=false ImportKeyToP12
@run main/othervm/timeout=30 ImportKeyToP12
There was a problem hiding this comment.
I'm getting this on a FIPS enabled system. Expected?
$ java -cp work/classes/ImportKeyToP12.java.d -Djava.security.debug=properties ImportKeyToP12
properties: reading security properties file: /home/sgehwolf/mandrel-fips/mandrel-build/conf/security/java.security
properties: {jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, fips.provider.3=SunEC, fips.provider.4=SunJSSE, fips.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg, fips.provider.2=SUN, jdk.security.legacyAlgorithms=SHA1, RSA keySize < 2048, DSA keySize < 2048, crypto.policy=unlimited, jceks.key.serialFilter=java.base/java.lang.Enum;java.base/java.security.KeyRep;java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*, login.configuration.provider=sun.security.provider.ConfigFile, security.overridePropertiesFile=true, jdk.tls.legacyAlgorithms=NULL, anon, RC4, DES, 3DES_EDE_CBC, security.provider.7=SunSASL, security.provider.8=XMLDSig, security.provider.9=SunPCSC, jdk.security.caDistrustPolicies=SYMANTEC_TLS, security.provider.1=SUN, security.provider.2=SunRsaSign, security.provider.3=SunEC, security.provider.4=SunJSSE, networkaddress.cache.negative.ttl=10, jdk.tls.alpnCharset=ISO_8859_1, security.provider.5=SunJCE, security.provider.6=SunJGSS, ssl.KeyManagerFactory.algorithm=SunX509, ssl.TrustManagerFactory.algorithm=PKIX, fips.provider.5=SunJCE, fips.provider.6=SunRsaSign, policy.allowSystemProperty=true, jdk.io.permissionsUseCanonicalPath=false, package.access=sun.misc.,sun.reflect., package.definition=sun.misc.,sun.reflect., security.provider.12=SunPKCS11, policy.provider=sun.security.provider.PolicyFile, policy.url.1=file:${java.home}/conf/security/java.policy, policy.url.2=file:${user.home}/.java.policy, securerandom.source=file:/dev/random, jdk.certpath.disabledAlgorithms=MD2, MD5, SHA1 jdkCA & usage TLSServer, RSA keySize < 1024, DSA keySize < 1024, EC keySize < 224, jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, RC4, DES, MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, policy.ignoreIdentityScope=false, keystore.type.compat=true, security.provider.11=JdkSASL, security.provider.10=JdkLDAP, jdk.sasl.disabledMechanisms=, fips.keystore.type=PKCS11, sun.security.krb5.maxReferrals=5, security.useSystemPropertiesFile=true, jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37, jdk.xml.dsig.secureValidationPolicy=disallowAlg http://www.w3.org/TR/1999/REC-xslt-19991116,disallowAlg http://www.w3.org/2001/04/xmldsig-more#rsa-md5,disallowAlg http://www.w3.org/2001/04/xmldsig-more#hmac-md5,disallowAlg http://www.w3.org/2001/04/xmldsig-more#md5,disallowAlg http://www.w3.org/2000/09/xmldsig#sha1,disallowAlg http://www.w3.org/2000/09/xmldsig#dsa-sha1,disallowAlg http://www.w3.org/2000/09/xmldsig#rsa-sha1,disallowAlg http://www.w3.org/2007/05/xmldsig-more#sha1-rsa-MGF1,disallowAlg http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha1,maxTransforms 5,maxReferences 30,disallowReferenceUriSchemes file http https,minKeySize RSA 1024,minKeySize DSA 1024,minKeySize EC 224,noDuplicateIds,noRetrievalMethodLoops, securerandom.drbg.config=, sun.security.krb5.disableReferrals=false, keystore.type=pkcs12, securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN, policy.expandProperties=true, krb5.kdc.bad.policy=tryLast}
properties: reading system security properties file /etc/crypto-policies/back-ends/java.config
properties: {jdk.jar.disabledAlgorithms=MD2, MD5, RSA keySize < 1024, DSA keySize < 1024, fips.provider.3=SunEC, fips.provider.4=SunJSSE, fips.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg, fips.provider.2=SUN, jdk.security.legacyAlgorithms=SHA1, RSA keySize < 2048, DSA keySize < 2048, crypto.policy=unlimited, jceks.key.serialFilter=java.base/java.lang.Enum;java.base/java.security.KeyRep;java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*, login.configuration.provider=sun.security.provider.ConfigFile, security.overridePropertiesFile=true, jdk.tls.legacyAlgorithms=, security.provider.7=SunSASL, security.provider.8=XMLDSig, security.provider.9=SunPCSC, jdk.security.caDistrustPolicies=SYMANTEC_TLS, security.provider.1=SUN, security.provider.2=SunRsaSign, security.provider.3=SunEC, security.provider.4=SunJSSE, networkaddress.cache.negative.ttl=10, jdk.tls.alpnCharset=ISO_8859_1, security.provider.5=SunJCE, security.provider.6=SunJGSS, ssl.KeyManagerFactory.algorithm=SunX509, ssl.TrustManagerFactory.algorithm=PKIX, fips.provider.5=SunJCE, fips.provider.6=SunRsaSign, policy.allowSystemProperty=true, jdk.io.permissionsUseCanonicalPath=false, package.access=sun.misc.,sun.reflect., package.definition=sun.misc.,sun.reflect., security.provider.12=SunPKCS11, policy.provider=sun.security.provider.PolicyFile, policy.url.1=file:${java.home}/conf/security/java.policy, policy.url.2=file:${user.home}/.java.policy, securerandom.source=file:/dev/random, jdk.certpath.disabledAlgorithms=MD2, SHA1, MD5, DSA, RSA keySize < 2048, jdk.tls.disabledAlgorithms=DH keySize < 2048, TLSv1.1, TLSv1, SSLv3, SSLv2, RSAPSK, TLS_RSA_WITH_AES_256_CBC_SHA256, TLS_RSA_WITH_AES_256_CBC_SHA, TLS_RSA_WITH_AES_128_CBC_SHA256, TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_GCM_SHA384, TLS_RSA_WITH_AES_128_GCM_SHA256, DHE_DSS, RSA_EXPORT, DHE_DSS_EXPORT, DHE_RSA_EXPORT, DH_DSS_EXPORT, DH_RSA_EXPORT, DH_anon, ECDH_anon, DH_RSA, DH_DSS, ECDH, 3DES_EDE_CBC, DES_CBC, RC4_40, RC4_128, DES40_CBC, RC2, HmacMD5, policy.ignoreIdentityScope=false, keystore.type.compat=true, security.provider.11=JdkSASL, security.provider.10=JdkLDAP, jdk.sasl.disabledMechanisms=, fips.keystore.type=PKCS11, sun.security.krb5.maxReferrals=5, security.useSystemPropertiesFile=true, jdk.tls.keyLimits=AES/GCM/NoPadding KeyUpdate 2^37, jdk.xml.dsig.secureValidationPolicy=disallowAlg http://www.w3.org/TR/1999/REC-xslt-19991116,disallowAlg http://www.w3.org/2001/04/xmldsig-more#rsa-md5,disallowAlg http://www.w3.org/2001/04/xmldsig-more#hmac-md5,disallowAlg http://www.w3.org/2001/04/xmldsig-more#md5,disallowAlg http://www.w3.org/2000/09/xmldsig#sha1,disallowAlg http://www.w3.org/2000/09/xmldsig#dsa-sha1,disallowAlg http://www.w3.org/2000/09/xmldsig#rsa-sha1,disallowAlg http://www.w3.org/2007/05/xmldsig-more#sha1-rsa-MGF1,disallowAlg http://www.w3.org/2001/04/xmldsig-more#ecdsa-sha1,maxTransforms 5,maxReferences 30,disallowReferenceUriSchemes file http https,minKeySize RSA 1024,minKeySize DSA 1024,minKeySize EC 224,noDuplicateIds,noRetrievalMethodLoops, securerandom.drbg.config=, sun.security.krb5.disableReferrals=false, keystore.type=pkcs12, securerandom.strongAlgorithms=NativePRNGBlocking:SUN,DRBG:SUN, policy.expandProperties=true, krb5.kdc.bad.policy=tryLast}
properties: Calling getSystemFIPSEnabled (libsystemconf)...
properties: getSystemFIPSEnabled: calling SECMOD_GetSystemFIPSEnabled
properties: getSystemFIPSEnabled: SECMOD_GetSystemFIPSEnabled returned 0x1
properties: Call to getSystemFIPSEnabled (libsystemconf) returned: true
properties: FIPS mode detected
properties: Removing provider: security.provider.7=SunSASL
properties: Removing provider: security.provider.8=XMLDSig
properties: Removing provider: security.provider.9=SunPCSC
properties: Removing provider: security.provider.1=SUN
properties: Removing provider: security.provider.2=SunRsaSign
properties: Removing provider: security.provider.3=SunEC
properties: Removing provider: security.provider.4=SunJSSE
properties: Removing provider: security.provider.5=SunJCE
properties: Removing provider: security.provider.6=SunJGSS
properties: Removing provider: security.provider.12=SunPKCS11
properties: Removing provider: security.provider.11=JdkSASL
properties: Removing provider: security.provider.10=JdkLDAP
properties: Adding provider 1: security.provider.1=SunPKCS11 ${java.home}/conf/security/nss.fips.cfg
properties: Adding provider 2: security.provider.2=SUN
properties: Adding provider 3: security.provider.3=SunEC
properties: Adding provider 4: security.provider.4=SunJSSE
properties: Adding provider 5: security.provider.5=SunJCE
properties: Adding provider 6: security.provider.6=SunRsaSign
properties: FIPS mode default keystore.type = PKCS11
properties: FIPS mode javax.net.ssl.keyStore = NONE
properties: FIPS mode javax.net.ssl.trustStoreType = pkcs12
properties: FIPS support enabled with plain key support
properties: FIPS support enabled.
SunPKCS11: SunPKCS11-NSS-FIPS
=========================================================================
Cipher PBE: PBEWithHmacSHA1AndAES_128
Mac PBE: HmacPBESHA1
Exception in thread "main" java.io.IOException: Integrity check failed: java.security.NoSuchAlgorithmException: Algorithm HmacPBESHA1 not available
at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2224)
at java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:221)
at java.base/java.security.KeyStore.load(KeyStore.java:1473)
at ImportKeyToP122.testWith(ImportKeyToP12.java:124)
at ImportKeyToP122.main(ImportKeyToP12.java:86)
at ImportKeyToP122.main(ImportKeyToP12.java:137)
at ImportKeyToP12.main(ImportKeyToP12.java:49)
Caused by: java.security.NoSuchAlgorithmException: Algorithm HmacPBESHA1 not available
at java.base/javax.crypto.Mac.getInstance(Mac.java:190)
at java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2198)
... 6 more
There was a problem hiding this comment.
Looks like a test problem. On a FIPS system HmacPBESHA1 is only provided by SunPKCS11-NSS-FIPS but that provider is removed prior the integrity check here:
https://github.com/franferrax/jdk/blob/RH2048582/test/jdk/sun/security/pkcs11/KeyStore/ImportKeyToP12.java#L124
There was a problem hiding this comment.
@jerboaa: I think it's worth giving a bit of context here.
For all the tests except this one, we've taken the following approach regarding assertion data: we try to obtain the
expected data (derived key, hash-based message authentication code, or cipher text) from SunJCE (our "trusted"
implementation), if SunJCE can't be instantiated, we fall back to test-embedded/hardcoded assertion data.
The idea behind this is to self-document the way we obtain the assertion data, allowing to easily create new tests [†]
that also work in FIPS-mode (where SunJCE is not available). This way, we can have upstream-ready tests (they are
just SunPKCS11 tests, to be proposed along with most of this PR work [‡]), that also run downstream and in FIPS mode.
In particular, for ImportKeyToP12.java, we faced the following issues (@martinuy knows the full details, since he developed this one):
- We didn't want to hardcode a certificate or a whole keystore
- Generating a certificate on-the-fly would require a significant amount of code (generating its key was the easy part)
- Implementing the test as a
SecmodTest(using an existing certificate fromtest/jdk/sun/security/pkcs11/Secmod) wouldn't work upstream because the certificate's key have theCKA_SENSITIVEattribute (and upstream, we don't have the work of RH2023467: Enable FIPS keys export #1, nor it is upstreamable) - PKCS#12 can't store a certificate's key if there is no certificate associated to it
Finally, we decided to just store a SecretKey (symmetric, so no certificates involved) and to rely on available providers (typically SunJCE) to make a cross-check: create a keystore with SunPKCS11's crypto, then read it with other available providers' crypto. That's why SunPKCS11 is removed from the providers, and why this test isn't meant to be run in FIPS mode. Please note that the test still exercises the SunPKCS11 PBA and PBE primitives used for PKCS#12 keystores, which are the ones used when in FIPS mode.
[†] New test creation steps
- Develop the test with an empty test-embedded fallback assertion data
- Run it in a FIPS-disabled OpenJDK (so SunJCE is available), and make sure it passes
- Copy the printed SunPKCS11's actual data (but asserted to be equal to SunJCE's expected data)
- Paste this data as the test-embedded fallback assertion data
- Now we are able to run the test in both FIPS and non-FIPS
[‡] So far, non-upstreamable bits include
- Little changes in
SunJCE.java - NSS FIPS-tests workaround by invoking
java.security.Security.getProviders(), we must proposeTestClass2as upstream'sTestClass, forTestClassin: (PBECipher,ImportKeyToP12,PBAMac,TestPBKD)
There was a problem hiding this comment.
OK, I understand. I've worked around this (on a FIPS system) by doing the verification steps in a separate Java program that is running in non-fips mode which seems a nice inter-op test too.
There was a problem hiding this comment.
[‡] So far, non-upstreamable bits include
- Little changes in
SunJCE.java- NSS FIPS-tests workaround by invoking
java.security.Security.getProviders(), we must proposeTestClass2as upstream'sTestClass, forTestClassin: (PBECipher,ImportKeyToP12,PBAMac,TestPBKD)
SunJCE.java is no longer touched, after rebasing to include #16 changes.
Yes, we know, almost everything in this PR is upstreamable, except for the things mentioned in [‡], plus b7675c2a87099211814dad03b5b2ea77990f87a5 & ee5209d5e3089dfa807c658fa11ef363b3679d10. ee5209d5e3089dfa807c658fa11ef363b3679d10 reuses the existing NOTE: the force-push is because I rebased this branch to include #5 changes |
Right, so are there upstream bugs for this?
This is not what we agreed when we discussed including this under such a short timeline. This new functionality should be covered by its own switch and be off by default, so people have to opt in to use it. |
This is what I proposed in the last meeting. We will use the existing FIPS switch so new PBE algorithms are not exposed under a non-FIPS configuration. For a non-FIPS case, there are no changes visible until this feature is pushed upstream and backported. For a FIPS scenario, the user can just not use PKCS#12 keystores if there were any crticial issues not caught in our testing. Please also notice that the "fips.keystore.type" security property can also be changed to the previous PKCS#11 value. Applications currently running in FIPS mode should experience no regressions as they are not using PKCS#12 keystores yet. Applications using PKCS#12 transitioning to FIPS mode may, in the worst case scenario, be unable to complete the transition as it's currently happening (a switch-disable won't make any difference apps in this category). In general, I prefer not to have a per-feature/per-fix whenever possible because it can potentially lead to a significant number causing more confusion. |
Not yet, @martinuy will create them when he finds the time. |
Co-Authored-By: Martin Balao <[email protected]>
Align it with the non-FIPS keystore type (pkcs12) now that it is supported
This is: RHEL is in FIPS mode (non default), and the OpenJDK has been configured to align with it (default)
gnu-andrew
left a comment
gnu-andrew
left a comment
There was a problem hiding this comment.
Yes, my concern is there are a lot of changes here that affect code that is used outside the FIPS mode and even some which is nothing to do with PKCS11. There is a lot of refactoring to move code from classes like PBES2Core to PBEUtil and I had to manually verify that the code in PBEUtil was the same as that removed from other classes, and things still operated in the same way. As far as I can see, it should all work the same. Maybe it would have been better to do this refactoring first separately? Is this a candidate for going upstream?
There are a few other things I'll comment on, but this is good to go in and start getting wider testing.
|
|
||
| /** | ||
| * Returns the string representation of CK_PKCS5_PBKD2_PARAMS. | ||
| * Returns the string representation of CK_ECDH1_DERIVE_PARAMS. |
There was a problem hiding this comment.
I'm a little bemused by these changes. I guess this is a copy-and-paste error.
| * typedef struct CK_PBE_PARAMS { | ||
| * CK_CHAR_PTR pInitVector; | ||
| * CK_CHAR_PTR pPassword; | ||
| * CK_BYTE_PTR pInitVector; |
There was a problem hiding this comment.
Is CK_PBE_PARAMS actually already in use? Will these type changes affect anything else?
There was a problem hiding this comment.
It seems that some of these classes (representing PKCS#11 structures) were added a long time ago, but never tested or used. In this case, we are sure CK_PBE_PARAMS had no usages because it didn't have a useful constructor (just Java's default).
We aligned it to be in sync with the C structure in pkcs11t.h (untouched by us):
jdk/src/jdk.crypto.cryptoki/share/native/libj2pkcs11/pkcs11t.h
Lines 1761 to 1770 in 1e26894
Those Java changes are paired with jPbeParamToCKPbeParamPtr() changes in p11_convert.c:
Finally, freeCKMechanismPtr() was adjusted in p11_util.c, since CK_PBE_PARAMS fields were being leaked:
|
|
||
| SynchronizedPKCS11(String pkcs11ModulePath, String functionListName) | ||
| throws IOException { | ||
| throws IOException, PKCS11Exception { |
There was a problem hiding this comment.
Is this going to cause compatibility issues again? Maybe the wrapper constructor should catch PKCS11Exception and wrap it in an IOException?
There was a problem hiding this comment.
PKCS11 & SynchronizedPKCS11 constructors are only used from PKCS11::getInstance(), which already throws PKCS11Exception in upstream code.
Both PKCS11::getInstance() and the constructors are part of a non-public API, only accesible by breaking encapsulation through an illegal reflective access (requiring --add-exports in modern Java versions), as in RH2036462: rh1991003 patch breaks sun.security.pkcs11.wrapper.PKCS11.getInstance().
But unlike PKCS11::getInstance(), direct usage of the constructors has an additional caveat: special care needs to be taken to avoid calling C_Initialize more than once per native .so. That makes PKCS11::getInstance() a much better candidate for this kind of unsupported usage.
I think it's not worth wrapping the PKCS11Exception in an IOException, but we started working in the upstream proposal of this, so we can see if the same concern is raised there.
| private static void d(String type, String algorithm, String className, | ||
| int[] m) { | ||
| register(new Descriptor(type, algorithm, className, null, m)); | ||
| register(new Descriptor(type, algorithm, className, null, m, null)); |
There was a problem hiding this comment.
Would it not just make sense to add a wrapper constructor that passes null to the new version? That would simplify this code and avoid a repeat of the issue we had when we changed a signature before.
There was a problem hiding this comment.
I'll apply this suggestion to the patch for OpenJDK upstream. Thanks.
* RH2048582: Support PKCS#12 keystores v16 This is superseded by 8301553: "Support Password-Based Cryptography in SunPKCS11" upstream Algorithms are now available both in and out of FIPS mode, as upstream * Change default keystore type for FIPS Align it with the non-FIPS keystore type (pkcs12) now that it is supported Co-authored-by: Martin Balao <[email protected]> Reviewed-by: @gnu-andrew
4 participants
Search this PR in Red Hat Jira
RH2048582: Support PKCS#12 keystores in FIPS mode [rhel-8, openjdk-17]
Description
The goal of this enhancement is to support PKCS#12 keystores in FIPS mode. In order to accomplish that, we need to extend the SunPKCS11 security provider to implement Password-Based encryption algorithms (PBE). These algorithms enable key derivation (from a human-readable password) for data encryption and authentication.
We aimed to leverage on defined PKCS#11 PBE mechanisms as much as possible, instead of calling low-level crypto primitives (such as hash functions) directly. For some algorithms, we used NSS Software Token custom mechanisms that are not part of the PKCS#11 standard -this type of mechanisms is used already in OpenJDK-.
The version of the PKCS#11 standard in which we based this work is v3.0. However, we have analyzed potential backward-compatibility issues and written best-effort code for older versions.
To prioritize which PBE mechanisms to implement, we focused on those required for PKCS#12 keystores that are current (such as
PBEWithHmacSHA256AndAES_256andHmacPBESHA256, which are Keytool defaults for encryption and authentication respectively when dealing with PKCS#12 keystores). Legacy mechanisms were not implemented, since they are discouraged, they might not be FIPS-compliant and keystore migration should be feasible at a reasonable cost.It is worth noticing that the code to handle the PKCS#12 format is already implemented in the SunJCE security provider. This code gets cryptographic services by means of the Java Cryptographic Architecture (JCA), to which SunPKCS11 will now provide implementations. Using SunJCE for PKCS#12 handling should be fine under FIPS mode because no crypto transformations are applied there: it's ASN.1 binary-format parsing. However, this implies that we need to install the SunJCE security provider in FIPS mode with enough granularity to disable all other non-PKCS#12 services.
Even when the FIPS configuration is Red Hat downstream only, the extension of the SunPKCS11 security provider will be proposed to OpenJDK upstream.
Useful pointers: