Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fido2 follow up #18637

Merged
merged 2 commits into from
Dec 19, 2022
Merged

Fido2 follow up #18637

merged 2 commits into from
Dec 19, 2022

Conversation

Ollrogge
Copy link
Member

Contribution description

This PR simplifies the flash handling code of the FIDO CTAP2 implementation. Specifically it removes the functions fido2_ctap_mem_get_flashpage_number_of_rk and fido2_ctap_mem_get_offset_of_rk_into_flashpage and handles all flash related logic in ctap_mem.c instead of distributed across ctap.c and ctap_mem.c

Testing procedure

  • tests/sys_fido2_ctap

@PeterKietzmann
Copy link
Member

I did not review the code changes but provide testing, executed on the nrf52840dk:

  • Created two accounts on webauthn.io, power off, power on and successfully authenticate both accounts.
  • make fido2-test is successful
results

========================================================= test session starts ==========================================================
platform linux -- Python 3.8.10, pytest-7.2.0, pluggy-1.0.0
rootdir: /RIOT/build/pkg/fido2_tests
plugins: rerunfailures-10.2, ordering-0.6, timeout-2.1.0
collected 140 items / 7 deselected / 133 selected                                                                                      

tests/standard/connect_test.py --- HID ---
[CtapHidDevice(/dev/hidraw0)]
.
tests/standard/fido2/test_get_assertion.py Resetting Authenticator...
.......................Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw1)]
.
tests/standard/fido2/test_getinfo.py ...........
tests/standard/fido2/test_make_credential.py Resetting Authenticator...
.................................MC [b'123456789abcdef0123456789abcdef0', {'id': 'example.org', 'name': 'ExampleRP'}, {'id': b'\xb5LH@\xed\xa60', 'name': 'Neely Sheeree Renee', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Neely Sheeree Renee'}, [{'type': 'public-key', 'alg': -7}], None, None, {'unknown': False}, None, None, None, <tests.vendor.solo.utils.DeviceSelectCredential object at 0x7f9ec94a5f70>]
.ed25519 is not supported.  Skip this test.
.
tests/standard/fido2/test_reset.py Resetting Authenticator...
.
tests/standard/fido2/test_reset_credential.py Resetting Authenticator...
Resetting Authenticator...
.
tests/standard/fido2/test_resident_key.py Resetting Authenticator...
.Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw2)]
.Resetting Authenticator...
.Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw3)]
.Resetting Authenticator...
Resetting Authenticator...
.Resetting Authenticator...
Resetting Authenticator...
.Resetting Authenticator...
...
            {'id': b'W\xcc\xbe\x96w2\xf6\x85\xa3\xb5L\xd3\xabW\xec@\x19\xa7!\x7f\xf7\xe0Ul\x9b\xc4_l\r>H\xe6O\x1b\x9a\xec\x04v\x90r\xcb\xffP\xf7\x18=\xfe?', 'name': 'Christyna Larina Thekla', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Christyna Larina Thekla'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.4457510366298426.com', 'name': 'Example'}
            

            {'id': b'\xb2\xe67\x8bfA J\x05T\xc4j\xda\x9aA\xdc\xc1\xdfOB\xa1ns\xaeO(\xd6\xdc~\xb6\xc5\x0c\xa9@\xcc\xe1j+\x93\x0b\x05k\xbc\xabtE\xb4\xa5\x0c\x1c\xfaL\\\xc2\x1c\xe7+\x90g', 'name': 'Maurise Rosabelle Nisse', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Maurise Rosabelle Nisse'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.4457510366298426.com', 'name': 'Example'}
            

            {'id': b'\xb7\xc7\xd7N\xe1z/\xf7\x80TGZ5/\xd2\x83\xdd\x1a\xe1K\xc9I ', 'name': 'Gretal Emlynne Philis', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Gretal Emlynne Philis'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.4457510366298426.com', 'name': 'Example'}
            
.s.sResetting Authenticator...
...
            {'id': b'\xb3& 3x\xe5\xf2\x07v\xa6\x9b;u*\xec\xd0foV\xda\xec_\x17\x9f#\xd5\xd4}dm\xc0\x1ci', 'name': 'Gaylene Josee Fidelity', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Gaylene Josee Fidelity'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.34758444793531695.com', 'name': 'Example'}
            

            {'id': b'I\xa8\xc9J\xd9\x13i\xa9O|\xe9\xa4\xa3\xa4\xe3\xda\xb1>\xf0\xf6\x1b\xab\xbc\xcf\xcd\xda\xc8\x81\xd0\xb2\xbdP\xd9\x19x]\x07\xd5\\\x9e\xa5\nv|a\xa2x\x0bnJ\xbaG', 'name': 'Meara Lyn Cecelia', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Meara Lyn Cecelia'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.34758444793531695.com', 'name': 'Example'}
            

            {'id': b'\x97\xa1\xbfO\x7fk\xfe\xdb\x114\xe07\xd1\xda\xd2z\xc1', 'name': 'Korella Adaline Kiersten', 'icon': 'https://www.w3.org/TR/webauthn/', 'displayName': 'Displayed Korella Adaline Kiersten'}
            b'123456789abcdef0123456789abcdef0'
            {'id': 'unique-0.34758444793531695.com', 'name': 'Example'}
            
.s.sssResetting Authenticator...
...
tests/standard/fido2/pin/test_lockout.py Resetting Authenticator...
Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw4)]
Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw5)]
Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw6)]
.
tests/standard/fido2/pin/test_pin.py Resetting Authenticator...
........Resetting Authenticator...
.Resetting Authenticator...
Check there is 7 pin attempts left
Check there is 6 pin attempts left
Please reboot authenticator and hit enter

--- HID ---
[CtapHidDevice(/dev/hidraw7)]
.
tests/standard/fido2/pin/test_set_pin.py Resetting Authenticator...
.Resetting Authenticator...
Resetting Authenticator...
....Resetting Authenticator...
Resetting Authenticator...
.
tests/standard/transport/test_hid.py .b'\x11\x11\x11\x11\x11\x11\x11\x11\x02\x00\x00\x00\x02\x00\x00\x00\r'
...Implements CBOR.
.U2F not implemented.
............
tests/standard/transport/test_nfc.py s

======================================= 126 passed, 7 skipped, 7 deselected in 166.66s (0:02:46) =======================================

  • make fido2-test-up does not seem to work, neither on master. All four LEDs just start to blink after running the command. @Ollrogge can you give it a look (probably beyond the scope of this PR)?

@Ollrogge
Copy link
Member Author

Ollrogge commented Nov 1, 2022

* `make fido2-test-up` does not seem to work, neither on master.  All four LEDs just start to blink after running the command. @Ollrogge can you give it a look (probably beyond the scope of this PR)?

The LEDs start blinking due to the reset command at the beginning. You need to press Button1 2 times to reset the authenticator. Afterwards the UP test starts. Tested it on master and on this branch and it still works.

@Ollrogge
Copy link
Member Author

Ollrogge commented Nov 1, 2022

I did not review the code changes but provide testing, executed on the nrf52840dk:

* Created two accounts on _webauthn.io_, power off, power on and successfully authenticate both accounts.

* `make fido2-test` is successful

results

And thanks for testing !

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is contained to fido2, so let's not stall this indefinitely

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 11, 2022
@benpicco
Copy link
Contributor

bors merge

bors bot added a commit that referenced this pull request Dec 19, 2022
18472: drivers/mrf24j40: add support for IEEE 802.15.4 Radio HAL r=benpicco a=jia200x



18637: Fido2 follow up r=benpicco a=Ollrogge



19056: pkg/lvgl: bump to 8.3.4 r=aabadie a=aabadie



19063: CI: no quickbuild on "bors try" r=aabadie a=kaspar030



Co-authored-by: Jose Alamos <jose@alamos.cc>
Co-authored-by: Ollrogge <nils-ollrogge@outlook.de>
Co-authored-by: Alexandre Abadie <alexandre.abadie@inria.fr>
Co-authored-by: Kaspar Schleiser <kaspar@schleiser.de>
@bors
Copy link
Contributor

bors bot commented Dec 19, 2022

Build failed (retrying...):

bors bot added a commit that referenced this pull request Dec 19, 2022
18472: drivers/mrf24j40: add support for IEEE 802.15.4 Radio HAL r=benpicco a=jia200x



18637: Fido2 follow up r=benpicco a=Ollrogge



Co-authored-by: Jose Alamos <jose@alamos.cc>
Co-authored-by: Ollrogge <nils-ollrogge@outlook.de>
@bors
Copy link
Contributor

bors bot commented Dec 19, 2022

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Dec 19, 2022

Build succeeded:

@bors bors bot merged commit ca3b259 into RIOT-OS:master Dec 19, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants