Merge pull request #16489 from Ollrogge/fido2_pr

FIDO2 support in RIOT

makefiles/pseudomodules.inc.mk

@@ -37,6 +37,7 @@ PSEUDOMODULES += evtimer_mbox
 PSEUDOMODULES += evtimer_on_ztimer
 PSEUDOMODULES += fmt_%
 PSEUDOMODULES += gcoap_dtls
+PSEUDOMODULES += fido2_tests
 PSEUDOMODULES += gnrc_dhcpv6_%
 PSEUDOMODULES += gnrc_ipv6_default
 PSEUDOMODULES += gnrc_ipv6_ext_frag_stats

pkg/fido2_tests/Makefile

@@ -0,0 +1,9 @@
+PKG_NAME=fido2_tests
+PKG_URL=https://github.com/solokeys/fido2-tests
+PKG_VERSION=3f7893d8d1a39b009cddad7913d3808ca664d3b7
+PKG_LICENSE=Apache-2.0 OR MIT
+
+include $(RIOTBASE)/pkg/pkg.mk
+
+all:
+	@

pkg/fido2_tests/patches/0001-Adaptions-for-RIOT-FIDO2-CTAP.patch

@@ -0,0 +1,226 @@
+From 445c1fe93f6d0edbd1c59f318703b070c8ee445f Mon Sep 17 00:00:00 2001
+From: Ollrogge <nils-ollrogge@outlook.de>
+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
+

sys/Kconfig

@@ -24,6 +24,7 @@ rsource "embunit/Kconfig"
 rsource "entropy_source/Kconfig"
 rsource "eepreg/Kconfig"
 rsource "event/Kconfig"
+rsource "fido2/Kconfig"
 rsource "fmt/Kconfig"
 rsource "frac/Kconfig"
 rsource "hashes/Kconfig"

sys/Makefile.dep

@@ -779,4 +779,35 @@ ifneq (,$(filter dbgpin,$(USEMODULE)))
   FEATURES_REQUIRED += dbgpin
 endif
 
+ifneq (,$(filter fido2_ctap_%,$(USEMODULE)))
+  USEMODULE += fido2_ctap_transport
+  USEMODULE += fido2_ctap
+  ifneq (,$(filter fido2_ctap_transport_hid,$(USEMODULE)))
+      USEMODULE += usbus_hid
+      DISABLE_MODULE += auto_init_usbus
+  endif
+endif
+
+ifneq (,$(filter fido2_ctap,$(USEMODULE)))
+  FEATURES_REQUIRED += periph_flashpage
+  FEATURES_REQUIRED += periph_gpio_irq
+
+  USEPKG += tiny-asn1
+  USEPKG += tinycbor
+  USEPKG += micro-ecc
+
+  INCLUDE += $(RIOTPKG)/tinycbor
+
+  USEMODULE += mtd_flashpage
+  USEMODULE += mtd_write_page
+  USEMODULE += ztimer_msec
+  USEMODULE += event
+  USEMODULE += event_timeout
+  USEMODULE += prng_sha256prng
+  USEMODULE += cipher_modes
+  USEMODULE += crypto_aes_256
+  USEMODULE += hashes
+  USEMODULE += fido2
+endif
+
 include $(RIOTBASE)/sys/test_utils/Makefile.dep

sys/fido2/Kconfig

@@ -0,0 +1,7 @@
+# Copyright (C) 2021 Freie Universität Berlin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+rsource "ctap/Kconfig"

sys/fido2/Makefile

@@ -0,0 +1,5 @@
+ifneq (,$(filter fido2_ctap%,$(USEMODULE)))
+  DIRS += ctap
+endif
+
+include $(RIOTBASE)/Makefile.base

sys/fido2/ctap/Kconfig

@@ -0,0 +1,96 @@
+# Copyright (C) 2021 Freie Universität Berlin
+#
+# This file is subject to the terms and conditions of the GNU Lesser
+# General Public License v2.1. See the file LICENSE in the top level
+# directory for more details.
+
+menuconfig KCONFIG_USEMODULE_FIDO2_CTAP
+    bool "FIDO2 CTAP"
+    depends on USEMODULE_FIDO2_CTAP
+    help
+        Configure a FIDO2 CTAP authenticator via KConfig.
+
+if KCONFIG_USEMODULE_FIDO2_CTAP
+
+config FIDO2_CTAP_STACK_SIZE
+    int "CTAP thread stack size"
+    default 15000
+
+config FIDO2_CTAP_DEVICE_AAGUID
+    string "AAGUID of the CTAP authenticator"
+    default "9c295865fa2c36b705a42320af9c8f16"
+    help
+        The AAGUID is identifying the type of the authenticator (e.g manufacturer
+        and model). The AAGUID needs to be 128 bits long. The default value here
+        is a fallback value that was randomly generated.
+
+config FIDO2_CTAP_DISABLE_UP
+    bool "Disable user presence tests"
+    help
+        When set, the authenticator will not ask for permission before creating
+        a new credential pair or authenticating.
+
+config FIDO2_CTAP_DISABLE_LED
+    bool "Disable LED animations"
+    help
+        When set, the authenticator will not use LED's.
+
+config FIDO2_CTAP_UP_TIMEOUT
+    int "Seconds until user presence test times out"
+    default 15
+
+config FIDO2_CTAP_UP_BUTTON_PORT
+    int "Port of user presence button"
+    depends on !FIDO2_CTAP_DISABLE_UP
+    default -1
+
+config FIDO2_CTAP_UP_BUTTON_PIN
+    int "Pin of user presence button"
+    depends on !FIDO2_CTAP_DISABLE_UP
+    default -1
+
+choice
+    bool "User presence button mode"
+    depends on !FIDO2_CTAP_DISABLE_UP
+    default FIDO2_CTAP_UP_BUTTON_MODE_IN_PU
+
+config FIDO2_CTAP_UP_BUTTON_MODE_IN_PU
+    bool "GPIO_IN_PU"
+    help
+        Configure as input with pull-up resistor
+
+config FIDO2_CTAP_UP_BUTTON_MODE_IN_PD
+    bool "GPIO_IN_PD"
+    help
+        Configure as input with pull-down resistor
+
+config FIDO2_CTAP_UP_BUTTON_MODE_IN
+    bool "GPIO_IN"
+    help
+        Configure as input without pull resistor
+
+endchoice
+
+choice
+    bool "User presence button pin flank"
+    depends on !FIDO2_CTAP_DISABLE_UP
+    default FIDO2_CTAP_UP_BUTTON_FLANK_FALLING
+
+config FIDO2_CTAP_UP_BUTTON_FLANK_FALLING
+    bool "GPIO_FALLING"
+
+config FIDO2_CTAP_UP_BUTTON_FLANK_RISING
+    bool "GPIO_RISING"
+
+endchoice
+
+config FIDO2_CTAP_FLASH_START_PAGE
+    int "First flash page to store data in"
+    default -1
+    help
+        Configuring this incorrectly can lead to firmware corruption so make sure
+        the flash page is located after the firmware.
+
+rsource "transport/Kconfig"
+
+endif # KCONFIG_USEMODULE_FIDO2_CTAP