Remove dependency on BouncyCastle
Closed this issue · 6 comments
As far as I can tell we are pulling a big dependency in the form of BouncyCastle:
https://github.com/status-im/react-native-status-keycard/blob/9c5b897991ea4512f3131155af5cc80ac7fc944a/android/build.gradle#L47
Purely because we use Hex.decode() and Hex.toHexString() in this library:
https://github.com/status-im/react-native-status-keycard/blob/7548329c597a6e1370010b8aa2d16a1d93610f11/android/src/main/java/im/status/ethereum/keycard/SmartCard.java#L528
https://github.com/status-im/react-native-status-keycard/blob/b9f242aa11afbb6a79c269459c48e32a6091176c/android/src/main/java/im/status/ethereum/keycard/Installer.java#L63
https://github.com/status-im/react-native-status-keycard/blob/7548329c597a6e1370010b8aa2d16a1d93610f11/android/src/main/java/im/status/ethereum/keycard/SmartCard.java#L168-L170
Seems silly to pull such a massive library just for decoding/encoding a hex.
Supersedes:
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
Okay, then if that is the case, then should we upgrade it?
Also, such an indirect runtime dependency should probably be clarified in a comment, preferably in build.gradle.
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.
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.