APDU parser has multiple errors

[micolous]

I found a couple of bugs while reviewing the OpenSK APDU parser, because while writing a FIDO client library, I noticed many FIDO implementations have buggy ISO 7816 implementations. I haven't run this on "real hardware" to try it out properly.

1. APDU parser does not handle extended form with N_c = 0 and N_e > 0 ("Case 2E"). It would error out with WRONG_LENGTH.

   Example request, where N_c = 0 and N_e = 0x1234:

   CLA INS P1 P2 0x00 0x12 0x34
   

   U2F GET_VERSION is impacted by this, other U2F commands and CTAP2 (over an ISO 7816 transport) always have command payloads.

   The parser appears to only handle "Case 2S" (short form N_c = 0 and N_e > 0), but supporting extended form is required for all transports, and supporting short form is only required over NFC.

2. APDU parser accepts malformed 3 byte L_c + 3 byte L_e.

   Per ISO 7816-4:2005 Section 5.1 (Command-response pairs), when N_c > 0 and N_e > 0 and using extended form ("Case 4E"), the correct representation is:

   CLA INS P1 P2 0x00 {Nc as uint16_be} {command bytes} {Ne as uint16_be}
   

   The implementation here appears to also incorrectly accept malformed requests of the form:

   CLA INS P1 P2 0x00 {Nc as uint16_be} {command bytes} 0x00 {Ne as uint16_be}
   

   The only reason L_e should be prefixed with 0x00 is if the L_c field is absent and L_e is in extended form ("Case 2E").

   Accepting malformed requests can cause implementers "testing against OpenSK" to make mistakes which are only exposed when testing with another FIDO implementation which follows ISO 7816 correctly. I've already seen subtle differences in existing FIDO certified devices with malformed inputs.

Note: "Case 2E", "Case 2S" and "Case 4E" references to ISO 7816-3:2006 section 12.1.3 (Decoding conventions for command APDUs); S = short form, E = extended form.

Case	Command Data Field	Response Length Field	Total length
Case 1	None	None	4 bytes
Case 2S	None	Present (1 byte)	5 bytes
Case 2E	None	Present (3 bytes)	7 bytes
Case 3S	Present (1 byte)	None	5 + (command data length) bytes
Case 3E	Present (3 bytes)	None	7 + (command data length) bytes
Case 4S	Present (1 byte)	Present (1 byte)	6 + (command data length) bytes
Case 4E	Present (3 bytes)	Present (2 bytes)	9 + (command data length) bytes

[kaczmarczyck]

Hi, thanks for the writeup, this is useful!
Our NFC implementation is still work in progress, and these bugs will be great test cases when we continue working on it.

[micolous]

By the way, after getting my hands on more FIDO2 keys, as far as I can tell pretty much only Yubikey implements the NFC side of things even close to correctly (there are bugs, just minor ones). Feitian, Hideez and Token2 keys don't implement extended L_c/L_e at all (as required by the CTAP spec), even though they are allegedly "certified"... so OpenSK is already doing better. 😄

Relatedly, I wasn't able to find where OpenSK's ATR was, but that needs to flag its extended L_c/L_e in the "card capabilities" field (per ISO 7816-4 §8.1.1.2.7); again that's another case where only Yubikey seems to do this correctly.