AndyQ / NFCPassportReader

NFCPassportReader for iOS 13
MIT License
733 stars 235 forks source link

Error identifying Active Authentication Hash Algorithm - Chinese ePassport #223

Open rbrouwer opened 2 months ago

rbrouwer commented 2 months ago

The solution of #194 broke AA for recent Chinese passports.

Those will return 0 bytes when giving them an Expected Response Length of 65536. When undoing the changes to secure messaging and setting the Expected Response Length to 256. Those work just fine again.

I guess behaviour such as in JMRTD' AAAPDUSender is wanted; Guessing the correct length and even retrying with 65536 when 256 did not make sense.

rbrouwer commented 2 months ago

Setting Expected Response Length to 231 on this line works for those documents as well. Guessing the extended format is not being liked.

AndyQ commented 2 months ago

Thats really annoying! will have a re-think

AndyQ commented 2 months ago

I've created a test branch - aa_test which makes the following changes:

If anyone can test this branch to see if it works fine that would be great!

rbrouwer commented 2 months ago

I do not believe that would work. Some documents (like that Australian passport in #194) will actually return OK with a partial signature (cutting off whatever did not fit). The implementation in aa_test would return that response and not go for an extended read.

This of course would work fine for this one document I now mentioned in this issue, but that other one would again break.

AndyQ commented 2 months ago

Hmmm thats a good point. Wonder if it would work the other way round? try first with extended read and if that failed then try standard? I've pushed up a small change to try that

rbrouwer commented 2 months ago

That will not work for my document as you would need to handle the NFCPasswordReaderError.ResponseError thrown from TagReader.send(). This Chinese document returns sw - 0x69, sw2 - 0x88 (SM data objects incorrect) when doing the extended version first.

AndyQ commented 2 months ago

Ah of course - small change! - currently doesn't check the error, just retries non-extended - any better?

I'm really interested if the passport will allow the normal read after the extended read fails!

rbrouwer commented 2 months ago

Well... Apparently that will give you: (Extended try) sw - 0x67, sw2 - 0x00, and no data

and then (Normal try) sw - 0x69, sw2 - 0x88, and no data

So apparently not.

furkanisik000 commented 1 month ago

?

baskurtbey commented 1 month ago

@rbrouwer Did you solve the problem

baskurtbey commented 1 month ago

The doInternalAuthentication function is actually not working. Active Authentication (AA) is not being created.

AndyQ commented 1 month ago

Well... Apparently that will give you: (Extended try) sw - 0x67, sw2 - 0x00, and no data

and then (Normal try) sw - 0x69, sw2 - 0x88, and no data

So apparently not.

Thats a shame! Not quite sure what to do here really. I could add a flag to enable extended mode BUT you'd have to know that the passport supports/doesn't support that. I'll did a little more into JMRTD code and see if I can see what they did to solve this. I had a quick look thorugh and they had some functions in bouncy castle that could get certain info about the keys that I haven't yet figured out how to do (I'm sure its possible though).

AndyQ commented 1 month ago

The doInternalAuthentication function is actually not working. Active Authentication (AA) is not being created.

Could you try with version 2.1.1 and see if that works for you (that doesn't enable extended read) and has been working for most passports (except newer Australian and probably other newer ones)?

baskurtbey commented 1 month ago

Hello, I tried but unfortunately there was an issue. It doesn't decode, most likely. The ID is a Turkish ID and it should normally be supported.

baskurtbey commented 1 month ago

It works seamlessly on Android using JMRTD.

AndyQ commented 1 month ago

So what I've done for the moment is instead of enabling extended mode for AA by default, we now have an additional flag set on PassportReader init - useExtendedMode. Not ideal but will think more.

Also, as per #224 - We're now using OpenSSL 1.1.2300 which is a signed release and includes a privacy policy.

Currently main branch only,

baskurtbey commented 1 month ago

The latest update solved my problem, thank you. You saved me from a big issue; I had been researching it for a week. 🙏❤️

danydev commented 3 weeks ago

@AndyQ looks like it's working for @baskurtbey maybe it's time for a release? ;)

Also, when we should use the flag for extended? Which passports as far as you know require it to read them correctly?

AndyQ commented 3 weeks ago

It's definitely getting closer. I'm still not 100% happy as I do t yet know what passport support extended reads and which don't! It looks like some Chinese ones don't but I'm sure there are others too. So I can't yet say when you should use extended read or not. JMRTD does some best guesses based on key lengths BITBi haven't yet figured out how to replicate that in OpenSSL (JMRTD uses bouncycastle which has different APIs).

tomgi commented 3 weeks ago

@andyq for what it's worth, ICAO specifies in https://www.icao.int/publications/documents/9303_p11_cons_en.pdf that

Note.— It should be noted that when using key lengths exceeding 1 848 bits (if Secure Messaging with 3DES is used) / 1 792 bits (if Secure Messaging with AES is used) in Active Authentication with Secure Messaging, Extended Length APDUs MUST be supported by the eMRTD chip and the Inspection System.

For Australian R-series EPassports, DG14 includes id-PACE-ECDH-GM-AES-CBC-CMAC-256 (0.4.0.127.0.7.2.2.4.2.4) as the only supported PACE algorithm. This is a 256-byte key, so based on that we know the chip MUST support extended length APDUs.

Logs from here are:

doPace - inpit parameters
paceOID - 0.4.0.127.0.7.2.2.4.2.4
parameterSpec - 933
mappingType - Generic Mapping
agreementAlg - ECDH
cipherAlg - AES
digestAlg - SHA-256
keyLength - 256