Skip to content

Commit

Permalink
FIDO2 support in RIOT
Browse files Browse the repository at this point in the history
  • Loading branch information
Ollrogge committed Sep 8, 2021
1 parent 8ca1520 commit e127a4d
Show file tree
Hide file tree
Showing 33 changed files with 7,893 additions and 1 deletion.
1 change: 1 addition & 0 deletions makefiles/pseudomodules.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions pkg/fido2_tests/Makefile
Original file line number Diff line number Diff line change
@@ -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:
@
226 changes: 226 additions & 0 deletions pkg/fido2_tests/patches/0001-Adaptions-for-RIOT-FIDO2-CTAP.patch
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions sys/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
31 changes: 31 additions & 0 deletions sys/Makefile.dep
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions sys/fido2/Kconfig
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 5 additions & 0 deletions sys/fido2/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
ifneq (,$(filter fido2_ctap%,$(USEMODULE)))
DIRS += ctap
endif

include $(RIOTBASE)/Makefile.base
96 changes: 96 additions & 0 deletions sys/fido2/ctap/Kconfig
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit e127a4d

Please sign in to comment.