Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DefaultSecurityProviderConfig with Bouncy Castle disabled #861

Merged
merged 4 commits into from
Jul 20, 2023

Conversation

exceptionfactory
Copy link
Contributor

This pull request address issue #782 with a new DefaultSecurityProviderConfig that extends DefaultConfig and disables Bouncy Castle Security Provider registration in a static initializer.

As described in the referenced issue, SecurityUtils.isBouncyCastleRegistered() checks current Security Providers and attempts to register Bouncy Castle according to the default settings. The DefaultConfig class checks the status of Bouncy Castle registration to determine whether to configure standard factories, but this introduces unnecessary coupling between Bouncy Castle registration and standard algorithm factories.

Changes in this pull request include removing the SecurityUtils.isBouncyCastleRegistered() check from the DefaultConfig constructor and registering all factories. The initCipherFactories() method already attempts to initialize configured Cipher Factories and disables factories when the corresponding cipher algorithm is not supported. The random factories do not depend on Bouncy Castle registration, and the Key File Provider factories may or may not need Bouncy Castle depending on the particular key protection algorithms. Removing the dependence of Bouncy Castle registration allows the rest of the library to attempt security operations as needed with the potential to support other Security Providers. It is important to note that the DefaultConfig will attempt to register the Bouncy Castle Provider when calling other SecurityUtils methods, but removing the registered check avoids tight coupling to the particular provider.

With the changes to DefaultConfig, the new DefaultSecurityProviderConfig includes a static initializer to call SecurityUtils.setRegisterBouncyCastle(false). This changes subsequent behavior and thus expects java.security settings or other external components to setup the providers necessary. SecurityUtils.setRegisterBouncyCastle() could be called without the introduction of a new configuration class, but new class provides a clear indication of expected behavior, and simple replacement for DefaultConfig when necessary. Additional adjustments include removing SecurityUtils.isBouncyCastleRegistered() from several locations where it should not be required.

@exceptionfactory
Copy link
Contributor Author

Thanks for rebasing this PR @hierynomus. What do you think about these changes?

As stated in the description, the net effect is removing Bouncy Castle registration has a hard-coded requirement for the standard algorithms and key providers.

At this time, Bouncy Castle is still required for some of the Private Key parsing, and also required for certain algorithms on Java 8, like ChaCha20-Poly1305 and X25519. However, it does move more in the direction of making it optional in some use cases.

@hierynomus hierynomus merged commit 3069138 into hierynomus:master Jul 20, 2023
7 checks passed
@hierynomus
Copy link
Owner

It's merged :) For now I think this is indeed the best to be done with removing the need for BC.

@exceptionfactory
Copy link
Contributor Author

Thanks @hierynomus!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants