BlockchainCommons / GordianQRTool-iOS

UR & QR code storage for iOS and MacOS
Other
5 stars 7 forks source link

CRITICAL BUG: Corrupt QR displayed in Apple from CA Covid-19 Record Source #78

Open ChristopherA opened 3 years ago

ChristopherA commented 3 years ago

I have finally my real CA covid-19 health care vaccination QR, however, when I read it in App Store release 1.0.3, the input QR value consists of shc:/ followed by a list of number (0-9) digits and the QR itself isn't very dense, however the QR display stored from the app's view and detail display panes are severely broken and corrupted — not only does it have two different densities (very dense on left 2/3 of QR square, and unreadably dense on right side of QR square).

Contact me for the two images, the input QR and the bad output QR, the shc: text.

shannona commented 3 years ago

The QR does read back in properly if you make it really big, but yeah, it's definitely too dense (and most people will just see it not read because of the size issue)

shannona commented 3 years ago

So, I don't think the QR is actually broken. I was able to reread it if I pushed the image size up far enough. The problem is that it's much more dense, increasing from 4 reference points across and down (16 total) in the original QR to 7 across and down (49) total in the one spit back out.

Some research suggests this is a relatively isolated problem. Many of the QRs that get stored reproduce exactly the same. (Tested: the DPAL seed, a DPAL SSKR share, and a 2FA seed.) The rest reproduce at the same size. (Tested: some encoded text and some encoded numbers.) I'm guessing this is different because something was done to tighten up the original QR, possibly related to the fact that it's purely numeric after the 'shc:/' prefix.

The QR display code is in Helpers -> QRGenerator.swift. It looks pretty normative:

       let data = textInput.trimmingCharacters(in: .whitespacesAndNewlines).data(using: .ascii)

        guard let filter = CIFilter(name: "CIQRCodeGenerator") else { return nil }
        filter.setValue(data, forKey: "inputMessage")
        filter.setValue("Q", forKey: "inputCorrectionLevel")

        let transform = CGAffineTransform(scaleX: 10.0, y: 10.0)

        guard let output = filter.outputImage?.transformed(by: transform) else { return nil }

I tried two experiments for size:

1) Reducing the correction level from Q (25%) to L (7%) reduced the size from 7 references to 6. 2) Capitalizing the lowe-case shc:/ prefix reduced the size from 7 references to 6. 3.) Doing both reduced the size from 7 references to 5.

So, none of that can account for the full change. My guess is that maybe the original was encoded purely numeric, if you can do that even with an alphabetic prefix, in which case we might need to be more careful about either storing or retrieving or data. But figuring out data types and encoding NSData correctly is starting to get over my head for my current knowledge of Swift.

@wolfmcnally, is this something you can help with?

Fonta1n3 commented 3 years ago

@ChristopherA the animate button is there for QR code's which are too big.

Really not sure how this is a critical bug?

ChristopherA commented 3 years ago

@Fonta1n3 The problem is this QR is not an UR, it is a standard use by California for COVID-19 credentials, that for some reason isn’t veyr dense in the original presented to us, but when we turn it around is quite a bit larger.

Fonta1n3 commented 3 years ago

@Fonta1n3 The problem is this QR is not an UR, it is a standard use by California for COVID-19 credentials, that for some reason isn’t veyr dense in the original presented to us, but when we turn it around is quite a bit larger.

For text that is not UR we convert it to data and then ur:bytes. You can animate any QR this way so that it is readable by any app which supports UR.

Fonta1n3 commented 3 years ago

For the public record I want to make it extremely clear I find it highly offensive to our basic human rights that the State is attempting to label people and divide society into those who are vaccinated and not. I have always been pro vaccine but this is reminiscent of Nazi Germany and I want no part in it. If the app is moving in the direction of explicitly helping to make use of these state provided QR codes to label people as we do cattle then I will no longer be a part of this project in any way shape or form.

ChristopherA commented 3 years ago

@Fonta1n3 I have written extensively about the perils of vaccine credentials, have shared annotated links on why, have advocated against bad practices of health credentials themselves in the W3C, and have publicly testified against their misuse in both California & Wyoming.

However, they are happening anyhow. So the goal of QR Tool is to allow people to protect themselves, by making it such that revealing them to others is under their control, and are encrypted under their own keys. Currently, several orgs including the CA Health Department suggest taking a photo of the QR and present it that way — not a good idea, but at least iPhones photo roll is encrypted. However, other orgs (NYC in particular) are suggesting other apps that are saving these QRs to their own service between presentations — a very bad idea.

We are not supporting the state to create and display these QR codes, we are supporting the individual user to put them under their personal control as much we can. It is quite imperfect, but better than nothing.

ChristopherA commented 3 years ago

PR #81 addresses the immediate problem of this problem in the short term, but it doesn't solve the underlying problem.

I am still concerned that our result is a much more dense QR presented by QR Tool, which may not work with some other QR readers.

Shannon thinks based on https://github.com/smart-on-fhir/health-cards/ that they are doing some custom QR encoding. They may be breaking their coding into two chunks, encoding the first as bytes, the second as numeric, for better compression. This technique could also happen with future QRs that have nothing to do with this particular example, and it also may be a trick we may want to puzzle out how to do with UR. @wolfmcnally also thinks that maybe moving over the https://github.com/nayuki/QR-Code-generator might help.

I'm keeping this issue open until we understand it better. We should also use this particular example QR as a test for a longer-term solutions to this problem.

ChristopherA commented 3 years ago

BTW, the shc: folk rejected my suggestion to upper case the URL scheme like we do to maximize QR compression. See https://github.com/smart-on-fhir/health-cards/issues/165

shannona commented 3 years ago

Given their segmentation, that makes sense. shc: vs SHC: is a very minor size difference when the rest of the data is encoded as a numeric segment.

The bigger problem is of course the bad interaction between URIs being canonical as lower-case and QRs requiring uppercase for alphanumeric encoding.

wolfmcnally commented 3 years ago

@Fonta1n3 @ChristopherA My QRCodeGenerator Swift package can now solve this issue in a completely generic way, with no specific reference to any particular format.

shannona commented 3 years ago

Terrific!

@ChristopherA let me know if you'll be having @wolfmcnally integrate QR Tool with the new package, or if you want me to learn how to do so.