Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RH2020290: Support TLS 1.3 in FIPS mode #13

Merged
merged 1 commit into from Aug 29, 2022

Conversation

franferrax
Copy link

@franferrax franferrax commented Jun 30, 2022

Search this PR in Red Hat Jira

RH2020290: Support TLS 1.3 in FIPS mode [rhel-8, openjdk-17]

Description

When OpenJDK runs on a FIPS-configured system, TLS 1.3 (implemented in the SunJSSE security provider) is disabled both on the server and client sides (RH1860986). The reason is that the PKCS#11 key derivation mechanism for TLS 1.3 is not supported in the SunPKCS11 security provider; and the SunJSSE code for key derivation would require to import plain secret keys into an NSS Software Token (blocked by RH1991003).

The goal of this task is to implement a solution to re-enable TLS 1.3 on both server and client sides when OpenJDK runs in FIPS mode.

@franferrax
Copy link
Author

Please note that as explained in #11 (1ˢᵗ comment), the Linux x64 + FIPS (jdk/tier1 part 2) step is failing because the java/util/logging/SimpleFormatterFormat.java test doesn't work with -Djava.security.debug=properties.

I haven't tried to fix it because I don't know if we are OK with removing -Djava.security.debug=properties, maybe there's an easy way to configure this per test, which I don't know.

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.

Ok, this is trivially fine, it's just a reversion of the appropriate parts of 0af22dc#diff-fc40f443cd9d2236d4279b83e6b6e662771d08faa571851f9de3de4925a2c36c

I was expecting more changes than this to make it work. Is that all in previous patches?

@gnu-andrew
Copy link

Please note that as explained in #11 (1ˢᵗ comment), the Linux x64 + FIPS (jdk/tier1 part 2) step is failing because the java/util/logging/SimpleFormatterFormat.java test doesn't work with -Djava.security.debug=properties.

I haven't tried to fix it because I don't know if we are OK with removing -Djava.security.debug=properties, maybe there's an easy way to configure this per test, which I don't know.

-Djava.security.debug=properties was added so we could debug any issues that arose from system security properties being enabled. I guess we could remove it and only add it back if we needed to diagnose issues. I wasn't expecting it to cause problems with tests. It seems odd that a test would try to parse debug output in the first place.

@gnu-andrew gnu-andrew merged commit 0bd5ca9 into rh-openjdk:fips-17u Aug 29, 2022
@franferrax
Copy link
Author

Ok, this is trivially fine, it's just a reversion of the appropriate parts of 0af22dc#diff-fc40f443cd9d2236d4279b83e6b6e662771d08faa571851f9de3de4925a2c36c

I was expecting more changes than this to make it work. Is that all in previous patches?

@gnu-andrew: Yes, the patches are RH1991003 + RH2007331   ⇒   abcd095 + fedora:java-17-openjdk:#17

I've just created this PR with @martinuy's patch from RH2020290. In that BZ ticket, he explains that SunJSSE code for key derivation required abcd095, which hadn't yet been developed at the moment 0af22dc was instroduced.

Also, he had found the problem with imported SecretKeys which were missing the CKA_SIGN attribute (SunPKCS11's import operation). Later, a customer found a very similar issue with generated SecretKeys (SunPKCS11's generate operation), so we solved both in its own issue, fedora:java-17-openjdk:#17.

@franferrax franferrax deleted the RH2020290 branch August 29, 2022 20:14
gnu-andrew pushed a commit that referenced this pull request Nov 23, 2022
#13 isn't a perfect revert of 0af22dc limited to SSLContextImpl.java and
SunJSSE.java, since it doesn't remove the SharedSecrets import. This was
already in this way in rh2020290-support_tls_1_3_in_fips.v1.patch, I'm
now realizing this when doing the OpenJDK 11 backport and retrying the
same approach in OpenJDK 17:
~~~
# Revert #13
git show 0bd5ca9 | git apply -R
# Redo #13 by reverting 0af22dc in SSLContextImpl.java and SunJSSE.java
git show 0af22dc |
  git apply -R --include=src/java.base/share/classes/sun/security/ssl/*
~~~

In #14, I forgot to delete the DHKF and DHKFLock static
variables from FIPSKeyImporter, which are no longer used,
see #14 (comment).

Reviewed-by: @gnu-andrew
gnu-andrew pushed a commit that referenced this pull request Aug 22, 2023
Co-authored-by: Martin Balao <mbalao@redhat.com>

Reviewed-by: @gnu-andrew
gnu-andrew pushed a commit that referenced this pull request Aug 22, 2023
#13 isn't a perfect revert of 0af22dc limited to SSLContextImpl.java and
SunJSSE.java, since it doesn't remove the SharedSecrets import. This was
already in this way in rh2020290-support_tls_1_3_in_fips.v1.patch, I'm
now realizing this when doing the OpenJDK 11 backport and retrying the
same approach in OpenJDK 17:
~~~
# Revert #13
git show 0bd5ca9 | git apply -R
# Redo #13 by reverting 0af22dc in SSLContextImpl.java and SunJSSE.java
git show 0af22dc |
  git apply -R --include=src/java.base/share/classes/sun/security/ssl/*
~~~

In #14, I forgot to delete the DHKF and DHKFLock static
variables from FIPSKeyImporter, which are no longer used,
see #14 (comment).

Reviewed-by: @gnu-andrew
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants