nanocoap: add coap_next_msg_id() #18991
Conversation
b75d365 to
d45ae80
Compare
| * | ||
| * @return A new message ID that can be used for a request or response. | ||
| */ | ||
| uint16_t coap_next_msg_id(void); |
There was a problem hiding this comment.
Could you change so that the src and dst endpoint are available to the implementation?
The standard suggest three levels of granularity:
- Having one counter for each distinct src <--> dst pair
- Having one counter per network interface / subnet /
- Having one single globel counter
The reason for having more counter is that message IDs must not be reused within some time. It easily becomes the limiting factor on 802.3 or 802.11 networks. So, having multiple counters can increase throughput.
I think it is fine to keep using a single counter. But having a way to extend would be nice.
There was a problem hiding this comment.
Then we better keep the separate implementations.
With nanocoap_sock it's trivial to attach such a counter to the nanocoap_sock_t struct.
If we want a generalized function we would need to work with arrays to look up the message ID, which sounds like a disproportionate overhead to me.
|
|
||
| static void auto_init_coap_random(void) | ||
| { | ||
| if (_msg_id == 0) { |
There was a problem hiding this comment.
Doesn't this assume that SRAM has a value of 0 on the first boot? To me understanding this is not necessarily the case and this us being relied upon by the PUF modules.
The PUF random seeding utility does track whether it is a cold boot. Maybe we should split this out an own (trivial) module and use that here?
There was a problem hiding this comment.
Yes that's why initially only wanted to use the .noinit section as it was done in nanocoap_sock.
@kfessel was worried that some MCUs would then always start with 0 on power-on-reset, so I added the random.
I wanted to keep the .noinit to keep sequence numbers sequential across reboots as I had issues with CoAP servers discarding old message IDs in the past (when they were reset after the reboot).
With always randomizing there is a small chance of this happening.
There was a problem hiding this comment.
I think the standard actually wants the first msg IDs to be randomized. I do agree with .noinit, but I doubt that checking it to be zero is a good way to determine it is a cold boot.
There was a problem hiding this comment.
With always randomizing there is a small chance of this happening.
especially since random seed might not be random but RANDOM_SEED_DEFAULT or cpuid in these cases .noinit seems a better bet to not reuse the same msg_id.
checking for 0 we catch the cpus that have their memory cells drift to 0 if powered off .
There was a problem hiding this comment.
checking for 0 we catch the cpus that have their memory cells drift to 0 if powered off .
Note: Production variance has a significant impact on whether a given SRAM cell tends ti drift to zero or one. A few cells end up being randomly zero or one. With large enough SRAM this is a viable source of entropy.
So, there really is no sense in checking for it being zero. One could profile the memory at the address this gets linked to for a specific SRAM and end up with a classification that would yield a probability for it being a cold boot.
We do have reliable ways to reliably tell the reset cause: Some MCUs readily tell so. For others, we can store a magic cookie value into a .noinit variable. If it is still stored there on boot, it was a warm boot.
We may already have a module for that, I think this is ringing a bell.
|
There has been a similar proposal in #16730. My preference would be that that either RIOT bless some mechanism of persisting data across reboots (which comes in various, possibly conflicting, sets of requirements1), or it be up to the application to do that persistence. Either way, while message-ID is one thing to persist, I'd rather have an opaque thing that the CoAP stack (or even the full OS) takes at startup and produces at any time, which may easily be just the MID for now (but may become larger as the stack takes on responsibilities, eg. when it enforces congestion avoidance, or when we want to enhance our RNG by keeping entropy across reboots). Footnotes
|
This reverts commit 074b2e2.
|
I still don't fully understand why randomness of the initial message ID is so important. All message IDs after that are sequential (even across requests) so why is it important that the first ID is randomized? |
|
The initial random value is in the spec to make blind attackers' lives
harder (if a device always comes up with 4 and you spam it with
RSTs of message ID 4, it will never get far).
|
The growth could also be randomized IMHO (as long as it does not contradict the standard). |
|
Growth better not be -- there is a rejected erratum (https://www.rfc-editor.org/errata/eid5429 from @maribu) even that it would be a MUST to go by 1. Still, some people want to implement message dedup based on a window.
|
|
Note that using an upcounting counter is the suggested implementation in the standard and everybody uses this. Some CoAP implementations rely one a counter being used to optimize duplicate detection. The QoS would drop when interacting with such implementations. The 32 bit entropy in the CoAP token required by the standard should help more than the 16 bit msg ID in annoying attackers anyway. |
|
A bit OT, but there's no guarantee of 32bit in the token. Message size sensitive applications will use the 0-length token as often as possible.
|
|
So to salvage this:
did I get that right? |
Hmm, I read the standard as that the Token is where you should put in your entropy against spoofing, if you want this (section 5.3.1):
But then, I do wonder what the use cases are where adding 32 bit of entropy to your message will make a lot of difference. I honestly see no use case were using CoAP over plain UDP is sensible except for networks with L2 security and a firewall that prevents plain UDP based CoAP communication with nodes outside of that network (such as nodes in the Internet). If you have a device capable enough to act as a border router, I would expect that to also to be capable enough to act as secure endpoint of CoAP communication (such as CoAP over OSCORE or DTLS). And that could expose any resource provided by the constrained devices incapable of DTLS or OSCORE in the L2 network with added security. |
|
closed in favor of #19178 |
Contribution description
Both nanoCoAP and GCoAP have the need for getting (somewhat) monotonic message IDs.
Currently both implement this as an internal function that they then kinda need to re-repose after all.
Clean this up by just adding a single public
coap_next_msg_id()function that can be used by anyone needing CoAP message IDs.Testing procedure
CoAP should function as before.
(
tests/gcoap_fileserverwill run integration test on CI)Issues/PRs references