Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libp11 does not support ECDH key derivation is OpenSSL versions < 1.1 #49

Closed
mouse07410 opened this issue Jan 1, 2016 · 47 comments
Closed
Assignees
Milestone

Comments

@mouse07410
Copy link
Contributor

Symptoms: trying to do encryption/decryption - which for ECC translates into shared key derivation - fails, because p11_ops.c (and/or p11_ec.c) does not implement ECDH1-DERIVE (ECDH1-COFACTOR-DERIVE) method. It seems to only support ECDSA now.

Example:

-passin arg     pass phrase source
$ openssl pkeyutl -engine pkcs11 -peerform engine -derive -inkey 384priv.pem -peerkey id_03 -hexdump
engine "pkcs11" set.
Error initializing context
140735231131728:error:260C0065:engine routines:ENGINE_get_pkey_meth:unimplemented public key method:tb_pkmeth.c:128:
140735231131728:error:0609D09C:digital envelope routines:INT_CTX_NEW:unsupported algorithm:pmeth_lib.c:164:
Usage: pkeyutl [options]
-in file        input file
-out file       output file
-sigfile file signature file (verify operation only)
-inkey file     input key
-keyform arg    private key format - default PEM
-pubin          input is a public key
-certin         input is a certificate carrying a public key
-pkeyopt X:Y    public key options
-sign           sign with private key
-verify         verify with public key
-verifyrecover  verify with public key, recover original data
-encrypt        encrypt with public key
-decrypt        decrypt with private key
-derive         derive shared secret
-hexdump        hex dump output
-engine e       use engine e, possibly a hardware device.
-passin arg     pass phrase source

The problem is not with pkeyutl key derivation itself, because it works fine when the keys are in the filesystem:

$ pkcs15-tool -read-public-key 03 -o 384token-pub.pem
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
Certificate with ID 'ead-public-key' not found.
$ pkcs15-tool --read-public-key 03 -o 384token-pub.pem
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
$ cat !!:$
cat 384token-pub.pem
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEDGS7L4T8k+bleEHixoBy/78g5c2T4gXh
jfbSp9mu7QFsQM+GUXPq/vXY/keAvMytU1/J1+IOs60fSojUHTTKBGP37eAirjbP
aPKoCOuwZfU2azEtyfqlEhVZF9RX3Sre
-----END PUBLIC KEY-----
$ openssl pkeyutl -derive -inkey 384priv.pem -peerkey 384token-pub.pem -hexdump
0000 - 49 9d 0c 63 f0 65 24 65-8e 36 43 dd e0 d9 38 61   I..c.e$e.6C...8a
0010 - 96 16 2a 03 49 e3 a1 cf-48 fd e0 f8 d4 b8 5d 1b   ..*.I...H.....].
0020 - e9 9e 31 53 a6 97 10 5f-cb c7 d2 15 ac 7d 84 b5   ..1S..._.....}..
$
@nmav
Copy link
Contributor

nmav commented Jan 1, 2016

While that may be a feature for the sake of completeness, ECDH is not really used by real world TLS. Its deployment is compares with static DH (i.e. 0). So I'd expect if anyone is interested on that feature for a particular application to contribute the code rather than adding code just for the sake of being "complete" in supported protocols.

@dengert
Copy link
Member

dengert commented Jan 1, 2016

ECDH has not been implemented on libp11 or engine_pkcs11 mainly
because OpenSSL had not added the ability to use an engine to do
ECDH. 
But that has changed as of last month. In the OpenSSL github master,
the ECDSA and ECDH methods are now combined in to one method
EC_KEY_METHOD
 and the EC_KEY_METHOD_set_compute_key() can be used to set a new
ECDH_compute_key routine. 
The engine libp11 and OpenSC code need to be re written to use the
new method for EC  and 1.1 also made a lot of other changes,
deprecating some routines
and not exposing BIGNUM routines as it has in the past. 

I started looking  using the OpenSSL github version of what will
become  1.1 with OpenSC, and so far 11 routines need changes  mostly
with BIGNUM and \#include statements.
Once I have a working opensc, then move on the engine_pkcs11  and
then libp11. 

Most tokens would only support a NULL KDF, I have not looked closely
at this  but it looks like OpenSSL can do all that in software so
libp11 and engine_pkcs11 only need to handle 
EVP_PKEY_ECDH_KDF_NONE. 

For testing ECDH, I would recommend using the OpenSSL cms  command,
that can also generate the  ephemeral ECDH
  keys and handle the input of the peer certificates 
  needed in the  ECDH_compute_key routines. 

http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Ar2.pdf
http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-73-4.pdf
https://tools.ietf.org/html/rfc5753

800-73-4  Section A.5.2.2 PIV KMK Specific ECDH Key Agreement
Schemes
shows how the PIV (or other smartcards) with static ECDH keys are
used with 800-56Ar2.
RFC 5753 Sections 3 and 3.1 discribe how  (method C(1, 1, ECC CDH))
in 800-56 can be used in CMS. 


I was planning after the holidays to spend more time on the OpenSSL
1.1, engine_pkcs11 and libp11 to add the 
needed calls to allow OpenSSL cms to use engine and libp11. 


On 12/31/2015 8:14 PM, Mouse wrote:


  Symptoms: trying to do encryption/decryption - which for ECC
    translates into shared key derivation - fails, because p11_ec.c
    does not implement ECDH-DERIVE (ECDH-COFACTOR-DERIVE) method. It
    seems to only support ECDSA now.
  Example:
  -passin arg     pass phrase source

$ openssl pkeyutl -engine pkcs11 -peerform engine -derive -inkey 384priv.pem -peerkey id_03 -hexdump
engine "pkcs11" set.
Error initializing context
140735231131728:error:260C0065:engine routines:ENGINE_get_pkey_meth:unimplemented public key method:tb_pkmeth.c:128:
140735231131728:error:0609D09C:digital envelope routines:INT_CTX_NEW:unsupported algorithm:pmeth_lib.c:164:
Usage: pkeyutl [options]
-in file input file
-out file output file
-sigfile file signature file (verify operation only)
-inkey file input key
-keyform arg private key format - default PEM
-pubin input is a public key
-certin input is a certificate carrying a public key
-pkeyopt X:Y public key options
-sign sign with private key
-verify verify with public key
-verifyrecover verify with public key, recover original data
-encrypt encrypt with public key
-decrypt decrypt with private key
-derive derive shared secret
-hexdump hex dump output
-engine e use engine e, possibly a hardware device.
-passin arg pass phrase source

  The problem is not with pkeyutl key derivation itself, because
    it works fine when the keys are in the filesystem:
  $ pkcs15-tool -read-public-key 03 -o 384token-pub.pem

Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
Certificate with ID 'ead-public-key' not found.
$ pkcs15-tool --read-public-key 03 -o 384token-pub.pem
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
$ cat !!:$
cat 384token-pub.pem
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEDGS7L4T8k+bleEHixoBy/78g5c2T4gXh
jfbSp9mu7QFsQM+GUXPq/vXY/keAvMytU1/J1+IOs60fSojUHTTKBGP37eAirjbP
aPKoCOuwZfU2azEtyfqlEhVZF9RX3Sre
-----END PUBLIC KEY-----
$ openssl pkeyutl -derive -inkey 384priv.pem -peerkey 384token-pub.pem -hexdump0000 - 49 9d 0c 63 f0 65 24 65-8e 36 43 dd e0 d9 38 61 I..c.e$e.6C...8a
0010 - 96 16 2a 03 49 e3 a1 cf-48 fd e0 f8 d4 b8 5d 1b ..*.I...H.....].
0020 - e9 9e 31 53 a6 97 10 5f-cb c7 d2 15 ac 7d 84 b5 ..1S..._.....}..
$

  —
    Reply to this email directly or view it on
      GitHub.









-- 

Douglas E. Engert DEEngert@gmail.com

@mouse07410
Copy link
Contributor Author

The engine libp11 and OpenSC code need to be re written to use the
new method for EC.

OpenSC would also have to be rewritten? Because it seems to support ECDH1-COFACTOR-DERIVE as is:

$ pkcs11-ec-derive-demo
Extracting public key from the token...
pkcs15-tool --read-public-key 03 -o /tmp/derive.31436.token.pub.pem
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
read EC key

Generating ephemeral ECC key pair on secp384r1...
openssl ecparam -name secp384r1 -genkey -out /tmp/derive.31436.priv.pem
openssl ec -in /tmp/derive.31436.priv.pem -pubout -outform DER -out /tmp/derive.31436.pub.der
read EC key
writing EC key

Generating random 250 bytes of Base64-encoded data...
openssl rand -base64 -out /tmp/derive.31436.text 250

Deriving shared key from ephemeral private and token public keys...
openssl pkeyutl -derive -inkey /tmp/derive.31436.priv.pem -peerkey /tmp/derive.31436.token.pub.pem -out /tmp/derive.31436.shared1

Encrypting data file with derived symmetric key in /tmp/derive.31436.shared1 and AES-CFB...
openssl enc -aes-256-cfb -e -a -kfile /tmp/derive.31436.shared1 -in /tmp/derive.31436.text -out /tmp/derive.31436.text.enc

Deriving shared symmetric key on the token, using ephemeral public key...
pkcs11-tool --slot 1 --module /Library/OpenSC/lib/opensc-pkcs11.so -l --derive -m ECDH1-COFACTOR-DERIVE -d 03 -i /tmp/derive.31436.pub.der -o /tmp/derive.31436.shared2
Logging in to "PIV_II (PIV Card Holder pin)".
Please enter User PIN:
Using derive algorithm 0x00001051 ECDH1-COFACTOR-DERIVE

Decrypting data file with derived symmetric key in /tmp/derive.31436.shared2 and AES-CFB...
openssl enc -aes-256-cfb -d -a -kfile /tmp/derive.31436.shared2 -in /tmp/derive.31436.text.enc -out /tmp/derive.31436.text.dec

Derived keys matched.
Decrypted file matches the original plaintext.

P.S. Happy New Year, guys!

@dengert
Copy link
Member

dengert commented Jan 1, 2016

Your test script looks like it works!

The OpenSC code should be updated to use OpenSSL 1.1 to avoid using
multiple versions of OpenSSL 
in libp11, engine and opensc.  There are 11 files I have found so
far. There are 5 opensc files that include openssl/ec.h
we need to look at these but may not require any changes.  

We may want to look at KDFs much like we do hashes... Use the
OpenSSL KDFs, within OpenSC pkcs11 or we could
require the caller to do KDF 

On 1/1/2016 11:12 AM, Mouse wrote:



    The engine libp11 and OpenSC code need to be re written to
      use the 
      new method for EC. 

  OpenSC would also have to be rewritten? Because it seems to
    support ECDH1-COFACTOR-DERIVE as is:
  $ pkcs11-ec-derive-demo

Extracting public key from the token...
pkcs15-tool --read-public-key 03 -o /tmp/derive.31436.token.pub.pem
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
read EC key

Generating ephemeral ECC key pair on secp384r1...
openssl ecparam -name secp384r1 -genkey -out /tmp/derive.31436.priv.pem
openssl ec -in /tmp/derive.31436.priv.pem -pubout -outform DER -out /tmp/derive.31436.pub.der
read EC key
writing EC key

Generating random 250 bytes of Base64-encoded data...
openssl rand -base64 -out /tmp/derive.31436.text 250

Deriving shared key from ephemeral private and token public keys...
openssl pkeyutl -derive -inkey /tmp/derive.31436.priv.pem -peerkey /tmp/derive.31436.token.pub.pem -out /tmp/derive.31436.shared1

Encrypting data file with derived symmetric key in /tmp/derive.31436.shared1 and AES-CFB...
openssl enc -aes-256-cfb -e -a -kfile /tmp/derive.31436.shared1 -in /tmp/derive.31436.text -out /tmp/derive.31436.text.enc

Deriving shared symmetric key on the token, using ephemeral public key...
pkcs11-tool --slot 1 --module /Library/OpenSC/lib/opensc-pkcs11.so -l --derive -m ECDH1-COFACTOR-DERIVE -d 03 -i /tmp/derive.31436.pub.der -o /tmp/derive.31436.shared2
Logging in to "PIV_II (PIV Card Holder pin)".
Please enter User PIN:
Using derive algorithm 0x00001051 ECDH1-COFACTOR-DERIVE

Decrypting data file with derived symmetric key in /tmp/derive.31436.shared2 and AES-CFB...
openssl enc -aes-256-cfb -d -a -kfile /tmp/derive.31436.shared2 -in /tmp/derive.31436.text.enc -out /tmp/derive.31436.text.dec

Derived keys matched.
Decrypted file matches the original plaintext.

    OpenSSL 1.1 also made a lot of other changes,
      deprecating some routines and not exposing BIGNUM routines as
      it has in the past.

  —
    Reply to this email directly or view
      it on GitHub.









-- 

Douglas E. Engert DEEngert@gmail.com

@dengert
Copy link
Member

dengert commented Jan 2, 2016

Here is were I would start looking.
./crypto/engine/eng_lib.c:138  in my source is at 137:

135     if (e->destroy)
136         e->destroy(e);
137     CRYPTO_free_ex_data(CRYPTO_EX_INDEX_ENGINE, e,
&e->ex_data);
138     OPENSSL_free(e);

The call to e->destroy(e);  should be calling in engine_pkcs11  
./src/hw_pkcs11.c    pkcs11_engine_destroy
that calls  in libp11 ./src/p11_ec.c  PKCS11_ecdsa_method_free
that calls in OpenSSL ECDSA_METHOD_free

Look at the use of ex_data in libp11 
./src/p11_ec.c

92 /*
 93  * Get EC key material and stach pointer in ex_data
 94  * Note we get called twice, once for private key, and once for
public
 95  * We need to get the EC_PARAMS and EC_POINT into both,
 96  * as lib11 dates from RSA only where all the pub key components
 97  * were also part of the privite key.  With EC the point
 98  * is not in the privite key, and the params may or may not be.
 99  *
100  */



193         if (sensitive || !extractable) {
194                 ECDSA_set_ex_data(ec, 0, key);
195                 EC_KEY_free(ec); /* drops our reference to it.
*/
196                 return 0;
197         }
198 
199         if (ec)
200             EC_KEY_free(ec);

You have been making a lot of changes in this area with public vs
private keys and sensitive.
 Its not clear if there is now a double free.

Running under valgrind might help. 






On 1/1/2016 10:06 PM, Mouse wrote:


  Alas, I don't have gdb, and lldb (gdb analog in Xcode toolset)
    refuses to run openssl with command line arguments. Here's what
    I got, building libp11 and openssl with "-g".
  Command execution:
  $ apps/openssl pkeyutl -engine pkcs11 -keyform engine -sign -inkey id_02 -in t384.dat -out t384.sig

engine "pkcs11" set.
PKCS#11 token PIN:
engine name = "pkcs11 engine"
Segmentation fault: 11
$ ls -l t384.sig
-rw-r--r-- 1 uri staff 103 Jan 1 23:02 t384.sig
$ dumpasn1 t384.sig
0 101: SEQUENCE {
2 49: INTEGER
: 00 CD 26 15 49 B1 2D 14 05 85 50 0D FC AD C7 31
: 2D 94 1B D9 D6 92 11 D2 74 E1 B0 CC 27 44 D1 46
: 13 20 F5 F4 EA 92 A7 80 7D 09 D5 88 86 5E 05 39
: 2D
53 48: INTEGER
: 0B 1D 76 83 03 40 FF 57 97 F9 33 18 5A 50 52 D3
: 78 71 36 45 C2 53 85 A2 22 96 A0 0E 8C 9E B3 CE
: 3C 78 2C 30 78 F8 B8 CA C2 B2 14 EA 73 6A 65 7E
: }

0 warnings, 0 errors.

  Crash report:
  Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type: EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000001021d1fc0

VM Regions Near 0x1021d1fc0:
VM_ALLOCATE 00000001020b4000-00000001020b5000 [ 4K] rw-/rwx SM=ALI
-->
VM_ALLOCATE 00000001025bb000-00000001025bf000 [ 16K] rw-/rwx SM=PRV

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 ??? 0x00000001021d1fc0 0 + 4330430400
1 openssl 0x0000000101b853d1 int_free_ex_data + 369 (ex_data.c:522)
2 openssl 0x0000000101c330b0 engine_free_util + 160 (eng_lib.c:138)
3 openssl 0x0000000101c33887 ENGINE_remove + 199 (eng_list.c:179)
4 openssl 0x0000000101c33c25 engine_list_cleanup + 21 (eng_list.c:89)
5 openssl 0x0000000101c331cb engine_cleanup_cb_free + 11 (eng_lib.c:199)
6 openssl 0x0000000101c413b0 sk_pop_free + 48 (stack.c:325)
7 openssl 0x0000000101c331ac ENGINE_cleanup + 28 (eng_lib.c:207)
8 openssl 0x0000000101af0362 main + 1122 (openssl.c:437)
9 libdyld.dylib 0x00007fff8e35c5ad start + 1

  —
    Reply to this email directly or view
      it on GitHub.









-- 

Douglas E. Engert DEEngert@gmail.com

@mouse07410
Copy link
Contributor Author

This method does not seem to do anything but calling PKCS11_ecdsa_method_free():

/* Destructor */
static int pkcs11_engine_destroy(ENGINE * e)
{

#ifndef OPENSSL_NO_EC
#ifndef OPENSSL_NO_ECDSA
        PKCS11_ecdsa_method_free();
#endif
#endif

        return 1;
}

And this method

void PKCS11_ecdsa_method_free(void)
{
        if (ops) {
        ECDSA_METHOD_free(ops);
        ops = NULL;
        }
}

does not seem to be our likely culprit, because it does not take any parameters (that could unexpectedly be NULL), and checks for ops being NULL before passing them to ECDSA_METHOD_free().

I don't know... All I can say is that adding a simple check for a NULL-pointer where I added it, alleviates the problem. Of course, if the experts in the code can find the exact ...free() function that breaks the tradition and does not check for NULL in its input parameters, great. I can't count myself among such experts. In the meanwhile, it seems worth it to add a NULL-pointer check before involving that free(), as its performance hit will likely be minimal, and its benefits are clear - it prevents the entire application from crashing if one of the functions to-be-called misbehaves (of course it should never happen, but as we see, it does).

@dengert
Copy link
Member

dengert commented Jan 2, 2016

You need to use valgrind or a debugger  and set breakpoints for any
of the _ex_data functions that engine or libp11 uses,
and look at what might be going on. If you can't do it on the MAC,
use a linux system.
You have a lot of changes in engine and lip11. I would suspect that
they are the cause of the problem.  

I can't debug it for you. 

On 1/2/2016 10:21 AM, Mouse wrote:


  This method does not seem to do anything but calling
    PKCS11_ecdsa_method_free():
  /* Destructor */

static int pkcs11_engine_destroy(ENGINE * e)
{

#ifndef OPENSSL_NO_EC
#ifndef OPENSSL_NO_ECDSA
PKCS11_ecdsa_method_free();
#endif
#endif

    return 1;

}

  And this method
  void PKCS11_ecdsa_method_free(void)

{
if (ops) {
ECDSA_METHOD_free(ops);
ops = NULL;
}
}

  does not seem to be our likely culprit, because it does not
    take any parameters (that could unexpectedly be NULL), and
    checks for ops being NULL before passing them to
    ECDSA_METHOD_free().
  I don't know...
  —
    Reply to this email directly or view
      it on GitHub.









-- 

Douglas E. Engert DEEngert@gmail.com

@mtrojnar mtrojnar added this to the 0.4.x milestone Jan 3, 2016
@mtrojnar mtrojnar self-assigned this Jan 3, 2016
@dengert
Copy link
Member

dengert commented Jan 3, 2016

Note that OpenSSL 1.1 combines ECDSA_METHOD and ECDH_METHOD into EC_KEY_METHOD
and adds EC_KEY_METHOD_set_compute_key to allow an engine
to provides its own ECDH_compute_key This is the missing piece for ECDH with the opensc engine.

Adding ECDH could wait till the cleanup of libp11 and engine is complete.

1.1 also has some other unexpected changes, like moving struct x509_st from x509.h to internal/x509_int.h

@mouse07410
Copy link
Contributor Author

Adding ECDH could wait till the cleanup of libp11 and engine is complete.

No objections from me, especially since this cleanup work seems to have already begun.

@mtrojnar
Copy link
Member

mtrojnar commented Jan 4, 2016

What are the practical applications for using ECDH key derivation instead of ECDHE?

@mouse07410
Copy link
Contributor Author

What are the practical applications for using ECDH key derivation instead of ECDHE?

Off-hand, first thing that comes to mind: ability to send (and receive!) encrypted email, in a way similar to how it is done with RSA. Except that in this case the sender generates a random ECC key pair instead of a random symmetric key (for obvious reasons). But the receiver only needs to publish his ECC "encryption" (or rather "derivation") key, no negotiation (and round-trips) involved.

@dengert
Copy link
Member

dengert commented Jan 4, 2016

RFC 5753 discribes how EC is used with a CMS that is used in E-mail. See the URLs earlier in this discusion. Sections 3 and 3.1 discribe how (method C(1, 1, ECC CDH)) in 800-56 can be used in CMS.

The engine and/or the libp11 do not need to generate or do any crypto with the ephemeral EC private key. The engine and/or libp11 only need to support ECC CDH when using the sendor's and/or receivers's EC private key on a card.

At the requset of a NIST developer, who was working on getting patches in Mozilla NSS to do ECDH:
On 08/04/2011 05:23 PM, Douglas E. Engert wrote:
Attached are OpenSC mods for ECDH, in two patches.
These were generated against Martin's git repository,
in the "proposed" branch from today.

His response was:
Thanks for sending me the patches. They work great! I downloaded OpenSC 0.12.2 and applied the patches, downloaded and unpacked the latest source RPM for OpenSC for my distribution, replaced the OpenSC source file from the RPM with the patched version of OpenSC 0.12.2, and then built the RPM. ...
However, with your signature patches applied, everything works great whether a PIV Card is in the reader or not.

His Mozilla patches and my OpenSC patches were then added to NSS and OpenSC.

@mouse07410
Copy link
Contributor Author

The engine and/or the libp11 do not need to generate or do any crypto with the
ephemeral EC private key.

Correct (I did not imply they would).

The engine and/or libp11 only need to support ECC CDH when using the sender's and/or
receivers's EC private key on a card.

Correct. Receiver's key for decrypting, and sender's key for signing.

@mtrojnar
Copy link
Member

mtrojnar commented Jan 4, 2016

ability to send (and receive!) encrypted email

Indeed. Thank you.

@mtrojnar mtrojnar assigned dengert and unassigned mtrojnar Jan 21, 2016
@mtrojnar
Copy link
Member

#60 implemented this feature as a side effect of porting to OpenSSL 1.1.0.

@mouse07410
Copy link
Contributor Author

@mtrojnar Naive question: will it (this feature) work with OpenSSL_1_0_2-stable?

Adding @dengert

@mtrojnar
Copy link
Member

I assumed that it will. @dengert is the right person to answer this question, as he currently develops EC functionality.

I currently spend most of my time on the engine_pkcs11 release planned for the next week. The next step, planned for the beginning of February, will be merging engine_pkcs11 into libp11.

@dengert
Copy link
Member

dengert commented Jan 24, 2016

I would say no. but...

OpenSSL-1.0.2 only added the functions need by an enging to hook the ECDSA_METHOD. They did not add the hooks for ECDH_METHOD. the "but..." is with access to OpenSSL source and internal OpenSSL header files it might be possible to hook the ECDH_METHOD.

In 1.1 OpenSSL combined the ECDSA_METHOD and ECDH_METHOD into one EC_KEY_METHOD
rewriting much of the interface too. (Including how to handle _ex_data for EC)

OpenSSL never exposed the internal strucuture of EC routines, and does not in 1.1 either.

OpenSC code prior to the recent changes for 1.1 had a way to compile engine using the internal OpenSSL header files. This same approach could be used again for 1.0.2 only to access ECDH.
The approach did not require the recompilation of OpenSSL, as all the code was in the OpenSC engine.

I don't even want to look at the 1.0.2 code to see if could be done untill @mtrojnar combines engine and libp11, and 1.1 is working. There are just to many changes going on. And even then any 1.0.2 ECDH changes would be not standard changes that no distro would even consider implementing.

Even if you got the OpenSC side working, there may still be issues with OpenSSL 1.0.2 using ECDH. You have been on the OPenSSL dev list, and look at the responses you have gotten about changes to 1.0.2. You might have to have local code changes to OpenSSL too.

@mtrojnar
Copy link
Member

OpenSSL-1.0.2 only added the functions need by an enging to hook the ECDSA_METHOD. They did not add the hooks for ECDH_METHOD.

What exactly do you mean by "hooks for ECDH_METHOD"? ENGINE_set_ECDH()?
https://github.com/openssl/openssl/blob/OpenSSL_1_0_2-stable/crypto/engine/engine.h#L557

int ENGINE_set_ECDH(ENGINE *e, const ECDH_METHOD *ecdh_meth);
int ENGINE_set_ECDSA(ENGINE *e, const ECDSA_METHOD *ecdsa_meth);

I don't even want to look at the 1.0.2 code to see if could be done untill @mtrojnar combines engine and libp11, and 1.1 is working.

I completely fail to understand how my work on merging engine_pkcs11 into lib11 affects your work on EC. There will still be two separate libraries. The changes in engine_pkcs11 are just 1 (one) line of code, namely, the invocation of ENGINE_set_ECDH().

And even then any 1.0.2 ECDH changes would be not standard changes that no distro would even consider implementing.

Could you elaborate on the required changes to OpenSSL 1.0.2? Some links to the related emails in the OpenSSL mailing list archive would be sufficient...

@mtrojnar mtrojnar reopened this Jan 24, 2016
@dengert
Copy link
Member

dengert commented Jan 24, 2016

@mouse07410, do you have output showing how the compilicate failed?
Are you using the same version of OpenSSL for both engine_pkcs11 and libp11?

@mtrojnar you said:
"I completely fail to understand how my work on merging engine_pkcs11 into lib11 affects your work on EC."

I am interpreting your work to merge engine_pkcs11 and libp11 to be they will end up in one package. PKCS11_get_ecdsa_method is only used with 1.0.2. PKCS11_get_ec_key_method is only used with 1.1 and above. Even though the functions start with PKCS11_ they are only intended to be used from the engine. If there is only one package, the package will be built with one version of OpenSSL.
Since they are not intended to be used outside engine, maybe the functions should not be named PKCS11_.

@mtrojnar
Copy link
Member

I am interpreting your work to merge engine_pkcs11 and libp11 to be they will end up in one package.

This will indeed be one package, just like OpenSSL is one package. Also, just like the OpenSSL produces two separate libraries: libcrypto and libssl, libp11 will also produce two libraries: libp11 (installed with other libraries) and libpkcs11 (installed with other OpenSSL engines). Just like in OpenSSL, there is absolutely no impact on the exported symbols: libcrypto still needs to export symbols only intended for libssl.

@mouse07410
Copy link
Contributor Author

@mouse07410, do you have output showing how the compile failed?

I think this belongs to a different thread/issue: #62

Apologies - please see the next comment.

Are you using the same version of OpenSSL for both engine_pkcs11 and libp11?

Yes of course. You really should have more faith in me. :-)
Now throughout my machine, it's OpenSSL_1_0_2f-dev.

But to bring hard evidence:

$ otool -L /opt/local/lib/libp11.dylib
/opt/local/lib/libp11.dylib:
    /opt/local/lib/libp11.2.dylib (compatibility version 6.0.0, current version 6.0.0)
    /opt/local/lib/libssl.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /opt/local/lib/libcrypto.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
$ otool -L /opt/local/lib/engines/libpkcs11.so
/opt/local/lib/engines/libpkcs11.so:
    /opt/local/lib/libp11.2.dylib (compatibility version 5.0.0, current version 5.0.0)
    /opt/local/lib/libssl.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /opt/local/lib/libcrypto.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
$ otool -L /opt/local/bin/openssl
/opt/local/bin/openssl:
    /opt/local/lib/libssl.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /opt/local/lib/libcrypto.1.0.0.dylib (compatibility version 1.0.0, current version 1.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
$ /opt/local/lib/openssl version
OpenSSL 1.0.2f-dev xx XXX xxxx
$

If the above is not conclusive - please let me know what kind of evidence you'd like.

@mouse07410
Copy link
Contributor Author

@mtrojnar @dengert here's the ECDH_compute_key() failing output, preceded by a successful key derivation by pkcs11-tool from OpenSC:

Deriving shared symmetric key on the token, using ephemeral public key...
pkcs11-tool --slot 1 --module /Library/OpenSC/lib/opensc-pkcs11.so -l --derive -m ECDH1-COFACTOR-DERIVE -d 03 -i /tmp/derive.37664.pub.der -o /tmp/derive.37664.shared2
Logging in to "PIV_II (PIV Card Holder pin)".
Please enter User PIN: 
Using derive algorithm 0x00001051 ECDH1-COFACTOR-DERIVE

Attempting to derive ECDH key on the token...
openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.37664.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
Public Key operation error
140735120921424:error:2B064064:lib(43):ECDH_compute_key:no private value:ech_ossl.c:136:

@dengert
Copy link
Member

dengert commented Jan 27, 2016

As expected, 1.0.2 will fail to use an engine to to ECDH.

I have told you that libp11 and opensc_engine when using OpenSSL 1.0.2 does not support engine use of ECDH because OpenSSL does not export a way to set the hooks needed for the engine to get control to do ECDH.
But OpenSSL 1.1 combines the ECDSA_METHOD and ECDH_METHOD and trhe hooks are there.

Also as I said I did not want to look at this problem untill at least libp11 and opensc_engine were working with 1.1. I have started looking at what it would take to get then working with 1.0.2.

@mouse07410
Copy link
Contributor Author

As expected, 1.0.2 will fail to use an engine to to ECDH.

:-(

I have told you that libp11 and opensc_engine when using OpenSSL 1.0.2 does not support
engine use of ECDH because OpenSSL does not export a way to set the hooks needed for
the engine to get control to do ECDH.

Being unfamiliar with how exactly the interface between OpenSSL and libp11/engine_pkcs11 works, I can't properly appreciate this info. And looking at the Alexander Gostrer's changes, he basically uncommented (*init) and (*finish) in the ech_locl.h file, and added a method ECDH_generate_key() whose purpose I did not understand. :-(

But OpenSSL 1.1 combines the ECDSA_METHOD and ECDH_METHOD and the hooks are there.

That is great - but mostly useless for the majority of users for (probably) a long period of time. Because now there are two kinds of users: those who need the applications they use to support OpenSSL (aka production environment, Web servers, etc. etc.), and those who don't. The first group is likely to be stuck with 1.0.2 for a while, so whatever good 1.1 brings up won't help them until the mainstream moves to 1.1. And the second group could just use OpenSC-based key derivation and be done with it (it works, as you can see above). I estimate the number/amount of those who can switch their production environment to 1.1 soon (within a few months) as nil to negligible.

Also as I said I did not want to look at this problem untill at least libp11 and opensc_engine
were working with 1.1. I have started looking at what it would take to get then working with 1.0.2.

Yes.

But I personally think that the priorities should be reversed - because you're working first on something that isn't likely to be usable for some time, and postponing something that could have value right now.

(Also, if what Alexander Gostrer did actually works - his changes to OpenSSL are literally miniscule, almost nothing. And we already have a working engine_pkcs11, so unlike him we don't need to write the entire engine from scratch.)

@dengert
Copy link
Member

dengert commented Jan 28, 2016

Here are the patches to allow ECDH to work with OpenSSL-1.0.2. It is based on routines that work
with OpenSSL-1.1. They are only usable with OpenSSL-1.0.2.

Testing has been done with 1.0.2d, and the cms decrypt command.

I consider these patches to be a hack, and should not end up in the master branch of libp11 or engine_opensc. Libp11 needs to be compiled with the OpenSSL-1.0.2 internal source file crypto/ecdh/ech_locl.h

See the commit messages for more information.

dengert@d8c8b10
dengert/engine_pkcs11@c7cff98

@mouse07410
Copy link
Contributor Author

Thank you! I will begin testing as soon as a working/build-able libp11 is pushed to Github (see #65 :-(

@mouse07410
Copy link
Contributor Author

First, thank you!

They [the patches] are only usable with OpenSSL-1.0.2.

Of course. I didn't see any notice of 1.0.2 discontinuance impending any time soon, did you? :-)

And when 1.0.2 disappears (probably around 2020) - all the code that relies on it would be either changed, or retired. But for the coming years I think it is safe to assume that these patches will be perfectly usable.

I've applied and tested your patches against the latest OpenSSL_1_0_2-stable in Github (safe to assume it's 1.0.2f). It appears to work fine:

......
Deriving shared symmetric key on the token, using ephemeral public key...
pkcs11-tool --slot 1 --module /Library/OpenSC/lib/opensc-pkcs11.so -l --derive -m ECDH1-COFACTOR-DERIVE -d 03 -i /tmp/derive.4261.pub.der -o /tmp/derive.4261.shared2
Logging in to "PIV_II (PIV Card Holder pin)".
Please enter User PIN: 
Using derive algorithm 0x00001051 ECDH1-COFACTOR-DERIVE

Attempting to derive ECDH key on the token...
openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.4261.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
0000 - f1 16 7c 90 49 17 f5 47-75 73 ba f4 f0 da 63 6b   ..|.I..Gus....ck
0010 - c4 75 b0 54 60 3e 45 df-24 43 62 1f 7e e6 39 a2   .u.T`>E.$Cb.~.9.
0020 - 88 d6 c9 89 e6 bb 23 99-6e 03 0a 79 21 00 b2 2d   ......#.n..y!..-
Decrypting data file with derived symmetric key in /tmp/derive.4261.shared2 and AES-CFB...
openssl enc -aes-256-cfb -d -a -kfile /tmp/derive.4261.shared2 -in /tmp/derive.4261.text.enc -out /tmp/derive.4261.text.dec

Derived keys /tmp/derive.4261.shared1 and /tmp/derive.4261.shared2 matched.
Decrypted file /tmp/derive.4261.text.dec matches the original plaintext /tmp/derive.4261.text.

I consider these patches to be a hack, and should not end up in the master branch
of libp11 or engine_opensc.

I am not sure I agree with such a harsh statement. 1.0.2 is going to be around, like it or not. ECDH capability should also be around. So in my opinion, as long as we care to be 1.0.2-compatible, it makes sense to go all the way, and support full functionality - including ECDH.

Libp11 needs to be compiled with the OpenSSL-1.0.2 internal source file
crypto/ecdh/ech_locl.h

Yes, this is a disadvantage. I still hope that OpenSSL dev team might be persuaded to make the needed interface visible, to allow us using stock openssl/*.h as opposed to going for the source. But that (having to go to the source) is the only disadvantage I see. And it's not that big, IMHO.

Doug, could you explain why you think that your perfectly working fix should not be in the master branch?

P.S. OpenSSL Release Strategy says:

Version 1.0.2 will be supported until 2019-12-31.

@mouse07410
Copy link
Contributor Author

Now - for the problem. ;)

Before, pkeyutl failed to do ECDH key derivation on the token, but with openssl/openssl#557 could use public key on the token to EC-derive from a private key in a file.

Now, thankfully pkeyutl successfully does ECDH derivation on the token - but somehow lost the ability to derive from a private key in a file and public key on the token:

$ openssl pkeyutl -engine pkcs11 -peerform engine -derive -keyform PEM -inkey /tmp/derive.96830.priv.pem -peerkey id_03 -hexdump
Initializing engine
engine "pkcs11" set.
Loading public key "id_03"
Looking in slot -1 for key: 03
Found 2 slots
[18446744073709551615] Virtual hotplug slot       no tok
[1] Yubico Yubikey NEO OTP+U2  login             (PIV_II (PIV Card Holder pin))
Found slot:  Yubico Yubikey NEO OTP+U2F+CCID
Found token: PIV_II (PIV Card Holder pin)
Found 4 certificates:
   1    Certificate for PIV Authentication (/CN=Me Mouse)
   2    Certificate for Digital Signature (/CN=Me Mouse)
   3    Certificate for Key Management (/CN=Me Mouse)
   4    Certificate for Card Authentication (/CN=Me Mouse)
Found 4 public keys:
   1    PIV AUTH pubkey
   2    SIGN pubkey
   3    KEY MAN pubkey
   4    CARD AUTH pubkey
Public Key operation error
$

I checked, and the PR is in the openssl-1.0.2, so it doesn't seem like it was lost along the way.
Certainly, it is more important to be able to derive on the token, because if worst comes to worst, pub key can be extracted by a separate command. But for the sake of neatness, perhaps we could fix this problem? I suspect something got lost in libp11/engine_pkcs11 related to difference in treating public key vs. private key. Probably it was automatically pulling the public key from the token before, but is failing to do so now...?

Any comments? Any place you'd recommend me to start looking at?

Update
@dengert, @mtrojnar
Just got an idea...

For RSA there are two methods for each key pair: encrypt <-> decrypt, sign <-> verify.

Are there two methods for ECDH-DERIVE, one that uses private key on the token, and the other one that uses public key? Because if not - then it would explain why ECDH-DERIVE works either with private key and fails with public, or works with public but fails with private.

If the above diagnosis is correct, then for a one-time command line invocation (like what pkeyutl is doing) we could probably fill that ops table based on the operation requested. Do you concur?

Can we address it for an application that might want repeated operations, some with public key and some with private..?

@mouse07410
Copy link
Contributor Author

When I install the older code (master branch of OpenSC/engine_pkcs11, and libp11-0_3_x branch of OpenSC/libp11), then the correct ECDH public key processing from the token is restored - but the ECDH private key-based derivation on the token is not there (debugging output => my instrumentation of apps/pkeyutl.c around lines 320-325):

$ apps/openssl pkeyutl -engine pkcs11 -keyform PEM -peerform engine -derive -inkey /tmp/derive.1017.priv.pem -peerkey "pkcs11:object=KEY%20MAN%20pubkey;object-type=public" -hexdump
engine "pkcs11" set.
pkeyutl: about to do pkey_op 400
pkeyutl: pkey_op 400 returned 1
pkeyutl: allocated 32 buf_out
pkeyutl: (2)pkey_op 400 returned 1
received buffer in hex:
74 68 17 49 f2 4a d0  f 73 4e ed c8 81 68 58 87 19 61 ba 11 d9 e8  5 d5 d7 fd cf e4 62 52 52 35
0000 - 74 68 17 49 f2 4a d0 0f-73 4e ed c8 81 68 58 87   th.I.J..sN...hX.
0010 - 19 61 ba 11 d9 e8 05 d5-d7 fd cf e4 62 52 52 35   .a..........bRR5
$ apps/openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.1017.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN:
pkeyutl: about to do pkey_op 400
pkeyutl: pkey_op 400 returned 1
pkeyutl: allocated 32 buf_out
pkeyutl: (2)pkey_op 400 returned 0
received buffer in hex:
 0  0  0  0  0  0  0 60 8a 12 35 58 f9  7  0 40  2  0  0  0  0  0  0  0  0  0  0  0  0  0  2  0
Public Key operation error
140735323983952:error:2B064064:lib(43):ECDH_compute_key:no private value:ech_ossl.c:136:
$

Re-installing the master branch of OpenSC/libp11 with the patches from @dengert enables ECDH private key-based derivation on the token, but loses public key-based derivation:

$ apps/openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.1017.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN:
pkeyutl: about to do pkey_op 400
pkeyutl: pkey_op 400 returned 1
pkeyutl: allocated 32 buf_out
pkeyutl: (2)pkey_op 400 returned 1
received buffer in hex:
74 68 17 49 f2 4a d0  f 73 4e ed c8 81 68 58 87 19 61 ba 11 d9 e8  5 d5 d7 fd cf e4 62 52 52 35
0000 - 74 68 17 49 f2 4a d0 0f-73 4e ed c8 81 68 58 87   th.I.J..sN...hX.
0010 - 19 61 ba 11 d9 e8 05 d5-d7 fd cf e4 62 52 52 35   .a..........bRR5
$ apps/openssl pkeyutl -engine pkcs11 -keyform PEM -peerform engine -derive -inkey /tmp/derive.1017.priv.pem -peerkey "pkcs11:object=KEY%20MAN%20pubkey;object-type=public" -hexdump
engine "pkcs11" set.
pkeyutl: about to do pkey_op 400
pkeyutl: pkey_op 400 returned 1
pkeyutl: allocated 32 buf_out
pkeyutl: (2)pkey_op 400 returned 0
received buffer in hex:
 0  0  0  0  0  0  0 40  0  0  0  0  0  0  0 40  2  0  0  0  0  0  0  0  0  0  0  0  0  0  2  0
Public Key operation error
$

It would be great if we could keep this newly-acquired ECDH private key-based derivation, and fix the public key-based... Must add that all of the above works as shown with openssl-1.0.2f, and 1.0.2g-dev.

Update
I compared with the libp11 master (latest) without Doug's patches. Against the stock (Macports) OpenSSL-1.0.2f. Same thing as above (public-based derive works, private-based doesn't). So it's definitely something with the patches. Doug, what in there could break the public key functionality for ECDH? Should something that it's doing be conditional upon whether the key is private?

ECDSA still works fine:

$ pkcs11-ecdsa-demo2 test-driver 
Using reader with a card: Yubico Yubikey 4 OTP+U2F+CCID
read EC key
openssl dgst -engine pkcs11 -keyform engine -sha384 -sign "pkcs11:object=SIGN%20key;object-type=private" -out /tmp/test-driver.sig test-driver
engine "pkcs11" set.
PKCS#11 token PIN: 
Signature for test-driver is stored in /tmp/test-driver.sig

pkcs11-tool -r -y pubkey -d 02 -o /tmp/derive.43374.02
Using slot 1 with a present token (0x1)
openssl dgst -verify /tmp/derive.43374.02 -keyform DER -sha384 -signature /tmp/test-driver.sig test-driver
Verified OK

openssl dgst -engine pkcs11 -keyform engine -sha384 -verify "pkcs11:object=SIGN%20pubkey;object-type=public" -signature /tmp/test-driver.sig test-driver
engine "pkcs11" set.
Verified OK

$

@mouse07410
Copy link
Contributor Author

Oh, and in case it matters, I'm getting these warnings compiling with the patches on:

...
p11_ec.c:297:8: warning: implicit declaration of function 'pkcs11_ecdh_derive_internal' is invalid in C99
      [-Wimplicit-function-declaration]
        ret = pkcs11_ecdh_derive_internal(&buf, &buflen, CKM_ECDH1_DERIVE,
              ^
p11_ec.c:302:8: warning: expression result unused [-Wunused-value]
                        ret -1;
                        ~~~ ^~
p11_ec.c:471:25: warning: incompatible pointer types assigning to 'int (*)(void *, size_t, const EC_POINT
      *, EC_KEY *, void *(*)(const void *, size_t, void *, size_t *))' from 'int (void *, size_t, const
      EC_POINT *, const EC_KEY *, void *(*)(const void *, size_t, void *, size_t *))'
      [-Wincompatible-pointer-types]
                ops_ecdh->compute_key = pkcs11_ec_ckey;
                                      ^ ~~~~~~~~~~~~~~
6 warnings generated.
...

@dengert
Copy link
Member

dengert commented Feb 1, 2016

Does not really matter, they are warnings. But see update of the ecdh-1.0.2 patch
dengert@ebdaf38
Tha makes sure pkcs11_ecdh_derive_intrernal is defined in the patch.

@dengert
Copy link
Member

dengert commented Feb 1, 2016

@mtrojnar I would like to see this issue closed. The code for OpenSSL-1.1 has been added.

If its not closed, at least rename it to:
"libp11 does not support ECDH key derivation is OpenSSL versions < 1.1"

@mouse07410
Copy link
Contributor Author

@dengert since 1.0.2 isn't going to disappear from the mainstream until at least the end of 2019, I don't think it is appropriate to close this issue simply because it may be resolved in the version that would become the mainstream who knows when. (I say "may" because the code has been added for 1.0.2 also. There's no reason that I see to expect that the code currently in 1.1 would work better than the one we have in 1.0.2.)

Yes, I think it's fine to rename it the way suggested.

P.S. I'd much appreciate if we could spend a bit more effort on finding the problem, rather than on what's the proper title for this issue. ;)

@dengert
Copy link
Member

dengert commented Feb 1, 2016

What is "the problem"?

OpenSSL is evolving, as is OpenSC, its just not at the rate you would like. I though the OpenSSL developers said to you they would not add new functionality to 1.0.2 which is what is needed to hook the method by external engines without using internal OpenSSL header files. I don't think OpenSC or libp11 should be trying to use internal OpenSSL header files in our releases. That will never end up in destros.

So you are left with local source code changes and compiling yourself. I have given you the basis for this with the ECDH-1.0.2. mods.

@mouse07410
Copy link
Contributor Author

What is "the problem"?

That currently I can get working either one of the following, but not both:

openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey id_03 -peerform DER -peerkey ephempub.der -hexdump

or

openssl pkeyutl -engine pkcs11 -peerform engine -derive -inkey ephempriv.pem -peerkey id_03 -hexdump

OpenSSL is evolving, as is OpenSC, its just not at the rate you would like.

And that is fine, especially since it's not in my power to affect in any way.

OpenSSL developers said to you they would not add new functionality to 1.0.2 which
is what is needed to hook the method by external engines without using internal
OpenSSL header files.

Yes (though it's still a question of whether exposing an existing interface without any changes to it means adding functionality :). But I'm perfectly OK to use that internal header file (luckily, only one seems necessary). I also hope that libp11 can include that file ech_locl.h in its source base to be used when it is compiled against 1.0.2.

So you are left with local source code changes and compiling yourself.

Yes. That is perfectly OK.

I have given you the basis for this with the ECDH-1.0.2. mods.

Yes, thank you! Now I'm trying to cross that "last bridge" and make both kinds of derivation work (see above).

It may be not possible - I don't know. In that case perhaps you could explain what makes it impossible...

P.S. So far I've been unable to test openssl-1.1, because on Mac OS X I seem unable to provide the appropriate shared libraries path (yes I know about DYLD_LIBRARY_PATH :), and am unable to wipe my current openssl (1.0.2) and break everything else to install 1.1 over it for testing. :-(

$ DYLD_LIBRARY_PATH=/Users/ur20980/src/openssl-1.1/lib:. ~/src/openssl-1.1/bin/openssl engine pkcs11
140735120921424:error:260B6091:engine routines:dynamic_load:version incompatibility:eng_dyn.c:493:
140735120921424:error:2606A074:engine routines:ENGINE_by_id:no such engine:eng_list.c:378:id=pkcs11
$ DYLD_LIBRARY_PATH=/Users/ur20980/src/openssl-1.1/lib:. ~/src/openssl-1.1/bin/openssl engine pkcs11
140735120921424:error:25066067:DSO support routines:dlfcn_load:could not load the shared library:dso_dlfcn.c:171:filename(/Users/ur20980/src/openssl-1.1/lib/engines/libpkcs11.dylib): dlopen(/Users/ur20980/src/openssl-1.1/lib/engines/libpkcs11.dylib, 2): Symbol not found: _BUF_strdup
  Referenced from: /Users/ur20980/src/openssl-1.1/lib/libp11.2.dylib
  Expected in: flat namespace
 in /Users/ur20980/src/openssl-1.1/lib/libp11.2.dylib
140735120921424:error:25070067:DSO support routines:DSO_load:could not load the shared library:dso_lib.c:227:
140735120921424:error:260B6084:engine routines:dynamic_load:dso not found:eng_dyn.c:453:
140735120921424:error:2606A074:engine routines:ENGINE_by_id:no such engine:eng_list.c:378:id=pkcs11
$ DYLD_LIBRARY_PATH=/Users/ur20980/src/openssl-1.1/lib:. ~/src/openssl-1.1/bin/openssl engine rdrand
(rdrand) Intel RDRAND engine
$ ls -l ~/src/openssl-1.1/lib/libp11.2.dylib 
-rwxr-xr-x  1 ur20980  MITLL\Domain Users  49628 Feb  1 18:44 /Users/ur20980/src/openssl-1.1/lib/libp11.2.dylib*
$ otool -L ~/src/openssl-1.1/lib/libp11.2.dylib 
/Users/ur20980/src/openssl-1.1/lib/libp11.2.dylib:
    /Users/ur20980/src/openssl-1.1/lib/libp11.2.dylib (compatibility version 6.0.0, current version 6.0.0)
    /Users/ur20980/src/openssl-1.1/lib/libssl.1.1.dylib (compatibility version 1.1.0, current version 1.1.0)
    /Users/ur20980/src/openssl-1.1/lib/libcrypto.1.1.dylib (compatibility version 1.1.0, current version 1.1.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
$

Has BUF_strdup() been replaced with CRYPTO_strdup() in 1.1? Should it be addressed in p11_cert.c, p11_key.c, etc.?

$ nm /opt/local/lib/libcrypto.dylib | grep strdup
00000000000d44c0 T _BUF_strdup
0000000000002100 T _CRYPTO_strdup
$ nm ~/src/openssl-1.1/lib/libcrypto.dylib | grep strdup
0000000000002710 T _CRYPTO_strdup
$

@mtrojnar mtrojnar changed the title libp11 does not support ECDH key derivation libp11 does not support ECDH key derivation is OpenSSL versions < 1.1 Feb 2, 2016
@mtrojnar
Copy link
Member

mtrojnar commented Feb 2, 2016

The best way to support ECDH key derivation in OpenSSL 1.0.2 would be to submit a pull request with the missing functions against OpenSSL 1.0.2. Adding a few missing functions to the API of a stable release should not break anything. In fact, it happened before.

If the OpenSSL maintainers refuse to merge this pull request then I am okay with including a local definition of ECDH_METHOD for OpenSSL 1.2 in p11_ec.c.

@mouse07410
Copy link
Contributor Author

The best way to support ECDH key derivation in OpenSSL 1.0.2 would be to submit a pull
request with the missing functions against OpenSSL 1.0.2.

I concur. @dengert you know both codebases (your patches to libp11, and openssl) better than me, and probably better than @mtrojnar. Would you be agreeable to create such a PR?

Adding a few missing functions to the API of a stable release should not break anything.
In fact, it happened before.

Again, I concur. However, the following dialog from openssl-dev mailing list with one of the developers/maintainers seems to suggest otherwise:

> Still, since openssl-1_0_2 does ECDH, and it exposes ‎ECDSA to external engine(s), how
> difficult would it be to add ECDH exposure? I suspect - a good deal easier than getting
> 1.1 replace 1.0.x as the de-facto deployment standard.

It’s a new feature, so it won’t come from OpenSSL.  Don’t know if that’s an issue or not.  Only fixes go into released branches.

and

> The fact that these mechanisms are half-done means to be that it’s a bug in need of fixing.

I doubt that anyone else on the team will find this argument compelling.

Other Dev Team members neither confirmed nor denied the above statements. ;)

@mtrojnar
Copy link
Member

mtrojnar commented Feb 2, 2016

ECDH_METHOD is definitely non-functional without API functions to instantiate it and access its data. I'm positive implementing these missing functions fixes a bug (design flaw?) rather than implements a new feature.

@mouse07410
Copy link
Contributor Author

I'm positive implementing these missing functions fixes a bug (design flaw?) rather
than implements a new feature.

I concur (and had stated that same thing in the past). The trick is convincing the OpenSSL Dev Team. ;)

In the meanwhile, I'd really love to get ECDH-DERIVE completely fixed in libp11, though one could reasonably argue that the more important part is done.

An application (or a user) may want to:

a. Derive ECDH key on the token - which did not work until you fixed it, thanks! or

b. Derive ECDH key from the ephemeral private key it received or generated and the public key on the token.

Here's the example of (a) (which I'm very grateful for fixing!):

$ openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey id_03 -peerform DER -peerkey ephempub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
0000 - 22 d8 ad 9f aa 8f f9 62-c0 6e 63 a4 8b 3c 64 ca   "......b.nc..<d.
0010 - 60 fe bb 80 d1 99 63 5e-a9 60 87 12 e2 0f 40 eb   `.....c^.`....@.
0020 - 8f a6 cc 56 31 83 1b c8-d4 e5 45 6b cd 2a fd 89   ...V1.....Ek.*..
$

Here's the example of (b) (which got broken, probably because currently the code can't distinguish between where the private key for this operation is):

$ openssl pkeyutl -engine pkcs11 -peerform engine -derive -inkey ephempriv.pem -peerkey id_03 -hexdump
engine "pkcs11" set.
Public Key operation error
$

The problem is - once the token gets involved/mentioned, the code automatically assumes that that's where the private key is going to be. Or at least tries to perform the operation as if that's where the private key was.

With RSA, it does not make this mistake: when Decrypt is requested it goes to the token, but I can request Encrypt - in which case it just takes the public key from the token and uses it in software.

@dengert, what is your opinion regarding how to address the above? Do/can we need to do something different within the method when it's -peerform engine as opposed to -keyform engine? I suspect that we should avoid setting up ecdh_method altogether when -peerform engine (and thus resort to the pre-patch behavior that was shown to work with -peerform engine), but not sure how to change the code to accomplish it.

@dengert
Copy link
Member

dengert commented Feb 4, 2016

In regards to pkeyutil, it the real world why would you have access to the token of the peer? The peer's pubkey is in the peer's certificate. In any protocol, the certificates or pubkey of an ephemeral EC key are what are sent to the other party.

In your testing, you could simulate this be getting the peer certificate or pubkey in a separate step and pass it to pkeyutil as a file.

All the OpenSSL utilities appear to be able to handle only a single engine at a time. As you point out there may be some confusion and any engine is considered usable for the private key operations. So you could submit a bug report to OpenSSL.

Even if they could handle more then one engine at a time, could engine_pkcs11 and libp11 be invoked as two separate engines?

PKCS#11 could have multiple tokens at the same time, but does this work correctly?

It could also be a design issue. I believe engines were originally designed to replace crypto routines because one may not trust the OpenSSL implementation. So pkeyutil may be assuming that the availability of an engine that can do derive can be used. You need to look at the code.

(You know I am retired, and am still on this project because of the PIV card and I wanted to see ECDH implemented in OpenSC and engine. Having ECDH in OpenSSL-1.1 satisfies that desire. I am not interested in strange ways to use pkeyutil and only marginally interested in any back ports of ECDH to 1.0.2.)

@mtrojnar mtrojnar assigned mtrojnar and unassigned dengert Feb 4, 2016
@dengert
Copy link
Member

dengert commented Feb 4, 2016

@mtrojnar
https://github.com/dengert/libp11/tree/ecdh-1.0.2
https://github.com/dengert/engine_pkcs11/tree/ecdh-102
contain the patches I had for 1.0.2. Let me know what you would like to do with them.

@mtrojnar
Copy link
Member

mtrojnar commented Feb 4, 2016

@dengert Could you please leave the ecdh-1.0.2 branch as it is? I'll be glad to merge your code after I finish the cleanup I'm working on right now. Thank you very much for your work on this feature!

@mouse07410
Copy link
Contributor Author

In regards to pkeyutl, it the real world why would you have access to the token of the peer?...
In any protocol, the certificate or pubkey of an ephemeral EC key are what are sent to the other party.

Good point.

In your testing, you could simulate this be getting the peer certificate or pubkey in a separate
step and pass it to pkeyutil as a file.

Yes, that's what I've been doing.

But there is certain elegance in not having to employ another package and another command-line tool. And everything else (RSA and ECDSA) already supports this kind of usage, so it would be nice to cover that "last mile". :-)

All the OpenSSL utilities appear to be able to handle only a single engine at a time.

Do you happen to know whether this applies to applications linked with OpenSSL libraries?

I want access to PKCS#11 tokens via engines work for "arbitrary" software linked with openssl libs, not only openssl CLI (dgst, pkeyutl, etc).

As you point out there may be some confusion and any engine is considered usable for the
private key operations. So you could submit a bug report to OpenSSL.

You see, the problem is that for RSA (sign/verify and encrypt/decrypt) and ECDSA (sign/verify) it works perfectly as is. I.e., I can sign using private key on the token, and verify using public key on the token without having to extract it first (same with RSA encrypt/decrypt). So I don't expect a favorable response from the OpenSSL team.

Even if they could handle more then one engine at a time, could engine_pkcs11 and libp11
be invoked as two separate engines?

I don't think I understand - I thought that engine_pkcs11 and libp11 are being merged? In fact, the master branch of libp11 that I'm using, already merged the two...? Could you explain please?

PKCS#11 could have multiple tokens at the same time, but does this work correctly?

I don't know - but I don't think so. Since there's no way to indicate which token (out of several available) you want, one would probably have to play with OpenSC configuration/parameters outside of OpenSSL. I haven't tried that (yet? as I have several tokens of three kinds).

You know I am retired, and am still on this project because of the PIV card and I wanted to
see ECDH implemented in OpenSC and engine.

Yes I do.

Having ECDH in OpenSSL-1.1 satisfies that desire.

But you don't know if it works. And probably won't know for some considerable time.

I am not interested in strange ways to use pkeyutil and only marginally interested in any
back ports of ECDH to 1.0.2.

Understood.

@mouse07410
Copy link
Contributor Author

@mtrojnar, request re-opening.

Current/latest master branch from OpenSC/libp11, does not work:

Attempting to derive ECDH key on the token...
openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.60770.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
Public Key operation error

$ openssl version
OpenSSL 1.0.2g-dev  xx XXX xxxx

The previous version (manually merged @dengert's patches) worked:

openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.62672.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
0000 - b3 41 58 6a 3e c5 68 23-2f fd ee 16 cb df 31 cf   .AXj>.h#/.....1.
0010 - 0c 0d a4 64 6c e6 33 c3-ed 6b 99 6f d3 c9 98 47   ...dl.3..k.o...G
0020 - a9 0a 45 93 c2 f1 10 32-ba 08 23 5a e1 f9 38 95   ..E....2..#Z..8.

Please let me know what info you'd need to chase problem this down.

P.S. It doesn't work the other way too, but as Doug pointed out, the following is not a practical scenario, so we can worry about it after the above is fixed:

openssl pkeyutl -engine pkcs11 -peerform engine -derive -inkey /tmp/derive.60770.priv.pem -peerkey "pkcs11:object=KEY%20MAN%20pubkey;object-type=public" -out /tmp/derive.60770.shared1
engine "pkcs11" set.
Public Key operation error

@mtrojnar
Copy link
Member

Fixed. Thank you.

@mouse07410
Copy link
Contributor Author

Thanks - it works now:

openssl pkeyutl -engine pkcs11 -keyform engine -derive -inkey "pkcs11:object=KEY%20MAN%20key;object-type=private" -peerform DER -peerkey /tmp/derive.74397.pub.der -hexdump
engine "pkcs11" set.
PKCS#11 token PIN: 
0000 - f5 0c 27 a8 4e ab 37 95-1f b7 df 3e 27 02 b5 3c   ..'.N.7....>'..<
0010 - e2 01 1c 24 e8 1c e8 de-f0 c2 76 01 e3 bb e8 b2   ...$......v.....
0020 - 89 e7 f8 ff 52 ea 85 6e-b9 52 bb 2b 07 a8 32 2e   ....R..n.R.+..2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants