Remove Bouncycastle as the default provider fixes #52

[brakthehack]

Previously, the library placed an artificial constraint needing
the BouncyCastle provider to perform cryptographic functions
when it was not necessary to do so.

This commit removes the requirement to both require BouncyCastle
as the sole provider, as well as requiring the BouncyCastle
dependency at runtime.

If the user wishes to use BouncyCastle, they should add the provider
as is done in the tests. The library will automatically use
whatever the preferred provider is instead of BouncyCastle.

[Ophirr33]

👍 , I agree

[pauldijou]

Just to be sure, the migration path for users would be to add BouncyCastle as a dependency and then do:

Security.addProvider(new BouncyCastleProvider())

Right?

[pauldijou]

Could you add a link to the original source code in the description? I would like to take a look. This line seems strange. Since expected == expected, it will most likely return true immediately from L22 and then negate it. Would be faster to just return false here.

[brakthehack]

Sure, not a problem. You can find the link here: https://github.com/bcgit/bc-java/blob/290df7b4edfc77b32d55d0a329bf15ef5b98733b/core/src/main/java/org/bouncycastle/util/Arrays.java#L145. FWIW, it looks like this snippet originates from an older commit and there has been some recent development in that area to improve the function. If you'd prefer a particular revision, I'd be happy to use that over this revision.

[Ophirr33]

@brakthehack it might be good to document what functionality in jwt relies on BouncyCastle. Is there a subset of functionality that doesn't rely on any additional security providers?

[brakthehack]

Just to be sure, the migration path for users would be to add BouncyCastle as a dependency and then do:

Security.addProvider(new BouncyCastleProvider())

Right?

You've got it.

[brakthehack]

@brakthehack it might be good to document what functionality in jwt relies on BouncyCastle. Is there a subset of functionality that doesn't rely on any additional security providers?

So, I'm not all that familiar with the code base so I'm not sure if I can totally confirm, but my take is that this change affects the algorithms used, which is specified by the user. For example, if the user's token uses RSA signatures, it will probably work out of the box. However, if we remove BC as the provider in the tests we see a couple of tests in the core project fail:

[info] JwtSpec:
[info] pdi.jwt.JwtSpec *** ABORTED ***
[info]   java.security.NoSuchAlgorithmException: ECDSA KeyPairGenerator not available
...
[info]   transcodeSignatureToConcat and transcodeSignatureToDER
[info]   - should be symmetric
[info]   - should be symmetric for generated tokens *** FAILED ***
[info]     java.security.NoSuchAlgorithmException: ECDSA KeyPairGenerator not available

The primary reason is NoSuchAlgorithmException because it appears the default provider on my system does not provide a generator for ECDSA algorithms.

So to answer your question, it's system-specific which is why we probably should shy away declaring what is and is not supported.