CrunchyBagel / TracePrivately

A privacy-focused app using Apple's soon-to-be-released contact tracing framework.
MIT License
351 stars 27 forks source link

Continue to submit new keys following a positive diagnosis #21

Open HendX opened 4 years ago

HendX commented 4 years ago

According to the documentation:

Keys will be reported for the previous 14 days of contact tracing. The app will also be launched every day after the daily tracing key changes to allow it to request again to get the key for each previous day for the next 14 days.

It's unclear though how this subsequent daily launch will work. Permission needs to be granted to retrieve the keys, but this makes it sound like the app will be woken in the background.

I believe this is a missing aspect of their documentation thus far.

tatey commented 4 years ago

Do you think the client will need to persist the submission ID after its initial submission from the server? Otherwise if new keys are uploaded how do you prove that they are associated with a positive test result without going through the whole confirmation process again?

HendX commented 4 years ago

Yes I agree with that. I will update the YAML so it can return such an identifier.

Additionally, the same submit endpoint will be used but with an option to include that identifier instead of re-verifying.

HendX commented 4 years ago

One other thing I noted here, that isn't in Apple's documentation:

https://blog.google/documents/55/Android_Contact_Tracing_API.pdf

In order to be whitelisted to use this API, apps will be required to timestamp
and cryptographically sign the set of keys before delivery to the server
with the signature of an authorized medical authority.

I'm not sure exactly how to handle this on the client side.

HendX commented 4 years ago

I've updated the YAML in c6573f3d788d49b2ef63b05b1408374d05bfda08 and 866b7270984f82ffa9568335e87c66741104f050.

Note that the iOS app and my server sample just have placeholders for now and aren't actually properly making use of it.

tatey commented 4 years ago

I've updated the YAML in c6573f3 and 866b727

Understood. I've open an issue to track this work https://github.com/tatey/trace_privately/issues/14.

HendX commented 4 years ago

I do have a security concern about just blindly submitted the ID with new keys, since it could potentially be extracted from the Core Data database, but obviously the device needs to be authenticated.

Perhaps the server needs to rate limit the submissions for an existing submission. For example, only accepted an average of 1 key per 24 hours from the date of the original submission, up to X days (14? 21?) until the diagnoses is no longer considered active.

Then in the case of abuse, it's only going to be fake keys that don't match against other users' databases so I don't believe it will actually cause any impact (such as false indicating an exposure).

HendX commented 4 years ago

Opened #37 related to this.

tatey commented 4 years ago

I do have a security concern about just blindly submitted the ID with new keys, since it could potentially be extracted from the Core Data database

Actually, this is a really good point that I overlooked. Right now I'm returning the primary key of the submission which is an auto incrementing ID from the database. Perhaps this should be some sort of token that only the client which made the submission would know. This would stop tampering with other submissions.

The other option is to make it so that you get an account behind the scenes and we limit it so that the account you're authenticating with can only updates its own submissions. I like this one less because then it's linking submissions and keys to an authenticated device.

HendX commented 4 years ago

Yea, I don't think using the auto-incrementing ID is a good idea. In sample implementation it generates a UUID and assigns it to the record and returns that

(Currently it's not storing it in the database, but I've made a note to do so. This would likely be an indexed field).

I don't think an account is necessary. If it's throttled then there's really not going to be much / any effective tampering. Not sure of the key data format from Apple yet, but some validation against that could also occur.

tatey commented 4 years ago

Yea, I don't think using the auto-incrementing ID is a good idea. In sample implementation it generates a UUID and assigns it to the record and returns that

My bad. I missed that. I've patched it so the server conforms to this behaviour.

I don't think an account is necessary. If it's throttled then there's really not going to be much / any effective tampering. Not sure of the key data format from Apple yet, but some validation against that could also occur.

Cool, agreed.