FHIR Chat · Quebec breach · smart/health-cards

Stream: smart/health-cards

Topic: Quebec breach

John Moehrke (Aug 29 2021 at 13:48):

What did Quebec use for their vaccine credential? Quebec could make obtaining QR code more complex in response to breaches, minister says
CBC News · https://www.cbc.ca/news/canada/montreal/vaccination-passport-safety-security-hacker-1.6155488

Josh Mandel (Aug 29 2021 at 14:24):

They're using SMART Health Cards, like California, New York, Louisiana (and Walmart, CVS, Epic, Cerner, and others). I don't think there have been details published about the specific issues mentioned in the article here, but based on early feedback there is an implementation issue and not a protocol issue.

John Moehrke (Aug 30 2021 at 17:14):

I expected that it was a validation, more specifically a lack of validation, issue. The model we use does enable the data to be seen without forcing a validation of the signature. Thus it is possible to have a poorly (maliciously) implemented reader that simply displays.
Which the best lesson-learned could be that there will need to be a set of testing (test plan) requirements that can be used to show that a "reader" application is doing all the right things, and not just displaying the content.

John Moehrke (Aug 30 2021 at 17:16):

an organization that is going to rely on these QR codes needs to have some way to prove that the app they are using is legitimately validating. Thus we need both good data and various bad data as part of a comprehensive test plan.

Christian Paquin (Aug 30 2021 at 17:57):

John Moehrke said:

Thus we need both good data and various bad data as part of a comprehensive test plan.

I think it's a good idea to have a threat modelling list for validation apps, to at least avoid accidental (vs. malicious) implementation errors. The validation SDK has a lot of test files that could be used to create such a test plan (invalid payload, signatures, etc.)

Marc-André Poulin (Aug 31 2021 at 22:27):

ESET security released a vulnerability report. Is it related to the SDK behavior?
https://www.welivesecurity.com/fr/2021/08/31/faille-preuve-vaccination-quebec-vaxicode-verif/

Marc-André Poulin (Aug 31 2021 at 22:35):

Translating the relevant part:

"The vulnerability resides in the fact that once downloaded a public key can be used to validate any other passport, without verifying the content of the issuer field (iss)."

John Moehrke (Sep 01 2021 at 12:40):

that is not a vulnerability, that is normal public/private key fact. Public keys are "public". Public keys enable signature validation.

John Moehrke (Sep 01 2021 at 12:50):

The problem is outlined in the article. It is that the app does not maintain a proper truststore of certificates worthy of trust for issuing vaccine certificates. The attack involves two QR codes, the first of which is intended to fail, but the failure of it does not cleanup and thus remnants of that first QR validation (the attackers public key is in the trust store), and thus the second QR succeeds. --- THUS this is just poor software design.

John Moehrke (Sep 01 2021 at 12:51):

likely only one QR was needed.. first scan fails, but individual insists they try again, and second scan succeeds.

John Moehrke (Sep 01 2021 at 12:53):

seems the app new version has fixed truststore with no ability to update public keys. this will fail when new issuers are needed, or when existing issuers need to get new certificates. -- again, poor software design.

Marc-André Poulin (Sep 01 2021 at 12:53):

@john agreed, I think you described the attack properly, but isn't the SDK downloading and adding the key to the keystore in the first place. Referring to the code here: https://github.com/smart-on-fhir/health-cards-validation-SDK/blob/a68f478a6c4607cfce9f60df7ce0c8afba49111f/src/jws-compact.ts#L210

Marc-André Poulin (Sep 01 2021 at 12:56):

The SDK doesn't appear to validate that the key used to validate is the same as the one provided by the iss field, allowing the attack to happen.

John Moehrke (Sep 01 2021 at 12:56):

if the reference code is susceptible to this, then it should be fixed... properly.

John Moehrke (Sep 01 2021 at 12:57):

The iss check is additional. the main problem was truststore management.

Josh Mandel (Sep 01 2021 at 13:36):

There are a few related issues here, stemming from the way that SMART issuer testing tools were incorporated into the Quebec verifier app. @Christian Paquin can provide more context, but the gist is:

The official testing tool is designed to help issuers debug their implementations, providing useful feedback about the pieces that are right and wrong. As such, it parses payloads leniently, to generate more useful developer facing warnings or expose downstream errors even if there are some early showstoppers. We did not design the testing tool with the idea that its code would be incorporated directly into verifier apps, but we should have anticipated that this would be appealing for verifiers. https://github.com/smart-on-fhir/health-cards-validation-SDK/pull/143 clarifies the design intent, and we are also planning to rename the tool itself to avoid ambiguity on this point (to wit, I'm referring to it as a testing tool here rather than a validation SDK).
As John points out, the testing tool uses a keystore in such a way that keys in the store are not bound to specific issuers. (For background: the testing tool was designed to run tests independently, so state would not accumulate in the keystore across invocations and typical usage. It is also designed to support issuers in early stage testing when they have not yet published keys on the web, so it provides additional mechanisms to load keys into its store.) In any case, the lack of binding between keys and issuers is a bug that we will fix ASAP (this should not change testing results when the testing tool is being used as intended).
We did not have any officially documented channel for reporting security issues with the testing tool; this has been fixed in https://github.com/smart-on-fhir/health-cards-validation-SDK/pull/146 (but of course individual verifier apps and issuers likely need their own security reporting channels; this is something we will continue to push on through the VCI directory; see https://github.com/the-commons-project/vci-directory/issues/52)

Marc-André Poulin (Sep 01 2021 at 13:47):

Makes sense, thank you @Josh Mandel

John Moehrke (Sep 01 2021 at 13:54):

all reference sample code should be assumed it will be used in production without correcting the residual TODO issues... Sad reality of the software world. Not unique to us here. Someone will eventually figure out how to prevent this.
Thanks for the clarity and the proper handling.

Christian Paquin (Sep 01 2021 at 14:49):

Marc-André Poulin said: