OpenSC / OpenSC.tokend

Tokend module for OS X with support for all cards supported by OpenSC
Other
36 stars 20 forks source link

Improves support for PIV tokens, adds SHA-2 and ECDSA #23

Closed mouse07410 closed 8 years ago

mouse07410 commented 8 years ago

This PR provides:

Improved support for PIV tokens, which was completely broken until @frankmorgner fixed it and updated his fork. Complete support for SHA-2, which does not work in other forks (as far as I was able to test). Support for ECDSA. Base code for ECDH: decrypt() now calls sc_pkcs15_derive(), but ECDH is not supported yet, and not been tested because there is no application that I know of that uses it. Would be happy to re-open this issue, and improve/fix the ECDH processing. RSA is fully supported, and has been tested with Keychain Access, Apple Mail on Yosemite and El Capitan, MS Outlook 2011 (Yosemite and El Capitan). Both interoperated with Thunderbird (same platforms - but Thunderbird does not use tokend). Also, interoperated with Blackberry Classic (a different S/MIME implementation, and a good validation test).

ECDSA was tested with Apple Mail against Thunderbird. MS Outlook 2011 could validate ECDSA signatures, but could not produce them (deficiency of the Outlook itself - it keeps asking for RSA-based signature algorithms even when the provided key is ECC).

frankmorgner commented 8 years ago

Here are my previous comments:

I hope someone with some spare time and the right token can finalize this PR

mouse07410 commented 8 years ago

Thanks for the comments.

Regarding separation of RSA and ECDSA - it would look nicer, but it does not work (as your commit to sha-2 branch shows). So I decided to stop with the working code rather than spending unpredictable time making it beautiful (especially since it isn't clear at all WHY that "separated" code doesn't work - I don't even know where to poke).

I will correct the indentation.

Regarding separating the defines for RSA and ECDSA - it would make the code more correct, but IMHO the likelihood for somebody at Apple coming and changing the defines in the dying product stated to be deprecated and on the way to be removed is zero. Therefore I'd rather not complicate the code by making each assignment into a multi-line if-statement. It would've been easier with two separate methods for RSA and ECDSA, but as stated above - alas it didn't work for reasons unknown. If somebody knows how to fix it for PIV tokens - I'd be very happy to embrace that change.

omnihil0 commented 8 years ago

This required two further edits to get working with an ECDSA token. First, OpenSCAttributeCoder.cpp needed to have the same fix applied to return field_length rather than modulus_length for kSecKeyKeySizeInBits. Second, the ECDSA keys still showed up as RSA keys in Keychain Access until editing OpenSCSchema.cpp's constructor to invoke mKeyAlgorithmCoder(uint32(CSSM_ALGID_ECDSA)). This second fix is surely the wrong thing in general, but allows ECDSA keys and certificates to work properly combined with mouse07410:master.

frankmorgner commented 8 years ago

The problem is that this PR is not in a state of being merged, see the comments above. If you mind creating a fresh PR rebased onto todays master, I can have a look again.

mouse07410 commented 8 years ago

This required two further edits to get working with an ECDSA token.

@omnihil0 I very much appreciate your fixes, and would like to see those edits.

First, OpenSCAttributeCoder.cpp needed to have the same fix applied to return field_length rather than modulus_length

Thank you - applied and pushed to Git.

Second, the ECDSA keys still showed up as RSA keys in Keychain Access until editing OpenSCSchema.cpp's constructor to invoke mKeyAlgorithmCoder(uint32(CSSM_ALGID_ECDSA)). This second fix is surely the wrong thing in general, but allows ECDSA keys and certificates to work properly combined with mouse07410:master.

Now this is a problem area. Since many PIV tokens are RSA, just making them all appear EC wouldn't be proper. On the other hand, I don't see how OpenSCToken.cpp (around line 341) can know what type of keys this token has at this time, and that's where OpenSCSchema() is constructed.

What we seem to need is a way to modify the type after the first key has been retrieved from the token. Would you concur?

@frankmorgner what is your take on this problem?

Update with more questions: @frankmorgner and @martinpaljak, it looks like to correctly address this issue we need to access the token itself (probably via libopensc stuff) somewhere near line 341 in OpenSCToken.cpp to retrieve information about public keys and/or certificates stored on the token. You could be of great help if you'd point me at:

I would really appreciate if you could guide me here, as my understanding of how Tokend works and interacts with the OS is (unfortunately) very limited, and this problem needs a fix, preferably sooner rather than later. Thanks for an help you can offer.

@dengert, I know you don't want to mess with C++ (and therefore with the Tokend code itself), but perhaps you can point me at the libopensc methods/functions that could give me information about the inserted token? I want/need to pull a certificate from the token (does not matter which one of the four), and read its type (RSA or EC) and CN, and I need to do it before Tokend initializes its schema (so probably cannot use Tokend mechanisms for doing that, and need to side-step them with direct OpenSC access to the token).

mouse07410 commented 8 years ago

@omnihil0 please check the current version of https://github.com/mouse07410/OpenSC.tokend - I believe it should address your needs:

ecc_key_in_keychain

I've successfully signed ECDSA email using Apple Mail and this key.

Now I need somebody to tell me how to properly decode sc_pkcs15_der *value of a certificate to extract Common Name from it.

dengert commented 8 years ago

Look at pkcs11-tool.c parse_certificate() It uses OpenSSL d2i_X509() and i2d_X509_NAME() Could then use: X509_NAME_get_index_by_NID(X509_get_subject_name(x509), NID_commonName, -1);

or look libopensc/pkcs15-cert.c sc_pkcs15_read_certificate it extracts the subject as DER ASN1. But I don't see an OpenSC non-OpenSSL routine to parse the X509_NAME to get the components.

On 5/24/2016 4:46 PM, Mouse wrote:

@omnihil0 https://github.com/omnihil0 please check the current version of https://github.com/mouse07410/OpenSC.tokend - I believe it should address your needs:

ecc_key_in_keychain https://cloud.githubusercontent.com/assets/5923577/15520699/9c4cb5be-21d5-11e6-9ae2-0d13f6104cec.png

I've successfully signed ECDSA email using Apple Mail and this key.

Now I need somebody to tell me how to properly decode |sc_pkcs15_der *value| of a certificate to extract Common Name from it.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/OpenSC/OpenSC.tokend/pull/23#issuecomment-221408516

Douglas E. Engert DEEngert@gmail.com

mouse07410 commented 8 years ago

Look at pkcs11-tool.c parse_certificate(). It uses OpenSSL d2i_X509() and i2d_X509_NAME() Could then use: X509_NAME_get_index_by_NID()

In that case I would just need to modify the Tokend build to make sure it is linked with OpenSSL shared libraries, right? (And of course add the correct include files)

or look libopensc/pkcs15-cert.c sc_pkcs15_read_certificate() - it extracts the subject as DER ASN1. But I don't see an OpenSC non-OpenSSL routine to parse the X509_NAME to get the components.

I'm trying this for now. If it doesn't work, or turns out too complicated - I will switch to the first option (getting OpenSSL involved).

Thank you!!

mouse07410 commented 8 years ago

Almost fixed it. :-)

For some reason, OpenSCToken::establish() reports that it got printName set to what I told it to, but what gets communicated back to Keychain Access (i.e., to the OS) is still the old stuff...

mouse07410 commented 8 years ago

With help of @dengert, Tokend now acquires commonName from one of the certificates on the token.

The problem is - somehow that name is not picked, and not displayed. The token is still shown in Keychain Access as "PIV_II".

@frankmorgner and @martinpaljak you have been working on OpenSC.tokend for a long time. Hopefully you care for it to be the best it can be. Please help me get that name (which I currently strlcpy() into printName argument of OpenSCToken::establish() within that method) displayed - aka set in the right place. I tried to set it before the call to Tokend::ISO7816Token::establish() (so the call has printName argument set to the desired value), and after that call - to no result. Debugging output shows that printName gets set to what I want it to be, but it does not have any effect.

I was unable to get any debugging output from anywhere in Tokend/Token.cpp. Should I move the commonName-retrieving code to Token::_establish() method instead??? That method (line 122) seems to invoke OpenSCToken::establish() method where I currently set printName, and the comment seems to say that if printName is set there (aka in Tokend/Token.cpp in method Token::_establish()), it will be used...

Please help.

dengert commented 8 years ago
Is tokend picking up some cached information from previous sessions,
or a cache stored internally?
OpenSCToken::probe uses the  mScP15Card->tokeninfo->label 
(i.e. "PIV_II")  and mScP15Card->tokeninfo->serial_number
to create a "tokenUid"  Is this then stored somewhere along with
readable versions if the previous printName? 

./Tokend/Token.cpp has:

 106 // Establish:
 107 // Okay, you're the one. The token is yours. Here's your GUID
and subservice ID
 108 // (in case you care); it'll get automatically inserted into
your MDS unless
 109 // you override it. If you can make up a nice, user-friendly
print name for
 110 // your token, return it in printName. If you can't, leave it
alone and
 111 // securityd will make something up for you.

looks like your are doing what it wants. 
Is securityd over riding this per line 111?

On 5/25/2016 9:49 PM, Mouse wrote:

  With help of @dengert,
    Tokend now acquires commonName from one of the
    certificates on the token.
  The problem is - somehow that name is not picked, and not
    displayed. The token is still shown in Keychain Access as
    "PIV_II".
  @frankmorgner
    and @martinpaljak
    you have been working on OpenSC.tokend for a long time.
    Hopefully you care for it to be the best it can be. Please help
    me get that name (which I currently strlcpy() into
    printName argument of OpenSCToken::establish()
    within that method. I tried to set it before the call to Tokend::ISO7816Token::establish()
    (so the call has printName argument set to the
    desired value), and after that call - to no result. Debugging
    output shows that printName gets set to what I
    want it to be, but it does not have any effect.
  I was unable to get any debugging output from anywhere in Tokend/Token.cpp.
    Should I move the commonName-retrieving code to Token::_establish()
    method instead??? That method (line 122) seems to invoke OpenSCToken::establish()
    method where I currently set printName, and the
    comment seems to say that if printName is set
    there, it will be used...
  Please help.
  —
    You are receiving this because you were mentioned.
    Reply to this email directly or view
      it on GitHub

-- 

Douglas E. Engert DEEngert@gmail.com

mouse07410 commented 8 years ago

Is tokend picking up some cached information from previous sessions, or a cache stored internally?

Yes it does. /var/db/TokenCache/tokens:

$ ls -alFR
total 0
drwx--x--x  3 root  wheel  102 May 26 11:25 ./
drwx--x--x  4 root  wheel  136 Jun  9  2015 ../
drwx--x--x  6 root  wheel  204 May 26 11:25 com.apple.tokend.opensc:3884649d7da5cf0709a162a629e13b31ce0f036b/

./com.apple.tokend.opensc:3884649d7da5cf0709a162a629e13b31ce0f036b:
total 16
drwx--x--x  6 root     wheel    204 May 26 11:25 ./
drwx--x--x  3 root     wheel    102 May 26 11:25 ../
drwx------  2 _tokend  _tokend   68 May 26 11:25 Cache/
-rw-rw-rw-  1 root     wheel      6 May 26 11:25 PrintName
-rw-rw-rw-  1 root     wheel      3 May 26 11:25 SSID
drwx------  2 _tokend  _tokend   68 May 26 11:25 Work/

./com.apple.tokend.opensc:3884649d7da5cf0709a162a629e13b31ce0f036b/Cache:
total 0
drwx------  2 _tokend  _tokend   68 May 26 11:25 ./
drwx--x--x  6 root     wheel    204 May 26 11:25 ../

./com.apple.tokend.opensc:3884649d7da5cf0709a162a629e13b31ce0f036b/Work:
total 0
drwx------  2 _tokend  _tokend   68 May 26 11:25 ./
drwx--x--x  6 root     wheel    204 May 26 11:25 ../
$ cat com.apple.tokend.opensc:3884649d7da5cf0709a162a629e13b31ce0f036b/PrintName
PIV_II$

But I'm cleaning up that directory every time before re-trying.

looks like your are doing what it wants.

That's what I thought...

Is securityd overriding this per line 111 (of file Tokend/Token.cpp)?

I wonder...

Silence of @martinpaljak and @frankmorgner is most disconcerting...

frankmorgner commented 8 years ago

Sorry, I don't currently have time for investigating problems in Tokend. But I'm glad to hear that you're making progress!

mouse07410 commented 8 years ago

I don't currently have time for investigating problems in Tokend

I was planning on doing the investigation myself. I was looking for explanation of the expected Tokend behavior from you and from @martinpaljak - because it is rather hard to determine what's wrong if you don't know what right looks like. For example, I've no idea why there's ISO7856Tokend in addition to Tokend class, or why mPrintName instance variable is present only in ISO7856Tokend, or why printName is passed around as a char array rather than a pointer...

But I'm glad to hear that you're making progress!

Thank you! It looks like I found the problem and got this working. Polishing and refactoring code now.

Update

With @dengert's help and encouragement, this part is complete.

Now https://github.com/mouse07410/OpenSC.tokend displays the name of the token (printName) as Subject->commonName, if it is available (i.e., if certificates are present on this token). It also creates Schema correctly now, setting it as ECC for tokens with Elliptic Curve-based keys, and RSA for tokens with RSA keys. As reported above, it's been successfully tested against MS Outlook (Blackberry too - but Blackberry has its own list of bugs, particularly for OpenPGP and ECC).

omnihil0 commented 8 years ago

@mouse07410, I have confirmed that your fixes do work. The latest code allows TLS client authentication in Safari and email signing in Mail.app with ECDSA keys without any modifications. Thank you for this effort.

mouse07410 commented 8 years ago

@omnihil0 thanks for checking.

My only remaining concern is S/MIME encryption/decryption using ECC certs (and ECDH). Thunderbird has a known problem with decryption using keys on hardware tokens (I personally observed it with ECC and RSA tokens using different middleware on different platforms, on Windows 7 and Mac OS X), so the only two remaining apps are Apple Mail (not very promising so far, but who knows) and MS Outlook...

Can you test it?

@dengert, do you have any suggestions/recommendations regarding above?

Update My test shows that none of the following - Apple Mail, MS Outlook 2011, latest Thunderbird - accept ECDH keys for S/MIME. They just refuse to use that cert to encrypt email, let alone decrypt... And this is even before we touch hardware tokens - this is with the key being in Keychain (Apple Mail and Outlook), or directly imported (Thunderbird).

@dengert, in your opinion, what Key Usage should the ECC KEY MANAGEMENT Key have? I think - only Key Agreement (like RSA KEY MAN Key should have only Key Encipherment). Do you concur?

Update2

Tried to play with OpenSSL-1.0.2h and a copy of a simple email message (not using tokens, just to see if ECC keys would work):

$ openssl smime -encrypt -aes128 -inform SMIME -in Cyph_Bot_test.eml -outform SMIME -out Cyph_Bot_test.smime.eml -subject SMIME_ECC ~/Documents/Certs/me_mouse_yubi_9d_.pem
Error creating PKCS#7 structure
140735083847760:error:21082096:PKCS7 routines:PKCS7_RECIP_INFO_set:encryption not supported for this key type:pk7_lib.c:542:
140735083847760:error:21073078:PKCS7 routines:PKCS7_encrypt:error adding recipient:pk7_smime.c:503:
$ openssl version
OpenSSL 1.0.2h  3 May 2016
$

Here's the source of pk7_lib.c:

533:    if (!pkey || !pkey->ameth || !pkey->ameth->pkey_ctrl) {
534:        PKCS7err(PKCS7_F_PKCS7_RECIP_INFO_SET,
535:                 PKCS7_R_ENCRYPTION_NOT_SUPPORTED_FOR_THIS_KEY_TYPE);
536:        goto err;
537:    }
538:
539:    ret = pkey->ameth->pkey_ctrl(pkey, ASN1_PKEY_CTRL_PKCS7_ENCRYPT, 0, p7i);
540:    if (ret == -2) {
541:        PKCS7err(PKCS7_F_PKCS7_RECIP_INFO_SET,
542:                 PKCS7_R_ENCRYPTION_NOT_SUPPORTED_FOR_THIS_KEY_TYPE);
543:        goto err;
544:    }

Update3 Per Dr. Henson, openssl smime works only with RSA keys. For ECC one should use openssl cms. It worked - see the posts below.

omnihil0 commented 8 years ago

@mouse07410, I have been trying to get S/MIME working as well. This actually works using your tokend fixes along with the 'openssl cms' utility from OpenSSL 1.1.0 (1.0.2h is crashing when attempting to decrypt). This only works after creating a certificate with the "KEY MAN" key. I have been unable to get any of the common mail applications to support encrypted emails. Given that OpenSSL's cms works, it seems to be an issue of application support for ECDH, PKCS#11, and/or separate signing and key agreement certificates.

dengert commented 8 years ago
The PIV  NIST standards indicate that there are 4 cert/key pairs.
The Sign Key is for signing. The Key Man key is for decryption (RSA)
or key derivation(EC). The PIV driver sets the PKCS#11 attributes
based on these intended usages. You can not use the same cert/key to
both sign ECDSA and key derive (ECDH).  Use the Sign Key to sign,
and the Key Man key to decrypt/derive. 
   

On 6/1/2016 4:21 PM, omnihil0 wrote:

  @mouse07410, I have been trying to
    get S/MIME working as well. This actually works using your
    tokend fixes along with the 'openssl cms' utility from OpenSSL
    1.1.0 (1.0.2h is crashing when attempting to decrypt). This only
    works after creating a certificate with the "KEY MAN" key. I
    have been unable to get any of the common mail applications to
    support encrypted emails. Given that OpenSSL's cms works, it
    seems to be an issue of application support for ECDH, PKCS#11,
    and/or separate signing and key agreement certificates.

Yes PIV NIST standards dictate "separate signing and key agreement
certificates". 

  —
    You are receiving this because you were mentioned.
    Reply to this email directly, view
      it on GitHub, or mute
      the thread.

-- 

Douglas E. Engert DEEngert@gmail.com

mouse07410 commented 8 years ago

I have been trying to get S/MIME working as well. This actually works using your tokend fixes along with the 'openssl cms' utility from OpenSSL 1.1.0 (1.0.2h is crashing when attempting to decrypt).

@omnihil0 could you please provide more details? How do you encrypt, and how do you decrypt? And how do you use email client (presumably Apple Mail)? OpenSSL has nothing to do with Tokend - only applications like Apple Mail, MS Outlook, Safari, and Keychain Access do.

Here's my escapades with OpenSSL-1.0.2h (on Mac OS X 10.11.5):

$ openssl version
OpenSSL 1.0.2h  3 May 2016
$ tail Cyph_Bot_test.eml
Message-id: <FBD81819-1B51-48CA-939B-03E6403F5A9A@us.army.mil>
Date: Sun, 02 Jun 2013 00:56:22 -0400
To: Cloud Mouse <mouse07410@me.com>
MIME-version: 1.0 (1.0)
X-Mailer: iPad Mail (10B329)

4DFJ3ECyu3XQmJJtPTXp1HJXeCSFnmL8euXcOSc1NGmDm9fqgR0RU+s0Rl1oggUJ

Sent from my iPad
$ openssl cms -engine pkcs11 -encrypt -aes256 -inform SMIME -in Cyph_Bot_test.eml -outform SMIME -out Cyph_Bot_test.smime.eml -subject SMIME_ECC ~/Documents/Certs/me_mouse_yubi_9d_.pem
engine "pkcs11" set.
$ tail Cyph_Bot_test.smime.eml
p7qKV4ttuid/6ynNbobYNgSUenzrmnbO0Z03KhglAy1l/om4crfK3+5r2UJ+5daf
9kL1EUrVy6flhE198793YTZJngi3zKFYk+BY2K8wNzLEoXAfJSY6a9z8RINZW9n8
dMlfziiY09T7oUBS0XF0UJ4DtPJDADCk/CKikbZRI+52OlWzklHaPRNiBwst3oMM
KTa1D2Ub26XlBWo5cnRyFydad3Db8RWPY3RoSVKRPqp1GYNcRyLAkTazOPRb24qe
+ec3hWY41iSvkEfwBGTAYp0DGqG/Lvm1CX6iABji3RBmoc3owYBIKieDBthwsvd+
lxxkX9IDnroUfCZcp7X7aIfJSiHbfw8O0+SJcXnTx/6E20woe7VDf5+7oPLWZKx9
Vw9AXKu/MOegPj6D6nQIZyE1GR7Txp6TSw4WD5M2vuCO7GkXXxHFA1IwuN9HI6QP
Dj6TZvv3xxbRunpovSPk/qgn0wEu+qV/XC4nqLW3OtNQ5sVqdD/t2WNlLWVpU2sW
3Jg+bQjC1KAqnpTDyT4y0jh1xaRSTQtjt8te0ejht/hemQy8pa9g+RUbxinb1w==

$ openssl cms -engine pkcs11 -keyform engine -decrypt -aes256 -inform SMIME -in Cyph_Bot_test.smime.eml -outform SMIME -out Cyph_Bot_test.decrypt.eml -recip ~/Documents/Certs/me_mouse_yubi_9d_.pem -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private"
engine "pkcs11" set.
PKCS#11 token PIN:
$ tail Cyph_Bot_test.decrypt.eml
Message-id: <FBD81819-1B51-48CA-939B-03E6403F5A9A@us.army.mil>
Date: Sun, 02 Jun 2013 00:56:22 -0400
To: Cloud Mouse <mouse07410@me.com>
MIME-version: 1.0 (1.0)
X-Mailer: iPad Mail (10B329)

4DFJ3ECyu3XQmJJtPTXp1HJXeCSFnmL8euXcOSc1NGmDm9fqgR0RU+s0Rl1oggUJ

Sent from my iPad
$

Note that if you don't specify -recip, decryption using OpenSSL-1.0.2h doesn't work for some reason (probably what you observed):

$ openssl cms -engine pkcs11 -keyform engine -decrypt -aes256 -inform SMIME -in Cyph_Bot_test.smime.eml -outform SMIME -out Cyph_Bot_test.decrypt1.eml -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private"
engine "pkcs11" set.
PKCS#11 token PIN:
Error decrypting CMS structure
140735083847760:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:evp_enc.c:529:
$

Running the above with -debug_decrypt reveals the problem:

$ openssl cms -engine pkcs11 -keyform engine -decrypt -debug_decrypt -aes256 -inform SMIME -in Cyph_Bot_test.smime.eml -outform SMIME -out Cyph_Bot_test.decrypt1.eml -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private"
engine "pkcs11" set.
PKCS#11 token PIN:
Error decrypting CMS using private key
140735083847760:error:2E072084:CMS routines:CMS_decrypt_set1_pkey:no matching recipient:cms_smime.c:661:
$

which contradicts what the man page says:

If the -decrypt option is used without a recipient certificate then an attempt is made to locate the 
recipient by trying each potential recipient in turn using the supplied private key. To thwart the MMA
attack (Bleichenbacher's attack on PKCS #1 v1.5 RSA padding) all recipients are tried whether they
succeed or not and if no recipients match the message is "decrypted" using a random key which will
typically output garbage. The -debug_decrypt option can be used to disable the MMA attack protection
and return an error if no recipient can be found: this option should be used with caution.

I wasn't able to specify certificate on the token itself - had to download it into a disk file first.

mouse07410 commented 8 years ago

Master branch replaced to become compatible with the current https://github.com/OpenSC/OpenSC.git master that had several changes and PRs applied after the above PR has been submitted.

This PR is still valid, and is the basis for an OpenSC.tokend that works correctly with PIV and CAC tokens.

For users seeking a tokend that works correctly with PIV and CAC cards and can accept RSA and ECC tokens with SHA-1 and SHA-2, it is recommended to use mouse07410/OpenSC.toked instead of the main OpenSC/OpenSC.tokend repo.