From 445c1fe93f6d0edbd1c59f318703b070c8ee445f Mon Sep 17 00:00:00 2001 From: Ollrogge Date: Tue, 7 Sep 2021 19:12:31 +0200 Subject: [PATCH] Adaptions for RIOT FIDO2 CTAP --- Makefile | 15 ++++++------ tests/conftest.py | 2 +- tests/standard/fido2/pin/test_pin.py | 24 ++++++++++++++++--- tests/standard/fido2/test_reset.py | 5 ++++ tests/standard/fido2/test_resident_key.py | 4 ++-- .../fido2/user_presence/test_user_presence.py | 10 +++++++- tests/standard/transport/test_hid.py | 11 +++++++++ 7 files changed, 57 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 85aa451..c101826 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: standard-tests vendor-tests +.PHONY: standard-tests PY_VERSION=$(shell python -c "import sys; print('%d.%d'% sys.version_info[0:2])") VALID=$(shell python -c "print($(PY_VERSION) >= 3.6)") @@ -16,24 +16,21 @@ else endif standard-tests: venv - $(BIN)/pytest tests/standard + $(BIN)/pytest --ignore=tests/standard/fido2v1 --ignore=tests/standard/fido2/extensions --ignore=tests/standard/u2f --ignore tests/standard/fido2/user_presence -k "not test_ctap1_interop and not test_rk_maximum_list_capacity_per_rp_nodisplay and not test_keep_alive" tests/standard/ -s -vendor-tests: venv - $(BIN)/pytest tests/vendor +up-tests: venv + $(BIN)/pytest tests/standard/fido2/user_presence -s # setup development environment venv: $(PYTHON) -m venv venv $(BIN)/python -m pip install -U pip $(BIN)/pip install -U -r requirements.txt - $(BIN)/pip install -U -r dev-requirements.txt - $(BIN)/pre-commit install # re-run if dependencies change update: $(BIN)/python -m pip install -U pip $(BIN)/pip install -U -r requirements.txt - $(BIN)/pip install -U -r dev-requirements.txt # ensure this passes before commiting check: @@ -48,3 +45,7 @@ black: isort: $(BIN)/isort -y --recursive tests/ + +clean: + rm -r venv + diff --git a/tests/conftest.py b/tests/conftest.py index 761a684..d13d6dc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -175,7 +175,7 @@ class MoreRobustPcscDevice(CtapPcscDevice): except CtapError: if self._capabilities == 0: raise ValueError("Unsupported device") - + def apdu_exchange(self, apdu, protocol = None): try: return super().apdu_exchange(apdu,protocol) diff --git a/tests/standard/fido2/pin/test_pin.py b/tests/standard/fido2/pin/test_pin.py index 78b09e3..f5ee4e4 100644 --- a/tests/standard/fido2/pin/test_pin.py +++ b/tests/standard/fido2/pin/test_pin.py @@ -60,7 +60,11 @@ class TestPin(object): with pytest.raises(CtapError) as e: device.client.pin_protocol.set_pin('1234') - assert e.value.code == CtapError.ERR.NOT_ALLOWED + ''' + CTAP spec states: "If a PIN has already been set, authenticator + returns CTAP2_ERR_PIN_AUTH_INVALID error." + ''' + assert e.value.code == CtapError.ERR.PIN_AUTH_INVALID def test_get_key_agreement_fields(self, CPRes): @@ -99,11 +103,25 @@ class TestPin(object): def test_zero_length_pin_auth(self, device, SetPinRes): with pytest.raises(CtapError) as e: reg = device.sendMC(*FidoRequest(SetPinRes, pin_auth=b"").toMC()) - assert e.value.code == CtapError.ERR.PIN_AUTH_INVALID + + ''' + CTAP spec states: If platform sends zero length pinAuth, authenticator + needs to wait for user touch and then returns either + CTAP2_ERR_PIN_NOT_SET if pin is not set or CTAP2_ERR_PIN_INVALID + if pin has been set. [...]" + ''' + assert e.value.code == CtapError.ERR.PIN_INVALID with pytest.raises(CtapError) as e: reg = device.sendGA(*FidoRequest(SetPinRes, pin_auth=b"").toGA()) - assert e.value.code == CtapError.ERR.PIN_AUTH_INVALID + + ''' + CTAP spec states: If platform sends zero length pinAuth, authenticator + needs to wait for user touch and then returns either + CTAP2_ERR_PIN_NOT_SET if pin is not set or CTAP2_ERR_PIN_INVALID + if pin has been set. + ''' + assert e.value.code == CtapError.ERR.PIN_INVALID def test_make_credential_no_pin(self, device, SetPinRes): with pytest.raises(CtapError) as e: diff --git a/tests/standard/fido2/test_reset.py b/tests/standard/fido2/test_reset.py index 508d755..adb2818 100644 --- a/tests/standard/fido2/test_reset.py +++ b/tests/standard/fido2/test_reset.py @@ -9,9 +9,14 @@ import tests def test_reset(device): device.reset() +''' +Not mentioned in any spec. +''' +''' def test_reset_window(device): print("Waiting 11s before sending reset...") time.sleep(11) with pytest.raises(CtapError) as e: device.ctap2.reset(on_keepalive=DeviceSelectCredential(1)) assert e.value.code == CtapError.ERR.NOT_ALLOWED +''' \ No newline at end of file diff --git a/tests/standard/fido2/test_resident_key.py b/tests/standard/fido2/test_resident_key.py index 2c5bece..32fe534 100644 --- a/tests/standard/fido2/test_resident_key.py +++ b/tests/standard/fido2/test_resident_key.py @@ -45,7 +45,7 @@ class TestResidentKeyPersistance(object): @pytest.mark.parametrize("do_reboot", [False, True]) def test_user_info_returned_when_using_allowlist(self, device, MC_RK_Res, GA_RK_Res, do_reboot): assert "id" in GA_RK_Res.user.keys() - + allow_list = [ { "id": MC_RK_Res.auth_data.credential_data.credential_id[:], @@ -66,7 +66,7 @@ class TestResidentKeyPersistance(object): class TestResidentKeyAfterReset(object): def test_with_allow_list_after_reset(self, device, MC_RK_Res, GA_RK_Res): assert "id" in GA_RK_Res.user.keys() - + allow_list = [ { "id": MC_RK_Res.auth_data.credential_data.credential_id[:], diff --git a/tests/standard/fido2/user_presence/test_user_presence.py b/tests/standard/fido2/user_presence/test_user_presence.py index c9904b2..0b74d24 100644 --- a/tests/standard/fido2/user_presence/test_user_presence.py +++ b/tests/standard/fido2/user_presence/test_user_presence.py @@ -34,7 +34,10 @@ class TestUserPresence(object): device.sendGA( *FidoRequest(GARes, timeout=event, on_keepalive=None).toGA() ) - assert e.value.code == CtapError.ERR.KEEPALIVE_CANCEL + ''' + The CTAP states that if no UP has been activated, CTAP2_ERR_OPERATION_DENIED should be returned. + ''' + assert e.value.code == CtapError.ERR.OPERATION_DENIED @pytest.mark.skipif( not "trezor" in sys.argv, reason="Only Trezor supports decline." @@ -71,6 +74,10 @@ class TestUserPresence(object): ) assert e.value.code == CtapError.ERR.INVALID_OPTION + ''' + This test makes no sense since device.sendGA is blocking + ''' + ''' def test_user_presence_permits_only_one_request(self, device, MCRes, GARes): print("ACTIVATE UP ONCE") device.sendGA(*FidoRequest(GARes).toGA()) @@ -81,3 +88,4 @@ class TestUserPresence(object): *FidoRequest(GARes, timeout=event, on_keepalive=None).toGA() ) assert e.value.code == CtapError.ERR.KEEPALIVE_CANCEL + ''' \ No newline at end of file diff --git a/tests/standard/transport/test_hid.py b/tests/standard/transport/test_hid.py index c79c933..6203a00 100644 --- a/tests/standard/transport/test_hid.py +++ b/tests/standard/transport/test_hid.py @@ -105,6 +105,16 @@ class TestHID(object): device.send_raw("\x01") device.send_data(CTAPHID.INIT, "\x11\x22\x33\x44\x55\x66\x77\x88") + ''' + CTAP spec states: If an application tries to access the device from a + different channel while the device is busy with a transaction, that request + will immediately fail with a busy-error message sent to the requesting channel. + + This test tries to send an init from a different cid while the authenticator + is busy with the ping. In my understanding, based on the sentence above, + this should throw an error. Therefore the test does not make sense. + ''' + ''' def test_ping_abort_from_different_cid(self, device, check_timeouts=False): oldcid = device.cid() newcid = "\x11\x22\x33\x44" @@ -123,6 +133,7 @@ class TestHID(object): # print('wait for timeout') cmd, r = device.recv_raw() # timeout response assert cmd == 0xBF + ''' def test_timeout(self, device): device.send_data(CTAPHID.INIT, "\x11\x22\x33\x44\x55\x66\x77\x88") -- 2.33.0