Securely Store Location Data + Other improvements #747

troach-sf · 2020-05-01T18:47:15Z

Securely store locations in encrypted Realm db

This PR securely stores all captured Geolocation coordinates in an encrypted Realm database.

Secures Locations imported via Google
Migrates old locations to new secure storage -> deletes old on success
Still allows for json export
Fixes bug where locations stopped being captured after device restart

Linked issues:

How to test

For debug builds, you can download the Realm database inside the apps storage. You can open the database using Realm Studio: https://realm.io/products/realm-studio/. Because the database is encrypted, you will need to log a hex-encoded encryption key.

Securely store locations in encrypted Realm db

troach-sf · 2020-05-01T18:47:53Z

@tstirrat @kenpugsley Can you assign yourselves as reviewers?

troach-sf · 2020-05-01T18:48:34Z

Encryption review is pending. Lets hold off on merging until we get an approval.

android/app/build.gradle

tstirrat

Great stuff. I've done a quick pass on general structure. But will do a second pass later tonight.

I'd love to see some unit tests on the native code before merging.

android/app/src/main/java/org/pathcheck/covidsafepaths/storage/Location.kt

tremblerz · 2020-05-04T06:27:24Z

Great work!

Ferrumofomega · 2020-05-04T14:37:49Z

This is wonderful stuff @troach-sf .

…ation-stroage

.github/workflows/test.yml

troach-sf · 2020-05-06T02:53:14Z

@tstirrat, I've completed testing on both sides. Looks like there is 1 conflicting file that needs to be resolved now. I'll merge that in tomorrow morning.

…ation-stroage # Conflicts: # app/views/LocationTracking.js

tstirrat · 2020-05-06T18:34:32Z

Thanks! How does one run the native tests?

It would be good to encode that into 2 more jobs in the test.yml workflow.. /cc @efalkner

troach-sf · 2020-05-06T19:20:59Z

I just wrote a document on how to manually capture encryption key for develop builds to allow for opening the database. This should help for manual tests. https://gist.github.com/troach-sf/f257bb7b80e6dddd4f3bade81b7b1410

For running the automatic tests.

Android:

cd android
../gradlew clean
../gradlew connectedAndroidTest

iOS:
Haven't tried from the command line but open the project, open the CovidSafePathsTests scheme, and click on the test icon in the sidebar and run all tests in RealmSecureStorageTest

troach-sf · 2020-05-06T19:21:47Z

Right now Android requires a device/emulator. I don't think Realm is compatible with Robolectric for unit testing without a device. That may make it tricky to get working on GH Actions.

kenpugsley · 2020-05-07T03:58:54Z

Note to others ... I'm currently working on doing some functional testing on this, and will try to run new native tests locally.

kenpugsley

Inline comments

app/services/LocationService.js

kenpugsley · 2020-05-07T05:52:35Z

cd android
../gradlew clean
../gradlew connectedAndroidTest

This doesn't work for me. It's failing in the geolocation tests ... any ideas?
> Task :@mauron85_react-native-background-geolocation:connectedDebugAndroidTest 12:47:08 V/ddms: execute: running am get-config 12:47:09 V/ddms: execute 'am get-config' on 'emulator-5554' : EOF hit. Read: -1 12:47:09 V/ddms: execute: returning 12:47:09 D/@mauron85_react-native-background-geolocation-debug-androidTest.apk: Uploading @mauron85_react-native-background-geolocation-debug-androidTest.apk onto device 'emulator-5554' 12:47:09 D/Device: Uploading file onto device 'emulator-5554' 12:47:09 D/ddms: Reading file permision of /Users/ken/code/covid-safe-paths/node_modules/@mauron85/react-native-background-geolocation/android/lib/build/outputs/apk/androidTest/debug/@mauron85_react-native-background-geolocation-debug-androidTest.apk as: rw-r--r-- 12:47:09 V/ddms: execute: running pm install -r -t "/data/local/tmp/@mauron85_react-native-background-geolocation-debug-androidTest.apk" 12:47:12 V/ddms: execute 'pm install -r -t "/data/local/tmp/@mauron85_react-native-background-geolocation-debug-androidTest.apk"' on 'emulator-5554' : EOF hit. Read: -1 12:47:12 V/ddms: execute: returning 12:47:12 V/ddms: execute: running rm "/data/local/tmp/@mauron85_react-native-background-geolocation-debug-androidTest.apk" 12:47:13 V/ddms: execute 'rm "/data/local/tmp/@mauron85_react-native-background-geolocation-debug-androidTest.apk"' on 'emulator-5554' : EOF hit. Read: -1 12:47:13 V/ddms: execute: returning 12:47:13 I/RemoteAndroidTest: Running am instrument -w -r com.marianhello.bgloc.react.test/android.support.test.runner.AndroidJUnitRunner on Nexus_4_API_27(AVD) - 8.1.0 12:47:13 V/ddms: execute: running am instrument -w -r com.marianhello.bgloc.react.test/android.support.test.runner.AndroidJUnitRunner 12:47:13 V/InstrumentationResultParser: INSTRUMENTATION_RESULT: shortMsg=Process crashed. 12:47:13 I/InstrumentationResultParser: test run failed: 'Instrumentation run failed due to 'Process crashed.''

troach-sf · 2020-05-07T06:28:40Z

@kenpugsley ah sorry, run :app:androidConnectedTest to limit to just our own tests. Good eye on the backfill. I discussed this with @Ferrumofomega and @tremblerz that we would only store real locations in the db. It would be easy enough to add backfilling in on the js layer when pulling locations. After discussing that a bit, I believe we came to an agreement that if there is a delay we shouldn't make assumptions someone stayed in one spot if the previous location had barely changed. I'll leave that up to you all to decide, but if we add it back in, rather than throwing assumed locations in the db, we should only add during processing.

kenpugsley · 2020-05-07T13:32:29Z

we should only add during processing.

I think that logic applies to the intersection, but this also have effects on the flow of data to safe paths. I see the point about not wanting location data that is inferred stored, but this will be a breaking change to the "contract" (not that there is one explicitly) with Safe Places. If we don't store the points in the local db, then in both the intersection as well as the data export we have to remember to backfill the gaps. Seems like a service layer thing to me.

kenpugsley

Approved. Based on thread in slack, going to look at the backfill separately.

tstirrat · 2020-05-09T21:26:57Z

Closing this in favor of #788 where we are automating tests and merging conflicts.

AdamLeonSmith · 2020-05-10T17:05:02Z

I just wrote a document on how to manually capture encryption key for develop builds to allow for opening the database. This should help for manual tests. https://gist.github.com/troach-sf/f257bb7b80e6dddd4f3bade81b7b1410

I'm getting compilation errors (Android) when I add this immediately after SecureRandom().nextBytes(newKey) in getEncryptionKey()

troach-sf · 2020-05-11T12:57:12Z

@AdamLeonSmith, I have updated the gist. There was a missing ")".

Feature/realm (#54)

f52841a

Securely store locations in encrypted Realm db

tstirrat added this to To do in Code reviews May 1, 2020

tstirrat reviewed May 1, 2020

View reviewed changes

android/app/build.gradle Outdated Show resolved Hide resolved

tstirrat reviewed May 1, 2020

View reviewed changes

android/app/src/main/java/org/pathcheck/covidsafepaths/storage/Location.kt Show resolved Hide resolved

tstirrat moved this from To do to In review in Code reviews May 1, 2020

kenpugsley self-requested a review May 1, 2020 23:50

tstirrat moved this from In review to Waiting on author in Code reviews May 2, 2020

Fix unintended comment move

3882088

Attempt to resolve GH Actions issue

f50f857

troach-sf force-pushed the feature/secure-location-stroage branch from 51ce4b2 to f50f857 Compare May 4, 2020 20:57

troach-sf added 4 commits May 5, 2020 09:33

Android testing refactor

3ba56ef

Merge remote-tracking branch 'origin/develop' into feature/secure-loc…

9d3ba81

…ation-stroage

Formatting fix

f0841b9

Google Takeout merge fix

b378fd6

tstirrat previously approved these changes May 5, 2020

View reviewed changes

Code reviews automation moved this from Waiting on author to Reviewer approved May 5, 2020

Fix Android tests and add iOS tests

f07bee6

troach-sf dismissed tstirrat’s stale review via f07bee6 May 6, 2020 01:22

Code reviews automation moved this from Reviewer approved to In review May 6, 2020

troach-sf added 2 commits May 5, 2020 21:28

iOS test cleanup

b769c4f

iOS test cleanup

8b4f3d1

efalkner reviewed May 6, 2020

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

remove verbose flag added to inspect GH Actions failure

169daa3

tstirrat moved this from In review to To do in Code reviews May 6, 2020

troach-sf mentioned this pull request May 6, 2020

Add retry logic to get desired location accuracy #770

Closed

Merge remote-tracking branch 'origin/develop' into feature/secure-loc…

ccad7dd

…ation-stroage # Conflicts: # app/views/LocationTracking.js

kujenga mentioned this pull request May 6, 2020

Implement on-device encryption for Location Tracking data safe-paths-contrib/covid-safe-paths#29

Closed

tstirrat previously approved these changes May 6, 2020

View reviewed changes

Code reviews automation moved this from To do to Reviewer approved May 6, 2020

kenpugsley requested changes May 7, 2020

View reviewed changes

app/services/LocationService.js Show resolved Hide resolved

Code reviews automation moved this from Approved, needs smoke test to In review May 7, 2020

thecoder8890 previously approved these changes May 7, 2020

View reviewed changes

Add backfilled locations to android with new source indicator

4764eef

troach-sf dismissed stale reviews from thecoder8890 and tstirrat via 4764eef May 7, 2020 20:44

kenpugsley self-requested a review May 7, 2020 20:52

kenpugsley approved these changes May 7, 2020

View reviewed changes

Code reviews automation moved this from In review to Approved, needs smoke test May 7, 2020

efalkner mentioned this pull request May 9, 2020

Securely Store Location Data + Other improvements #788

Merged

tstirrat closed this May 9, 2020

Code reviews automation moved this from Approved, needs smoke test to Done May 9, 2020

tstirrat mentioned this pull request May 9, 2020

Android location saving after restart part 2 #527

Closed

tstirrat mentioned this pull request May 12, 2020

TESTING: Verify logging resumes on reboot #72

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Securely Store Location Data + Other improvements #747

Securely Store Location Data + Other improvements #747

troach-sf commented May 1, 2020 •

edited by tstirrat

troach-sf commented May 1, 2020

troach-sf commented May 1, 2020

tstirrat left a comment

tremblerz commented May 4, 2020

Ferrumofomega commented May 4, 2020

troach-sf commented May 6, 2020

tstirrat commented May 6, 2020

troach-sf commented May 6, 2020

troach-sf commented May 6, 2020

kenpugsley commented May 7, 2020

kenpugsley left a comment

kenpugsley commented May 7, 2020

troach-sf commented May 7, 2020

kenpugsley commented May 7, 2020 •

edited

kenpugsley left a comment

tstirrat commented May 9, 2020

AdamLeonSmith commented May 10, 2020

troach-sf commented May 11, 2020

Securely Store Location Data + Other improvements #747

Securely Store Location Data + Other improvements #747

Conversation

troach-sf commented May 1, 2020 • edited by tstirrat

Linked issues:

How to test

troach-sf commented May 1, 2020

troach-sf commented May 1, 2020

tstirrat left a comment

Choose a reason for hiding this comment

tremblerz commented May 4, 2020

Ferrumofomega commented May 4, 2020

troach-sf commented May 6, 2020

tstirrat commented May 6, 2020

troach-sf commented May 6, 2020

troach-sf commented May 6, 2020

kenpugsley commented May 7, 2020

kenpugsley left a comment

Choose a reason for hiding this comment

kenpugsley commented May 7, 2020

troach-sf commented May 7, 2020

kenpugsley commented May 7, 2020 • edited

kenpugsley left a comment

Choose a reason for hiding this comment

tstirrat commented May 9, 2020

AdamLeonSmith commented May 10, 2020

troach-sf commented May 11, 2020

troach-sf commented May 1, 2020 •

edited by tstirrat

kenpugsley commented May 7, 2020 •

edited