Arg0s1080 / mrz

Machine Readable Zone generator and checker for official travel documents sizes 1, 2, 3, MRVA and MRVB (Passports, Visas, national id cards and other travel documents)
GNU General Public License v3.0
328 stars 122 forks source link

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

Open mjl opened 4 years ago

mjl commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 <

mjl commented 3 years ago

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

mjl commented 3 years ago

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 commented 3 years ago

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

rutgervandriel commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Thanks for the hard work!