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

CC_MD5 deprecation migration #415

Open
safaiyeh opened this issue Aug 10, 2020 · 8 comments
Open

CC_MD5 deprecation migration #415

safaiyeh opened this issue Aug 10, 2020 · 8 comments
Labels
help wanted :octocat: platform: iOS This is iOS specific platform: macOS This is macOS specific

Comments

@safaiyeh
Copy link

Current behavior

Currently Async Storage iOS relies on RCTMD5Hash which uses CC_MD5 to create a file hash https://github.com/react-native-community/async-storage/blob/af2664e5334175a180d71e22fe10e184904d63ff/ios/RNCAsyncStorage.m#L365

CC_MD5 was deprecated in iOS 13 due to not being cryptographically correct.

'CC_MD5' is deprecated: first deprecated in macOS 10.15 - This function is cryptographically broken and should not be used in security contexts. Clients should migrate to SHA256 (or stronger).

This initially brought up in React Native core, facebook/react-native#29590

Expected behavior

Either migrate to a SHA256 encryption or use a cryptographically correct MD5 hash package.

I'm assuming a SHA256 encryption would be better as it won't introduce a new dependency. This might be just a change in React Native core, unless it would be better for Async Storage to own that functionality.

I'll make the PR for these, just want to confirm which direction is best to go.

Repro steps

Build React Native app with async-storage & targeting iOS 13.

'CC_MD5' is deprecated: first deprecated in macOS 10.15 - This function is cryptographically broken and should not be used in security contexts. Clients should migrate to SHA256 (or stronger).

Warning should show up.

Environment

  • Async Storage version: 1.11.0
  • React-Native version: 0.63
  • Platform tested: iOS
  • Logs/Error that are relevant: N/A
@jonthanon
Copy link

What does "a cryptographically correct MD5 hash package" mean? My understanding is that MD5 itself is considered broken and not collision-resistant, which is the reason for the deprecation.

@safaiyeh
Copy link
Author

I talked to @krizzu and our current plan is to migrate to a SHA256 hash

@safaiyeh
Copy link
Author

@jonthanon an alternative I found was using https://github.com/onmyway133/SwiftHash but it would be introducing another dependency.

@jonthanon
Copy link

Right, but the MD5 algorithm itself is compromised, regardless of the implementation. (Sorry, I think I'm communicating this poorly.) Either way, SHA256 sounds good!

@safaiyeh
Copy link
Author

Ah makes sense! So MD5 in general is wrong, the only option is to migrate to SHA256.

@krizzu
Copy link
Member

krizzu commented Aug 13, 2020

@safaiyeh Thanks for raising this up. Yes, we agreed that SHA256 is better, among those two.
Because we're changing the filenames used, we have to create a migration process too. Here's how I see it:

On startup, check storage folder for current files. If no files, just create SHA256-based filename and continue as normal (because that means it's fresh install).
And scenario where there are files already in directory: If it's one file, compare its name to generated SHA256. If it matches, continue as normal. Otherwise copy file with new SHA256 name.
If there are multiple files, it means that previous scenario took place (one old MD5 and new SHA256). So just pick SHA256 file
I don't think there'd be a situation where more than 2 files would be present

@safaiyeh
Copy link
Author

Just a quick update, my focus is first finishing off this PR react-native-device-info/react-native-device-info#1057, it will take me a week.

Then I'll fully focus on finishing this task.

@github-actions
Copy link

This issue has been marked as stale due to inactivity. Please respond or otherwise resolve the issue within 7 days or it will be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted :octocat: platform: iOS This is iOS specific platform: macOS This is macOS specific
Projects
None yet
Development

No branches or pull requests

4 participants