RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice

Claes Redestad redestad at openjdk.java.net
Tue Apr 12 15:44:34 UTC 2022


On Tue, 12 Apr 2022 15:19:41 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 73:
>> 
>>> 71: 
>>> 72:     static AlgorithmConstraints wrap(AlgorithmConstraints userSpecifiedConstraints) {
>>> 73:         if (userSpecifiedConstraints == DEFAULT) {
>> 
>> Just thinking out loud: It seems all this does when `userSpecifiedConstraints` is a `SSLAlgorithmConstraints` is force the `enableX509..` flag to `true`. So in addition to the obvious thing for `DEFAULT`, you could also return `DEFAULT` for `DEFAULT_SSL_ONLY`. Or more generally: if `userSpecifiedConstraints instanceof SSLAlgorithmConstraints` then you could either return `userSpecifiedConstraints` as-is if `enabledX509DisabledAlgConstraints` is `true` or else return a clone of it with `enabledX509DisabledAlgConstraints` set to `true`.
>
> While this is technically true, `SSLAlgorithmConstraints` is an internal class, so it's very unlikely that we will ever get `SSLAlgorithmConstraints` other than `DEFAULT` here.

Right, I see even `DEFAULT_SSL_ONLY` is only used statically in one place. 

So the patch is probably good enough. Out of scope here, but if these permits-calls are (somewhat) performance-sensitive and the `DEFAULT` object is likely the only instance of `SSLAlgorithmConstraints` we'll ever see then perhaps it should be a specialized implementation that avoid the always-null `userSpecifiedConstraints != null` and `peerSpecifiedConstraints != null` checks.

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

PR: https://git.openjdk.java.net/jdk/pull/8199



More information about the security-dev mailing list