GIDS Decipher fix for TPM #1881

dengert · 2019-12-04T01:00:36Z

If GIDS decipher APDU fails with status '65 00' it will be retried without the leading "Padding Indication" byte. This is done by a new gids_decipher routine was created based in the iso7816.c version.

Extended APDU is turned off as this may not be supported on some cards. Chaining is used instead, as it is supported on all cards.

Some card->flags are defined in case there are issues with extended or max_send_size.

Tested with pkcs11-tool --test --login on Windows 10 with a TPM 2.0 module via "Microsoft Virtual Smart Card".

On branch gids-decipher
Changes to be committed:
modified: src/libopensc/card-gids.c

Checklist

PKCS#11 module is tested

dengert · 2019-12-04T01:39:36Z

@vletoux @el-oso Can you test this fix. You can find the *.msi files under the "All checks have passed" click on "show all checks". Click on "Details" on the AppVeyor line. Under "Jobs" click on each of the VSVER=12 lines, then click on "Artifacts" to find one of the MSI files.

vletoux · 2019-12-04T17:50:36Z

It is OK for the TPM chip.

C:\Program Files\OpenSC Project\OpenSC\tools>pkcs11-tool.exe --test --login
Using slot 3 with a present token (0xc)
Logging in to "GIDS card (UserPIN)".
Please enter User PIN: C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  ERR: C_GenerateRandom failed: CKR_FUNCTION_NOT_SUPPORTED (0x54)
Digests:
  all 4 digest functions seem to work
  MD5: OK
  SHA-1: OK
  RIPEMD160: OK
Signatures (currently only for RSA)
  testing key 0 (93be0ded-5552-4fbc-811b-eb8812b9deff)
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
Verify (currently only for RSA)
  testing key 0 (93be0ded-5552-4fbc-811b-eb8812b9deff)
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
Decryption (currently only for RSA)
  testing key 0 (93be0ded-5552-4fbc-811b-eb8812b9deff)
    RSA-PKCS: OK
1 errors

Better for the GIDS applet. An error is shown but I don't know if it is related to the applet or the driver

C:\Program Files\OpenSC Project\OpenSC\tools>pkcs11-tool.exe --test --login
Using slot 0 with a present token (0x0)
Logging in to "GIDS card (UserPIN)".
Please enter User PIN: C_SeedRandom() and C_GenerateRandom():
  seeding (C_SeedRandom) not supported
  ERR: C_GenerateRandom failed: CKR_FUNCTION_NOT_SUPPORTED (0x54)
Digests:
  all 4 digest functions seem to work
  MD5: OK
  SHA-1: OK
  RIPEMD160: OK
Signatures (currently only for RSA)
  testing key 0 (07cc67a2-e725-4f97-aa4c-6d1a00886da9)
  all 4 signature functions seem to work
  testing signature mechanisms:
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
    SHA256-RSA-PKCS: OK
Verify (currently only for RSA)
  testing key 0 (07cc67a2-e725-4f97-aa4c-6d1a00886da9)
    RSA-PKCS: OK
    SHA1-RSA-PKCS: OK
    MD5-RSA-PKCS: OK
    RIPEMD160-RSA-PKCS: OK
Decryption (currently only for RSA)
  testing key 0 (07cc67a2-e725-4f97-aa4c-6d1a00886da9)
    RSA-PKCS: error: PKCS11 function C_Decrypt failed: rv = CKR_DATA_LEN_RANGE (0x21)
Aborting.

vletoux · 2019-12-04T18:03:40Z

Here is the instructions sent by OpenSC => failed

10 2A 80 86 FF 00 0B A8 75 AD 85 F5 4D F3 AA 3E
D1 44 50 DF 71 B1 19 72 2E 84 BA 7F 42 4B CF A9
26 F1 59 80 6E AC DA 93 F0 D4 3A C4 E9 15 87 1D
E5 35 21 78 CA E0 B3 A7 F1 DB 82 D3 D7 B0 0C 55
3E 7C FA 3B 93 58 8F E9 74 13 AC A7 1A 55 BC 03 
30 62 A4 83 EB 0D B5 86 50 AB 3B 0F FA 36 88 ED
9E A8 C8 D9 37 D5 CF AB 67 6A 2D 2F EC 81 94 7D 
82 C3 F9 26 AC 3F 1A 28 D2 BB 9B C3 BE 9B A4 E7 
71 4C 47 E8 B6 84 34 62 26 24 15 E7 75 53 11 0E 
95 2A 9B BC 8C 37 CC D9 B7 00 49 53 06 2B AC 0A 
7E 8A 71 D3 4A 37 E1 B3 FA 54 F7 24 79 5B 94 DC 
7D C8 A8 54 43 91 DE C0 8E 0C D2 FE A3 99 28 F5 
F5 E6 A5 93 5C 62 A6 A2 6F 46 52 E1 55 C0 22 4A
E3 C6 6B EC 86 F6 AA 40 1A DC 1B 46 68 05 83 CE
55 1B 20 EE 65 E6 9F 25 AD 7E 1D 0D FF D2 FC 04 
49 28 9D B8 C4 42 F1 34 A5 D9 01 41 FB 5C D0 91
12 79 5C AE

90 00 ..

00 2A 80 86 02 01 53 00
67 00

Here are the instructions sent by certutil -scinfo


0000  10 2a 80 86 f0 cb 86 71 fc 69 0d 42 10 0c a7 f5 fa d3 70 4c 66 e3 39 8d 03 b9 de 7a f5 95 e0 50 99 ba 78 aa e6 43 e8 64 2d d0 f0 7f 2e c1 81 d4 7b 2a c1 ad  .*.....q.i.B......pLf.9....z...P..x..C.d-.......{*..
0034  10 29 47 5b a8 dd 80 25 d1 21 01 58 06 12 9b fc ff e3 69 a1 d4 07 b7 70 ec 57 c3 52 83 27 1f 83 14 65 db 9e 31 8b 5c ca bc e8 8a c6 4a c5 86 95 db d4 99 ef  .)G[...%.!.X......i....p.W.R.'...e..1.\.....J.......
0068  63 e8 ad 20 d6 44 b3 19 34 df c8 a1 b4 f7 88 17 b5 b1 7a cf b7 71 e0 ea c8 21 94 00 9e 68 e9 79 59 3f aa 23 f5 d7 bf bd 1d f0 dc 87 e6 b9 0a 60 95 0d b3 60  c.. .D..4.........z..q...!...h.yY?.#...........`...`
009c  86 4d ad 2c ed fd f9 8a 96 55 3f e0 e5 4d a6 a9 c4 db 6c c8 91 a7 bd 4b 34 83 be 78 b0 d5 b2 01 88 5d 76 25 6a 8a 8b a8 f6 7c 6d 7e 3d ca e9 60 f1 85 38 35  .M.,.....U?..M....l....K4..x.....]v%j....|m~=..`..85
00d0  f2 e1 21 6f 4f 10 98 9f 7b 5a 2b a1 50 6a 4f ea 7a 2b e6 83 16 4a 40 fe b1 03 0d 1f c3 ab 67 78 a6 6a 9d 63 21

90 00

0000  00 2a 80 86 10 ba 8f 6b 01 ec 3d fc 66 bc ac 3e 42 e4 91 14 cb 00                                                                                            .*.....k..=.f..>B.....                 

0000  bb ae 97 a9 7d 0b 16 e5 97 92 2b da 62 83 1b b5 90 00                                                                                                        ....}.....+.b.....

In summary, OpenSC send then 260 + 8 = 268 bytes
certutil -scinfo send then 245 + 22 = 267 bytes
The slipt is not the same and one additional byte is sent.

dengert · 2019-12-04T18:56:22Z

The failure looks like the max_send_size is 255, vs 240. This could be a reader or card restriction. Does the applet, reader or card have any restriction?I know you said some do not support extended.

You can test by changing the opensc.conf to something like this:
opensc-gids.conf.txt Set ATR to your card. Will also produce an opensc-debug.log

If the restriction is for the card, we can force it. Or if it can be read from the card, we can use what the card says.

dengert · 2019-12-04T19:02:46Z

Note that GIDS docx says:

"If the commands are to be sent without secure messaging, the length of block shall be set to a maximum of 0xFF = 255 bytes." So it should have worked.

vletoux · 2019-12-04T20:47:26Z

So problem is the additional byte whose value is 0 at the start of the APDU. Not the way it is chained

dengert · 2019-12-04T21:32:24Z

I looked at this again. I don't thing it is a max_send_size.

Microsoft Virtual Smart Card with TPM with PI byte (00) and chaining 255 + 16 = 257 returned "6D 00". A different failure which the PR will then try without the PI, sending 255 + 15 = 256 bytes.

The failure in #1881 (comment) has the PI byte (00) and chaining 255 + 16 = 257. with status error "67 00" = Le wrong length. But first chain returns 90 00 and second APDU returns the "67 00".

Certutil sends no PI byte and chains 240 + 10 = 256 and it works. So I don't think the max_send_size is an issue

So each implementation handled the error differently.

The PR only retries without the PI, if the first APDU chain failed with "6D 00" Is there any implementation that may expect a PI byte?

I can change the PR to never send the PI in that case or change to try without PI first then
try with PI.

What do you think?

vletoux · 2019-12-04T21:37:14Z

I think the minidriver does handle both chaining and extended APDU and that the implementation is different. A bug.
And I based my implementation on what MS did, so I incorporated the bug.
You can try what you told.
I got 15 minutes to test before going to sleep

dengert · 2019-12-05T02:44:57Z

@vletoux Please try the new artifacts. They do not send a PI byte for Decipher.

vletoux · 2019-12-05T10:15:42Z

Tested both 1024 & 2048 keys on both TPM and GIDS card
=> everything is working

Jakuje · 2019-12-04T12:25:25Z

src/libopensc/card-gids.c

+
+	/* for testing ... */
+	if (card->flags & 0x00010000) {
+	card->caps = SC_CARD_CAP_APDU_EXT;


wrong indentation

Jakuje · 2019-12-05T13:06:24Z

src/libopensc/card-gids.c

+
+	sbuf[0] = 0; /* padding indicator byte, 0x00 = No further indication */
+	memcpy(sbuf + 1, crgram, crgram_len);
+	apdu.data = sbuf + 1; /* Skip padding indication not needed unless SM */


Do I see right that you add the padding indcator byte here and than not use it in the APDU?

Yes, The GIDS docx talks about the PI being present, but the Microsoft "Virtual Smart Card (and @vletoux applet) will not work if the PI is present in decipher. iso7816-4 hints that it may be optional. The GIDS docx says PI is needed if Secure Messaging is being used but the driver does not currently support SM.

The gids_decipher was copied from iso7816.c which always adds a PI of 0x00.
I left the code in case we ever add SM to the driver. debug logs show certutil does not use SM. and only works if the PI is not present.

I could just have easily not allocated the sbuf and used something like: apdu.data = crgram;

Suggestions?

I do not know anything about GIDS, but if it is just more code (copied from ISO), which is not used, I would rather see it simplified without the needless allocations and copying. If the SM will be implemented some time, adding the padding indicator will probably be least work.

frankmorgner · 2019-12-05T13:44:42Z

So the only difference between the current behavior is to not send the padding indicator, right? That's strange, because Generic Identity Device Specification Version 2.0 says you should send the Data to be deciphered (Pl || cryptogram) (I'd guess they meant a capital I instead of a small ell). Maybe Windows already adds the padding indicator, did you test PKCS#11 as well?

(the debug flags should be removed before merging.)

frankmorgner · 2019-12-05T13:49:35Z

Just realized that Vincent's output above indicates that Windows' default implementation does not send the padding indicator...

However, @vletoux, your GIDSApplet does expect the padding indicator...

That's everything very strange

vletoux · 2019-12-05T13:52:27Z

This is the first byte of the final APDU. Not the first byte of the content sent inside the APDU.

dengert · 2019-12-05T14:00:15Z

As a side note, GIDS docx says both extended APDU and chaining must be supported, and will indicate in the EF.ATR in tag "Card capabilities tag". But the driver does test the EF.ATR.
So chaining is used as it appears to be the most likely to be supported. certutil use chaining.

The GIDS docx has a lot that is not implemented in the current driver.

It appears to me decipher command has never worked with the driver so this PR gets that part working.

dengert · 2019-12-05T14:11:38Z

The testing I have been I can do is using Microsoft "virtual Smart Card" with a TPM on Windows 10. `certutil -v -scinfo will verifying the key by deciphering some data using the Microsoft minidriver.
(The OpenSC minidriver is not used in any of the testing.)
@vletoux has also confirmed that the PI must not be present. The OpenSC PKCS11 will only work if the PI is absent. @vletoux has also confirmed that his applet works only if the the PI is not present.

frankmorgner · 2019-12-05T14:16:36Z

I see, thanks for the clearification. Please add a comment to note the difference between specification and implementation regarding the padding indicator, remove the testing flag and simplify your code as requested by Jakub. If done, this PR is good to be merged.

GIDS decipher APDU fails with status '65 00' or '67 00' if "Padding Indication" byte is present. Debug logs of Microsoft certutil -v -scinfo using Microsoft drivers show that for a decipher, the "Padding Indication" is not present. It maybe needed if Secure Messaging is added later. Extended APDU is turned off as this may not be supported on some cards. Chaining is used used instead, it works on all cards. RAW RSA is turned off, it is supported. Tested with pkcs11-tool on Windows 10 with a TPM 2.0 module. On branch gids-decipher Changes to be committed: modified: src/libopensc/card-gids.c Date: Tue Dec 3 18:08:32 2019 -0600 interactive rebase in progress; onto 01678e8 Last commands done (3 commands done): squash c968d0d GIDS No Padding Indication Byte squash 0fa940fc Take 3 No commands remaining. You are currently rebasing branch 'gids-decipher' on '01678e87'.

dengert mentioned this pull request Dec 5, 2019

Towards new release 0.20.0 #1782

Closed

Jakuje reviewed Dec 5, 2019

View reviewed changes

dengert force-pushed the gids-decipher branch from c968d0d to fd1330d Compare December 5, 2019 15:16

frankmorgner merged commit 5fa6330 into OpenSC:master Dec 5, 2019

dengert deleted the gids-decipher branch May 10, 2022 21:56

xhanulik mentioned this pull request Jul 12, 2022

Prepare for release 0.23.0 #2426

Closed

GIDS Decipher fix for TPM #1881

GIDS Decipher fix for TPM #1881

Uh oh!

Conversation

dengert commented Dec 4, 2019

Checklist

Uh oh!

dengert commented Dec 4, 2019

Uh oh!

vletoux commented Dec 4, 2019

Uh oh!

vletoux commented Dec 4, 2019

Uh oh!

dengert commented Dec 4, 2019

Uh oh!

dengert commented Dec 4, 2019

Uh oh!

vletoux commented Dec 4, 2019

Uh oh!

dengert commented Dec 4, 2019

Uh oh!

vletoux commented Dec 4, 2019

Uh oh!

dengert commented Dec 5, 2019

Uh oh!

vletoux commented Dec 5, 2019

Uh oh!

Jakuje Dec 4, 2019

Choose a reason for hiding this comment

Uh oh!

Jakuje Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

dengert Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

Jakuje Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

frankmorgner commented Dec 5, 2019

Uh oh!

frankmorgner commented Dec 5, 2019

Uh oh!

vletoux commented Dec 5, 2019

Uh oh!

dengert commented Dec 5, 2019

Uh oh!

dengert commented Dec 5, 2019

Uh oh!

frankmorgner commented Dec 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants