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

Remove dependency on BouncyCastle #47

Closed
jakubgs opened this issue Dec 6, 2022 · 6 comments
Closed

Remove dependency on BouncyCastle #47

jakubgs opened this issue Dec 6, 2022 · 6 comments

Comments

@bitgamma
Copy link
Contributor

bitgamma commented Dec 6, 2022

BouncyCastle is used as the crypto provider through JCE, so while the depedency is not apparent from code (except for those two methods which of course do not warrant by themselves pulling that lib), we actually need it at runtime for the crypto stuff to work properly

@jakubgs
Copy link
Author

jakubgs commented Dec 6, 2022

Okay, then if that is the case, then should we upgrade it?

@jakubgs
Copy link
Author

jakubgs commented Dec 6, 2022

Also, such an indirect runtime dependency should probably be clarified in a comment, preferably in build.gradle.

@jakubgs
Copy link
Author

jakubgs commented Dec 6, 2022

The current version is 1.60, which came out in Jul 06, 2018:
https://mvnrepository.com/artifact/org.bouncycastle/bcprov-jdk15on/1.60

Which apparently is affected by CVE-2020-15522:

Bouncy Castle BC Java before 1.66, BC C# .NET before 1.8.7, BC-FJA before 1.0.1.2, 1.0.2.1, and BC-FNA before 1.0.1.1 have a timing issue within the EC math library that can expose information about the private key when an attacker is able to observe timing information for the generation of multiple deterministic ECDSA signatures.

If we use it more widely at runtime that might be something we care about.

@bitgamma
Copy link
Contributor

bitgamma commented Dec 6, 2022

the reason why it hasn't been upgraded is that trying to do so generated builds of status-mobile which crashed at runtime. Since status-mobile is indirectly dependent on bouncycastle through other dependencies as well, I guessed it was some kind of conflict between required versions.

I have checked that CVE but it doesn't affect us because we don't use BouncyCastle for ECDSA signing. We use it for verifying, for ECDH, AES, DES and SHA in the context of Keycard. Outside Keycard I don't know exactly where it is used, but all blockchain related crypto is done in status-go so it is probably used for TLS (again, ECDH).

We could try again upgrading it and investigating what breaks the app at runtime, but unless there is a pressing reason to upgrade I would say it is low-prio.

@jakubgs
Copy link
Author

jakubgs commented Dec 6, 2022

Thanks for explaining, then I will reopen the issue:

Could you please also paste your comment there?

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

No branches or pull requests

2 participants