Merge branch 'development' into eddsa

Signed-off-by: Pol Henarejos <pol.henarejos@cttc.es>

src/fido/cbor_make_credential.c

@@ -156,51 +156,6 @@ int cbor_make_credential(const uint8_t *data, size_t len) {
     uint8_t rp_id_hash[32];
     mbedtls_sha256((uint8_t *) rp.id.data, rp.id.len, rp_id_hash, 0);
 
-    int curve = -1, alg = 0;
-    if (pubKeyCredParams_len == 0) {
-        CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER);
-    }
-
-    for (int i = 0; i < pubKeyCredParams_len; i++) {
-        if (pubKeyCredParams[i].type.present == false) {
-            CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER);
-        }
-        if (strcmp(pubKeyCredParams[i].type.data, "public-key") != 0) {
-            continue;
-        }
-        if (pubKeyCredParams[i].alg == FIDO2_ALG_ES256) {
-            curve = FIDO2_CURVE_P256;
-        }
-        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES384) {
-            curve = FIDO2_CURVE_P384;
-        }
-        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES512) {
-            curve = FIDO2_CURVE_P521;
-        }
-        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES256K) {
-            curve = FIDO2_CURVE_P256K1;
-        }
-        else if (pubKeyCredParams[i].alg == FIDO2_ALG_EDDSA) {
-            curve = FIDO2_CURVE_ED25519;
-        }
-        else if (pubKeyCredParams[i].alg == 0) { // no present
-            curve = -1;
-        }
-        else {
-            curve = 0;
-        }
-        if (curve > 0) {
-            alg = pubKeyCredParams[i].alg;
-            break;
-        }
-    }
-    if (curve == 0) {
-        CBOR_ERROR(CTAP2_ERR_UNSUPPORTED_ALGORITHM);
-    }
-    else if (curve == -1) {
-        CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER);
-    }
-
     if (pinUvAuthParam.present == true) {
         if (pinUvAuthParam.len == 0 || pinUvAuthParam.data == NULL) {
             if (check_user_presence() == false) {
@@ -222,6 +177,58 @@ int cbor_make_credential(const uint8_t *data, size_t len) {
             }
         }
     }
+
+    int curve = -1, alg = 0;
+    if (pubKeyCredParams_len == 0) {
+        CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER);
+    }
+
+    for (int i = 0; i < pubKeyCredParams_len; i++) {
+        if (pubKeyCredParams[i].type.present == false) {
+            CBOR_ERROR(CTAP2_ERR_INVALID_CBOR);
+        }
+        if (pubKeyCredParams[i].alg == 0) {
+            CBOR_ERROR(CTAP2_ERR_INVALID_CBOR);
+        }
+        if (strcmp(pubKeyCredParams[i].type.data, "public-key") != 0) {
+            CBOR_ERROR(CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
+        }
+        if (pubKeyCredParams[i].alg == FIDO2_ALG_ES256) {
+            if (curve <= 0) {
+                curve = FIDO2_CURVE_P256;
+            }
+        }
+        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES384) {
+            if (curve <= 0) {
+                curve = FIDO2_CURVE_P384;
+            }
+        }
+        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES512) {
+            if (curve <= 0) {
+                curve = FIDO2_CURVE_P521;
+            }
+        }
+        else if (pubKeyCredParams[i].alg == FIDO2_ALG_ES256K) {
+            if (curve <= 0) {
+                curve = FIDO2_CURVE_P256K1;
+            }
+        }        
+        else if (pubKeyCredParams[i].alg == FIDO2_ALG_EDDSA) {
+            if (curve <= 0) {
+                curve = FIDO2_CURVE_ED25519;
+            }
+        }
+        else {
+            CBOR_ERROR(CTAP2_ERR_CBOR_UNEXPECTED_TYPE);
+        }
+        if (curve > 0 && alg == 0) {
+            alg = pubKeyCredParams[i].alg;
+        }
+    }
+    if (curve <= 0) {
+        CBOR_ERROR(CTAP2_ERR_UNSUPPORTED_ALGORITHM);
+    }
+
     if (options.present) {
         if (options.uv == ptrue) { //5.3
             CBOR_ERROR(CTAP2_ERR_INVALID_OPTION);

tests/conftest.py

@@ -25,7 +25,7 @@ from fido2.attestation import FidoU2FAttestation
 from fido2.ctap2.pin import ClientPin
 from fido2.server import Fido2Server
 from fido2.ctap import CtapError
-from fido2.webauthn import CollectedClientData, AttestedCredentialData
+from fido2.webauthn import CollectedClientData, PublicKeyCredentialParameters, PublicKeyCredentialType
 from utils import *
 from fido2.cose import ES256
 import sys
@@ -116,6 +116,10 @@ class Device():
         self.__rp = rp
         self.__attestation = attestation
         self.__server = Fido2Server(self.__rp, attestation=self.__attestation)
+        self.__server.allowed_algorithms = [
+            PublicKeyCredentialParameters(PublicKeyCredentialType.PUBLIC_KEY, p['alg'])
+            for p in self.__client._backend.info.algorithms
+        ]
 
     def client(self):
         return self.__client

tests/pico-fido/test_020_register.py

@@ -20,6 +20,7 @@
 
 from fido2.client import CtapError
 from fido2.cose import ES256, ES384, ES512, EdDSA
+from utils import ES256K
 import pytest
 
 
@@ -121,7 +122,7 @@ def test_bad_type_pubKeyCredParams(device):
         device.doMC(key_params=["wrong"])
 
 @pytest.mark.parametrize(
-    "alg", [ES256.ALGORITHM, ES384.ALGORITHM, ES512.ALGORITHM, EdDSA.ALGORITHM]
+    "alg", [ES256.ALGORITHM, ES384.ALGORITHM, ES512.ALGORITHM, ES256K.ALGORITHM, EdDSA.ALGORITHM]
 )
 def test_algorithms(device, info, alg):
     if ({'alg': alg, 'type': 'public-key'} in info.algorithms):
@@ -131,14 +132,14 @@ def test_missing_pubKeyCredParams_type(device):
     with pytest.raises(CtapError) as e:
         device.doMC(key_params=[{"alg": ES256.ALGORITHM}])
 
-    assert e.value.code == CtapError.ERR.MISSING_PARAMETER
+    assert e.value.code == CtapError.ERR.INVALID_CBOR
 
 def test_missing_pubKeyCredParams_alg(device):
     with pytest.raises(CtapError) as e:
         device.doMC(key_params=[{"type": "public-key"}])
 
     assert e.value.code in [
-        CtapError.ERR.MISSING_PARAMETER,
+        CtapError.ERR.INVALID_CBOR,
         CtapError.ERR.UNSUPPORTED_ALGORITHM,
     ]
 
@@ -150,7 +151,7 @@ def test_unsupported_algorithm(device):
     with pytest.raises(CtapError) as e:
         device.doMC(key_params=[{"alg": 1337, "type": "public-key"}])
 
-    assert e.value.code == CtapError.ERR.UNSUPPORTED_ALGORITHM
+    assert e.value.code == CtapError.ERR.CBOR_UNEXPECTED_TYPE
 
 def test_exclude_list(resetdevice):
     resetdevice.doMC(exclude_list=[{"id": b"1234", "type": "rot13"}])

tests/pico-fido/test_021_authenticate.py

@@ -20,7 +20,7 @@
 
 from fido2.client import CtapError
 from fido2.cose import ES256, ES384, ES512, EdDSA
-from utils import verify
+from utils import verify, ES256K
 import pytest
 
 def test_authenticate(device):
@@ -49,7 +49,7 @@ def test_empty_allowList(device):
     assert e.value.code == CtapError.ERR.NO_CREDENTIALS
 
 @pytest.mark.parametrize(
-    "alg", [ES256.ALGORITHM, ES384.ALGORITHM, ES512.ALGORITHM, EdDSA.ALGORITHM]
+    "alg", [ES256.ALGORITHM, ES384.ALGORITHM, ES512.ALGORITHM, ES256K.ALGORITHM, EdDSA.ALGORITHM]
 )
 def test_algorithms(device, info, alg):
     if ({'alg': alg, 'type': 'public-key'} in info.algorithms):

tests/utils.py

@@ -19,6 +19,11 @@
 
 
 from fido2.webauthn import AttestedCredentialData
+from fido2.cose import CoseKey
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives.asymmetric import ec
+from fido2.utils import bytes2int, int2bytes
+from cryptography.hazmat.backends import default_backend
 import random
 import string
 import secrets
@@ -175,3 +180,29 @@ class Timeout(object):
         if self.timer:
             self.timer.cancel()
             self.timer.join()
+
+class ES256K(CoseKey):
+    ALGORITHM = -47
+    _HASH_ALG = hashes.SHA256()
+
+    def verify(self, message, signature):
+        if self[-1] != 8:
+            raise ValueError("Unsupported elliptic curve")
+        ec.EllipticCurvePublicNumbers(
+            bytes2int(self[-2]), bytes2int(self[-3]), ec.SECP256K1()
+        ).public_key(default_backend()).verify(
+            signature, message, ec.ECDSA(self._HASH_ALG)
+        )
+
+    @classmethod
+    def from_cryptography_key(cls, public_key):
+        pn = public_key.public_numbers()
+        return cls(
+            {
+                1: 2,
+                3: cls.ALGORITHM,
+                -1: 8,
+                -2: int2bytes(pn.x, 32),
+                -3: int2bytes(pn.y, 32),
+            }
+        )