search

arekinath / PivApplet

PIV applet for JavaCard 2.2.2 and 3.0.4+ with full ECDSA/ECDH support
111 stars 37 forks source link

fix bug where responses are truncated under certain circumstances #72

Closed swissbit-csteuer closed 1 year ago

swissbit-csteuer commented 1 year ago

If a SGList has multiple write buffers, SGList#readInto should iterate through all write buffers and read the remaining bytes into the destination buffer until len bytes have been read, or no more bytes remain. Before this fix, SGList#readInto stopped iterating through the write buffers at the first write buffer that had no remaining bytes. However, it can happen that an empty write buffer is between two write buffers with remaining bytes:

WPTR_BUF -> buffer_x   (remaining: 10)
            buffer_x+1 (remaining:  0)
            buffer_x+2 (remaining: 10)

In that case, the bytes in buffer_x+2 are not read and the applet returns a truncated response (e.g. when generating a keypair)

swissbit-csteuer commented 1 year ago

Hi, I found this bug when trying to generate keys with the OpenSC piv-tool on Linux. The bug only happens on certain APDU sequences. E.g. key generation on the same host works with the yubico-piv-tool but not with the OpenSC piv-tool.

Here is the sequence of APDUs with which the bug occurs:

APDU 1 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 08 ..?..\.~.

Incoming APDU (2 bytes):
69 86 i.

APDU 2 (SELECT)

Outgoing APDU (15 bytes):
00 A4 04 00 09 A0 00 00 03 08 00 00 10 00 00 ...............

Incoming APDU (129 bytes):
61 81 7C 4F 0B A0 00 00 03 08 00 00 10 00 01 00 a.|O............
79 07 4F 05 A0 00 00 03 08 50 1B 50 69 76 41 70 y.O......P.PivAp
70 6C 65 74 20 76 30 2E 39 2E 30 2F 52 45 65 53 plet v0.9.0/REeS
41 78 4C 72 61 44 5F 50 1E 67 69 74 68 75 62 2E AxLraD_P.github.
63 6F 6D 2F 61 72 65 6B 69 6E 61 74 68 2F 50 69 com/arekinath/Pi
76 41 70 70 6C 65 74 AC 06 80 01 03 06 01 00 AC vApplet.........
06 80 01 0C 06 01 00 AC 06 80 01 07 06 01 00 AC ................
06 80 01 11 06 01 00 AC 06 80 01 14 06 01 00 90 ................
00                                              .

APDU 3 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 07 08 ..?..\._...

Incoming APDU (10 bytes):
53 32 F0 15 A0 00 00 01 61 2C S2......a,

APDU 4

Outgoing APDU (5 bytes):
00 C0 00 00 2C ....,

Incoming APDU (46 bytes):
16 FF 02 2A A2 AB 70 03 9C 55 10 DD F0 64 20 EC ...*..p..U...d .
EB F1 01 21 F2 01 21 F3 00 F4 00 F5 01 10 F6 00 ...!..!.........
F7 00 FA 00 FB 00 FC 00 FD 00 FE 00 90 00       ..............

APDU 5 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 07 34 ..?..\._..4

Incoming APDU (54 bytes):
53 32 F0 15 A0 00 00 01 16 FF 02 2A A2 AB 70 03 S2.........*..p.
9C 55 10 DD F0 64 20 EC EB F1 01 21 F2 01 21 F3 .U...d ....!..!.
00 F4 00 F5 01 10 F6 00 F7 00 FA 00 FB 00 FC 00 ................
FD 00 FE 00 90 00                               ......

APDU 6 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 08 ..?..\.~.

Incoming APDU (10 bytes):
7E 12 4F 0B A0 00 00 03 61 0C ~.O.....a.

APDU 7

Outgoing APDU (5 bytes):
00 C0 00 00 0C .....

Incoming APDU (14 bytes):
08 00 00 10 00 01 00 5F 2F 02 40 00 90 00 ......._/.@...

APDU 8 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 14 ..?..\.~.

Incoming APDU (22 bytes):
7E 12 4F 0B A0 00 00 03 08 00 00 10 00 01 00 5F ~.O............_
2F 02 40 00 90 00                               /.@...

APDU 9 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 08 ..?..\.~.

Incoming APDU (10 bytes):
7E 12 4F 0B A0 00 00 03 61 0C ~.O.....a.

APDU 10

Outgoing APDU (5 bytes):
00 C0 00 00 0C .....

Incoming APDU (14 bytes):
08 00 00 10 00 01 00 5F 2F 02 40 00 90 00 ......._/.@...

APDU 11 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 14 ..?..\.~.

Incoming APDU (22 bytes):
7E 12 4F 0B A0 00 00 03 08 00 00 10 00 01 00 5F ~.O............_
2F 02 40 00 90 00                               /.@...

APDU 12 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 07 08 ..?..\._...

Incoming APDU (10 bytes):
53 32 F0 15 A0 00 00 01 61 2C S2......a,

APDU 13

Outgoing APDU (5 bytes):
00 C0 00 00 2C ....,

Incoming APDU (46 bytes):
16 FF 02 2A A2 AB 70 03 9C 55 10 DD F0 64 20 EC ...*..p..U...d .
EB F1 01 21 F2 01 21 F3 00 F4 00 F5 01 10 F6 00 ...!..!.........
F7 00 FA 00 FB 00 FC 00 FD 00 FE 00 90 00       ..............

APDU 14 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 07 34 ..?..\._..4

Incoming APDU (54 bytes):
53 32 F0 15 A0 00 00 01 16 FF 02 2A A2 AB 70 03 S2.........*..p.
9C 55 10 DD F0 64 20 EC EB F1 01 21 F2 01 21 F3 .U...d ....!..!.
00 F4 00 F5 01 10 F6 00 F7 00 FA 00 FB 00 FC 00 ................
FD 00 FE 00 90 00                               ......

APDU 15 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 00 ..?..\.~.

Incoming APDU (22 bytes):
7E 12 4F 0B A0 00 00 03 08 00 00 10 00 01 00 5F ~.O............_
2F 02 40 00 90 00                               /.@...

APDU 16 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 0C 08 ..?..\._...

Incoming APDU (10 bytes):
53 08 C1 01 00 C2 01 00 61 02 S.......a.

APDU 17

Outgoing APDU (5 bytes):
00 C0 00 00 02 .....

Incoming APDU (4 bytes):
FE 00 90 00 ....

APDU 18 (GET DATA)

Outgoing APDU (11 bytes):
00 CB 3F FF 05 5C 03 5F C1 0C 0A ..?..\._...

Incoming APDU (12 bytes):
53 08 C1 01 00 C2 01 00 FE 00 90 00 S...........

APDU 19 (GET DATA)

Outgoing APDU (9 bytes):
00 CB 3F FF 03 5C 01 7E 00 ..?..\.~.

Incoming APDU (22 bytes):
7E 12 4F 0B A0 00 00 03 08 00 00 10 00 01 00 5F ~.O............_
2F 02 40 00 90 00                               /.@...

APDU 20 (GENERAL AUTH GET CHALLENGE)

Outgoing APDU (10 bytes):
00 87 03 9B 04 7C 02 81 00 00 .....|....

Incoming APDU (14 bytes):
7C 0A 81 08 F9 C0 62 13 58 52 89 65 90 00 |.....b.XR.e..

APDU 21 (GENERAL AUTH RESPONSE)

Outgoing APDU (17 bytes):
00 87 03 9B 0C 7C 0A 82 08 C9 26 EC 6D DA 47 1F .....|....&.m.G.
6F                                              o

Incoming APDU (2 bytes):
90 00 ..

APDU 22 (GENERATE RSA-2048 KEYPAIR IN SLOT 9A)

Outgoing APDU (11 bytes):
00 47 00 9A 05 AC 03 80 01 07 00 .G.........

Incoming APDU (11 bytes):
7F 49 82 01 09 81 82 01 00 90 00 .I.........

As you can see the response to the generate-key command is truncated after 9 bytes.

When processing APDU 3, where piv-tool tries to read the Card Capability Container, the applet allocates write buffer 1 of the outgoing SGList.

When writing the modulus of the generated RSA key into the outgoing SGList there are only 9 bytes left in write buffer 0, therefore SGList#startReserve switches to buffer 1 for the remaining bytes. It tries to expand buffer 1, but after expanding the buffer there are still not enough bytes available, therefore it switches to buffer 2, allocates it and writes the remaining bytes into the buffer.

Now we have the situation described above. Since nothing was written into buffer 1 it has zero remaining bytes while buffer 0 has 9 bytes remaining and buffer 2 has 246 bytes remaining.

The yubico-piv-tool never reads the Card Capability Container and therefore buffer 1 is not allocated until the RSA key is generated.

I'm not sure if my analysis is 100% correct, since it was quite hard for me to follow the code.

dengert commented 1 year ago

In APDU 3 the current OpenSC card-piv.c tries to use GET DATA to read the read the first 8 bytes of the CCC object, which the card returned but also indicated there were more bytes available even though it only asked for 8. The OpenSC uses a get response in APDU 4 to read the rest of the object.

In APDU 5 it asks for the CCC with the actual size of the CCC (including the 5C tag and length)

It asks for first 8 bytes to determine the size of the object for all objects to determine the size. This is a trick that works on all PIV cards, but it may be causing some problem with PivApplet.

I don't really know how PivApplet SGlist buffers work but https://github.com/OpenSC/OpenSC/pull/2053 includes code to not use this trick, but instead allocate large buffer so any object can be read without first knowing the size of the object.

swissbit-csteuer commented 1 year ago

https://github.com/OpenSC/OpenSC/pull/2053 would probably solve the problem for this sequence of APDUs, but I fear it will still happen with other sequences that do not use the size trick. E.g., the problem described in https://github.com/arekinath/PivApplet/issues/51 seems to be caused by the same bug.

swissbit-csteuer commented 1 year ago

I forgot to create a separate branch for this change in our fork so that this PR is now tracking our master branch. I will close this PR and open a new one so that we can continue pushing other unrelated changes to the master branch of our fork. Sorry for the inconvenience.