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

gcoap: ensure response address is the same as request address #18026

Merged
merged 6 commits into from
May 10, 2022

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Apr 27, 2022

Contribution description

If a node has multiple addresses we must reply to a request with the same address on which the request was received.

Testing procedure

Run GCoAP on a node that has multiple addresses assigned to it's interface, or a second interface with a global address:

2022-04-27 18:01:21,824 # Iface  7 
2022-04-27 18:01:21,828 #           Long HWaddr: AA:EC:E6:8B:F9:BA:BD:C8 
2022-04-27 18:01:21,831 #           MTU:65535  HL:64  RTR  
2022-04-27 18:01:21,833 #           RTR_ADV  
2022-04-27 18:01:21,835 #           Link type: wired
2022-04-27 18:01:21,841 #           inet6 addr: fe80::a8ec:e68b:f9ba:bdc8  scope: link  VAL
2022-04-27 18:01:21,847 #           inet6 addr: fd11::a8ec:e68b:f9ba:bdc8  scope: global  VAL
2022-04-27 18:01:21,852 #           inet6 addr: fdea:dbee:f::1  scope: global  VAL
2022-04-27 18:01:21,855 #           inet6 group: ff02::2
2022-04-27 18:01:21,858 #           inet6 group: ff02::1
2022-04-27 18:01:21,861 #           inet6 group: ff02::1:ffba:bdc8
2022-04-27 18:01:21,864 #           inet6 group: ff02::1:ff00:1

2022-04-27 21:02:37,436 # Iface  6  HWaddr: FC:C2:3D:0B:D4:4F 
2022-04-27 21:02:37,441 #           L2-PDU:1500  MTU:1492  HL:255  RTR  
2022-04-27 21:02:37,444 #           Source address length: 6
2022-04-27 21:02:37,446 #           Link type: wired
2022-04-27 21:02:37,452 #           inet6 addr: fe80::fec2:3dff:fe0b:d44f  scope: link  VAL
2022-04-27 21:02:37,459 #           inet6 addr: 2001:16b8:4536:a600:fec2:3dff:fe0b:d44f  scope: global  VAL
2022-04-27 21:02:37,462 #           inet6 group: ff02::2
2022-04-27 21:02:37,464 #           inet6 group: ff02::1
2022-04-27 21:02:37,468 #           inet6 group: ff02::1:ff0b:d44f

We should be able to reach the CoAP server with any of these addresses:

2022-04-27 18:06:12,488 # > url get coap://[fe80::a8ec:e68b:f9ba:bdc8%4]/.well-known/core
2022-04-27 18:06:12,505 # offset 000: </fw>

2022-04-27 18:06:22,593 # > url get coap://[fd11::a8ec:e68b:f9ba:bdc8]/.well-known/core
2022-04-27 18:06:22,609 # offset 000: </fw>

2022-04-27 18:06:29,575 # > url get coap://[fdea:dbee:f::1]/.well-known/core
2022-04-27 18:06:29,590 # offset 000: </fw>

2022-04-27 21:03:26,429 # > url get coap://[2001:16b8:4536:a600:fec2:3dff:fe0b:d44f]/.well-known/core
2022-04-27 21:03:26,444 # offset 000: </fw>

And on the other interface, from Linux

% aiocoap-client coap://[2001:16b8:4536:a600:fec2:3dff:fe0b:d44f]/.well-known/core

application/link-format content was re-formatted
</fw>

On master we get -EPROTO from sock_udp_recv_buf_aux() because the response address does not match the request.

2022-04-27 18:09:47,333 # > url get coap://[fe80::a8ec:e68b:f9ba:bdc8%4]/.well-known/core
2022-04-27 18:09:47,348 # offset 000: </fw>

2022-04-27 18:09:52,285 # > url get coap://[fd11::a8ec:e68b:f9ba:bdc8]/.well-known/core
2022-04-27 18:09:52,299 # offset 000: </fw>

2022-04-27 18:09:55,108 # > url get coap://[fdea:dbee:f::1]/.well-known/core
[no response - error returned]

2022-04-27 21:05:19,315 # > url get coap://[2001:16b8:4536:a600:fec2:3dff:fe0b:d44f]/.well-known/core
[no response - error returned]

Let's try adding a second address to the Ethernet interface:

2022-04-27 21:08:45,396 # > ifconfig 6 add 2001:16b8:4536:a600::1234
2022-04-27 21:08:45,401 # success: added 2001:16b8:4536:a600::1234/64 to interface 6

2022-04-27 21:08:48,112 # > ifconfig 6
2022-04-27 21:08:48,116 # Iface  6  HWaddr: FC:C2:3D:0B:D4:4F 
2022-04-27 21:08:48,120 #           L2-PDU:1500  MTU:1492  HL:255  RTR  
2022-04-27 21:08:48,123 #           Source address length: 6
2022-04-27 21:08:48,125 #           Link type: wired
2022-04-27 21:08:48,131 #           inet6 addr: fe80::fec2:3dff:fe0b:d44f  scope: link  VAL
2022-04-27 21:08:48,138 #           inet6 addr: 2001:16b8:4536:a600:fec2:3dff:fe0b:d44f  scope: global  VAL
2022-04-27 21:08:48,144 #           inet6 addr: 2001:16b8:4536:a600::1234  scope: global  VAL
2022-04-27 21:08:48,147 #           inet6 group: ff02::2
2022-04-27 21:08:48,150 #           inet6 group: ff02::1
2022-04-27 21:08:48,153 #           inet6 group: ff02::1:ff0b:d44f
2022-04-27 21:08:48,157 #           inet6 group: ff02::1:ff00:1234

(with CFLAGS += -DCONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF=3)

% ping 2001:16b8:4536:a600::1234      :(
PING 2001:16b8:4536:a600::1234(2001:16b8:4536:a600::1234) 56 data bytes
64 bytes from 2001:16b8:4536:a600::1234: icmp_seq=1 ttl=255 time=0.921 ms
64 bytes from 2001:16b8:4536:a600::1234: icmp_seq=2 ttl=255 time=0.367 ms
^C
--- 2001:16b8:4536:a600::1234 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 1001ms
rtt min/avg/max/mdev = 0.367/0.644/0.921/0.277 ms

% aiocoap-client coap://[2001:16b8:4536:a600::1234]/.well-known/core

WARNING:coap:Received Type.ACK from <UDP6EndpointAddress [2001:16b8:4536:a600:fec2:3dff:fe0b:d44f] (locally 2001:16b8:4536:a600:feb8:2c03:3621:69ed%eno1)>, but could not match it to a running exchange.
WARNING:coap:Received Type.ACK from <UDP6EndpointAddress [2001:16b8:4536:a600:fec2:3dff:fe0b:d44f] (locally 2001:16b8:4536:a600:feb8:2c03:3621:69ed%eno1)>, but could not match it to a running exchange.
^C%                                                                                                     3 benpicco@rechenknecht2k11 ~/dev/RIOT (git)-[gcoap_add_fix] % 

Issues/PRs references

@benpicco benpicco requested review from chrysn, cgundogan, maribu and miri64 and removed request for haukepetersen, miri64, cgundogan and PeterKietzmann April 27, 2022 16:07
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Apr 27, 2022
maribu
maribu previously approved these changes Apr 27, 2022
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. The change is very un-scary and I trust the provided testing

@maribu maribu enabled auto-merge April 27, 2022 16:47
@benpicco 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 27, 2022
@chrysn
Copy link
Member

chrysn commented Apr 27, 2022

Hm, doesn't changing the sock->local.addr modify the whole local socket (affecting future packets to the other address)? I thought that the outgoing message would need to use the aux interface as well.

@OlegHahm
Copy link
Member

I'm confused - why is the source IP address not set by the network layer correctly?

@maribu maribu disabled auto-merge April 27, 2022 18:14
@chrysn
Copy link
Member

chrysn commented Apr 27, 2022 via email

@maribu
Copy link
Member

maribu commented Apr 27, 2022

Sounds like we need to be able to select the address to send from via am argument instead. I think it should be possible via the aux struct as well.

But personally, I think the behaviour here is fine. IMO code that previously relied on a specific IP address being used for sending from an unbound socket is buggy. (But I have to grant that it is indeed difficult to write correct code for sending from an unbound socket when the sender address matters, though.)

@OlegHahm
Copy link
Member

Because it's sending a message from an unbound listener to a peer w/o knowing it's a response.

Does this matter? In my understanding https://www.ietf.org/rfc/rfc6724.txt describes how to select the source address for outgoing IPv6 packets - unless an upper layer protocol has other requirements. So, I don't really understand why CoAP should have special requirements here.

@benpicco
Copy link
Contributor Author

benpicco commented Apr 27, 2022

It's not CoAP that has this requirement but the sock API.

@OlegHahm
Copy link
Member

It's not CoAP that has this requirement but the sock API.

That smells like a design issue with sock then. Why should the transport layer API set the IP source address?

@benpicco
Copy link
Contributor Author

benpicco commented Apr 27, 2022

Well, aiocoap-client on Linux also doesn't like responses from a different address:

add a second address to the Ethernet interface

2022-04-27 21:08:45,396 # > ifconfig 6 add 2001:16b8:4536:a600::1234
2022-04-27 21:08:45,401 # success: added 2001:16b8:4536:a600::1234/64 to interface 6

2022-04-27 21:08:48,112 # > ifconfig 6
2022-04-27 21:08:48,116 # Iface  6  HWaddr: FC:C2:3D:0B:D4:4F 
2022-04-27 21:08:48,120 #           L2-PDU:1500  MTU:1492  HL:255  RTR  
2022-04-27 21:08:48,123 #           Source address length: 6
2022-04-27 21:08:48,125 #           Link type: wired
2022-04-27 21:08:48,131 #           inet6 addr: fe80::fec2:3dff:fe0b:d44f  scope: link  VAL
2022-04-27 21:08:48,138 #           inet6 addr: 2001:16b8:4536:a600:fec2:3dff:fe0b:d44f  scope: global  VAL
2022-04-27 21:08:48,144 #           inet6 addr: 2001:16b8:4536:a600::1234  scope: global  VAL
2022-04-27 21:08:48,147 #           inet6 group: ff02::2
2022-04-27 21:08:48,150 #           inet6 group: ff02::1
2022-04-27 21:08:48,153 #           inet6 group: ff02::1:ff0b:d44f
2022-04-27 21:08:48,157 #           inet6 group: ff02::1:ff00:1234

ping works

% ping -c 3 2001:16b8:4536:a600::1234   
PING 2001:16b8:4536:a600::1234(2001:16b8:4536:a600::1234) 56 data bytes
64 bytes from 2001:16b8:4536:a600::1234: icmp_seq=1 ttl=255 time=0.349 ms
64 bytes from 2001:16b8:4536:a600::1234: icmp_seq=2 ttl=255 time=0.322 ms
64 bytes from 2001:16b8:4536:a600::1234: icmp_seq=3 ttl=255 time=0.298 ms

--- 2001:16b8:4536:a600::1234 ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2056ms
rtt min/avg/max/mdev = 0.298/0.323/0.349/0.020 ms

CoAP request fails

% aiocoap-client coap://[2001:16b8:4536:a600::1234]/.well-known/core

WARNING:coap:Received Type.ACK from <UDP6EndpointAddress [2001:16b8:4536:a600:fec2:3dff:fe0b:d44f] (locally 2001:16b8:4536:a600:feb8:2c03:3621:69ed%eno1)>, but could not match it to a running exchange.
WARNING:coap:Received Type.ACK from <UDP6EndpointAddress [2001:16b8:4536:a600:fec2:3dff:fe0b:d44f] (locally 2001:16b8:4536:a600:feb8:2c03:3621:69ed%eno1)>, but could not match it to a running exchange.
^C                                                                                  

with this patch

% aiocoap-client coap://[2001:16b8:4536:a600::1234]/.well-known/core

application/link-format content was re-formatted
</fw>

@maribu
Copy link
Member

maribu commented Apr 27, 2022

A CoAP server really has to respond with the IP address and port it received the request on, except for request's received on multicast IP addresses, in which case the response must be send from a unicast address instead.

Otherwise a client would be even more exposed to spoofing attacks than it already is with CoAP over plain UDP.

@chrysn
Copy link
Member

chrysn commented Apr 27, 2022

That smells like a design issue with sock then. Why should the transport layer API set the IP source address?

It's not a fundamental issue with our sock API, it's a problem with every sock API -- and the solution is straightforward (and we have it, as does POSIX with its RCVPKTINFO).

CoAP thinks in terms of requests and responses, which is something sockets don't do, they either connect/bind or send arbitrary unrelated messages. A different API like TAPS might do this better, but that's a long way until that gets established, used widely, and I don't even know if it really works for embedded.

But be that as it is, I think this can be as easy as what Ben proposed, just by setting the address in aux after extending the aux handler in sock_udp_sendv_aux to use the local address from there.

@OlegHahm
Copy link
Member

@chrysn, I'm not sure I'm familiar with RCVPKTINFO in POSIX. Do you have a link?

Just to get this straight: a UDP client (in this case a CoAP client) sends a request from address A to a CoAP server listening at address X. The server application sends a response to X. In the current implementation IPv6 would perform the source address selection according to RFC 6724 and may end up to choose a different IP address (B) that is also configured for one of its interfaces because it produces a better match to X than A. Correct?

@OlegHahm
Copy link
Member

In the current implementation IPv6 would perform the source address selection according to RFC 6724 and may end up to choose a different IP address (B) that is also configured for one of its interfaces because it produces a better match to X than A.

It doesn't have to be a better match, both matches can be equally good - in this case, two global addresses. Which one is chosen depends then entirely on the order in which they were assigned.

As far as I remember RFC 6724 always determines an order. There are no equally good matches. And the order of assignment is completely irrelevant. Otherwise we have a bug in the implementation of this RFC.

@benpicco
Copy link
Contributor Author

There are no equally good matches. And the order of assignment is completely irrelevant. Otherwise we have a bug in the implementation of this RFC.

If you have 2001:db8::1 and 2001:db8::2 how are they ordered?
Also this is completely irrelevant here: The IP layer has no idea that an outgoing packet was the response to any incoming packet and thus can not make that choice if the address was unspecified by the application.

@chrysn
Copy link
Member

chrysn commented Apr 28, 2022 via email

@OlegHahm
Copy link
Member

If you have 2001:db8::1 and 2001:db8::2 how are they ordered?

Check https://datatracker.ietf.org/doc/html/rfc6724#section-5 but in most realistic use cases you won't have two differing host IDs for the same network (unless you're using privacy extensions, but then the selection should also be clear).

Also this is completely irrelevant here: The IP layer has no idea that an outgoing packet was the response to any incoming packet and thus can not make that choice if the address was unspecified by the application.

Well, the network layer does make a choice - just apparently the wrong one in some cases.

@OlegHahm
Copy link
Member

Okay, thanks for the pointer @chrysn. I think I do now understand the problem and agree with the proposed solution.

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

see inline

sys/net/application_layer/gcoap/gcoap.c Outdated Show resolved Hide resolved
sys/net/gnrc/sock/udp/gnrc_sock_udp.c Outdated Show resolved Hide resolved
@maribu maribu dismissed their stale review May 5, 2022 07:29

stale

@maribu
Copy link
Member

maribu commented May 10, 2022

Code looks good to me. Please squash!

Have you tested this? Otherwise I'd test first and ACK afterwards.

@benpicco
Copy link
Contributor Author

Still works:

> ifconfig 5 add 2001:16b8:45c2:fc00::1234
2022-05-10 09:22:12,151 # ifconfig 5 add 2001:16b8:45c2:fc00::1234
2022-05-10 09:22:12,156 # success: added 2001:16b8:45c2:fc00::1234/64 to interface 5

> ifconfig
2022-05-10 09:25:30,836 # Iface  5  HWaddr: FC:C2:3D:23:22:DF 
2022-05-10 09:25:30,842 #           L2-PDU:1500  MTU:1492  HL:255  Source address length: 6
2022-05-10 09:25:30,844 #           Link type: wired
2022-05-10 09:25:30,850 #           inet6 addr: fe80::fec2:3dff:fe23:22df  scope: link  VAL
2022-05-10 09:25:30,857 #           inet6 addr: 2001:16b8:45c2:fc00:fec2:3dff:fe23:22df  scope: global  VAL
2022-05-10 09:25:30,863 #           inet6 addr: 2001:16b8:45c2:fc00::1234  scope: global  VAL
2022-05-10 09:25:30,866 #           inet6 group: ff02::1
2022-05-10 09:25:30,870 #           inet6 group: ff02::1:ff23:22df
2022-05-10 09:25:30,873 #           inet6 group: ff02::1:ff00:1234
$ aiocoap-client coap://[2001:16b8:45c2:fc00:fec2:3dff:fe23:22df]/.well-known/core
application/link-format content was re-formatted
</cli/stats>; ct=0; rt=count; obs,
</riot/board>

$ aiocoap-client coap://[2001:16b8:45c2:fc00::1234]/.well-known/core
application/link-format content was re-formatted
</cli/stats>; ct=0; rt=count; obs,
</riot/board>

$ aiocoap-client coap://[fe80::fec2:3dff:fe23:22df%eno1]/.well-known/core 
application/link-format content was re-formatted
</cli/stats>; ct=0; rt=count; obs,
</riot/board>

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. Code looks good, trusting your testing.

@maribu maribu enabled auto-merge May 10, 2022 07:37
@maribu maribu merged commit eadd282 into RIOT-OS:master May 10, 2022
@benpicco benpicco deleted the gcoap_add_fix branch May 10, 2022 08:58
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
bors bot added a commit that referenced this pull request Feb 13, 2023
18257: drivers/wdt: add periph_wdt_auto_start for early watchdog r=benpicco a=benpicco



19272: gcoap: Do not send responses from multicast addresses r=chrysn a=chrysn

### Contribution description

Since #18026, CoAP requests to multicast addresses (eg. `ff02::1`) came back from that exact address, which Linux rightfully just drops.

The fix uses the existing multicast check from #17978 (thanks `@benpicco` for making me write this as dedicated function, I just had to generalize it removing one struct layer), and foregoes setting the source address when responding to multicasts.

### Testing procedure

* Run the gcoap example
* Send a CoAP request to a multicast address RIOT listens to, eg. `./aiocoap-client coap://'[ff02::1%tapbr0]'/.well-known/core --non`

Before, this got no response (while you see it arrive on wireshark). After, you get a correct response with two lines of note:

```
WARNING:coap:Sending request to multicast via unicast request method
Response arrived from different address; base URI is coap://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core
```

(The former is aiocoap telling us that we're not using the nonexistent multicast API so it's really more of an anycast, the latter is useful factual information).

Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: chrysn <chrysn@fsfe.org>
bors bot added a commit that referenced this pull request Feb 14, 2023
19272: gcoap: Do not send responses from multicast addresses r=benpicco a=chrysn

### Contribution description

Since #18026, CoAP requests to multicast addresses (eg. `ff02::1`) came back from that exact address, which Linux rightfully just drops.

The fix uses the existing multicast check from #17978 (thanks `@benpicco` for making me write this as dedicated function, I just had to generalize it removing one struct layer), and foregoes setting the source address when responding to multicasts.

### Testing procedure

* Run the gcoap example
* Send a CoAP request to a multicast address RIOT listens to, eg. `./aiocoap-client coap://'[ff02::1%tapbr0]'/.well-known/core --non`

Before, this got no response (while you see it arrive on wireshark). After, you get a correct response with two lines of note:

```
WARNING:coap:Sending request to multicast via unicast request method
Response arrived from different address; base URI is coap://[fe80::3c63:beff:fe85:ca96%tapbr0]/.well-known/core
```

(The former is aiocoap telling us that we're not using the nonexistent multicast API so it's really more of an anycast, the latter is useful factual information).

Co-authored-by: chrysn <chrysn@fsfe.org>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants