Yubico / ykneo-openpgp

OpenPGP applet for the YubiKey NEO
https://developers.yubico.com/ykneo-openpgp/
GNU General Public License v2.0
215 stars 67 forks source link

(resubmitting) Discretionary data objects nesting fix #22

Closed josecastillo closed 9 years ago

josecastillo commented 9 years ago

Resubmitting this so my changes are on a clean branch. More detail here.

Per the OpenPGP card specification (4.3.1 "DOs for GET DATA") and ISO 7816-4 (5.4.4.3 "Independent tag allocation schemes"), discretionary data objects in application related data must be nested within the 0x73 discretionary data objects tag. The current implementation almost does this; unfortunately, the discretionary data objects tag has a hard-coded length of 0, so when the objects themselves follow they are not nested:

Output of GET DATA 6E, vanilla Yubikey NEO
6E 81DD                                                    # Application Related Data, length 221
 ├─4F   10 D2 76 00 01 24 01 02 00 00 06 03 01 51 21 00 00 # Application AID
 ├─5F52 0F 00 73 00 00 80 00 00 00 00 00 00 00 00 00 00    # Historical Bytes
 ├─73   00                                                 # Discretionary Data Objects of length 0
 ├─C0   0A F0 00 00 FF 04 C0 00 FF 00 FF                   # Extended capabilities
 ├─C1   06 01 08 00 00 11 03                               # Algorithm Attributes
 ├─C2   06 01 08 00 00 11 03                               # Algorithm Attributes
 ├─C3   06 01 08 00 00 11 03                               # Algorithm Attributes
 ├─C4   07 01 7F 7F 7F 03 03 03                            # PW Status Bytes
 ├─C5   3C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 # Fingerprints
 ├─C6   3C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 # CA Fingerprints
 └─CD   0C 00 00 00 00 00 00 00 00 00 00 00 00             # Key generation timestamps

This patch correctly sets the length of the discretionary data objects tag, bringing the behavior of the application in line with the spec:

Output of GET DATA 6E, with patch applied
6E 81DE                                                    # Application Related Data, length 222
 ├─4F   10 D2 76 00 01 24 01 02 00 00 00 00 00 00 01 00 00 # Application AID
 ├─5F52 0F 00 73 00 00 80 00 00 00 00 00 00 00 00 00 00    # Historical Bytes
 └─73 81B7                                                 # Discretionary Data Objects of length 183
    ├─C0 0A F0 00 00 FF 04 C0 00 FF 00 FF                  # Extended capabilities
    ├─C1 06 01 08 00 00 11 03                              # Algorithm Attributes
    ├─C2 06 01 08 00 00 11 03                              # Algorithm Attributes
    ├─C3 06 01 08 00 00 11 03                              # Algorithm Attributes
    ├─C4 07 00 7F 7F 7F 03 03 03                           # PW Status Bytes
    ├─C5 3C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 # Fingerprints
    ├─C6 3C 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 # CA Fingerprints
    └─CD 0C 00 00 00 00 00 00 00 00 00 00 00 00            # Key generation timestamps

While the gnupg desktop application seems to accept this DO whether the length is set correctly or not, the "official" cards available from Kernel Concepts do set the length properly, and this change brings the applet in line with the relevant specifications.

jas4711 commented 9 years ago

Thanks -- is there an easy way to test this? Piping stuff into gpg-connect-agent for example? How do you send the GET DATA command?

jas4711 commented 9 years ago

Looking at the patch, there is nothing guaranteeing that the object size is actually 128..255, is there? what happens if the stored object is just 60 bytes? What happens if it is 300 bytes?

josecastillo commented 9 years ago

You can issue the GET DATA command by entering raw APDUs to the gpg-connect-agent, but the easiest way to see this output is to invoke gpg --verbose --verbose --card-status (with two --verbose's); the raw application related data DO will be dumped out on the terminal.

As for the length, technically the 0x81 prefix doesn't require the value be 128..255. The specification says that a two byte value can represent any length from 0..255, so there's nothing illegal about encoding a length of 60 as 813C. But in this case, we know it will be 183 (0xB7); the size of each of the discretionary data objects is fixed in the specification, and that's reflected in the code. Those fixed sizes, plus tags and lengths, are what we add up to get the length in the end.

jas4711 commented 9 years ago

That command doesn't print anything for me -- do I have to create some application data first? How?

I think if we can understand how to reproduce a difference between the kernelceoncept cards and ours, there shouldn't be a problem applying the patch, but I want to make sure I understand first.

jas4711 commented 9 years ago

This seems related to pull request #21 by the way? Is there overlap here?

josecastillo commented 9 years ago

The gpg-connect-agent is probably the best bet then; invoke gpg-connect-agent --hex and at the prompt you can get application related data with scd apdu 00 ca 00 6e ff. You shouldn't need to do any card customization at all; the returned application related data will just have zeroes for all the fingerprints and timestamps.

21 is a separate issue, but it's also a difference from the kernel concepts card. I'm not sure if #21 is a violation of the spec, though, which is why I didn't mess with it. The spec suggests that constructed DO's "are returned including their tag and length", which your implementation does but the kernel concepts card doesn't. The kernel concepts card returns essentially the same output sans the first three bytes.