Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BACKPORT] sys/net/nanocoap: align request handling with spec #21166

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

maribu
Copy link
Member

@maribu maribu commented Jan 27, 2025

Backport of sys/net/nanocoap: align request handling with spec #21163


Contribution description

  • Do not reply with a reset message to a reset or an ACK message
  • Reply with a reset message when not able to process a CON/NON message (not even a suitable error reply)

Testing procedure

$ sudo ip tuntap add tap0 mode tap user $(whoami)
$ sudo ip link set tap0 up
$ make BOARD=native64 -C examples/nanocoap_server -j flash term
$ cat test.py                     
import socket, aiocoap
m = aiocoap.Message()
m.code = 0
m.mid = 0
m.mtype = aiocoap.RST
s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
s.connect(('fe80::9826:30ff:feb8:31f4', 5683, 0, socket.if_nametoindex('tap0')))
s.send(m.encode())
s.recv(1024)
$ python3 test.py

With master:

$ tshark -i tap0 -Y "coap"
Capturing on 'tap0'
    3 0.000397706 fe80::9826:30ff:feb8:31f3 56273 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 RST, MID:0, Empty Message
    4 0.000675868 fe80::9826:30ff:feb8:31f4 5683 56273 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:0, Empty Message

Note the RST "reply" to the RST

With this PR:

$ tshark -i tap0 -Y "coap"
Capturing on 'tap0'
    7 3.994693734 fe80::9826:30ff:feb8:31f3 54684 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 RST, MID:0, Empty Message

Note that no RST "reply" is send to the RST

Issues/PRs references

None

- Do not reply with a reset message to a reset or an ACK message
- Reply with a reset message when not able to process a CON/NON message
  (not even a suitable error reply)
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master labels Jan 27, 2025
@github-actions github-actions bot added Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System labels Jan 27, 2025
@maribu
Copy link
Member Author

maribu commented Jan 27, 2025

Tested:

First (packet 3) a RST injected to the server (correctly got no RST as reply), afterwards pings send and correctly got RSTs.

$ tshark -i tap0 -Y "coap"
Capturing on 'tap0'
    3 1.922254924 fe80::9826:30ff:feb8:31f3 48728 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 RST, MID:0, Empty Message
    6 22.301734177 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17073, Empty Message
    7 22.302041251 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17073, Empty Message
    8 23.302259263 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17074, Empty Message
    9 23.302580815 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17074, Empty Message
   10 24.304082908 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17075, Empty Message
   11 24.304431790 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17075, Empty Message
   12 25.306029669 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17076, Empty Message
   13 25.306453672 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17076, Empty Message
   14 26.308035518 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17077, Empty Message
   15 26.308374502 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17077, Empty Message
   16 27.309967650 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17078, Empty Message
   17 27.310318876 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17078, Empty Message
   18 28.311765589 fe80::9826:30ff:feb8:31f3 58943 5683 fe80::9826:30ff:feb8:31f4 CoAP 66 CON, MID:17079, Empty Message
   19 28.312233154 fe80::9826:30ff:feb8:31f4 5683 58943 fe80::9826:30ff:feb8:31f3 CoAP 66 RST, MID:17079, Empty Message

Copy link
Contributor

@MrKevinWeiss MrKevinWeiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks for retesting (as this was not just a simple backport)! ACK!

@riot-ci
Copy link

riot-ci commented Jan 27, 2025

Murdock results

✔️ PASSED

0d6cb6f sys/net/nanocoap: align request handling with spec

Success Failures Total Runtime
10271 0 10271 09m:29s

Artifacts

@MrKevinWeiss MrKevinWeiss added this pull request to the merge queue Jan 27, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 27, 2025
@maribu maribu added this pull request to the merge queue Jan 27, 2025
Merged via the queue into RIOT-OS:2025.01-branch with commit d06a7d9 Jan 27, 2025
28 checks passed
@maribu maribu deleted the backport-pr-21163 branch January 27, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants