I propose to move out the FIDO 2 implementation out of RIOT and would like to gather feedback on this. Specifically, this would mean dropping the FIDO 2 implementation from the RIOT repo (both repos and modules) and suggesting the author to maintain an out-of-tree repo with external modules and an app to provide this. This external repo could then be referenced in awesome-riot-os.
I believe the RIOT repo is not the place the FIDO 2 app should reside for the following reasons:
First, in my opinion a FIDO 2 implementation is too specific to be useful for others. Hence, it is not useful for an OS to provide.
Second, the implementation is not particularly secure for a number of reasons:
it has not received thorough reviewing, especially not with a focus on security
the design choice to implement highly security sensitive software in C does not match the state of art
RIOT is not exactly state of the art for highly security sensitive software. RIOT does make compromises when it comes to security to meet other goals (e.g. does not require MPUs, is written in portable rather than memory safe language, does not implement a clear separation between userspace threads and kernel space, …)
It is clearly documented that the FIDO 2 implementation is experimental not ready for use. However, I don’t really see a path for it to leave the “experimental” stage. So the FIDO 2 code is so specific that it can only be used to provide a FIDO 2 stick, but it will probably never reach a state in which it can be trusted to be actually used as FIDO 2 stick.
And finally, if it is in the RIOT repo, it needs to be maintained. I have a lot of sympathy for creating tech demos based on RIOT for fun as I also do this. But having those demos outside of the main RIOT repo allows to pin a specific RIOT release and not cause maintenance overhead in upstream RIOT.
So: If nobody speaks up (lets say until end of this year) against removing the FIDO 2 code from the RIOT repo, I would open a PR that drops it.
Marian via RIOT notifications@riot-os.org wrote:
> I propose to move out the FIDO 2
> implementation out of RIOT
> and would like to gather feedback on this. Specifically, this would
> mean dropping the FIDO 2 implementation from the RIOT repo (both repos
> and modules) and suggesting the author to maintain an out-of-tree repo
This sounds like a good plan for the reasons you gave.
Will the CI system be able to continue to build this “example”?
First of all, I find it a little hurtful that I was not concacted directly and asked for my opinion on this. I implemented the FIDO2 port for RIOT and I am still actively contributing changes to it. So it is not like I disappeared.
I am very open to having the FIDO2 implementation be thoroughly reviewed and would gladly implement any changes / discuss approaches to improve the security. Furthermore, I was also planning on adding support for the PSA crypto API to encrypt all FIDO2 credentials stored in flash memory with a key stored inside a secure element.
Yes the FIDO2 implementation is very specific, but it is also a very nice application to test and improve other parts of RIOT such as the Flash API, the PSA crypto API or USB HID. I personally contributed / fixed bugs in these subsystems while working on FIDO2 and I am sure there are more places where possible improvements to RIOT will become apparent while further working on FIDO2.
First of all, I find it a little hurtful that I was not concacted directly and asked for my opinion on this. I implemented the FIDO2 port for RIOT and I am still actively contributing changes to it. So it is not like I disappeared.
sorry for hurting you feelings. I can assure you, that this was not the intention. I am aware that working hard on code and not seeing it merged upstream or removed from upstream is does not feel exactly great.
I think we need to have the discussion on what we merge into and maintain within the RIOT repository publicly and with the whole community. This is why I reached out to the whole community, rather than to just you.
I am very open to having the FIDO2 implementation be thoroughly reviewed
That is great. But who will do the reviews? We have very limited reviewing capacity and I’d argue that this capacity better spend elsewhere: Other code is less specific and more widely useful. And for the given use case of FIDO2, the review needs to be extremely thorough.
Yes the FIDO2 implementation is very specific, but it is also a very nice application to test and improve other parts of RIOT such as the Flash API, the PSA crypto API or USB HID. I personally contributed / fixed bugs in these subsystems while working on FIDO2 and I am sure there are more places where possible improvements to RIOT will become apparent while further working on FIDO2.
Thanks for that.
Note that contributing fixes to RIOT does not require applications to be upstream, though. In fact, we get a lot of fixes upstream for bugs that should up in downstream apps.
Downstream apps will not be build by Murdock, so the answer is no.
This can be somewhat mitigated by setting up a CI pipeline in the downstream repository. Given that downstream repos will contain fewer apps (typically one or two), the load would also be lot less and a trivial CI setup would be sufficient.
But of course, changes that break downstream apps would get merged upstream without noticing in upstream.
Marian via RIOT notifications@riot-os.org wrote:
> But of course, changes that break downstream apps would get merged
> upstream without noticing in upstream.
What I care about is that the quality of the examples remain high, and that
means that they don’t break, or if they do, that we know that they do, and we
can mark them as broken.
What if we had RIOT-examples, which contained many examples, but which was
also regularly CI tested? Perhaps it might be checked out as a submodule, or
maybe RIOT-OS would be a submodule of it.
We once had GitHub - RIOT-OS/applications: Some useful RIOT applications - I guess this could be revived. We could try to revive this and this time make it part of the release specs, so that the RIOT submodule get’s indeed bumped to the new release whenever we do a release and the broken examples do get fixed.
. But in all seriousness, I am not sure how it will be different this time…