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.

Sign up for GitHub

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

Jump to bottom

For more security spongycastle -> bouncycastle #1870

Closed
Neustradamus opened this issue Apr 13, 2020 · 8 comments · Fixed by #1881
Closed

For more security spongycastle -> bouncycastle #1870

Neustradamus opened this issue Apr 13, 2020 · 8 comments · Fixed by #1881
Assignees
@TranceLove
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task (low) This isn't a bug, but should be dealt with.
Milestone
v3.5

Comments

@Neustradamus
Copy link

Neustradamus commented Apr 13, 2020 •
edited
Loading

For more security, can you change old spongycastle (based on old bouncycastle) to bouncycastle?
@TranceLove
Copy link
Collaborator

Hmmm... this is a hard one.

We chose to use spongycastle since we need to use updated BouncyCastle for updated crypto code not available from stock BouncyCastle, which versions differ with different Android versions.

Happy to know if there is a way to use updated BouncyCastle without letting Android apps ignore the version we want to use, but always fall back to Android's own BouncyCastle.

@Neustradamus
Copy link
Author

@TranceLove: Thanks for your comment!
Currently Bouncy Castle last version is 1.65 and Spongy Castle last version is 1.58.
Spongy Castle is a fork of Bouncy Castle.
Please look previous links :)

Since several years, a lot of projects have already moved from Spongy Castle to Bouncy Castle because the project is dead.

You can see discussion "34" in https://github.com/rtyley/spongycastle/issues.

@TranceLove
Copy link
Collaborator

Thanks @Neustradamus, @rtyley/spongycastle#34 and a quick Google/StackOverflow both pointed to a node.js module to help building bouncycastle at their own namespace: @jbuhacoff/nodejs-mybc-util

Let's see when I have time to make our own bouncycastle jars.

@TranceLove TranceLove added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task (low) This isn't a bug, but should be dealt with. labels Apr 14, 2020
@TranceLove TranceLove added this to the v3.5 milestone Apr 14, 2020
@TranceLove TranceLove self-assigned this Apr 14, 2020
@Neustradamus
Copy link
Author

@TranceLove: nodejs-mybc-util is optional because a lot of projects have been migrated without...

TranceLove added a commit to TranceLove/AmazeFileManager that referenced this issue Apr 15, 2020
@TranceLove
Replace spongycastle with latest release version
9863933 
Fixes TeamAmaze#1870.

Steps to reproduce the required jars: (assume Ubuntu here)

1. install openjdk-8-jdk, maven, ant, git
2a. curl -sL https://deb.nodesource.com/setup_12.x | sudo -E bash -
2b. sudo apt-get install -y nodejs
3. install mybc: sudo npm install -g @jbuhacoff/mybc
4. git clone https://github.com/bcgit/bc-java && cd bc-java
5. git checkout r1v65 (as of Apr 2020, latest release version is 1.65)
6. mybc prebuild --providerName SC --packageName org.spongycastle --displayName SpongyCastle
7. bash ./build15+
8. mybc postbuild --groupId com.madgag.spongycastle
9. find the jars in $HOME/.m2/repository, copy bc{prix,prov}-jdk15on-{latestReleaseVersion}.jar to app/libs, replace any previous versions of the jars
10. resync, rebuild as usual

Tested on Fairphone 3 running LineageOS 16 GSI establishing SSH connection to home server; also tested with Robolectric and Espresso tests that depends on crypto libraries (with the help of TeamAmaze#1831).
@TranceLove TranceLove mentioned this issue Apr 15, 2020
Closed
@Neustradamus
Copy link
Author

@TranceLove: Not possible to add directly in gradle?

@TranceLove
Copy link
Collaborator

TranceLove commented Apr 19, 2020 •
edited
Loading

@TranceLove: Not possible to add directly in gradle?

Sorry for overlooking - that the discussion mentioned above actually pointed out that there is no namespace collision since Android 3.0.

I then tried a bit, seems promising. But is getting Espresso test failures on Kitkat - seems not directly related to migration to BouncyCastle though coding problem actually, otherwise is fine.

Will retract #1874 after tests passed using changeset from #1831.

TranceLove added a commit to TranceLove/AmazeFileManager that referenced this issue Apr 20, 2020
@TranceLove
Migrate to vanilla Bouncycastle
51c404d 
Fixes TeamAmaze#1870.

Per

- Remove stock Bouncycastle provider bundled with Android
- Register updated Bouncycastle provider as first crypto provider available
- Remove proguard config related to Spongycastle
- Update code to obtain crypto provider directly without prefix
@TranceLove TranceLove mentioned this issue Apr 20, 2020
Merged
@VishalNehra
Copy link
Member

Great, thanks for looking into this @TranceLove

bowiechen pushed a commit that referenced this issue May 16, 2020
@TranceLove @bowiechen
Migrate to vanilla Bouncycastle
0927acc 
Fixes #1870.

Per

- Remove stock Bouncycastle provider bundled with Android
- Register updated Bouncycastle provider as first crypto provider available
- Remove proguard config related to Spongycastle
- Update code to obtain crypto provider directly without prefix
@Neustradamus
Copy link
Author

@TranceLove, @VishalNehra, @bowiechen: Thanks a lot!
But not complete: https://github.com/TeamAmaze/AmazeFileManager/search?q=spongycastle&unscoped_q=spongycastle

TranceLove added a commit to TranceLove/AmazeFileManager that referenced this issue May 16, 2020
@TranceLove
Remove "SpongyCastle"
44ac49e 
A continuation of TeamAmaze#1870, remove any other SpongyCastle references in code comments
@TranceLove TranceLove mentioned this issue May 16, 2020
Merged
TranceLove added a commit to TranceLove/AmazeFileManager that referenced this issue May 23, 2020
@TranceLove
Migrate to vanilla Bouncycastle
1700e46 
Fixes TeamAmaze#1870.

Per

- Remove stock Bouncycastle provider bundled with Android
- Register updated Bouncycastle provider as first crypto provider available
- Remove proguard config related to Spongycastle
- Update code to obtain crypto provider directly without prefix
TranceLove added a commit to TranceLove/AmazeFileManager that referenced this issue May 23, 2020
@TranceLove
Remove "SpongyCastle"
4fd36f6 
A continuation of TeamAmaze#1870, remove any other SpongyCastle references in code comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@TranceLove TranceLove

Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Issue-Task (low) This isn't a bug, but should be dealt with.
Projects
None yet
Milestone
v3.5
3 participants
@Neustradamus @TranceLove @VishalNehra