search

ukhsa-collaboration / COVID-19-app-Android-BETA

Source code of the Beta of the NHS COVID-19 Android app
https://covid19.nhs.uk/
MIT License
776 stars 149 forks source link

It appears Covid19 status from notifications may be inferred from network metadata by privileged network actors #52

Closed markalanrichards closed 4 years ago

markalanrichards commented 4 years ago

Describe the bug I noticed in from code review that the notification flow, upon receiving data from Firebase messaging, may make a few web requests:

There may be enough metadata in the timing and sizes of responses and dns queries on the network to determine or just guess (perhaps wrongly) a user is submitting a diagnosis - and the guess is perhaps as worrying as a true determination as the network actor may react in the same manner.

A network actor might be your VPN provider, ISP, employer, landlord, the housemate who set up the router, your local pub whoever controls or monitors the WiFi or mobile network. Some will likely be able to identify a user from a devices hostname or mac address when they use the WiFi.

Expected behaviour Some of this request data is unnecessarily reactive to the notification event; the advice content could be pre-emptively cached (perhaps from a nightly download) I'm guessing the total size of the pages is tiny or could be and etags would keep full downloads to a minimum.

The domain names chosen could be less obvious: if a cloud provider is handling requests, do they have an ambiguous domain you can hide behind? If not, why not use a domain like gov.uk where it would be less clear whether someone is submitting a Covid19 diagnosis or using another government service like getting a new driving licence.

if any network metadata risks remain, they should be documented clearly so the user can make a choice; like preferring to submit data whilst on 4g only and not WiFi..

edent commented 4 years ago

Thanks Mark, that's an interesting point. We'll feed it into the risk assessment and see what we can do.

On your suggestions:

I don't think our technical architecture is set up to be able to use, for example, https://nhs.uk/?api=xyz.. - but I'll check.

Using an ambiguous domain is also a problem. Most mobile networks have zero-rated NHS domains - see https://www.gov.uk/government/news/mobile-networks-remove-data-charges-for-online-nhs-coronavirus-advice So if we used a non NHS domain, people may not be able to access the service if they're out of credit with their network provider.

Thanks for raising the issue.

markhankins commented 4 years ago

I'm not too worried about actors like ISP's or pub landlords, but there could also be an issue here with MITM attacks and people harvesting data for the purposes of blackmail etc.

markalanrichards commented 4 years ago

@markhankins

I think the mitm risk is greater for any http requests and I cannot tell if ACKNOWLEDGEMENT_URL is http or https so it makes it hard to judge if all bases are covered in this flow. There is evidence of http usage in the app ( https://github.com/nhsx/COVID-19-app-Android-BETA/blob/c8f564223d2dcaa47ea90341e11b1498b4065ad3/app/src/main/java/uk/nhs/nhsx/sonar/android/app/util/BrowserUtils.kt#L51 ).

https content is encrypted, but SNI may leak the domain name too, depending on how TLS is setup.

If root cas are installed on devices, then mitm is possible and I think that risk is more likely a concern for employer networks (vpns and office wifi) where many large companies (and thus large employers) have global transparent proxies they like to enrol corporate devices into so they can monitor security threats on the network.

edent commented 4 years ago

@markalanrichards good spot on that http typo. That's just for displaying the info pages, and will automatically redirect to https. I'll make sure it's updated before launch though.

timb-machine commented 4 years ago

@markalanrichards good spot on that http typo. That's just for displaying the info pages, and will automatically redirect to https. I'll make sure it's updated before launch though.

@edent That's a duplicate of #3 I think. Someone has already submitted https://github.com/nhsx/COVID-19-app-Android-BETA/pull/6 to fix it.

timb-machine commented 4 years ago

Describe the bug I noticed in from code review that the notification flow, upon receiving data from Firebase messaging, may make a few web requests:

  • A request sent to the ACKNOWLEDGMENT_URL in Notificationhandler
  • AtRiskActivity may be then loaded which has links to load gov.uk and nhs.uk content
  • AtRiskActivity flow suggests a Diagnosis submission to api.svc-covid19.nhs.uk for users feeling unwell

There may be enough metadata in the timing and sizes of responses and dns queries on the network to determine or just guess (perhaps wrongly) a user is submitting a diagnosis - and the guess is perhaps as worrying as a true determination as the network actor may react in the same manner.

A network actor might be your VPN provider, ISP, employer, landlord, the housemate who set up the router, your local pub whoever controls or monitors the WiFi or mobile network. Some will likely be able to identify a user from a devices hostname or mac address when they use the WiFi.

Do you have any PCAPs that exhibit the issue? I'd like to archive a practical example for posterity.

markalanrichards commented 4 years ago

@markalanrichards good spot on that http typo. That's just for displaying the info pages, and will automatically redirect to https. I'll make sure it's updated before launch though.

@edent That's a duplicate of #3 I think. Someone has already submitted #6 to fix it.

@timb-machine yes it is @edent I wasn't attempting to get that fixed in this PR, but glad to see there are efforts to do so. The problem I was hoping to communicate is that the technical failure of having an http link reach your master code, demonstrates a QC and potentially QA problem in how you're developing the software, so unless changes to improve QC and QA are shared, then the public should be very cautious to trust that a URL we cannot see (like the ACKNOWLEDGMENT_URL which I think comes from the notification) and any future URLs that may be added, will be https. For instance if (I haven't counted) 1/10 URLs in the master branch are http; then perhaps at best we should only have a ~90% confidence that other or new URLs will be https.

I think a good change to make to the code, is that the rest and web client functionality include a whitelist of https URL prefixes that it will communicate with (like perhaps only https://nhs.uk) and that if any links are supplied at runtime which don't match this, they are ignored; this should be a very simple if statement in the web open and rest client functionality.

markalanrichards commented 4 years ago

Describe the bug I noticed in from code review that the notification flow, upon receiving data from Firebase messaging, may make a few web requests:

  • A request sent to the ACKNOWLEDGMENT_URL in Notificationhandler
  • AtRiskActivity may be then loaded which has links to load gov.uk and nhs.uk content
  • AtRiskActivity flow suggests a Diagnosis submission to api.svc-covid19.nhs.uk for users feeling unwell

There may be enough metadata in the timing and sizes of responses and dns queries on the network to determine or just guess (perhaps wrongly) a user is submitting a diagnosis - and the guess is perhaps as worrying as a true determination as the network actor may react in the same manner. A network actor might be your VPN provider, ISP, employer, landlord, the housemate who set up the router, your local pub whoever controls or monitors the WiFi or mobile network. Some will likely be able to identify a user from a devices hostname or mac address when they use the WiFi.

Do you have any PCAPs that exhibit the issue? I'd like to archive a practical example for posterity.

@timb-machine Nope, from what I can tell, the backend is not open sourced so I haven't attempted to run the application against anything. I'm hoping the backend source code is provided soon so it can be reviewed too.

edent commented 4 years ago

I'm pasting this message in every active GitHub issue, so you may receive duplicate notifications.

Today, I'm happy to announce that NHSX has released the full git commit history for the Isle of Wight Beta apps.

As discussed, we have redacted API keys, sensitive domain names, and some of the developers' personal details. I am still waiting on final approval to publish the server-side code.

I would like to personally thank the community for your comments, bug reports, and vulnerability disclosures. They all went into helping the development process.

The beta trial of this app has now ended and we've moved to the next phase of app development. It is our intention to publish the source code of future apps as the binaries are released to the public.

Once again, thank you for being part of this.

Terence Eden Head of Open Technology - NHSX