On Fri, 6 Jun 2025 20:01:17 GMT, Hai-May Chao <hc...@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. I made the toggle specific to SunX509 Key Manager as suggested. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25016#issuecomment-2985653083
