8272875: Change the default key manager to PKIX

[artur-oracle]

The current key manager is SunX509, which is configured in the java.security. The SunX509 algorithm does not check the local certificate. The PKIX algorithm should be preferred now so that the default key manager could be more robust.

Compatibility considerations:

1. Customers using local certificates signed using algorithms prohibited by the default configuration (notably MD5 and SHA1) no longer will be able to use such certificates without modifying algorithm constraints in java.security config file.

2. Performance impact: there is about x2 performance decrease for full (non-resume) TLS handshake:

SUNX509
Benchmark (resume) (tlsVersion) Mode Cnt Score Error Units
SSLHandshake.doHandshake true TLSv1.2 thrpt 15 19758.012 ± 758.237 ops/s
SSLHandshake.doHandshake true TLS thrpt 15 1861.695 ± 14.681 ops/s
SSLHandshake.doHandshake false TLSv1.2 thrpt 15 1186.962 ± 12.085 ops/s
SSLHandshake.doHandshake false TLS thrpt 15 1056.288 ± 7.197 ops/s
Finished running test 'micro:java.security.SSLHandshake'

PKIX
Benchmark (resume) (tlsVersion) Mode Cnt Score Error Units
SSLHandshake.doHandshake true TLSv1.2 thrpt 15 19724.887 ± 393.636 ops/s
SSLHandshake.doHandshake true TLS thrpt 15 1848.927 ± 22.946 ops/s
SSLHandshake.doHandshake false TLSv1.2 thrpt 15 511.684 ± 5.405 ops/s
SSLHandshake.doHandshake false TLS thrpt 15 490.698 ± 6.453 ops/s
Finished running test 'micro:java.security.SSLHandshake'

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Change requires CSR request JDK-8355219 to be approved
• Commit message must refer to an issue

Issues

• JDK-8272875: Change the default key manager to PKIX (Enhancement - P3)
• JDK-8355219: Change the default key manager to PKIX (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/24756/head:pull/24756
$ git checkout pull/24756

Update a local copy of the PR:
$ git checkout pull/24756
$ git pull https://git.openjdk.org/jdk.git pull/24756/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 24756

View PR using the GUI difftool:
$ git pr show -t 24756

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/24756.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back abarashev! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@artur-oracle The following labels will be automatically applied to this pull request:

• core-libs
• net
• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (2b0c5525)
• 01: Full - Incremental (e5e83514)
• 00: Full (19a2ad1d)

[ascarpino]

Do we understand why this is so much slower? I wouldn't have thought extra checking would cause this big of a performance hit.

[AlanBateman]

/label remove core-libs

[openjdk]

@AlanBateman
The core-libs label was successfully removed.

[artur-oracle]

Do we understand why this is so much slower? I wouldn't have thought extra checking would cause this big of a performance hit.

Yes, it looks that way. SunX509 KeyManager is really simple, so adding certificate validation can decrease the performance significantly. The discussion of #17956 contains an extensive performance analyses.

[djelinski]

The discussion of #17956 contains an extensive performance analyses.

TL;DR: PKCS12 decrypts the private key before every use. The performance hit comes from applying PBKDF2 to the key encryption password.

SunX509 caches the private keys during initialization. PKIX always reads them directly from the keystore.

[ascarpino]

The discussion of #17956 contains an extensive performance analyses.

TL;DR: PKCS12 decrypts the private key before every use. The performance hit comes from applying PBKDF2 to the key encryption password.

SunX509 caches the private keys during initialization. PKIX always reads them directly from the keystore.

Is this a reflection of the perf test and not something seen in the real world, or something that needs to be fixed before or soon after this PR is integrated?

There doesn't seem to be much concern about a 2x slowdown which I'm a bit surprised about.

[djelinski]

This slowdown is seen in the real world. It is concerning, but not easily fixable.

I am not in the TLS server business at the moment, but the cases I used to work with [*] were perfectly well served by SunX509, so I guess some users will just keep using that. The fix for the PKIX+PKCS12 speed is not exactly easy. The options we explored were either incompatible with the existing implementation, or introduced subtle bugs in some corner cases.

[*] The servers I used to work with had either only one certificate, or one RSA and one EC certificate. We had to manually disable the TLS_RSA and TLS_ECDH ciphers, but these are disabled by default today. SunX509 serves that situation pretty well.

[artur-oracle]

I have a few points for making the change:

1. On my laptop the handshake time increased from 1ms to 2ms. So while it's a x2 increase it's not going to be noticeable.
2. I'm not 100% sure, but from what I read at least the half of the TLS connections these days are of resume type, and the performance for those is unchanged. Here is a good article from CloudFlare on this topic: https://blog.cloudflare.com/tls-session-resumption-full-speed-and-secure. They set session ticket lifetime to 18h.
3. Unlike SunX509, PKIX KeyManager checks local certificate signature algorithms against local algorithm constraints and also against peer-supported algorithms supplied by the peer. So technically we are in violation of TLSv1.3 RFC when using SunX509 because we ignore peer-supported certificate signature schemes. Also we don't respect our own algorithm constraints in java.security for local certificates which is the behavior users may expect.

[djelinski]

Well, technically the current 1ms takes into account both the client side and the server side of the handshake. Last time I checked, they were more or less evenly split. The 1ms slowdown will happen entirely on the server side, so it's more of a 3x slowdown.

Most of the time an extra millisecond doesn't matter, but it can be a factor in the server startup time on a busy server. We currently invalidate all session resumption tickets during server restart.

Re your point 3: what's the observable difference? If the certificate we can offer doesn't meet the peer's expectations, will we fail the handshake on the server side instead of failing on the client side, or is it something else?

[artur-oracle]

Well, technically the current 1ms takes into account both the client side and the server side of the handshake. Last time I checked, they were more or less evenly split. The 1ms slowdown will happen entirely on the server side, so it's more of a 3x slowdown.

Yes, good point, thanks!

Most of the time an extra millisecond doesn't matter, but it can be a factor in the server startup time on a busy server. We currently invalidate all session resumption tickets during server restart.

I see. But at the same time my understanding is that PKIX KeyManager's boot-up should be faster because it doesn't cache the whole KeyStore.

Re your point 3: what's the observable difference? If the certificate we can offer doesn't meet the peer's expectations, will we fail the handshake on the server side instead of failing on the client side, or is it something else?

Yes, we fail on the server side if we can't offer a certificate passing peer-supported algorithms in "signature_algorithms_cert" extension.

I put together a prototype of yet another KeyManager which can be a compromise solution: it's basically a SunX509 with algorithm constraints: #25016
Performance is pretty much the same as SunX509, constraints checking doesn't seem to take much time.

[bridgekeeper]

@artur-oracle This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[artur-oracle]

/touch

[openjdk]

@artur-oracle The pull request is being re-evaluated and the inactivity timeout has been reset.