From a1a75e4ab57aae6bb2e6291c8a7429eaeb9dce1f Mon Sep 17 00:00:00 2001
From: Conor Patrick <conorpp94@gmail.com>
Date: Thu, 21 Mar 2019 12:47:15 -0400
Subject: [PATCH] check errors

---
 fido2/ctap.c       |  2 +-
 fido2/ctap_parse.c |  4 +--
 tools/ctap_test.py | 82 +++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/fido2/ctap.c b/fido2/ctap.c
index fb47271..bebc9ab 100644
--- a/fido2/ctap.c
+++ b/fido2/ctap.c
@@ -354,7 +354,7 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf
         else
         {
             printf1(TAG_CTAP, "saltAuth is invalid\r\n");
-            return CTAP1_ERR_OTHER;
+            return CTAP2_ERR_EXTENSION_FIRST;
         }
 
         // Generate credRandom
diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c
index fbe3fc0..64cd2f6 100644
--- a/fido2/ctap_parse.c
+++ b/fido2/ctap_parse.c
@@ -601,11 +601,11 @@ uint8_t ctap_parse_hmac_secret(CborValue * val, CTAP_hmac_secret * hs)
             case EXT_HMAC_SECRET_SALT_ENC:
                 salt_len = 64;
                 ret = cbor_value_copy_byte_string(&map, hs->saltEnc, &salt_len, NULL);
-                check_ret(ret);
-                if (salt_len != 32 && salt_len != 64)
+                if ((salt_len != 32 && salt_len != 64) || ret == CborErrorOutOfMemory)
                 {
                     return CTAP1_ERR_INVALID_LENGTH;
                 }
+                check_ret(ret);
                 hs->saltLen = salt_len;
                 parsed_count++;
             break;
diff --git a/tools/ctap_test.py b/tools/ctap_test.py
index 4ded73e..984879a 100755
--- a/tools/ctap_test.py
+++ b/tools/ctap_test.py
@@ -786,6 +786,7 @@ class Tester:
 
         salt1 = b"\x5a" * 32
         salt2 = b"\x96" * 32
+        salt3 = b"\x03" * 32
 
         # self.testReset()
 
@@ -808,6 +809,16 @@ class Tester:
             assert "hmac-secret" in reg.auth_data.extensions
             assert reg.auth_data.extensions["hmac-secret"] == True
 
+        reg = self.testMC(
+            "Send MC with fake extension set to true, expect SUCCESS",
+            cdh,
+            rp,
+            user,
+            key_params,
+            expectedError=CtapError.ERR.SUCCESS,
+            other={"extensions": {"tetris": True}},
+        )
+
         with Test("Get shared secret"):
             key_agreement, shared_secret = (
                 self.client.pin_protocol._init_shared_secret()
@@ -818,14 +829,18 @@ class Tester:
                 default_backend(),
             )
 
-        for salt_list in ((salt1,), (salt1, salt2)):
+        def get_salt_params(salts):
             enc = cipher.encryptor()
             salt_enc = b""
-            for salt in salt_list:
+            for salt in salts:
                 salt_enc += enc.update(salt)
             salt_enc += enc.finalize()
 
             salt_auth = hmac_sha256(shared_secret, salt_enc)[:16]
+            return salt_enc, salt_auth
+
+        for salt_list in ((salt1,), (salt1, salt2)):
+            salt_enc, salt_auth = get_salt_params(salt_list)
 
             auth = self.testGA(
                 "Send GA request with %d salts hmac-secret, expect success"
@@ -855,12 +870,71 @@ class Tester:
                 dec = cipher.decryptor()
                 key = dec.update(ext["hmac-secret"]) + dec.finalize()
 
+                print(shannon_entropy(ext["hmac-secret"]))
                 if len(salt_list) == 1:
                     assert shannon_entropy(ext["hmac-secret"]) > 4.6
                     assert shannon_entropy(key) > 4.6
                 if len(salt_list) == 2:
-                    assert shannon_entropy(ext["hmac-secret"]) > 5.6
-                    assert shannon_entropy(key) > 5.6
+                    assert shannon_entropy(ext["hmac-secret"]) > 5.4
+                    assert shannon_entropy(key) > 5.4
+
+        salt_enc, salt_auth = get_salt_params((salt3,))
+
+        auth = self.testGA(
+            "Send GA request with hmac-secret missing keyAgreement, expect error",
+            rp["id"],
+            cdh,
+            other={"extensions": {"hmac-secret": {2: salt_enc, 3: salt_auth}}},
+        )
+        auth = self.testGA(
+            "Send GA request with hmac-secret missing saltAuth, expect MISSING_PARAMETER",
+            rp["id"],
+            cdh,
+            other={"extensions": {"hmac-secret": {1: key_agreement, 2: salt_enc}}},
+            expectedError=CtapError.ERR.MISSING_PARAMETER,
+        )
+        auth = self.testGA(
+            "Send GA request with hmac-secret missing saltEnc, expect MISSING_PARAMETER",
+            rp["id"],
+            cdh,
+            other={"extensions": {"hmac-secret": {1: key_agreement, 3: salt_auth}}},
+            expectedError=CtapError.ERR.MISSING_PARAMETER,
+        )
+
+        bad_auth = list(salt_auth[:])
+        bad_auth[len(bad_auth) // 2] = bad_auth[len(bad_auth) // 2] ^ 1
+        bad_auth = bytes(bad_auth)
+
+        auth = self.testGA(
+            "Send GA request with hmac-secret containing bad saltAuth, expect EXTENSION_FIRST",
+            rp["id"],
+            cdh,
+            other={
+                "extensions": {
+                    "hmac-secret": {1: key_agreement, 2: salt_enc, 3: bad_auth}
+                }
+            },
+            expectedError=CtapError.ERR.EXTENSION_FIRST,
+        )
+
+        salt4 = b"\x5a" * 16
+        salt5 = b"\x96" * 64
+        for salt_list in ((salt4,), (salt4, salt5)):
+            salt_enc, salt_auth = get_salt_params(salt_list)
+
+            salt_auth = hmac_sha256(shared_secret, salt_enc)[:16]
+            auth = self.testGA(
+                "Send GA request with incorrect salt length %d, expect INVALID_LENGTH"
+                % len(salt_enc),
+                rp["id"],
+                cdh,
+                other={
+                    "extensions": {
+                        "hmac-secret": {1: key_agreement, 2: salt_enc, 3: salt_auth}
+                    }
+                },
+                expectedError=CtapError.ERR.INVALID_LENGTH,
+            )
 
     def test_fido2_other(self,):