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

Optional data hash is checked even if optional data is empty #21

Open
mjl opened this issue Aug 4, 2020 · 10 comments
Open

Optional data hash is checked even if optional data is empty #21

mjl opened this issue Aug 4, 2020 · 10 comments
Assignees
Labels
bug Something isn't working CHECKER MRZ.CHECKER Issues GENERATOR MRZ.GENERATOR Issues

Comments

@mjl
Copy link

mjl commented Aug 4, 2020

While working with austrian ID cards and passports, I noticed that many of them fail the "optional data hash" check. For example (name and ID number redacted):

P<AUTPL***<<**********<<<<<<<<<<<<<<<<<<<<<<
U******9<4AUT9211041F2505060<<<<<<<<<<<<<<<2
type TD3

--> False 
fields(surname='PL***', name='*********', country='AUT', nationality='AUT', birth_date='921104', 
expiry_date='250506', sex='F', document_type='P', document_number='U******9', optional_data='', 
birth_date_hash='1', expiry_date_hash='0', document_number_hash='4', optional_data_hash='<', 
final_hash='2')

[('final hash', True), ('document number hash', True), ('birth date hash', True), ('expiry date hash', True),
('optional data hash', False), ('document type format', True), ('valid country code', True), 
('valid nationality code', True), ('birth date', True), ('expiry date', True), ('valid genre format', True), 
('identifier', True), ('document number format', True)]

and

P<AUTK******<<*********<<<<<<<<<<<<<<<<<<<<<
U******3<0AUT8402183M2610209<<<<<<<<<<<<<<<0
type TD3

--> False 
fields(surname='K******', name='*********', country='AUT', nationality='AUT', birth_date='840218',
expiry_date='261020', sex='M', document_type='P', document_number='U******3', optional_data='',
birth_date_hash='3', expiry_date_hash='9', document_number_hash='0', optional_data_hash='<', 
final_hash='0')

[('final hash', True), ('document number hash', True), ('birth date hash', True), ('expiry date hash', True),
('optional data hash', False), ('document type format', True), ('valid country code', True), 
('valid nationality code', True), ('birth date', True), ('expiry date', True), ('valid genre format', True), 
('identifier', True), ('document number format', True)]

I suppose, that should not be checked if there is no optional_data and/or if the optional_data_hash is '<'?

@Arg0s1080 Arg0s1080 self-assigned this Aug 6, 2020
@Arg0s1080 Arg0s1080 added CHECKER MRZ.CHECKER Issues pending Something is pending verification labels Aug 6, 2020
@Arg0s1080
Copy link
Owner

Hi @mjl

First I'd like to apologize for being late to reply. I've been (almost) offline for a few days.

Mmmm no. That should not happen. In the examples you have shown, optional_data field is empty and therefore < should return True.

I'm almost sure this didn't happen before, so there is surely an error. Some things have been changed and one bug may have occurred in the process, although I find it strange that it was not detected with unittests

Please, allow me a few days to investigate it.

Regards

@Arg0s1080 Arg0s1080 added bug Something isn't working and removed pending Something is pending verification labels Aug 6, 2020
@Arg0s1080
Copy link
Owner

I had already gone to bed but I could not resist and i have checked it xDD

ICAO specs for optional_data hash in TD3 hash are clear:

4.2.2.1 Data structure of the upper machine readable line

[...]

43 Check digit
When the personal number field is not
used and filler characters (<) are used
in positions 29 to 42, the check digit
may be zero or the filler character (<) at
the option of the issuing State or
organization

So it's an obvious BUG

I will try to fix it as soon as I can

Thanks!

@Arg0s1080
Copy link
Owner

Fixed (at least part 1).

The problem is that ICAO is ambiguous in its specifications, leaving the application of the rules at each State.

In this case, most countries choose to use 0 as hash because chars < are treated as 0 (another spec)

The bug for Checker is fixed:

from mrz.checker.td3 import TD3CodeChecker

mrz_a = ("P<CANMARTIN<<SARAH<<<<<<<<<<<<<<<<<<<<<<<<<<\n"
         "ZE000509<9CAN8501019F2301147<<<<<<<<<<<<<<08")

mrz_b = ("P<CANMARTIN<<SARAH<<<<<<<<<<<<<<<<<<<<<<<<<<\n"
         "ZE000509<9CAN8501019F2301147<<<<<<<<<<<<<<<8")

print(TD3CodeChecker(mrz_a))
print(TD3CodeChecker(mrz_b))

Output:

True
True

The problem with Generator remains. Actually, when mrz code is generated, if optional_data is empty, 0 is generated as hash without the possibility of choosing <

@Arg0s1080 Arg0s1080 reopened this Aug 6, 2020
@Arg0s1080 Arg0s1080 added the GENERATOR MRZ.GENERATOR Issues label Aug 6, 2020
@mjl
Copy link
Author

mjl commented Aug 7, 2020

Wow, thanks for the quick fix. Note however that the issue is not worth losing sleep on :-)

@mjl
Copy link
Author

mjl commented Aug 25, 2020

Heh, note that your test is wrong, it should end with '<<8' if you want to test for optional hash == '<'

        # optional data hash == '<'
        mrz2 = ("P<CANMARTIN<<SARAH<<<<<<<<<<<<<<<<<<<<<<<<<<\n"
                "ZE000509<9CAN8501019F2301147<<<<<<<<<<<<<<08")
        test2 = bool(TD3CodeChecker(mrz2))

@Arg0s1080
Copy link
Owner

Ooops! Was a typo. Commited. I'll push it when i'm done with what i'm doing. Thank you!

@rutgervandriel
Copy link

I can confirm that this is/was (present in release 0.5.8) an issue also with German passports.

I found a sample German passport here (Which I found from this site). Furthermore I have tested it with an actual German passport.

Your commit seems to resolve the issue, thanks for that! I have one question though, when can we expect this fix to land in a release?

@Arg0s1080
Copy link
Owner

Hi!

Yes, that bug was in versions =<0.5.8. The next version has not yet been released due to some changes i'm working on now.

I commited changes a few days ago, so, cloning repo now should work:

from mrz.checker.td3 import TD3CodeChecker


print(TD3CodeChecker("P<D<<MUSTERMANN<<ERIKA<<<<<<<<<<<<<<<<<<<<<<\n"
                     "C01XYCCG91D<<6408125F2702283<<<<<<<<<<<<<<<8"))

print(TD3CodeChecker("P<D<<MUSTERMANN<<ERIKA<<<<<<<<<<<<<<<<<<<<<<\n"
                     "C01XYCCG91D<<6408125F2702283<<<<<<<<<<<<<<08"))

Output:

True
True

Best regards!

@Arg0s1080
Copy link
Owner

I can confirm that this is/was (present in release 0.5.8) an issue also with German passports.

I found a sample German passport here (Which I found from this site). Furthermore I have tested it with an actual German passport.

Your commit seems to resolve the issue, thanks for that! I have one question though, when can we expect this fix to land in a release?

v0.6.1 has been released. This bug is fixed for MRZ Checker

If you prefer can use Pypi

@rutgervandriel
Copy link

Thanks for the hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CHECKER MRZ.CHECKER Issues GENERATOR MRZ.GENERATOR Issues
Projects
None yet
Development

No branches or pull requests

3 participants