Fido2 follow up continued
Contribution description
This PR adds changes to make the FIDO2 API usable without a transport layer as well as improve the overall usability. With this come changes specific to the native target, to account for the fact that mtd flash handling is file backed in this case.
This also includes breaking changes to the public API:
- All public methods defined in "sys/include/fido2/ctap.h" now return a
ctap_status_code_tinstead of the size of the response. - The size of the FIDO2 response is now contained within the response structure
ctap_resp_t.
Users of the FIDO2 API need to adjust their applications to now expect a status
code as the return value of functions such as fido2_ctap_handle_request and
expect the length of the response in resp->len.
To test the usability of FIDO2 without a transport layer, this PR also adds another test module which tests the CTAP implementation without transport layer.
With the new test module being added, the old tests are renamed to
sys_fido2_ctap_hid to highlight the fact that they test the CTAP2 implementation
using CTAPHID as transport binding.
Furthermore, this PR removes the dependency of ctap_hid for ztimer64 as
having 64 bit timestamps is not required.
Testing procedure
tests/sys_fido2_ctaptests/sys_fido2_ctap_hid
Issues/PRs references
Depends on PR #18637 Issue regarding file backed flash memory on native: #19559
With #18637 having been merged a while a go, I would like to get this in as well. Rebased on latest master.
@chrysn Can we get this in ?
link to #19559 native is using file backed mtd
i think one of the test application is "copied" ( old copy should be deleted) to the new path structure one is not moved yet.
I am not sure how many users of this module are out there, this changes a lot of api from return the size of data to return an error code (misinterpretation might not be nice).
i think one of the test application is "copied" ( old copy should be deleted) to the new path structure one is not moved yet.
Ah yes, I missed the directory structure changes. Will fix
I am not sure how many users of this module are out there, this changes a lot of api from return the size of data to return an error code (misinterpretation might not be nice).
Currently I don't think very many. I personally don't know of anybody. One reason for this was (at least the last time I checked) that many applications lacked FIDO2 support and only support FIDO U2F, which is not implemented in RIOT currently. I am planning to put some more work into the FIDO2 implementation in order to get a fully working RIOT security key at some point :)
Either way, I don't think the API change will affect many people.
please add a "cleanup" commit that fixes longlines ( >100 chars) and consecutive empty lines if possible
please add a "cleanup" commit that fixes longlines ( >100 chars) and consecutive empty lines if possible
Fixed for every location that is not a structure documentation
@Ollrogge @chrysn
I just looked at the code the changes seem sane and build tests are happy. As @Ollrogge is the main consumer of this code and he is the original author I would just accept it if nothing comes up within 2 weeks, please remind me.
@kfessel its been 2 weeks :)
:heavy_check_mark: all files are reviewed :heavy_check_mark: no opposition in 2 weeks
please squash your commits ( to a level of your liking)
✔️ all files are reviewed ✔️ no opposition in 2 weeks
please squash your commits ( to a level of your liking)
Done
Murdock results
:heavy_check_mark: PASSED
db95ef6f104dcc6a87ba0c7d040e8cd6391708fd sys/fido2: Small fix to dependencies && cleanup
| Success | Failures | Total | Runtime |
|---|---|---|---|
| 10249 | 0 | 10249 | 19m:27s |
Artifacts
if fixup chages are related to a previous commit -- please prepare them to be integrated with these commits (aka fixup)
tests/sys/fido2_ctap/ > BOARD=native64 make failed
https://ci.riot-os.org/results/da5dcdb3166b445dbec7480ea226323c/output/builds/tests/sys/fido2_ctap/native64:gnu.txt
This is better than before and successfully went through CI
I assume the author tested it.
Done. Ran all tests and everything still works as expected :)
@kfessel thanks for reviewing !