Clean up encoding of EC_POINT for key loading vs derivation based on the standard

Author: Danielle Madeley

README.rst

@@ -306,10 +306,11 @@ Python version:
 * 3.5 (with `aenum`)
 * 3.6
 
-PKCS#11 version:
+PKCS#11 versions:
 
-* 2.2
-* 2.4
+* 2.11
+* 2.20
+* 2.40
 
 Feel free to send pull requests for any functionality that's not exposed. The
 code is designed to be readable and expose the PKCS #11 spec in a

docs/opensc.rst

@@ -95,7 +95,7 @@ exporting keys.
 RSA
 ~~~
 
-PyCrypto example:
+`PyCrypto` example:
 
 ::
 
@@ -123,7 +123,7 @@ PyCrypto example:
 ECDSA
 ~~~~~
 
-oscrypto example:
+`oscrypto` example:
 
 ::
 
@@ -147,12 +147,73 @@ oscrypto example:
     key = load_public_key(encode_ec_public_key(pub))
     ecdsa_verify(key, signature, b'Data to sign', 'sha1')
 
+ECDH
+~~~~
+
+Smartcard-HSM can generate a shared key via ECDH key exchange.
+
+.. warning::
+
+    Where possible, e.g. over networks, you should use ephemeral keys,
+    to allow for perfect forward secrecy. Smartcard HSM's ECDH is only useful
+    when need to repeatedly retrieve the same shared secret, e.g. encrypting
+    files in a hybrid cryptosystem.
+
+`cryptography` example:
+
+::
+
+    from cryptography.hazmat.backends import default_backend
+    from cryptography.hazmat.primitives.asymmetric import ec
+    from cryptography.hazmat.primitives.serialization import \
+        Encoding, PublicFormat, load_der_public_key
+
+    # Retrieve our keypair, with our public key encoded for interchange
+    alice_priv = self.session.get_key(key_type=KeyType.EC,
+                                        object_class=ObjectClass.PRIVATE_KEY)
+    alice_pub = self.session.get_key(key_type=KeyType.EC,
+                                        object_class=ObjectClass.PUBLIC_KEY)
+    alice_pub = encode_ec_public_key(alice_pub)
+
+    # Bob generates a keypair, with their public key encoded for
+    # interchange
+    bob_priv = ec.generate_private_key(ec.SECP256R1,
+                                        default_backend())
+    bob_pub = bob_priv.public_key().public_bytes(
+        Encoding.DER,
+        PublicFormat.SubjectPublicKeyInfo,
+    )
+
+    # Bob converts Alice's key to internal format and generates their
+    # shared key
+    bob_shared_key = bob_priv.exchange(
+        ec.ECDH(),
+        load_der_public_key(alice_pub, default_backend()),
+    )
+
+    key = alice_priv.derive_key(
+        KeyType.GENERIC_SECRET, 256,
+        mechanism_param=(
+            KDF.NULL, None,
+            # SmartcardHSM doesn't accept DER-encoded EC_POINTs for derivation
+            decode_ec_public_key(bob_pub, encode_ec_point=False)
+            [Attribute.EC_POINT],
+        ),
+    )
+    alice_shared_key = key[Attribute.VALUE]
+
+When decoding the other user's `EC_POINT` for passing into the key derivation
+the standard says to pass a raw octet string (set `encode_ec_point` to False),
+however some PKCS #11 implementations require a DER-encoded octet string
+(i.e. the format of the :attr:`pkcs11.constants.Attribute.EC_POINT` attribute).
+
 Encrypting Files
 ----------------
 
 The device only supports asymmetric mechanisms. To do file encryption, you
-will need to generate AES keys locally, which you can encrypt with the
-public key (this is how the Nitrokey storage key works).
+will need to generate AES keys locally, which you can encrypt with your RSA
+public key (this is how the Nitrokey storage key works); or by using ECDH
+to generate a shared secret from a locally generated public key.
 
 Debugging
 ---------

pkcs11/util/ec.py

@@ -35,21 +35,32 @@ def encode_named_curve_parameters(oid):
     return encoder.encode(ecParams)
 
 
-def decode_ec_public_key(der):
+def decode_ec_public_key(der, encode_ec_point=True):
     """
     Decode a DER-encoded EC public key as stored by OpenSSL into a dictionary
     of attributes able to be passed to :meth:`pkcs11.Session.create_object`.
 
+    .. note:: encode_ec_point
+
+        For use as an attribute `EC_POINT` should be DER-encoded (True).
+
+        For key derivation implementations can vary.  Since v2.30 the
+        specification says implementations MUST accept a raw `EC_POINT` for
+        ECDH (False), however not all implementations follow this yet.
+
     :param bytes der: DER-encoded key
+    :param encode_ec_point: See text.
     :rtype: dict(Attribute,*)
     """
     asn1, _ = decoder.decode(der, asn1Spec=rfc3280.SubjectPublicKeyInfo())
 
     assert asn1['algorithm']['algorithm'] == id_ecPublicKey, \
         "Wrong algorithm, not an EC key!"
 
-    ecpoint = \
-        encoder.encode(OctetString(value=asn1['subjectPublicKey'].asOctets()))
+    ecpoint = asn1['subjectPublicKey'].asOctets()
+
+    if encode_ec_point:
+        ecpoint = encoder.encode(OctetString(value=ecpoint))
 
     return {
         Attribute.KEY_TYPE: KeyType.EC,
@@ -63,6 +74,7 @@ def encode_ec_public_key(key):
     """
     Encode a DER-encoded EC public key as stored by OpenSSL.
 
+
     :param PublicKey key: RSA public key
     :rtype: bytes
     """

tests/test_public_key_external.py

@@ -7,7 +7,7 @@
     encode_named_curve_parameters,
 )
 
-from . import TestCase, requires
+from . import TestCase, requires, Is
 
 
 class ExternalPublicKeyTests(TestCase):
@@ -101,7 +101,10 @@ def test_ecdh(self):
             KeyType.GENERIC_SECRET, 256,
             mechanism_param=(
                 KDF.NULL, None,
-                decode_ec_public_key(bob_pub)[Attribute.EC_POINT],
+                # N.B. it seems like SoftHSMv2 requires an EC_POINT to be
+                # DER-encoded, which is not what the spec says
+                decode_ec_public_key(bob_pub, encode_ec_point=Is.softhsm2)
+                [Attribute.EC_POINT],
             ),
             template={
                 Attribute.SENSITIVE: False,