Fix length of discretionary data objects tag in application related data

[josecastillo]

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.

[josecastillo]

Closing this pull request temporarily, as I failed to update the test cases to account for this change.

[josecastillo]

This pull request should be good to go, let me know if you have any questions.

[jas4711]

Thanks. Do you agree to license your contribution under the copyright/license that this project is published under?

[josecastillo]

Absolutely, I agree to license my contribution under the copyright and license terms of the ykneo-openpgp project.

[jas4711]

Btw, evaluating this, we wondered what the original problem was? Can you describe some set of commands that trigger a problem?

[josecastillo]

As I mentioned, the gnupg application doesn’t seem to have an issue with this; I noticed it because I was writing some software to communicate with OpenPGP smart cards, following the specification closely. Testing with several different cards, the Kernel Concepts card worked fine, as did another JavaCard implementation here; it was only the NEO that failed. You can see here how the length is set in the other JavaCard implementation.

It looks like gnupg doesn’t choke on this because its method of handling nested tags is to look first at nesting level 0, then recursively look deeper if not found. Since your implementation does have these objects at depth 0, it finds them and goes about its business. Nonetheless, the spec indicates that these objects should be nested, and software adhering closely to the spec will expect that. Currently it’s only an implementation quirk of gnupg that allows the zero-length field to work.

[asheiduk]

I stumbled across this issue too using custom code.

[josecastillo]

Since I'm doing some other things in my master branch here, I'm going to close this pull request; didn't realize github would incorporate later commits automatically. Still maintain that the initial request, fixing the nesting issue, would be a good fix, but I don't want to bug y'all with the additional stuff I'm doing.

[klali]

Hello,

While we haven't really got around to looking at this properly and merging it I think it makes sense to keep the pull requests sepparate. Maybe you can add this stuff to a sepparate branch to keep the pull request clean?

/klas

[josecastillo]

Sure thing, I've created a separate branch for this change, and opened a new pull request here.