zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.88k stars 6.63k forks source link

RFC: Replace TinyCBOR with ZCBOR within Zephyr #40591

Closed de-nordic closed 2 years ago

de-nordic commented 2 years ago

Problem description

Currently Zephyr uses specific variant of TinyCBOR (https://github.com/zephyrproject-rtos/tinycbor), that has been imported to support mcumgr library from internals of Mynewt (https://github.com/apache/mynewt-core/tree/master/encoding/tinycbor); the Zephyr/mynewt variant is not fully API compatible with the original implementation that comes from Intel (https://github.com/intel/tinycbor). In short; the TinyCBOR module we have has been made internal source code of Mynewt then extracted as module/library to Zephyr. The mcuboot (and tfm) have already moved to ZCBOR (https://github.com/NordicSemiconductor/zcbor/) , for SMP protocol implementation) and currently the only in-tree usage of TinyCBOR is mcumgr, so also SMP implementation. This means that we already, considering the mcuboot to be Zephyr application, have two CBOR libraries/implementations.

The zcbor is lighter in code than the TinyCBOR and quite API compatible with the latter. Also the zcbor allows to use CDDL to describe frames and auto generate code for CBOR stream parsing.

Replacement would also mean removal of the TinyCBOR as a module and inclusion of zcbor in its place.

An implementation of OSCORE+EDHOC is available for Zephyr using zcbor.

Concerns and Unresolved Questions

How to deal with 3'rd party modules that use different CBOR implementations?

Reference materials

Zephyr TinyCBOR fork: https://github.com/zephyrproject-rtos/tinycbor Zephyr TinyCBOR fork origins: https://github.com/apache/mynewt-core/tree/master/encoding/tinycbor Intel TinyCBOR (the real one): https://github.com/intel/tinycbor CDDL: https://github.com/NordicSemiconductor/cddl-gen CDDL is now zcbor: https://github.com/NordicSemiconductor/zcbor/ QCBOR: https://github.com/laurencelundblade/QCBOR

Other CBOR libarary introduction requests:

QCBOR (as a module) https://github.com/zephyrproject-rtos/zephyr/pull/40297

carlescufi commented 2 years ago

Note: TF-M is using QCBOR: https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/lib/ext/qcbor Note: MCUBoot uses zcbor

microbuilder commented 2 years ago

Something we'd been working with internally at Linaro is getting this up to date to work with MbedTLS 3.0, and enable COSE support for SIGN and ENCRYPT on top of CBOR: https://github.com/lindemer/cozy

But after going down that route, I think we've going to try to use QCBOR on the Zephyr side as well, plus https://github.com/laurencelundblade/t_cose for the COSE signature validation on top of QCBOR.

Given the important of TF-M to Nordic, and to other members like Linaro, it might make more sense to be investing in a common library on both sides of the S/NS divide, and in t_cose on top of that for COSE functionality?

microbuilder commented 2 years ago

@d3zd3z Curious if you have some feedback on the effort required to migrate MCUBoot to QCBOR in the future to align MCUBoot/TF-M/Zephyr on a common library? Was that ruled out as an option in the past for a specific reason?

rerickson1 commented 2 years ago

@lairdjm

carlescufi commented 2 years ago

@d3zd3z Curious if you have some feedback on the effort required to migrate MCUBoot to QCBOR in the future to align MCUBoot/TF-M/Zephyr on a common library? Was that ruled out as an option in the past for a specific reason?

The original migration from TinyCBOR to zcbor was done by @oyvindronningstad FYI, so it might be better to ask him. Also, MCUBoot AFAIK uses the advanced CDDL codegen features from zcbor, something that QCBOR doesn't support.

microbuilder commented 2 years ago

Is there a COSE option for zcbor? It seems like something worth keeping in mind if you care about signing or encrypting CBOR payloads, or decoding them inside Zephyr.

de-nordic commented 2 years ago

The original migration from TinyCBOR to zcbor was done by @oyvindronningstad FYI, so it might be better to ask him. Also, MCUBoot AFAIK uses the advanced CDDL codegen features from zcbor, something that QCBOR doesn't support.

Yes, the MCUBOOT uses CDDL to generate CBOR decoder for the requests. This is also something that would be useful for the MCUMGR where cborattr provides similar feature, where the decoding is done basing on request description done with specific C structures that describe expected structure and how to handle fields.

de-nordic commented 2 years ago

Is there a COSE option for zcbor? It seems like something worth keeping in mind if you care about signing or encrypting CBOR payloads, or decoding them inside Zephyr.

As far as I can tell the COSE supports NanoCBOR only. (oh no... another CBOR)

microbuilder commented 2 years ago

Is there a COSE option for zcbor? It seems like something worth keeping in mind if you care about signing or encrypting CBOR payloads, or decoding them inside Zephyr.

As far as I can tell the COSE supports NanoCBOR only. (oh no... another CBOR)

t_cose is built on top of MbedTLS and QCBOR, and we're looking at extending that with more than SIGN support on the TF-M side, but whatever solution ends up making the most sense for Zephyr should take COSE into consideration.

carlescufi commented 2 years ago

Is there a COSE option for zcbor? It seems like something worth keeping in mind if you care about signing or encrypting CBOR payloads, or decoding them inside Zephyr.

There is an implementation of COSE in the OSCORE library that will be submitted to Zephyr. This is the COSE code that we plan to use for Zephyr instead of t_cose.

microbuilder commented 2 years ago

Is there a COSE option for zcbor? It seems like something worth keeping in mind if you care about signing or encrypting CBOR payloads, or decoding them inside Zephyr.

There is an implementation of COSE in the OSCORE library that will be submitted to Zephyr. This is the COSE code that we plan to use for Zephyr instead of t_cose.

Do you know what it supports? SIGN1, SIGN, ENCRYPT, ENCRYPT0? There are a lot of incomplete COSE libraries out there, and most need some additions to support both SIGN and ENCRYPT, so if we have to add code to more than one implementation it's better to have a clear understanding of that beforehand.

microbuilder commented 2 years ago

It looks like it only support ENCRYPT0 (https://datatracker.ietf.org/doc/html/rfc8613#section-5), meaning a single recipient. That's better than nothing, but limits if you have multiple recipients where you will have to include one shared secret for each recipient.

I don't see any mention of SIGN1 or SIGN, but presumably SIGN1 is used during the encryption process (single signature attached to the basic COSE structure).

nordicjm commented 2 years ago

It looks like it only support ENCRYPT0 (https://datatracker.ietf.org/doc/html/rfc8613#section-5), meaning a single recipient. That's better than nothing, but limits if you have multiple recipients where you will have to include one shared secret for each recipient.

I don't see any mention of SIGN1 or SIGN, but presumably SIGN1 is used during the encryption process (single signature attached to the basic COSE structure).

mcumgr is command and response over the interface it was received on, so I'm not understanding why multiple recipients would be a problem if there is only a single recipient?

carlescufi commented 2 years ago

mcumgr is command and response over the interface it was received on, so I'm not understanding why multiple recipients would be a problem if there is only a single recipient?

I think the concern is not about mcumgr, but rather about future protocols using these CBOR/COSE libraries

StefanHri commented 2 years ago

OSCORE and EDHOC use both COSE and CBOR. In our implementation (uoscore-uedhoc ) we use zcbor to build the required COSE structures, which are a subset of the COSE specification. The COSE-related functions are however not exposed to the uoscore-uedhoc user and therefore they are not directly usable. For OSCORE and EDHOC is in my opinion sufficient to have only a CBOR library.

carlescufi commented 2 years ago

For OSCORE and EDHOC is in my opinion sufficient to have only a CBOR library.

Thanks for the input. Let's be conservative about this. Right now we have no need for a fully-fledged COSE library in Zephyr. If we introduce one in the future then OSCORE can make use of it instead of implementing it internally.

microbuilder commented 2 years ago

For OSCORE and EDHOC is in my opinion sufficient to have only a CBOR library.

Thanks for the input. Let's be conservative about this. Right now we have no need for a fully-fledged COSE library in Zephyr. If we introduce one in the future then OSCORE can make use of it instead of implementing it internally.

It's something we were actively working on right now, to try to enable decoding COSE SIGNed and ENCRYPTed payloads in Zephyr. Given that this already exists in TF-M (COSE signatures are used in attestation tokens, for example), reusing QCBOR and extending t_cose seemed the least amount of work on the NS side, but I'm not opposed to something different in Zephyr.

But from the position of maintaining TF-M, I'd lightly object to something where there wasn't a viable path to enable COSE support on the NS/Zephyr side. It sounds like OSCORE is doing all the COSE encoding by hand, which seems cumbersome and error-prone if we have to implement all the cryptographic calls ourselves and format things byte by byte.

I know this is of consequence to the TF-M Firmware Update service, for example, since there has been some discussion of using COSE ENCRYPTed firmware images, although it's still at the research or proof of concept phase.

I'm not going to block some other library going in, obviously, but I do think it would be a shame to be working on two different CBOR and COSE libraries.

microbuilder commented 2 years ago

To clarify ... I don't object to migrating to zcbor, which is a better situation that what we have today, but for COSE we may end up with one of:

They're all valid options if moving to QCBOR isn't feasible today.

But I do think COSE is something that will show up in a handful of TF-M related projects in the near future, so we should keep it in mind.

StefanHri commented 2 years ago

I think having a COSE library in Zephyr will be very nice because there are as well other drafts going on in IETF that use COSE which may become interesting for Zephyr as well. However for now I am perfectly fine with zcbor. If at some point a COSE library is available we can migrate the related code in our OSCORE and EDHOC implementations.

carlescufi commented 2 years ago

Thanks for the input @microbuilder, you make good points.

To clarify ... I don't object to migrating to zcbor, which is a better situation that what we have today, but for COSE we may end up with one of:

  • Two CBOR libraries later to enable t_cose (etc.), which depends on QCBOR

Given that Zephyr doesn't allow multiple implementation of the same functionality to coexist (unless one is deprecated of course), this is unlikely to be an option.

  • Writing a new COSE library that makes used of zcbor

Depending on how things evolve this might be a good option.

  • Updating something like t_cose to make use of zcbor beneath the hood

This would probably work as well.

They're all valid options if moving to QCBOR isn't feasible today.

It's not that moving to QCBOR is not feasible, is that QCBOR and t_cose are small projects that are not widely used and that are outside of Zephyr's control. Given that there is no immediate requirement for the functionality they offer in the non-secure image (aside from OSCORE and EDHOC) I would rather be cautious and not pull in a 3rd-party project like QCBOR at this point.

But I do think COSE is something that will show up in a handful of TF-M related projects in the near future, so we should keep it in mind.

Definitely. But I don't want to put the cart before the horses, we've been bitten by this in the past.

de-nordic commented 2 years ago

I have looked at the https://github.com/lindemer/cozy and its use of NanoCBOR. The NanoCBOR looks quite similar to zcbor and TinyCBOR: all of them use context/state and have API calls that add simple types. I was not able yet to look at the decode side of cozy, but the encode side seems quite neat and should be easy to make CBOR implementation agnostic. It should be possible to redefine the cbor context/state type according to used CBOR implementation and provide header files that would be defining intermediate CBOR API calls inlining used API (zcbor, NanoCBOR) function calls.

The https://github.com/laurencelundblade/t_cose looks more complicated in that matter, and uses some QCBOR specific calls that does not seem to have equivalence in other implementations (for example QCBORDecode_GetNthTagOfLast).

d3zd3z commented 2 years ago

As far as MCUboot, we're unlikely to want to use CBOR out of the Zephyr tree, anyway, since we support more than just Zephyr (even if the current code is Zephyr specific, it'd be nice if it wasn't).

microbuilder commented 2 years ago

@de-nordic We were able to update cozy to work with MbedTLS 3.0 without too much trouble, where we needed to decode a COSE packet and verify the signature. The library seems to have been abandonned, but adopting it in the zephyr community is an option.

I'm OK with almost any sensible library ... I'd just like to avoid a situation where three(+) different companies are working on three different libraries, such as Arm on t_cose and QCBOR for TF-M, Nordic with zcbor and ??? for COSE, Linaro with cozy and NanoCBOR (although we have been looking at moving to t_cose and QCBOR just to align with Arm engineering efforts in TF-M).

microbuilder commented 2 years ago

As far as MCUboot, we're unlikely to want to use CBOR out of the Zephyr tree, anyway, since we support more than just Zephyr (even if the current code is Zephyr specific, it'd be nice if it wasn't).

I understand MCUBoot won't want to pull in the Zephyr dependency, but it does seem like a shame not to pool engineering effort to improve ONE set of CBOR + COSE libraries since, at least on the COSE side, they all tend to be quite weak (and neglected) today and have large holes such as supporting SIGN but not ENCRYPT, or only one cipher for ENCRYPT or only ENCRYPT1, etc.

d3zd3z commented 2 years ago

I understand MCUBoot won't want to pull in the Zephyr dependency, but it does seem like a shame not to pool engineering effort to improve ONE set of CBOR + COSE libraries since, at least on the COSE side, they all tend to be quite weak (and neglected) today and have large holes such as supporting SIGN but not ENCRYPT, or only one cipher for ENCRYPT or only ENCRYPT1, etc.

This will become more relevant to MCUboot if/when it starts to get SUIT support, which is built around COSE.