nanocoap: follow-up fixes #17950

benpicco · 2022-04-14T13:43:56Z

Contribution description

NanoCoAP should use random message IDs and randomize the retransmit timeout.
This PR takes care of this.

Testing procedure

Block-wise does still work:

main(): This is RIOT! (Version: 2022.07-devel-63-g12dbc-nanocoap-fixes)
nanocoap test app
All up, running the shell now
> url get coap://[fe80::581a:98ff:fe23:2d6c%5]/riot/ver
offset 000: This is RIOT (Version: 2022.07-d
offset 032: evel-64-g414f73-nanocoap_vfs) ru
offset 064: nning on a native board with a n
offset 096: ative MCU.

Issues/PRs references

sys/net/application_layer/nanocoap/sock.c

sys/net/application_layer/nanocoap/nanocoap.c

benpicco · 2022-04-20T17:17:40Z

Sorry that this has grown larger again, @fjmolinas discovered that retransmissions were broken, which prompted me to dig through the code and clean it up some more.

Retransmissions should now work properly, but it seems like we might have discovered a bug in ethos along the way.

fjmolinas · 2022-04-21T08:10:08Z

Sorry that this has grown larger again, @fjmolinas discovered that retransmissions were broken, which prompted me to dig through the code and clean it up some more.

Indeed I had come across issues when running examples/suit_update test, I have now ran it 3 times on this PR with success, while on master it always fails on the second update.

fjmolinas

This PR has grown, and I would like some more context for some of the commits, and I think it would be best to split the improvements from the follow-up fixes to #17474.

d96ccaf: I understand the reasoning, but I would like to know what prompted you to add this (what success code other than 205 where you getting?)
25c9b5a: I think this should be split.
75787a7: this one also is better as its own PR
ba15a6e: can we split was is cosmetic and functional?
90ca43d: commit message does not seem to align with the commit itself, related to the commit I only see an assert, while there is some new logic to re-try reception without a re-request.

sys/net/application_layer/nanocoap/sock.c

fjmolinas · 2022-04-21T09:16:26Z

sys/net/application_layer/nanocoap/sock.c

-    pktpos += coap_opt_put_uri_pathquery(pktpos, &lastonum, path);
-    pktpos += coap_opt_put_uint(pktpos, lastonum, COAP_OPT_BLOCK2,
-                                (num << 4) | blksize);
+    buf += coap_build_hdr(pkt.hdr, COAP_TYPE_CON, NULL, 0, COAP_METHOD_GET, _get_id());


Seems like we should actually be setting the token (which is 0 right now) to maybe num and then check on that instead of else if (coap_get_id(pkt) != id), would that be correct @maribu?

Note that a zero length token is still a valid token. The standard asks for 32 bit of entropy in the Token if a device is connected to the Internet, though:

A client that is connected to the general Internet SHOULD use at least 32 bits of randomness, keeping in mind that not being directly connected to the Internet is not necessarily sufficient protection against spoofing. (Note that the Message ID adds little in protection as it is usually sequentially assigned, i.e., guessable, and can be circumvented by spoofing a separate response.)

https://datatracker.ietf.org/doc/html/rfc7252#section-5.3.1

Note that a zero length token is still a valid token

Yep

The standard asks for 32 bit of entropy in the Token if a device is connected to the Internet, though

In that case, the default token length should at least be 4bytes, thinking of gcoap default

RIOT/sys/include/net/gcoap.h

Lines 482 to 490 in e64b1bd

/**

* @ingroup net_gcoap_conf

* @brief Length in bytes for a token

*

* Value must be in the range 0 to @ref GCOAP_TOKENLEN_MAX.

*/

#ifndef CONFIG_GCOAP_TOKENLEN

#define CONFIG_GCOAP_TOKENLEN (2)

#endif

Although for this specific use-case, I guess if a block option is set it could check that the blknum matches what is expected, keeping the token field to 0.

Do the individual blocks require different tokens or is this considered a single transfer, so the same token for all blocks?

It would be different tokens for each block request, but I think you could leave it empty and check in the nanocoap_sock_request_cb call coap_get_block2(pkt, &block2); on the request and response, and if its there is a block option and the blknum does not match retry reception. But this optimization is maybe a bit of a hack to save the token bytes...

It is up to the client and needs to aid the client to identify the right response. With NSTART=1 only duplicates (trivial to detect with blockwise) and spoofed responses are needed to be told apart from the right one. In case of UDP transport, one cannot really defend against spoofed responses, but having same entropy in the Token at least requires the adversary to be able to listen to the requests. The standard specifically says 32 bits of randomness and not reusing Tokens for the same source and destination endpoints is what SHOULD be done.

The common strategy is to just use a properly seeded PRNG for generating the token and hope for the best.

Since we don't include a token yet in the request, how about we move this to a follow-up PR?

fjmolinas

Some comments regarding the last commits, otherwise now need a rebase.

sys/net/application_layer/nanocoap/sock.c

benpicco · 2022-04-22T08:17:22Z

units need fixing to ms

do they? sock_udp_recv_buf() expects µs regardless

fjmolinas · 2022-04-22T08:25:06Z

units need fixing to ms

do they? sock_udp_recv_buf() expects µs regardless

Ahh... well then maybe that one is fine, but the interval ones are definitively wrong.

So we don't have to wait a full interval again if we got a wrong message without resend.

benpicco · 2022-04-22T08:36:31Z

the interval ones are definitively wrong

Uh, which do you mean?

fjmolinas · 2022-04-22T08:44:06Z

the interval ones are definitively wrong

Uh, which do you mean?

Ah wait no, I mis-read where the US_PER_MS ,multiplier was, my bad

fjmolinas · 2022-04-22T08:46:16Z

* [d96ccaf](https://github.com/RIOT-OS/RIOT/commit/d96ccafc9efb07ae98714aef99ef0bd246951b1f): I understand the reasoning, but I would like to know what prompted you to add this (what success code other than 205 where you getting?)

I think this is still pending

* [90ca43d](https://github.com/RIOT-OS/RIOT/commit/90ca43d5b0ed1d1a844929e4c4fa46a2c4747cb9): commit message does not seem to align with the commit itself, related to the commit I only see an assert, while there is some new logic to re-try reception without a re-request.

This is still un-addressed

benpicco · 2022-04-22T08:53:58Z

I merged the first one with b20f899. The reasoning is that the (user) callback will evaluate the response code and can better decide if the function was a success than the wrapper function.

The second one I hoped the added comment could explain.
The re-try without resend was always there, it would just always fail as sock_udp_recv_buf() when called with the old ctx would just release the ctx and return 0.
The sock_udp_recv_buf() API is a bit tricky: You get a pointer to the network buffer and you are supposed to call the function in a loop, the idea being that a packet could be fragmented and you get the next data fragment or NULL with the next call, NULL meaning you have reached the end - but you are supposed to always call sock_udp_recv_buf() until you have reached NULL.

Now the way the function is implemented for GNRC and LWIP, there will never be a second data fragment. This is what enables this (slight ab)use here in the first place: If there were more fragments, we would still need a temporary buffer.
But I think it's safe to assume that if we do application layer fragmentation with CoAP we don't have to rely on IP layer fragmentation, if this ever gets implemented this way for sock_udp_recv_buf() in the future.

fjmolinas · 2022-04-22T09:22:22Z

Thanks for the details @benpicco, I had missed the first part

Now the way the function is implemented for GNRC and LWIP, there will never be a second data fragment. This is what enables this (slight ab)use here in the first place: If there were more fragments, we would still need a temporary buffer.
But I think it's safe to assume that if we do application layer fragmentation with CoAP we don't have to rely on IP layer fragmentation, if this ever gets implemented this way for sock_udp_recv_buf() in the future.

I think then this 'abuse' probably needs a comment as well. I would even prefer an assert on this in the case a new network stack would behave differently.

benpicco · 2022-04-22T09:38:21Z

Blockwise still works after all that 😄

tests/nanocoap_cli

2022-04-22 11:36:24,876 # main(): This is RIOT! (Version: 2022.07-devel-167-g5deea-nanocoap-fixes)
2022-04-22 11:36:24,878 # nanocoap test app
2022-04-22 11:36:24,881 # All up, running the shell now
> ifconfig
2022-04-22 11:36:37,697 # ifconfig
2022-04-22 11:36:37,700 # shell: command not found: ifconfig
> help
2022-04-22 11:36:39,985 # help
2022-04-22 11:36:39,987 # Command              Description
2022-04-22 11:36:39,991 # ---------------------------------------
2022-04-22 11:36:39,994 # client               CoAP client
2022-04-22 11:36:39,998 # url                  CoAP client URL request
2022-04-22 11:36:40,001 # server               CoAP server
2022-04-22 11:36:40,004 # inet6                IPv6 addresses
> inet6
2022-04-22 11:36:41,681 # inet6
2022-04-22 11:36:41,687 #           inet6 addr: fe80::fec2:3dff:fe23:22df  scope: link  VAL
2022-04-22 11:36:41,694 #           inet6 addr: 2001:16b8:45b5:f00:fec2:3dff:fe23:22df  scope: global  VAL
> url get coap://coap.me
2022-04-22 11:36:52,034 # url get coap://coap.me
2022-04-22 11:36:52,142 # offset 000: </test>;rt="test";ct=0,</validat
2022-04-22 11:36:52,174 # offset 032: e>;rt="validate";ct=0,</hello>;r
2022-04-22 11:36:52,205 # offset 064: t="Type1";ct=0;if="If1",</bl%C3%
2022-04-22 11:36:52,269 # offset 096: A5b%C3%A6rsyltet%C3%B8y>;rt="bl�offset 128: �bærsyltetøy";ct=0,</sink>;rt=
2022-04-22 11:36:52,300 # offset 160: "sink";ct=0,</separate>;rt="sepa
2022-04-22 11:36:52,332 # offset 192: rate";ct=0,</large>;rt="Type1 Ty
2022-04-22 11:36:52,363 # offset 224: pe2";ct=0;sz=1700;if="If2",</sec
2022-04-22 11:36:52,395 # offset 256: ret>;rt="secret";ct=0,</broken>;
2022-04-22 11:36:52,426 # offset 288: rt="Type2 Type1";ct=0;if="If2 If
2022-04-22 11:36:52,457 # offset 320: 1",</weird33>;rt="weird33";ct=0,
2022-04-22 11:36:52,489 # offset 352: </weird44>;rt="weird44";ct=0,</w
2022-04-22 11:36:52,520 # offset 384: eird55>;rt="weird55";ct=0,</weir
2022-04-22 11:36:52,551 # offset 416: d333>;rt="weird333";ct=0,</weird
2022-04-22 11:36:52,582 # offset 448: 3333>;rt="weird3333";ct=0,</weir
2022-04-22 11:36:52,613 # offset 480: d33333>;rt="weird33333";ct=0,</1
2022-04-22 11:36:52,644 # offset 512: 23412341234123412341234>;rt="123
2022-04-22 11:36:52,675 # offset 544: 412341234123412341234";ct=0,</lo
2022-04-22 11:36:52,707 # offset 576: cation-query>;rt="location-query
2022-04-22 11:36:52,738 # offset 608: ";ct=0,</create1>;rt="create1";c
2022-04-22 11:36:52,769 # offset 640: t=0,</large-update>;rt="large-up
2022-04-22 11:36:52,801 # offset 672: date";ct=0,</large-create>;rt="l
2022-04-22 11:36:52,832 # offset 704: arge-create";ct=0,</query>;rt="q
2022-04-22 11:36:52,863 # offset 736: uery";ct=0,</seg1>;rt="seg1";ct=
2022-04-22 11:36:52,894 # offset 768: 40,</path>;rt="path";ct=40,</loc
2022-04-22 11:36:52,926 # offset 800: ation1>;rt="location1";ct=40,</m
2022-04-22 11:36:52,957 # offset 832: ulti-format>;rt="multi-format";c
2022-04-22 11:36:52,989 # offset 864: t=0,</3>;rt="3";ct=50,</4>;rt="4
2022-04-22 11:36:53,020 # offset 896: ";ct=50,</5>;rt="5";ct=50
> url get coap://coap.me/large
2022-04-22 11:37:04,267 # url get coap://coap.me/large
2022-04-22 11:37:04,295 # offset 000: 
2022-04-22 11:37:04,298 #      0                   1     
2022-04-22 11:37:04,328 # offset 032:               2                 
2022-04-22 11:37:04,355 # offset 064:   3
2022-04-22 11:37:04,357 #     0 1 2 3 4 5 6 7 8 9 0 1 
2022-04-22 11:37:04,387 # offset 096: 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 
2022-04-22 11:37:04,414 # offset 128: 8 9 0 1
2022-04-22 11:37:04,416 #    +-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,446 # offset 160: -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,473 # offset 192: -+-+-+-+-+-+
2022-04-22 11:37:04,475 #    |Ver| T |  TKL  
2022-04-22 11:37:04,505 # offset 224: |      Code     |          Messa
2022-04-22 11:37:04,533 # offset 256: ge ID           |
2022-04-22 11:37:04,534 #    +-+-+-+-+-+
2022-04-22 11:37:04,564 # offset 288: -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,592 # offset 320: -+-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,593 #    |   To
2022-04-22 11:37:04,622 # offset 352: ken (if any, TKL bytes) ...
2022-04-22 11:37:04,623 #    +
2022-04-22 11:37:04,652 # offset 384: -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,682 # offset 416: -+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,709 # offset 448: 
2022-04-22 11:37:04,711 #    |   Options (if any) ...
2022-04-22 11:37:04,711 #    
2022-04-22 11:37:04,741 # offset 480: +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
2022-04-22 11:37:04,770 # offset 512: +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
2022-04-22 11:37:04,797 # offset 544: +
2022-04-22 11:37:04,800 #    |1 1 1 1 1 1 1 1|    Payloa
2022-04-22 11:37:04,828 # offset 576: d (if any) ...
2022-04-22 11:37:04,829 #    +-+-+-+-+-+-+-
2022-04-22 11:37:04,859 # offset 608: +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-
2022-04-22 11:37:04,887 # offset 640: +-+-+-+-+-+-+-+-+-+
2022-04-22 11:37:04,887 # 
2022-04-22 11:37:04,887 # [...]
2022-04-22 11:37:04,888 # 
2022-04-22 11:37:04,888 #    T
2022-04-22 11:37:04,917 # offset 672: oken Length (TKL):  4-bit unsign
2022-04-22 11:37:04,947 # offset 704: ed integer.  Indicates the lengt
2022-04-22 11:37:04,974 # offset 736: h of
2022-04-22 11:37:04,977 #       the variable-length T
2022-04-22 11:37:05,006 # offset 768: oken field (0-8 bytes).  Lengths
2022-04-22 11:37:05,034 # offset 800:  9-15 are
2022-04-22 11:37:05,036 #       reserved, MUST N
2022-04-22 11:37:05,066 # offset 832: OT be sent, and MUST be processe
2022-04-22 11:37:05,094 # offset 864: d as a message
2022-04-22 11:37:05,096 #       format erro
2022-04-22 11:37:05,123 # offset 896: r.
2022-04-22 11:37:05,123 # 
2022-04-22 11:37:05,125 #    Code:  8-bit unsigned int
2022-04-22 11:37:05,155 # offset 928: eger, split into a 3-bit class (
2022-04-22 11:37:05,182 # offset 960: most
2022-04-22 11:37:05,184 #       significant bits) and
2022-04-22 11:37:05,213 # offset 992:  a 5-bit detail (least significa
2022-04-22 11:37:05,241 # offset 1024: nt bits),
2022-04-22 11:37:05,243 #       documented as c.
2022-04-22 11:37:05,272 # offset 1056: dd where c is a digit from 0 to 
2022-04-22 11:37:05,299 # offset 1088: 7 for the 3-bit
2022-04-22 11:37:05,301 #       subfield a
2022-04-22 11:37:05,330 # offset 1120: nd dd are two digits from 00 to 
2022-04-22 11:37:05,359 # offset 1152: 31 for the 5-bit
2022-04-22 11:37:05,360 #       subfield.
2022-04-22 11:37:05,390 # offset 1184:   The class can indicate a reque
2022-04-22 11:37:05,418 # offset 1216: st (0), a success
2022-04-22 11:37:05,420 #       response
2022-04-22 11:37:05,449 # offset 1248:  (2), a client error response (4
2022-04-22 11:37:05,478 # offset 1280: ), or a server error
2022-04-22 11:37:05,479 #       respo
2022-04-22 11:37:05,508 # offset 1312: nse (5).  (All other class value
2022-04-22 11:37:05,537 # offset 1344: s are reserved.)  As a
2022-04-22 11:37:05,538 #       spe
2022-04-22 11:37:05,567 # offset 1376: cial case, Code 0.00 indicates a
2022-04-22 11:37:05,597 # offset 1408: n Empty message.  In case of a
2022-04-22 11:37:05,597 #  
2022-04-22 11:37:05,626 # offset 1440:      request, the Code field ind
2022-04-22 11:37:05,686 # offset 1472: icates the Request Method; in ca
2022-04-22 11:37:05,713 # offset 1504: se of a
2022-04-22 11:37:05,715 #       response a Respons
2022-04-22 11:37:05,743 # offset 1536: e Code.  Possible values are mai
2022-04-22 11:37:05,771 # offset 1568: ntained in the
2022-04-22 11:37:05,772 #       CoAP Code R
2022-04-22 11:37:05,801 # offset 1600: egistries (Section 12.1).  The s
2022-04-22 11:37:05,829 # offset 1632: emantics of requests
2022-04-22 11:37:05,830 #       and r
2022-04-22 11:37:05,859 # offset 1664: esponses are defined in Section 
2022-04-22 11:37:05,885 # offset 1696: 5.
2022-04-22 11:37:05,886 # 
2022-04-22 11:37:05,886 # 
>

fjmolinas · 2022-04-22T11:50:37Z

examples/suit_update PASS

suit seq_no > > > > suit seq_no seq_no: 0x62629555 > ifconfig ifconfig Iface 4 HWaddr: AA:04:26:28:76:39 L2-PDU:1500 MTU:1500 HL:64 RTR Source address length: 6 Link type: wired inet6 addr: fe80::a804:26ff:fe28:7639 scope: link VAL pinging node... PING fe80::a804:26ff:fe28:7639%riot0(fe80::a804:26ff:fe28:7639%riot0) 56 data bytes --- fe80::a804:26ff:fe28:7639%riot0 ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 0ms rtt min/avg/max/mdev = 42.645/42.645/42.645/0.000 ms pinging node succeeded. suit inet6 addr: fe80::2 scope: link VAL inet6 group: ff02::2 inet6 group: ff02::1 inet6 group: ff02::1:ff28:7639 inet6 group: ff02::1:ff00:2 > suit Usage: suit fetch <manifest url> compiling /home/francisco/workspace/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr... make: Nothing to be done for 'all'. creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627926.bin... creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627926.bin... suit: generating key in /home/francisco/workspace/RIOT/keys published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.latest.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.latest.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot0.1650627926.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot1.1650627926.bin" aiocoap-client -m POST "coap://[fe80::a804:26ff:fe28:7639%riot0]/suit/trigger" \ --payload "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" && \ echo "Triggered [fe80::a804:26ff:fe28:7639%riot0] to update." Triggered [fe80::a804:26ff:fe28:7639%riot0] to update. suit seq_no > suit: received URL: "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" suit_coap: trigger received suit_coap: downloading "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" suit_coap: got manifest with size 497 suit: verifying manifest signature Unable to validate signature: -2 compiling /home/francisco/workspace/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr... make: Nothing to be done for 'all'. creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627925.bin... creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627925.bin... published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.1650627925.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627925.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.latest.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.latest.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627925.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot0.1650627925.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627925.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot1.1650627925.bin" aiocoap-client -m POST "coap://[fe80::a804:26ff:fe28:7639%riot0]/suit/trigger" \ --payload "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627925.bin" && \ echo "Triggered [fe80::a804:26ff:fe28:7639%riot0] to update." Triggered [fe80::a804:26ff:fe28:7639%riot0] to update. suit_parse() failed. res=-6 suit: received URL: "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627925.bin" suit_coap: trigger received suit_coap: downloading "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627925.bin" suit_coap: got manifest with size 497 suit: verifying manifest signature suit: validated manifest version )Manifest seq_no: 1650627925, highest available: 1650627925 seq_nr <= running image compiling /home/francisco/workspace/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr... make: Nothing to be done for 'all'. creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627926.bin... creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627926.bin... published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.latest.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.latest.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot0.1650627926.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627926.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot1.1650627926.bin" aiocoap-client -m POST "coap://[fe80::a804:26ff:fe28:7639%riot0]/suit/trigger" \ --payload "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" && \ echo "Triggered [fe80::a804:26ff:fe28:7639%riot0] to update." Triggered [fe80::a804:26ff:fe28:7639%riot0] to update. )suit_parse() failed. res=-5 suit: received URL: "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" suit_coap: trigger received suit_coap: downloading "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627926.bin" suit_coap: got manifest with size 497 suit: verifying manifest signature suit: validated manifest version )Manifest seq_no: 1650627926, highest available: 1650627925 suit: validated sequence number )Formatted component name: Comparing manifest offset 1000 with other slot offset Comparing manifest offset 20800 with other slot offset validating vendor ID Comparing 547d0d74-6d3a-5a92-9662-4881afd9407b to 547d0d74-6d3a-5a92-9662-4881afd9407b from manifest validating vendor ID: OK validating class id Comparing 8818989e-a257-5994-ac9a-554b77898083 to 8818989e-a257-5994-ac9a-554b77898083 from manifest validating class id: OK Comparing manifest offset 1000 with other slot offset Comparing manifest offset 20800 with other slot offset SUIT policy check OK. Formatted component name: riotboot_flashwrite: initializing update to target slot 1 Fetching firmware |█████████████████████████| 100% Finalizing payload store Verifying image digest Starting digest verification against image Install correct payload Verifying image digest Starting digest verification against image Install correct payload Image magic_number: 0x544f4952 Image Version: 0x62629556 Image start address: 0x00020900 Header chksum: 0xebfd9e5c suit_coap: rebooting... ----> ethos: hello received Failed to send flush request: Operation not permitted gnrc_uhcpc: Using 4 as border interface and 0 as wireless interface. current-slot gnrc_uhcpc: only one interface found, skipping setup. main(): This is RIOT! (Version: 2022.07-devel-167-g5deea-pr-17950) RIOT SUIT update example application Running from slot 1 Image magic_number: 0x544f4952 Image Version: 0x62629556 Image start address: 0x00020900 Header chksum: 0xebfd9e5c suit_coap: started. Starting the shell > > > ifconfig current-slot Running from slot 1 > ifconfig Iface 4 HWaddr: AA:04:26:28:76:39 L2-PDU:1500 MTU:1500 HL:64 RTR Source address length: 6 Link type: wired inet6 addr: fe80::a804:26ff:fe28:7639 scope: link VAL pinging node... PING fe80::a804:26ff:fe28:7639%riot0(fe80::a804:26ff:fe28:7639%riot0) 56 data bytes --- fe80::a804:26ff:fe28:7639%riot0 ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 0ms rtt min/avg/max/mdev = 41.401/41.401/41.401/0.000 ms pinging node succeeded. compiling /home/francisco/workspace/RIOT/dist/tools/riotboot_gen_hdr/bin/genhdr... make: Nothing to be done for 'all'. creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627927.bin... creating /home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627927.bin... published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.1650627927.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627927.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/suit_files/riot.suit.latest.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.latest.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot0.1650627927.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot0.1650627927.bin" published "/home/francisco/workspace/RIOT/examples/suit_update/bin/samr21-xpro/riotboot_files/slot1.1650627927.bin" as "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/slot1.1650627927.bin" aiocoap-client -m POST "coap://[fe80::a804:26ff:fe28:7639%riot0]/suit/trigger" \ --payload "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627927.bin" && \ echo "Triggered [fe80::a804:26ff:fe28:7639%riot0] to update." Triggered [fe80::a804:26ff:fe28:7639%riot0] to update. inet6 addr: fe80::2 scope: link VAL inet6 group: ff02::2 inet6 group: ff02::1 inet6 group: ff02::1:ff28:7639 inet6 group: ff02::1:ff00:2 > suit: received URL: "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627927.bin" suit_coap: trigger received suit_coap: downloading "coap://[fd00:dead:beef::1]/fw/suit_update/samr21-xpro/riot.suit.1650627927.bin" suit_coap: got manifest with size 497 suit: verifying manifest signature suit: validated manifest version )Manifest seq_no: 1650627927, highest available: 1650627926 suit: validated sequence number )Formatted component name: Comparing manifest offset 1000 with other slot offset validating vendor ID Comparing 547d0d74-6d3a-5a92-9662-4881afd9407b to 547d0d74-6d3a-5a92-9662-4881afd9407b from manifest validating vendor ID: OK validating class id Comparing 8818989e-a257-5994-ac9a-554b77898083 to 8818989e-a257-5994-ac9a-554b77898083 from manifest validating class id: OK Comparing manifest offset 1000 with other slot offset SUIT policy check OK. Formatted component name: riotboot_flashwrite: initializing update to target slot 0 Fetching firmware |█████████████████████████| 100% Finalizing payload store Verifying image digest Starting digest verification against image Install correct payload Verifying image digest Starting digest verification against image Install correct payload Image magic_number: 0x544f4952 Image Version: 0x62629557 Image start address: 0x00001100 Header chksum: 0xfbffa65b suit_coap: rebooting... ----> ethos: hello received Failed to send flush request: Operation not permitted gnrc_uhcpc: Using 4 as border interface and 0 as wireless interface. current-slot gnrc_uhcpc: only one interface found, skipping setup. main(): This is RIOT! (Version: 2022.07-devel-167-g5deea-pr-17950) RIOT SUIT update example application Running from slot 0 Image magic_number: 0x544f4952 Image Version: 0x62629557 Image start address: 0x00001100 Header chksum: 0xfbffa65b suit_coap: started. Starting the shell > > > ifconfig current-slot Running from slot 0 > ifconfig Iface 4 HWaddr: AA:04:26:28:76:39 L2-PDU:1500 MTU:1500 HL:64 RTR Source address length: 6 Link type: wired inet6 addr: fe80::a804:26ff:fe28:7639 scope: link VAL pinging node... PING fe80::a804:26ff:fe28:7639%riot0(fe80::a804:26ff:fe28:7639%riot0) 56 data bytes --- fe80::a804:26ff:fe28:7639%riot0 ping statistics --- 1 packets transmitted, 1 received, 0% packet loss, time 0ms rtt min/avg/max/mdev = 41.390/41.390/41.390/0.000 ms pinging node succeeded. TEST PASSED

fjmolinas

ACK, issues I had are fixes, and code changes now look good to me, not sure if @maribu or @chrysn still want to take another look.

benpicco · 2022-04-22T21:14:57Z

@maribu said I can go ahead on Matrix, there are more follow-ups to come anyway - thank you all for the review and help bringing nanoCoAP forward 🎉

benpicco requested review from miri64, haukepetersen, PeterKietzmann and cgundogan as code owners April 14, 2022 13:43

benpicco requested review from chrysn, kaspar030 and maribu April 14, 2022 13:44

github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Apr 14, 2022

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 14, 2022

maribu reviewed Apr 15, 2022

View reviewed changes

sys/net/application_layer/nanocoap/sock.c Outdated Show resolved Hide resolved

chrysn reviewed Apr 16, 2022

View reviewed changes

sys/net/application_layer/nanocoap/nanocoap.c Outdated Show resolved Hide resolved

benpicco removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 16, 2022

benpicco mentioned this pull request Apr 16, 2022

nanocoap_sock: add nanocoap_sock_block_request() #17958

Merged

benpicco force-pushed the nanocoap-fixes branch from 76be802 to aadb304 Compare April 20, 2022 17:14

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 20, 2022

benpicco requested a review from fjmolinas April 20, 2022 17:15

benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Apr 20, 2022

benpicco force-pushed the nanocoap-fixes branch from aadb304 to ba15a6e Compare April 20, 2022 17:58

fjmolinas reviewed Apr 21, 2022

View reviewed changes

sys/net/application_layer/nanocoap/sock.c Show resolved Hide resolved

maribu reviewed Apr 21, 2022

View reviewed changes

sys/net/application_layer/nanocoap/sock.c Outdated Show resolved Hide resolved

fjmolinas mentioned this pull request Apr 21, 2022

makefiles: allow to override suit manifest payloads #16771

Merged

fjmolinas reviewed Apr 21, 2022

View reviewed changes

fjmolinas pushed a commit to fjmolinas/RIOT that referenced this pull request Apr 21, 2022

MERGECOMMIT: nanocoap: follow-up fixes RIOT-OS#17950

896b9f3

fjmolinas mentioned this pull request Apr 21, 2022

examples/suit_update/test: use 'suit seq_no' to get version #17973

Merged

benpicco mentioned this pull request Apr 21, 2022

nanocoap: follow-up fixes (uncontroversial parts) #17977

Merged

fjmolinas reviewed Apr 22, 2022

View reviewed changes

sys/net/application_layer/nanocoap/sock.c Show resolved Hide resolved

sys/net/application_layer/nanocoap/sock.c Outdated Show resolved Hide resolved

benpicco force-pushed the nanocoap-fixes branch from 1aa9a93 to a5acea3 Compare April 22, 2022 08:19

benpicco and others added 5 commits April 22, 2022 10:35

nanocoap_sock: ensure ctx is reset before receiving new frame

c22b124

nanocoap_sock: compare response token

e2fe9d0

nanocoap_sock: calculate deadline from interval

bd7fe3b

So we don't have to wait a full interval again if we got a wrong message without resend.

nanocoap_sock: clean up the request function

85833ab

nanocoap_sock: pass on error code from user callback

b20f899

benpicco force-pushed the nanocoap-fixes branch from a5acea3 to b20f899 Compare April 22, 2022 08:36

benpicco added 2 commits April 22, 2022 11:34

nanocoap_sock: document (ab)use of sock_udp_recv_buf(), add asssert()

bb4c597

tests/nanocoap_cli: add optional DNS support

5deea31

benpicco requested review from smlng, leandrolanzieri, aabadie and MichelRottleuthner as code owners April 22, 2022 09:36

github-actions bot added the Area: tests Area: tests and testing framework label Apr 22, 2022

benpicco mentioned this pull request Apr 22, 2022

net/nanocoap: remove coap_pkt token ptr #17983

Merged

fjmolinas approved these changes Apr 22, 2022

View reviewed changes

benpicco merged commit c831a99 into RIOT-OS:master Apr 22, 2022

benpicco deleted the nanocoap-fixes branch April 22, 2022 21:13

benpicco mentioned this pull request Apr 29, 2022

nanocoap_sock: only abort nanocoap_sock_get_blockwise() on negative error #18037

Merged

chrysn added this to the Release 2022.07 milestone Aug 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nanocoap: follow-up fixes #17950

nanocoap: follow-up fixes #17950

benpicco commented Apr 14, 2022

benpicco commented Apr 20, 2022

fjmolinas commented Apr 21, 2022

fjmolinas left a comment

fjmolinas Apr 21, 2022

maribu Apr 21, 2022

fjmolinas Apr 21, 2022

benpicco Apr 21, 2022

fjmolinas Apr 21, 2022

maribu Apr 21, 2022

benpicco Apr 21, 2022

fjmolinas left a comment

benpicco commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022 •

edited

Loading

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022 •

edited by fjmolinas

Loading

fjmolinas commented Apr 22, 2022

fjmolinas left a comment

benpicco commented Apr 22, 2022 •

edited

Loading

	/**
	* @ingroup net_gcoap_conf
	* @brief Length in bytes for a token
	*
	* Value must be in the range 0 to @ref GCOAP_TOKENLEN_MAX.
	*/
	#ifndef CONFIG_GCOAP_TOKENLEN
	#define CONFIG_GCOAP_TOKENLEN (2)
	#endif

nanocoap: follow-up fixes #17950

nanocoap: follow-up fixes #17950

Conversation

benpicco commented Apr 14, 2022

Contribution description

Testing procedure

Issues/PRs references

benpicco commented Apr 20, 2022

fjmolinas commented Apr 21, 2022

fjmolinas left a comment

Choose a reason for hiding this comment

fjmolinas Apr 21, 2022

Choose a reason for hiding this comment

maribu Apr 21, 2022

Choose a reason for hiding this comment

fjmolinas Apr 21, 2022

Choose a reason for hiding this comment

benpicco Apr 21, 2022

Choose a reason for hiding this comment

fjmolinas Apr 21, 2022

Choose a reason for hiding this comment

maribu Apr 21, 2022

Choose a reason for hiding this comment

benpicco Apr 21, 2022

Choose a reason for hiding this comment

fjmolinas left a comment

Choose a reason for hiding this comment

benpicco commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022 • edited Loading

fjmolinas commented Apr 22, 2022

benpicco commented Apr 22, 2022 • edited by fjmolinas Loading

fjmolinas commented Apr 22, 2022

fjmolinas left a comment

Choose a reason for hiding this comment

benpicco commented Apr 22, 2022 • edited Loading

benpicco commented Apr 22, 2022 •

edited

Loading

benpicco commented Apr 22, 2022 •

edited by fjmolinas

Loading

benpicco commented Apr 22, 2022 •

edited

Loading