X509Factory cache control

[Sean Mullan]

Hi Eirik,

Thanks for thinking about this issue. First, I agree that some changes 
are probably useful, but we probably need to discuss this more, and 
maybe try to address the more pressing issues first. Some of my thoughts:

- I don't think the cache should be removed. I think it can improve 
performance (especially for certs), and is not an unreasonable thing for 
the JDK to try to help with.

- I do think there should be a *supported* way to turn each of the 
caches off individually. If an application has their own caching 
mechanism, then they should be able to use it instead of the JDKs.

- We have evidence from at least 2 customers that the CRL cache is the 
more problematic issue. I don't remember the history, but I think the 
"750" size may have been chosen arbitrarily to match the certificate 
cache. As a short-term fix, I would probably be ok with lowering this 
default value to something more reasonable, but I am not sure what that 
should be offhand.

- I think knobs to control the cache properties sound like they would be 
useful, but I also think many users would not know what to use as 
reasonable values, and may end up just turning off the cache and seeing 
if that helps.

- I'm a little surprised the SoftReferences still caused so many entries 
to be retained. A comment (which may be out of date) in 
sun.security.util.Cache.java says that they may be purged too eagerly, 
which would seem to have helped the issues that the customers reported. 
Some testing may be in order to see what the current behavior is with 
different VM settings.

--Sean

On 4/24/23 2:32 AM, Eirik Bjørsnøs wrote:
> Hi,
> 
> When reaching out to the BouncyCastle community regarding the deprecated 
> javax.security.cert APIs, I got some interesting feedback from Matti 
> Aarnio of Methics Ltd:
> 
>     Long ago we did encounter problems with JRE's X509CertImpl.java
>       class, and more so with X509CRLImpl.java. Both have internal
>     caches of binary form input that is then converted to parsed forms.
> 
>     Especially the CRL cache did not (still does not?) have nice "that
>     version is obsoleted, drop it to garbage collection" handling of
>     parsed CRLs, while it had way too large cache size.
>     A server running for months and getting a new CRL every 12 hours
>     eventually grew the memory footprint of obsolete parsed data up to
>     ridiculous amounts of gigabytes of heap footprint.
> 
>     Once we realized that also certificate parser had such a cache and
>     that there is no official way to control that cache, we became very
>     much averse of using those APIs. 
> 
> It seems the lack of cache control of these static implementation caches 
> can be problematic, especially in the CRL case. The default for both 
> caches is a max number of entries of 750 with an unlimited lifetime. The 
> inability of expiring outdated CRLs seems particularly problematic 
> for Matti.
> 
> A similar issue was reported in JDK-8059007 back in 2014, with the 
> following comment from Sean:
> 
>     We need to introduce some system properties (or something similar)
>     to allow the crl/cert caches to be adjusted. Changing to RFE.
> 
> 
> While it would certainly be nice to add some form of cache control, the 
> solution space is large:
> 
> -1: Remove the cache. One could claim that the parser/factory level is 
> the wrong level to introduce parsing. There are so many ways to tune 
> parsing, there exists many excellent libraries. If apps need caching, 
> they should take care of it at the app level.
> 
> 0: Do nothing, there haven't been that many complaints
> 
> 1: Add an option to entirely disable the cache such that apps can do 
> their own caching (or not) without wasting space in the default cache.
> 
> 2: Add two system properties to control max entries of the cert and crl 
> cache. Setting these to 0 would disable the cache.
> 
> 3: Add four system properties to control max entries and max lifetime 
> for each of the cert and crl caches. Setting max entries to 0 would 
> disable a cache.
> 
> 4: Tune the default max entries and lifetime to make everyone happy.
> 
> 5: Add a pluggable API to control caching. Think 
> AbstractCacheControllingMaxEntrySizeProxyFactoryContext :-)
> 
> 6: Something I did not consider.
> 
> -1 is probably not desired/possible because of the behavioural change. 1 
> is simple but very constrained. 2 and 3 are similar in shape where 3 
> allows more flexibility introducing lifetime. 4 seems like wishful 
> thinking. You could probably guess I'm not a big fan of 5. 6 seems 
> promising :-)
> 
> I have a sketch implementation of 3 available, but it would be great to 
> gather some opinions before I put more work into this:
> 
> https://github.com/openjdk/jdk/compare/master...eirbjo:jdk:x509factory-cache-config <https://github.com/openjdk/jdk/compare/master...eirbjo:jdk:x509factory-cache-config>
> 
> Thanks,
> Eirik.
> 
> [1] https://bugs.openjdk.org/browse/JDK-8059007 
> <https://bugs.openjdk.org/browse/JDK-8059007>
> 
> 
>