Skip to content

Conversation

@franferrax
Copy link

@franferrax franferrax commented Aug 9, 2022

Search this PR in Red Hat Jira

RH2092507: P11Key.getEncoded does not work for DH keys in FIPS mode

Description

This work fixes the P11Key.getEncoded() issue by enabling SunJCE's secret key factories in FIPS mode (except for PBKDF2 secret key factories, which perform key derivation [†], a cryptographic operation that should only be handled by SunPKCS11-NSS-FIPS).

This issue was introduced in 6e74f28 (RH2052070), when a stripped/locked-down version of the SunJCE provider was added to java.security in FIPS mode (by means of the if (!systemFipsEnabled) { /* disabled code */ } patching). SunPKCS11's P11Key.getEncoded() internally uses SunJCE's KeyFactory to encode Diffie-Hellman public keys despite SunJCE not being listed in java.security [‡]. Before 6e74f28, SunJCE didn't need to be stripped/locked-down, so SunPKCS11 was directly using it unpatched, but now that it is listed in java.security (i.e. publicly available as a provider), the lock-down patching is mandatory. Consequently, we are just enabling SunJCE key factories without cryptographic code, required by SunPKCS11 keys encoding (such as the KeyFactory for "DiffieHellman"/"DH").


[†] See the following code:

// PBKDF2
psA("SecretKeyFactory", "PBKDF2WithHmacSHA1",
"com.sun.crypto.provider.PBKDF2Core$HmacSHA1",
null);

return new PBKDF2KeyImpl(ks, prfAlgo);

this.key = deriveKey(prf, passwdBytes, salt, iterCount, keyLength);

[‡] See the following code:

// see JCA spec
public final byte[] getEncoded() {
byte[] b = getEncodedInternal();
return (b == null) ? null : b.clone();
}

KeyFactory kf = KeyFactory.getInstance
("DH", P11Util.getSunJceProvider());

static Provider getSunJceProvider() {
Provider p = sunJce;
if (p == null) {
synchronized (LOCK) {
p = getProvider
(sunJce, "SunJCE", "com.sun.crypto.provider.SunJCE");
sunJce = p;
}
}
return p;
}
@SuppressWarnings("removal")
private static Provider getProvider(Provider p, String providerName,
String className) {
if (p != null) {
return p;
}
p = Security.getProvider(providerName);
if (p == null) {
try {
final Class<?> c = Class.forName(className);
p = AccessController.doPrivileged(
new PrivilegedAction<Provider>() {
public Provider run() {
try {
@SuppressWarnings("deprecation")
Object o = c.newInstance();
return (Provider) o;
} catch (Exception e) {
throw new ProviderException(
"Could not find provider " +
providerName, e);
}
}
}, null, new RuntimePermission(
"accessClassInPackage." + c.getPackageName()));
} catch (ClassNotFoundException e) {
// Unexpected, as className is not a user but a
// P11Util-internal value.
throw new ProviderException("Could not find provider " +
providerName, e);
}
}
return p;
}

Copy link

@gnu-andrew gnu-andrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this actually relates to RHELPLAN-130890 which introduced the cryptographic restrictions when in FIPS mode, as a replacement for the upstream "experimental FIPS mode" that was removed in OpenJDK 13.
The AlgorithmParameters change in RHELPLAN-112660 is actually a similar case where the restrictions were found to be too limiting.

If these key factories are actually permissible, then this patch looks fine.

@gnu-andrew gnu-andrew merged commit 84266ee into rh-openjdk:fips-17u Aug 11, 2022
@franferrax
Copy link
Author

I believe this actually relates to RHELPLAN-130890 which introduced the cryptographic restrictions when in FIPS mode, as a replacement for the upstream "experimental FIPS mode" that was removed in OpenJDK 13. The AlgorithmParameters change in RHELPLAN-112660 is actually a similar case where the restrictions were found to be too limiting.

RHELPLAN-130890 / 765baf2 only touches the SUN and SunEC security providers. In this issue, the SunPKCS11 security provider is managing to internally access a SunJCE provider instance to encode Diffie-Hellman public keys as X.509 (through SunJCE's KeyFactory service for the "DH" algorithm). This had been happening, even when SunJCE wasn't publicly available in FIPS mode, as we can see in the fips.provider.n list from java.security, at the moment it was introduced in RHELPLAN-14794 / 61c564d, where it didn't include SunJCE.

In RHELPLAN-112660 / 6e74f28, SunJCE was added as fips.provider.5, making it publicly available in FIPS mode. For this reason, it was also stripped-down with the if (!systemFipsEnabled) { /* FIPS disabled code */ } patching. But we stripped it too much, since we hadn't noticed the "DH" KeyFactory internal usage by SunPKCS11.

If these key factories are actually permissible, then this patch looks fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants