Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add saaaanchez test QR, text and result #9

Merged
merged 2 commits into from
Nov 3, 2021

Conversation

Jakub-KK
Copy link
Contributor

@Jakub-KK Jakub-KK commented Nov 2, 2021

Adding a sample created by truncating "pass-valido" that is recognized as valid by apps (base45 decodes without errors).

The sample is suspicious only because of non-official surname "Saaaanchez" and being found as part of problematic "pass-valido". It is possible that this sample is not fraudulent, and was issued for real person, it is impossible to tell for sure.

@denysvitali
Copy link
Owner

denysvitali commented Nov 2, 2021

Uhm, did you run ./verify.sh?

@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 2, 2021

I used ehn-sign-verify-python-trivial to validate signature and get all the information (also openssl x509 ... to get key info), didn't have time to use your tools yet. I tried to put information about sample in correct format.

@denysvitali denysvitali merged commit cc6cc1a into denysvitali:master Nov 3, 2021
@denysvitali
Copy link
Owner

The issue is that certificate is most probably v1.0, whilst nowadays we use v1.3 IIRC

@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 4, 2021

@denysvitali is it about 061f6ae#commitcomment-59268556?
If so, the base45 data is the same in both QR codes so the schema is not an issue (I used awesome Cognex android app to verify data on both QR codes)
I guess that Polish DCC verifier has a broken QR code reader, or the QR code generator that was used to create new QR code (061f6ae) for saaaanchez does something weird.
No big deal, just FYI :)

Commit comment 061f6ae#commitcomment-59268556 moved here for discussion continuity:
I'm getting strange result with Polish DCC verifier app - the QR code from this commit results in "Invalid structure of QR Code" error in the app. I confirmed with generic QR code scanner (Cognex app) that this code contains exactly the same data as the QR code from commit 9124af0 that it replaced, that was generated using python "qrcode" module (https://github.com/lincolnloop/python-qrcode). Other QRs from RESULTS page scan without problems.
Two possibilities: either non-standard encoding of data in new QR added in this commit, or broken QR decoder in Polish DCC verifier app (cannot check unde the hood b/c it's not open source)
And BTW, "Personal Name" is missing.

@denysvitali
Copy link
Owner

That explains it! Wow!

I'm sorry then for re-generating the QR code!

I'm also sorry for replying here to your commit comment, GitHub on Android doesn't show those :(

I'll revert my commit that changes the QR.

denysvitali added a commit that referenced this pull request Nov 4, 2021
The issue generated by the Polish app seems related to
the parsing of the QR code,
instead of the data contained in it.

This commit reverts the QR code to be able to
reproduce the issue with the Polish app.
@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 4, 2021

No problem at all. I'll try to raise the issue about this new saaaanchez QR code with the PL authorities, there should be no such problems with QR code that is readable by other apps in the field. Did you verify this new code with VerificaC19 app?

@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 4, 2021

@denysvitali no need to answer my question, I saw ministero-salute/it-dgc-verificaC19-android#185 (comment) :)

@denysvitali
Copy link
Owner

@denysvitali no need to answer my question, I saw ministero-salute/it-dgc-verificaC19-android#185 (comment) :)

Sadly both the italian and swiss app are affected. I feel stupid for not checking the QR code itself. I assumed the QR decoder was pretty standard and bug-free.

@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 4, 2021

They use bog standard zxing component as you can see in https://github.com/ministero-salute/it-dgc-verificaC19-android/blob/develop/app/src/main/java/it/ministerodellasalute/verificaC19/ui/main/codeReader/CodeReaderFragment.kt
I wonder what triggers the bug... Maybe you should put an issue about this on https://github.com/zxing/zxing?

@Jakub-KK
Copy link
Contributor Author

Jakub-KK commented Nov 5, 2021

Nice find @denysvitali ministero-salute/it-dgc-verificaC19-android#185 (comment)
Sample is not fraudulent (part of test suite) and should be deleted from repo.

@denysvitali
Copy link
Owner

I've got an external tip 😅.

I'll remove it later :)

denysvitali added a commit that referenced this pull request Nov 5, 2021
This certificate, despite being weird and being
signed by a production key of AD, is in fact
genuine and used as part of the EU DGC Quality Assurance tests.

This is very unfortunate as these certificates
should have never been signed with a productive key
to begin with.

For this reason, this certificate isn't forged and
thus it is now removed with this commit.

References:
- #9
- ministero-salute/it-dgc-verificaC19-android#185
- admin-ch/CovidCertificate-App-Android#306
- eu-digital-green-certificates/dcc-quality-assurance#183
@denysvitali
Copy link
Owner

Took me a while, sorry. I now removed both saaaanchez and pass-valido as they're both valid certificates (one really valid, the other with data added at the end).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants