search

react-native-async-storage / async-storage

An asynchronous, persistent, key-value storage system for React Native.
https://react-native-async-storage.github.io/async-storage/
MIT License
4.68k stars 465 forks source link

CC_MD5 deprecation migration #415

Open safaiyeh opened 4 years ago

safaiyeh commented 4 years ago

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, https://github.com/facebook/react-native/issues/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

jonthanon commented 4 years ago

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 commented 4 years ago

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

safaiyeh commented 4 years ago

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

jonthanon commented 4 years ago

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 commented 4 years ago

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

krizzu commented 4 years ago

@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 commented 4 years ago

Just a quick update, my focus is first finishing off this PR https://github.com/react-native-community/react-native-device-info/pull/1057, it will take me a week.

Then I'll fully focus on finishing this task.

github-actions[bot] commented 3 years ago

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.