Skip to content

Fido2 follow up#18637

Merged
bors[bot] merged 2 commits intoRIOT-OS:masterfrom
Ollrogge:fido2_follow_up
Dec 19, 2022
Merged

Fido2 follow up#18637
bors[bot] merged 2 commits intoRIOT-OS:masterfrom
Ollrogge:fido2_follow_up

Conversation

@Ollrogge
Copy link
Copy Markdown
Member

@Ollrogge Ollrogge commented Sep 23, 2022

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

@github-actions github-actions bot added the Area: sys Area: System label Sep 23, 2022
@Ollrogge Ollrogge mentioned this pull request Sep 28, 2022
Merged
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Oct 25, 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 

========================================================= 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
Copy Markdown
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
Copy Markdown
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 !

benpicco
benpicco approved these changes Dec 11, 2022
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor

benpicco commented Dec 19, 2022

bors merge

bors bot added a commit that referenced this pull request Dec 19, 2022
@bors @jia200x
@Ollrogge @aabadie @kaspar030
Merge #18472 #18637 #19056 #19063
d88a1e5 
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 <[email protected]>
Co-authored-by: Ollrogge <[email protected]>
Co-authored-by: Alexandre Abadie <[email protected]>
Co-authored-by: Kaspar Schleiser <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Dec 19, 2022

Build failed (retrying...):

bors bot added a commit that referenced this pull request Dec 19, 2022
@bors @jia200x @Ollrogge
Merge #18472 #18637
f3cfb5f 
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 <[email protected]>
Co-authored-by: Ollrogge <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Dec 19, 2022

Build failed (retrying...):

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Dec 19, 2022

Build succeeded:

@bors bors bot merged commit ca3b259 into RIOT-OS:master Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@chrysn chrysn Awaiting requested review from chrysn

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Ollrogge @PeterKietzmann @benpicco