smallrye / smallrye-jwt

Apache License 2.0
75 stars 48 forks source link

java.security.Provider contention #179

Open darranl opened 4 years ago

darranl commented 4 years ago

I have just been performing some profiling with SmallRye JWT within WildFly, the test has been pretty primitive hitting the server hard with simple HTTP requests containing a JWT token.

TBH most of the results look as I would expect, most of the time was spend in the operations relating to the handling of the signatures and as that is all that was happening to the requests that is as expected.

However one point of contention that comes up is syncrhonization within java.security.Provider.

Stack Trace Count Duration java.security.Provider.getService(String, String) 166,894 6,144,694,265,281 sun.security.jca.ProviderList$ServiceList.tryGet(int) 156,244 5,798,524,329,105 sun.security.jca.ProviderList$ServiceList.access$200(ProviderList$ServiceList, int) 156,244 5,798,524,329,105 sun.security.jca.ProviderList$ServiceList$1.hasNext() 156,244 5,798,524,329,105 java.security.Signature.getInstance(String) 156,244 5,798,524,329,105 org.jose4j.jws.BaseSignatureAlgorithm.getSignature(ProviderContext) 156,244 5,798,524,329,105 org.jose4j.jws.BaseSignatureAlgorithm.verifySignature(byte[], Key, byte[], ProviderContext) 156,244 5,798,524,329,105 org.jose4j.jws.JsonWebSignature.verifySignature() 156,244 5,798,524,329,105 org.jose4j.jwt.consumer.JwtConsumer.processContext(JwtContext) 156,244 5,798,524,329,105 org.jose4j.jwt.consumer.JwtConsumer.process(String) 156,244 5,798,524,329,105 io.smallrye.jwt.auth.principal.DefaultJWTTokenParser.parse(String, JWTAuthContextInfo) 156,244 5,798,524,329,105

Just raising this issue as if there is any approach where the Signature can be cached without needing to keep hitting the factory it could bypass this contention.

sberyozkin commented 4 years ago

Hi @darranl I'm not sure individual Signature objects are thread safe but this code shows that a Provider instance can be passed in.

So we can have initialized Providers statically, right now RS256 (and also ES256) are supported, and pass the right one to the Jose4j verification code. Have a look please

sberyozkin commented 4 years ago

But it would likely require tweaking the parser code a bit for the algorithm property be checked which might involve some double parsing. Perhaps a more effective approach is to have a Map of algorithm to Provider in org.jose4j.jws.BaseSignatureAlgorithm but it would require a jose4j PR which is doable too

darranl commented 4 years ago

Just finishing off some other tasks and will come and take a closer look - but +1 the actual Signature may not be re-usable but there may be some options to optimise how it is obtained to avoid hitting the synchronized code on each request.

sberyozkin commented 4 years ago

But it would likely require tweaking the parser code a bit for the algorithm property be checked which might involve some double parsing.

Ignore it please, we set a whitelist algorithm to a pre configured value, so it would be safe to pick up a statically initialized Provider from a Map with only two pairs (RS256, ES256 the keys) and pass it along :-)

sberyozkin commented 4 years ago

Hi @darranl I've looked through the Jose4j code, right now there is no way to bypass it. I'm planning to do some other Jose4j PR, so I suppose going forward we can customize Jose4 ProviderContext with Provider instance, etc, if it will help to completely eliminate the sync