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

Check IPv6 address after processing HBH #861

Merged
merged 4 commits into from
Dec 23, 2023

Conversation

thvdveld
Copy link
Contributor

@thvdveld thvdveld commented Nov 28, 2023

Now, the destination address of the IP packet is checked if it is part of the interface addresses or the multicast addresses, only after processing the hop-by-hop header, as this header must be processed by every node.

  1. I modified the has_multicast_group function to also check IPv6 multicast addresses, such as the ALL_NODES, ALL_RPL_NODES (only when using RPL) and the IPv6 solicited address.
  2. For some HBH options, an ICMPv6 parameter problem must be transmitted RFC2460 Section 4.2. In some cases even when the destination address is a multicast address. Therefore also did point (3).
  3. ICMPv6 messages were not transmitted when the destination address was a multicast address. I am not sure why we did it like that. When the destination address is multicast, I use the first IPv6 address from the interface. The selection of the correct IPv6 source address is something that still needs work in smoltcp.

RFC8200 Section 4 says the following about hop-by-hop headers:

Extension headers (except for the Hop-by-Hop Options header) are not
processed, inserted, or deleted by any node along a packet's delivery
path, until the packet reaches the node (or each of the set of nodes,
in the case of multicast) identified in the Destination Address field
of the IPv6 header.

Therefore, when an hop-by-hop extension is present, we should process it. Then we check if the packet reached its destination. If not, we discard the packet if we are not using a routing protocol. In case of RPL, we route the packet to the correct next node.

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (8a86a5a) 79.74% compared to head (1001586) 79.74%.

Files Patch % Lines
src/iface/interface/ipv6.rs 86.66% 10 Missing ⚠️
src/iface/interface/mod.rs 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #861      +/-   ##
==========================================
- Coverage   79.74%   79.74%   -0.01%     
==========================================
  Files          78       78              
  Lines       28131    28165      +34     
==========================================
+ Hits        22433    22459      +26     
- Misses       5698     5706       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@whitequark
Copy link
Contributor

@thvdveld Unfortunately this PR deals squarely with the bits of IPv6 I do not have a good understanding of.

@thvdveld thvdveld requested review from Dirbaio and removed request for whitequark November 28, 2023 12:24
@thvdveld
Copy link
Contributor Author

@whitequark no problem!

@thvdveld thvdveld requested a review from a team November 29, 2023 09:19
@thvdveld thvdveld force-pushed the check-ipv6-address branch 2 times, most recently from b2bc796 to c69e04b Compare December 1, 2023 12:39
has_multicast_group was only used for IPv4 with IGMP. However, this is
also needed for IPv6, where we check the ALL_NODES, ALL_RPL_NODES and
the solicited multicast addresses.

In case of IPv4, when the `proto-igmp` feature is not enabled, this
function will return false.
With IPv6, the hop-by-hop option should be checked allong every node on
the route. Only then the IPv6 destination address is checked.

ip(hbh): return Param Problem for some options

For some options, a ICMP parameter problem needs to be transmitted back
to the source.
@Dirbaio Dirbaio added this pull request to the merge queue Dec 23, 2023
Merged via the queue into smoltcp-rs:main with commit f08e687 Dec 23, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants