JDK-8345114 : Enhance OCSP, CRL and Certificate Fetch Timeouts
  • Type: CSR
  • Component: security-libs
  • Sub-Component: java.security
  • Priority: P4
  • Status: Closed
  • Resolution: Withdrawn
  • Fix Versions: 17-pool
  • Submitted: 2024-11-27
  • Updated: 2024-12-26
  • Resolved: 2024-12-26
Related Reports
CSR :  
JDK-8338808 - Enhance OCSP, CRL and Certificate Fetch Timeouts
Description
Summary
-------

Allow the existing com.sun.security.ocsp.timeout, com.sun.security.crl.timeout and com.sun.security.crl.readtimeout System properties to be specified in milliseconds by appending "ms" to the end of the numeric value. In addition three new properties will be introduced. The first is com.sun.security.ocsp.readtimeout which will provide a timeout specifically for the reading of an OCSP response after a connection has been established. The other two properties are for certificate fetching timeouts based on an X.509 certificate's AIA extension: com.sun.security.cert.timeout and com.sun.security.cert.readtimeout. All three properties have the same syntax requirements as the existing timeout properties above. The syntax specifics are detailed in the Solution section.

Note, this backport CSR is same as original CSR (JDK-8300722).

Problem
-------

The existing property value must be a decimal integer to be interpreted in seconds. Given the average latency for OCSP, certificate, and CRL fetches can be less than one second it is sensible to allow the user to specify a timeout period with millisecond granularity. While OCSP and CRLs have timeout properties already, CA issuer certificate fetching can only be done through API calls and no System properties exist as they do for OCSP and CRL fetches. This would introduce timeout properties that behave in a similar fashion to the OCSP and CRL timeout properties that prevent indefinite stalls during fetches.

Solution
--------

 - Enhance the allowed syntax for the existing `com.sun.security.ocsp.timeout`, `com.sun.security.crl.timeout` and `com.sun.security.crl.readtimeout` as detailed below.
 - Create three new properties `com.sun.security.ocsp.readtimeout`, `com.sun.security.cert.timeout` and `com.sun.security.cert.readtimeout` with the same syntax as those above.
  - `com.sun.security.ocsp.readtimeout` will hold the timeout value for reading an OCSP response after a connection has been established.  The `com.sun.security.crl.timeout` property will now handle only the timeout for establishing the TCP connection to the OCSP responder.  Previously the latter property's timeout value was applied to both kinds of timeouts.  This new property allows users to independently control the two kinds of timeouts similar to how other connect/read timeout property pairs are handled.
   - `com.sun.security.cert.timeout` and `com.sun.security.cert.readtimeout` handle timeouts for establishing the TCP connection and certificate reading, respectively, when following an X.509 certificate's AIA extension.

For all properties, existing and new, the proposed expanded syntax will conform to the following:

- A decimal integer: This will maintain the existing behavior of being interpreted in seconds.  This ensures backward compatibility.  If a non-numeric or negative value is supplied, the default timeout value of 15 seconds will be applied.  The default value is also the current behavior.
- A decimal integer ending in "s" (case-insensitive, no space) appended to it.  This will also be interpreted in seconds.
- The user may specify a decimal integer value with "ms" (case-insensitive, no space) appended to it.  This will be interpreted by the OCSP or URICertStore subsystems as milliseconds.  For example, a value of "2500ms" will be a 2.5 second timeout.
- As with the current behavior, non-numeric, non-decimal (e.g. hexadecimal values prepended by "0x", etc) values will be interpreted as illegal and will default to the 15 second timeout.  The same is true for negative values.
- Whether the value is interpreted in seconds or milliseconds, a value of zero will disable the timeout.
- For the newly proposed certificate fetching properties, the `com.sun.security.enableAIAcaIssuers` property must be set to true in order for fetching to occur and these property timeouts to be enabled.

Specification
-------------

There are no specification changes since the classes that act upon these properties are not exported through the java.base module.
Comments
Hello [~jnibedita], [~fferrari], [~jnimeh] Thank you for the positive discussion. I've moved the CSR https://bugs.openjdk.org/browse/JDK-8337407 to the Proposed state. It addresses option B's solution for this enhancement.
23-12-2024
Thanks [~jnimeh]. Option B sounds good to content. [~abakhtin] would you please propose the CSR JDK-8337407 and I will close this CSR as duplicate.
23-12-2024
Hello [~jnibedita], [~fferrari] and [~abakhtin], First off, Happy Holidays! It wouldn't feel like the holiday season without an end-of-year technical decision with a ticking clock attached to it, right? :) It sounds like most folks recognize that the on-the-ground technical risk of this is small whichever way we go, based on the comments that most folks don't have strong feelings about any of the options. It feels like we're a little stuck on the decision so why don't we proceed with 17 having the mainline change plus the compat fix (basically option B). After it is integrated and backported as far as it plans to go, then for JDK 25 (the next LTS release) I will make a small bugfix to address the deviation between the original mainline fix and the enhanced 17. That could get backported to the post JDK-17 LTS releases (and whatever non-LTS releases the sustaining team wants). It will be a one or two line change in mainline and should backport very easily. As long as the behavior ultimately is synchronized between the JDK-17 and JDK-21 (and however farther back the backport is planned) and 25+ I think we'll be in good shape in the end. Can we all be content with that? Am I missing anything in terms of things to consider?
23-12-2024
[~fferrari] I could understand your point of compatibility but I am also thinking in perspective of consistency/deviation of change history. I am open for either default alignment backport or adjust 21+ in separate enhancement. [~jnimeh] would you like to give your comment here please. Note: Would like to expedite the CSR review and close in priority, as having a customer requirement to backport the enhancement for Jan CPU. However we feel 17u backport of the enhancement for April CPU with a follow-on BPR for Jan CPU.
23-12-2024
[~jnibedita]: I'm not for nor against aligning with mainline, I think it should be acceptable, as long as you acknowledge the risks and are ok with them. In any case, I'm not a Reviewer (not even a Committer yet), so my opinion isn't very relevant. Are you planning to ship the backport in the next January CPU (17.0.14-oracle)? Rampdown finishes on December 31, and [JDK-8338808](https://bugs.openjdk.org/browse/JDK-8338808 "Enhance OCSP, CRL and Certificate Fetch Timeouts") actually targets the April CPU (17.0.15-oracle).
20-12-2024
Since a customer has requested inclusion of JDK-8179502 backport in Jan CPU 2025, would like to expedite the CSR review and close. We can align the change as mainline JDK and in a separate bug the discussed change can be adjusted from feature release and URs.
20-12-2024
Hmm. I don't have that strong of an opinion here. My default choice would be to align the backport with the behavior from the initial release this feature went in. If that should be adjusted, can be adjusted in the current feature release and backported, etc.
12-12-2024
Hi [~jnimeh] and [~fferrari], Thank you for considering and discussing all options. As mentioned by Francisco, `the behavioral deviation should be more acceptable at a major version upgrade than between two 17 versions`. So, I think for this particular backport, option B is preferable to bring the necessary functionality into OpenJDK17 without breaking behavior, leaving the possibility of adjusting 21+ as a separate enhancement (if required).
09-12-2024
Hi [~jnimeh], I agree with all your points, and I'm also concerned about the deviation between _17_ and _21+_, but I don't have a strong opinion. On one hand, the default value for `com.sun.security.ocsp.readtimeout` has been 15 seconds in every JDK where this property existed so far. On the other, [~goetz] has a point in that the correct backwards-compatible default should have been `com.sun.security.ocsp.timeout`. As we didn't realize this earlier, all we can do is to deal with some compatibility vs consistency trade-offs: **A)** If this backport makes _17_ behave as _21+_ (current oracle backport proposal without anything else), users could get an OCSP read-timeout of 15 seconds after upgrading to a newer _17_, while they were previously getting what was configured as the OCSP (connect and read) timeout. This can degrade the performance if the OCSP (connect and read) timeout was smaller than 15 seconds, or otherwise lead to more read-timeouts. **B)** If this backport makes _17_ use the backwards-compatible default (current OpenJDK backport proposal without anything else), users should remain unaffected after upgrading to a newer _17_, but could find the problem latter when upgrading to _21+_. **C)** If _17_ uses the backwards-compatible default, but we also make this change in _21+_ ([~jnimeh]'s offered alternative), users of _21+_ setting the OCSP connect-timeout and expecting the read-timeout to default to 15 seconds may experience issues when upgrading to a newer _21+_. As [~jnimeh] pointed out, **C** would be cleaner if achieved by combining **A** with a separate enhancement for 25, backported to _17_ and _21+_. I agree that combining **B** with a separate enhancement for 25, backported to only _21+_ would ofuscate the chain of events. Consistency-wise, **A** is the best candidate: each of the three pairs of timeout properties behave the same in _17_ and _21+_, additionally, they follow the precedent set by the CRL timeout properties (available even in _8_). **C** has some inconsistencies across properties: in _17_ and _21+_, the CRL timeout properties behave like the CERT timeout ones, but the OCSP timeout properties behave differently. **B** has some inconsistencies across versions and properties: _17_ is like **C**, _21+_ is like **A**. In terms of compatibility, **B** is better than **A**, as the behavioural deviation should be more acceptable at a major version upgrade than between two _17_ versions. **C** compares with **B** for _17_, and introduces a minor compatibility risk for _21+_ (IMHO is unlikely that _21+_ users are setting an OCSP connect-timeout while leaving the read-timeout as 15 seconds). In exchange, **C** reduces a longer term compatibility risk when upgrading from _17_ to _21+_. Finally, I have also considered making all the read-timeout properties default to the value of the corresponding connect-timeout ones. However, as mentioned, the CRL timeout properties are available even in _8_ and have set a behavioral precedent and expectation.
04-12-2024
Agree with points from [~jnimeh]. While on CSR, can have one for 17-pool but need to conclude if we are deviating from mainline or continue same as mainline and in separate bug can address the default value to align between the readtimeout and timeout OCSP properties. I prefer to align as mainline JDK.
04-12-2024
[~fferrari] I understand the desire for the default value to align between the `readtimeout` and `timeout` OCSP properties. In JDK-8337407 I even said it made sense (and it still does). However I'm beginning to rethink whether doing it in 17 in this backport is the best course of action. Since it didn't go into 21 originally, for better or worse, it creates a deviation in behavior between earlier revs and later ones. Francisco, I believe you alluded to that, but maybe you were speaking about 17 vs.17-oracle. I'm concerned about the deviation between 17 and 21+. It seems like it would be more sensible to make 17 and earlier align with what went into 21, keeping the changes to each version basically the same, then make a small enhancement to JDK mainline to make the default `readtimeout` be `timeout` and backport that to all interested versions. When one goes back and looks at the commit history across all these versions it will be easier to see the chain of events, rather than trying to patch-forward this default change to fill in the 21-24+ missing versions after doing the default `readtimeout/timeout` changes initially in JDK 17. I'm definitely open to dissenting opinions on this and unfortunately time is short, but I can definitely do the change going forward for JDK mainline quickly since it is a one liner. The question would be how quick we could get CSRs through given JDK-24's timetable. Or perhaps we could do it for 25. We definitely want it resolved in an LTS release.
03-12-2024
Thank you all for your clarifications, to add some context, the difference with the OpenJDK backport CSR is in the default value for `com.sun.security.ocsp.readtimeout`. The original CSR (and this backport CSR) uses 15 seconds as the default, whereas the OpenJDK backport CSR uses the value of `com.sun.security.ocsp.timeout`. This is done in the sake of backwards compatibility, as noticed by [~goetz] [here](https://github.com/openjdk/jdk17u-dev/pull/2754#issuecomment-2468336412). I think it would be ideal to reach an agreement on this decision, so that both JDKs behave the same.
03-12-2024
The preferred outcome is for Oracle JDK 17u and OpenJDK 17u to have the same changes for "Enhance OCSP, CRL and Certificate Fetch Timeouts." That can be accomplished by a single CSR with a fix value of "17-pool". The Skara bots interpret "17-pool" as applying to both Oracle JDK 17 and OpenJDK 17. It is possible, but neither desirable nor necessary, for a CSR to have a fixVersion field set to "17-pool, 17-pool-oracle" when the change in both sibling release trains is the same. Since JDK-8337407 was created before this CSR, I propose the changes undergo any further discussions there.
02-12-2024
I believe you can have multiple versions in the Fix Version field of the CSR so long as the change is the same in each train. It is discussed here: https://wiki.openjdk.org/display/csr/CSR+FAQs
02-12-2024
I think we should get to a common agreement of how to backport this change. If so, we can go with one CSR as it is the case most of the time.
02-12-2024
Hi [~jnibedita], please also take a look at the discussion in the OpenJDK 17 backport CSR: https://bugs.openjdk.org/browse/JDK-8337407 As it currently is, we have diverging behavior between the Oracle JDK 17 backport CSR and the OpenJDK 17 backport CSR. I don't have experience with this situation, should each backport have its own CSR? Or should there exist a single one? CC: [~jnimeh], [~abakhtin], [~goetz]
29-11-2024