AlgorithmConstraints caching [ was Re: RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice]

[Anthony Scarpino]

Hi Sean,

Caching is an interesting idea.  I've wondered for a while off and on 
about how to speed it up, but hadn't come up with a solution I liked. 
The complication with caching is while something like an algorithm name 
only could be easy in a hashmap, it gets more complicated when you get 
into key sizes. Such as, how to represent RSA 1k being disallowed and 
but 2k allowed.. or certificate usage..

Tony

On 4/13/22 2:03 AM, Sean Coffey wrote:
> On Tue, 12 Apr 2022 11:28:12 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:
> 
>> During TLS handshake, hundreds of constraints are evaluated to determine which cipher suites are usable. Most of the evaluations are performed using `HandshakeContext#algorithmConstraints` object. By default that object contains a `SSLAlgorithmConstraints` instance wrapping another `SSLAlgorithmConstraints` instance. As a result the constraints defined in `SSLAlgorithmConstraints` are evaluated twice.
>>
>> This PR improves the default case; if the user-specified constraints are left at defaults, we use a single `SSLAlgorithmConstraints` instance, and avoid duplicate checks.
> 
> Nice work. I've been looking at this area myself in recent weeks also while debugging some JDK 11u performance issues. JFR shows this area as costly. Some other JDK fixes in this area have already improved performance. This will certainly help. Pasting a stacktrace[1] from an old Oracle JDK 11.0.12 report for history purposes.
> 
> I think there are other improvements that can be made in the TLS constraints code. Caching the last known values from a constraints permits test is something I've begun looking into.
> 
> [1]
> Stack Trace	Count	Percentage
> boolean java.lang.StringLatin1.regionMatchesCI(byte[], int, byte[], int, int)	1637	100 %
> boolean java.lang.String.regionMatches(boolean, int, String, int, int)	1637	100 %
> boolean java.lang.String.equalsIgnoreCase(String)	1637	100 %
> boolean sun.security.util.AbstractAlgorithmConstraints.checkAlgorithm(List, String, AlgorithmDecomposer)	1631	99.6 %
> boolean sun.security.util.DisabledAlgorithmConstraints.permits(Set, String, AlgorithmParameters)	1631	99.6 %
> boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, AlgorithmParameters)	1631	99.6 %
> boolean sun.security.ssl.SSLAlgorithmConstraints.permits(Set, String, AlgorithmParameters)	836	51.1 %
> boolean sun.security.ssl.HandshakeContext.isActivatable(CipherSuite, AlgorithmConstraints, Map)	428	26.1 %
> List sun.security.ssl.HandshakeContext.getActiveCipherSuites(List, List, AlgorithmConstraints)	418	25.5 %
> void sun.security.ssl.HandshakeContext.<init>(SSLContextImpl, TransportContext)	418	25.5 %
> void sun.security.ssl.ClientHandshakeContext.<init>(SSLContextImpl, TransportContext)	418	25.5 %
> void sun.security.ssl.TransportContext.kickstart()	418	25.5 %
> void sun.security.ssl.SSLSocketImpl.startHandshake(boolean)	418	25.5 %
> void sun.security.ssl.SSLSocketImpl.startHandshake()	418	25.5 %
> 
> -------------
> 
> Marked as reviewed by coffeys (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/8199