RFR: 8353113: Peer supported certificate signature algorithms are not being checked with default SunX509 key manager [v3]

[Hai-May Chao]

On Thu, 15 May 2025 19:29:07 GMT, Sean Mullan <mullan at openjdk.org> wrote:

> > > It is nice to refactor the common code for algorithm constraints checking into a new class, `X509KeyManagerConstraints.java`, used by both `SunX509KeyManagerImpl` and `X509KeyManagerImpl`. However, it looks like a new system property, "jdk.tls.keymanager.disableConstraintsChecking", is introduced, and it will affect both `SunX509KeyManagerImpl` and `X509KeyManagerImpl`. Should the property be a switch for SunX509 KeyManager, not a general toggle for all KeyManager? Avoiding its misuse for `X509KeyManagerImpl` that may lead to disable the existing RFC compliant algorithm constraints checking? It might be preferable to keep the property logic in `SunX509KeyManagerImpl` (not in the common code).
> > 
> > 
> > @haimaychao Thanks for looking into it! Yes, it will disable constraints checking for both key managers and I did it this way on purpose. I think it will be simpler and less confusing to the end users. This system property is off by default and my assumption is that if end users want to disable KM algorithm constraints checking they would expect it to be disabled system-wide. Making this toggle SunX509-specific is a trivial change if we have a consensus on this.
> > @seanjmullan What do you think?
> 
> Need to think about it some more, but I am kind of leaning towards it only affecting SunX509. The main benefit of the property is to workaround any compatibility issues where current code is not ready for the change. Any application already using the PKIX TrustManager already has this checking enabled/enforced.

I'd agree. As I mentioned in my earlier comment, if the new system property ends up toggling behavior in both `SunX509KeyManager` and `X509KeyManagerImpl`, we could run into an unintended side effect. While we're adding compliant algorithm constraints checking to `SunX509KeyManager`, turning on the property to disable it for compatibility reasons would also disable the already-existing checking in `X509KeyManagerImpl`. The applications already relying on the stricter checks in `X509KeyManagerImpl` might lose that enforcement unintentionally.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25016#issuecomment-2950663297