-
Notifications
You must be signed in to change notification settings - Fork 254
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
Error identifying Active Authentication Hash Algorithm - Australian ePassport #194
Comments
DG15 ends with a trailer. Using that trailer the correct RSA-HASH type is selected (here). Apparently that trailer did not make sense. So unless you know what bytes you received and what the hashing algorithm should have been, it will be difficult to say anything about it. |
Hi @harry-anderson, if you can let me know what the hash type byte data is then I can look to see what I'm missing. According to the ICAO Specs, "For interoperability reasons, only SHA-1, SHA-224, SHA-256, SHA-384 and SHA-512 are supported as hash functions for Active Authentication with RSA" so will be interesting to see what you have. e.g. change NFCPassportModel : verifyActiveAuthentication (around line 347) to: |
Hi @AndyQ , Thanks for you message, I made the change to log the hash type byte as you mention Log.error( "Error identifying Active Authentication RSA message digest hash algorithm - hashTypeByte = \(hashTypeByte) \(String(format:"%02X", hashTypeByte))" ) here is what I am getting:
In testing I found that Im getting a different value each time
Let me know if there is anything else you need 👍 |
Hmm still a little unsure - would it be possible to log the whole of the decryptedSig signature? If you aren't happy posting it here please drop me an email |
@AndyQ Here you go:
Thanks in advance |
Hmmm I'm a little stumped here! Going through docs and other code, unless I'm missing something I have no idea what Algorithm is being used here. I did see some mention of it possibly now also using rsspss but as yet no other details (https://crypto.stackexchange.com/questions/75058/using-epassport-with-active-chip-authentication-to-sign-documents) One thing to possibly also try, can you try the ReadID app and see if that passes Active Authentication? |
Just my 2 cents, #174 (fixed in 2.1.1) also presented itself in similar way. The active authentication algorithm was misidentified because the correct byte that is supposed to select the algorithm was not actually read. |
Interesting - I wonder if this is also the case - @harry-anderson could you try turning on full logging and seeing if we get a similar code 0x61 when reading the AA signature? Also, are you using 2.1.1? |
I doubt it would be #174. That would cause "Error reading tag: sw1 - 0x61, sw2 - 0x" to appear, which does not in this case. (TagReader.swift#L284) In this case the response seems to have received the signature "OK". Therefore we reach NFCPassportModel.swift#L305 , which you can see in supplied logs. It seems very likely that the response is indeed not the full response and we therefore do not see the expected trailer. In which case the expectedResponseLength on TagReader.swift#L55 is causing the response to be cut short. |
I was kind of leaning in that direction but wasn't quite sure what affect that would have on existing reads. I'll give it a go on my test passports see what happens. @harry-anderson if you could also try that and see if it gives you more data (you'll need to be on latest version though for it to try to automatically read the extra data). |
Ok I switch to the
Sorry I am not sure how to turn on full logging, can you point me to the file to change this? Here is what I am getting:
|
@harry-anderson, on TagReader.swift#L55 did you try to change expectedResponseLength to -1? Or did you not do that in the log above? |
@rbrouwer Above I had not made the change. I changed this line as you mention:
Here is the log with that change:
Thanks |
@harry-anderson sorry for the delay - that looks like you changed the wrong line - looks like you changed the one in the sendMSEKAT function - rather than the one in the doInternalAuthentication function. Could you check? |
@AndyQ all good, and sorry, I had indeed changed the wrong line. I fixed it now, changed func doInternalAuthentication( challenge: [UInt8] ) async throws -> ResponseAPDU {
let randNonce = Data(challenge)
let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: -1)
return try await send( cmd: cmd )
} Here is the log:
|
OK, so thats out then, looks like it didn't even get the signature back this time (which is odd esp as no error). Am a little stumped at the moment. |
We're recently run into the same issue and together with @donbobka we've made some discoveries that we hope will help in solving this issue 🙂 Australian R series passport:
diff --git a/Sources/NFCPassportReader/SecureMessaging.swift b/Sources/NFCPassportReader/SecureMessaging.swift
index 575185a..765745b 100644
--- a/Sources/NFCPassportReader/SecureMessaging.swift
+++ b/Sources/NFCPassportReader/SecureMessaging.swift
@@ -88,7 +88,7 @@ public class SecureMessaging {
// otherwise its a single byte of size
let size = do87.count + do97.count + do8e.count
var dataSize: [UInt8]
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 256 { // possibly should be apdu.expectedResponseLength > 231 to account for SM envelope
dataSize = [0x00] + intToBin(size, pad: 4)
} else {
dataSize = intToBin(size)
@@ -98,7 +98,7 @@ public class SecureMessaging {
// If the data is more that 255, specify the we are using extended length (0x00, 0x00)
// Thanks to @filom for the fix!
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 256 { // possibly should be apdu.expectedResponseLength > 231 to account for SM envelope
protectedAPDU += [0x00,0x00]
} else {
protectedAPDU += [0x00]
diff --git a/Sources/NFCPassportReader/TagReader.swift b/Sources/NFCPassportReader/TagReader.swift
index 8791622..ac8366b 100644
--- a/Sources/NFCPassportReader/TagReader.swift
+++ b/Sources/NFCPassportReader/TagReader.swift
@@ -52,7 +52,7 @@ public class TagReader {
func doInternalAuthentication( challenge: [UInt8] ) async throws -> ResponseAPDU {
let randNonce = Data(challenge)
- let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 256)
+ let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 65536)
return try await send( cmd: cmd )
}
We used to have the same issue on android using jmrtd library and it was just recently fixed in https://sourceforge.net/p/jmrtd/code/1878/ - seems like they try to predict the AA signature size based on the AA public key from DG15. We might submit a PR with a proper fix based on jmrtd's approach in the following days/weeks, but we would also be very happy for you to do it, since we don't have access to too many passport to test against and our understanding of the ICAO standards and NFC tech is also quite limited 😅 |
Thanks for that, I'll try using enhanced mode on my test passports and seeing what happens. |
I did this for the reading of the data groups and it works fine (Swedish National ID) and much faster. It could be a good idea to have this as the default and then reintroduce the fallback mechanism to drop to 0xA0 (160) on failure. |
It is sort of there:
But what about if we are not using BAC but a PACE only document? |
I tried out the changes mentioned by @tomgi and AA check now succeeds! 🥳 Changes: index 575185a..958b391 100644
--- a/Sources/NFCPassportReader/SecureMessaging.swift
+++ b/Sources/NFCPassportReader/SecureMessaging.swift
@@ -88,7 +88,7 @@ public class SecureMessaging {
// otherwise its a single byte of size
let size = do87.count + do97.count + do8e.count
var dataSize: [UInt8]
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 256 {
dataSize = [0x00] + intToBin(size, pad: 4)
} else {
dataSize = intToBin(size)
@@ -98,7 +98,7 @@ public class SecureMessaging {
// If the data is more that 255, specify the we are using extended length (0x00, 0x00)
// Thanks to @filom for the fix!
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 256 {
protectedAPDU += [0x00,0x00]
} else {
protectedAPDU += [0x00]
diff --git a/Sources/NFCPassportReader/TagReader.swift b/Sources/NFCPassportReader/TagReader.swift
index 8791622..2212a41 100644
--- a/Sources/NFCPassportReader/TagReader.swift
+++ b/Sources/NFCPassportReader/TagReader.swift
@@ -52,7 +52,7 @@ public class TagReader {
func doInternalAuthentication( challenge: [UInt8] ) async throws -> ResponseAPDU {
let randNonce = Data(challenge)
- let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 256)
+ let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 65536)
return try await send( cmd: cmd )
} Snippet from the logs:
|
@tomgi for completeness I also tried setting --- a/Sources/NFCPassportReader/SecureMessaging.swift
+++ b/Sources/NFCPassportReader/SecureMessaging.swift
@@ -88,7 +88,7 @@ public class SecureMessaging {
// otherwise its a single byte of size
let size = do87.count + do97.count + do8e.count
var dataSize: [UInt8]
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 231 {
dataSize = [0x00] + intToBin(size, pad: 4)
} else {
dataSize = intToBin(size)
@@ -98,7 +98,7 @@ public class SecureMessaging {
// If the data is more that 255, specify the we are using extended length (0x00, 0x00)
// Thanks to @filom for the fix!
- if size > 255 {
+ if size > 255 || apdu.expectedResponseLength > 231 {
protectedAPDU += [0x00,0x00]
} else {
protectedAPDU += [0x00]
diff --git a/Sources/NFCPassportReader/TagReader.swift b/Sources/NFCPassportReader/TagReader.swift
index 8791622..2212a41 100644
--- a/Sources/NFCPassportReader/TagReader.swift
+++ b/Sources/NFCPassportReader/TagReader.swift
@@ -52,7 +52,7 @@ public class TagReader {
func doInternalAuthentication( challenge: [UInt8] ) async throws -> ResponseAPDU {
let randNonce = Data(challenge)
- let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 256)
+ let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 65536)
return try await send( cmd: cmd )
} Logs:
But If I remove the --- a/Sources/NFCPassportReader/TagReader.swift
+++ b/Sources/NFCPassportReader/TagReader.swift
@@ -52,7 +52,7 @@ public class TagReader {
func doInternalAuthentication( challenge: [UInt8] ) async throws -> ResponseAPDU {
let randNonce = Data(challenge)
- let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 256)
+ let cmd = NFCISO7816APDU(instructionClass: 00, instructionCode: 0x88, p1Parameter: 0, p2Parameter: 0, data: randNonce, expectedResponseLength: 65536)
return try await send( cmd: cmd )
} Logs:
Hope that helps |
Thats great news, I'll test out across my dev passports hopefully today (work has got in the way this week!) and see if thats a compatible change. |
That tested fine on all my test passports (going back to expiring in 2015). I've committed the change to main - but not yet done a release. If you could please check out the main branch and confirm it still works for you that would be great. I think I'm willing to push this up even with the slight risk it breaks with old passports. Thanks @tomgi, @donbobka, @harry-anderson for the fix and testing |
I tested it out with the latest commit |
I think it could be good to first check if extended mode is supported:
|
useExtended is just a flag so I can test without this functionality |
Sadly the current fix does break AA for Chinese passports (recent models). I believe a solution such as in JMRTD might be needed given you cannot just expect extended format to just work for all. |
Re-opening this - will take a look when I get a chance |
I had made #223 for that. Feel free to close either. Duplicates are not helpful. |
I'm going to modify this change. It really needs a little re-think, but for the moment I will put it behind a flag. |
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! |
Hi @AndyQ, I tested out branch
|
One thing, I've just change things slightly so that extended mode isn't the default as this breaks other passports. I've still some more investigation to do but for the moment I've added a new flag to PassportReader - useExtendedMode. This defaults to false if not specified. Currently in main branch only at the moment. |
I used the sample application (SPM) to scan an Australian ePassport Issued 2023 and found Passive Authentication succeeds while Active Authentication fails with the following error printed in the session output:
Any idea what is happening here?
Thanks 🙏
The text was updated successfully, but these errors were encountered: