Cyclic3
commented
4 years ago I initial met this issue with some scepticism, but after been reading though the code for the last few hours (and by god could it use some comments), I've come to the same conclusion. I'll give some more technical details to those who wish to verify it or explain how we've completely misunderstood some integral part of the code, which is completely possible. Of course I don't believe that this is some secret government scheme to track us, because the UK government has far better places to hide trackers than in open-source code on GitHub. This is probably an honest mistake, either on our part, or the dev's.
\<MASSIVE_DISCLAIMER>
I am in no way a Kotlin dev, and this could all be completely wrong!!! Someone who knows all of this better than me should check my links to confirm (or refute!) my understanding. I'll try to keep this updated with any more information I get, either through comments or further code inspection.
\</MASSIVE_DISCLAIMER>
The code refers to the 2 hour poll loop as "WorkManager" [code], so I will too.
-
I didn't look at QR code scanning, but I'm going to assume that's uninteresting.
-
The WorkManager calls the checker code for the risky venues [code], which downloads the list of ids, locally compares the client's ids with downloaded ones, and if it finds matching ones, triggers the circuit breaker initial tasks [code]
-
This initial tasks function creates a worker on the WorkManager, which only runs if the network is connected, and supplies it with these ids [code]
-
This worker jumps through some hoops [code] and iterates through the shared ids. For each id:
-
It passes the id to the mentioned API call [code]. This API call is a
POSTrequest tocircuit-breaker/venue/request[code], and is not really documented anywhere. That's not unique (all the code is undocumented), but leaves it's purpose rather vague. The optimist in me suggests that this is just a redundant check that the token is valid, but the pessimist suggests that this is a way to alert the remote server about possible venue exposure without the user's consent. Either way, unless I've completely missed the point (which is, again, very possible), it has no place in the app and should removed. -
If it succeeded, the user notification code is run, and we finish this iteration [code]
-
If it is "pending", which I assume indicates some form of server overload, a poll is added to the WorkManager with some form of request identification token [code] [code] [code]. If the remote server returns a value, the poll loop stops, with a successful value creating a prompt like an initial success would. However (according to my limited understanding), the silent exception catch [code] combined with a failed request would allow this to loop every 2 hours, uniquely identifying the user to the remote server. If this is the case, this definitely needs to be addressed! It is possible that this is supposed to handle network failure, but a catch-all solution like this is too easily exploited by purposefully sending bad HTTP responses.
- If an exception is thrown during the handling of the initial approval request, it is unmarked as a risky venue id [code], giving credibility to the idea that this is some form of redundant verification system. However, this behaviour is different with the "try again" philosophy of the poll loop, which is rather odd.
-
It would be great if someone could explain the intended purpose of this API call (and maybe even some documentation!), and also if my assessments are correct.
CovidSecure
nhs-covid19
The risky venues circuit breaker functions by submitting the venue IDs of each risky venue that a user has visited to an API endpoint. This partially exposes the user's venue check-in history based on which venue IDs are submitted, and is contrary to the various documentation which states that the user's recorded venue IDs will never leave their device.
This data could be used for gathering statistics and performing analyses without users' knowledge or consent. In conjunction with the user's IP address, the data could also be linked to a specific user. Similar data could also theoretically be gathered for any specific non-risky venue by placing it on the risky venues list and deactivating the circuit breaker for that venue to prevent a notification from being triggered.
This breaks the security model around which the app is built, being that all processing should happen on the user's device and no sensitive (i.e. non-analytic) data should ever be uploaded thereby abating possible concerns that the app may be used for mass data gathering or espionage by making it technically impossible to do so. In the interest of transparency, the risky venues circuit breaker component should be removed or redesigned in a way that does not expose the user's data or, if this is not feasible, a clear notice about the operation of this function should be given in the documentation.