[ip6] filter host originated multicast packets #9735
Conversation
Size Report of OpenThread
|
tests/scripts/thread-cert/backbone/test_mlr_multicast_routing_self_originated_packet.py
Outdated
Show resolved
Hide resolved
src/core/net/ip6.cpp
Outdated
| VerifyOrExit((header.GetDestination().IsMulticastLargerThanRealmLocal() && | ||
| Get<BackboneRouter::MulticastListenersTable>().HasListener(header.GetDestination())), | ||
| error = kErrorDrop); |
There was a problem hiding this comment.
This seems dropping all multicast packets within realm local scope. Is this desired behavior? My understanding is that the filter should only apply to packets destined to multicast scope greater than realm local.
There was a problem hiding this comment.
I think when untrusted source is sending to multicast scope <= realm-local, then we should filter it so misbehaving applications can't flood the thread network. The application can still communicate with Thread network with subscribed multicast addresses, similar to the other devices on the backbone interface, their packet sent to multicast address <= realm-local scope are not forwarded to Thread network.
There was a problem hiding this comment.
I'm inclined not to dropping such packets. If such packets are dropped, that means we cannot run ping command targeting ff02::1 on Thread interface on a Linux based BR. That's kind of confusing to me.
tests/scripts/thread-cert/backbone/test_mlr_multicast_routing_self_originated_packet.py
Outdated
Show resolved
Hide resolved
tests/scripts/thread-cert/backbone/test_mlr_multicast_routing_self_originated_packet.py
Outdated
Show resolved
Hide resolved
tests/scripts/thread-cert/backbone/test_mlr_multicast_routing_self_originated_packet.py
Outdated
Show resolved
Hide resolved
sunytt
force-pushed
the
filter-multicast-from-host
branch
from
5aba64d
to
cff5312
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9735 +/- ##
==========================================
+ Coverage 85.68% 85.70% +0.01%
==========================================
Files 555 556 +1
Lines 73829 73969 +140
==========================================
+ Hits 63262 63395 +133
- Misses 10567 10574 +7
|
sunytt
force-pushed
the
filter-multicast-from-host
branch
2 times, most recently
from
6b3c0ec
to
e0f1686
Compare
sunytt
force-pushed
the
filter-multicast-from-host
branch
from
e0f1686
to
0b17b23
Compare
| @@ -1068,6 +1072,26 @@ Error Ip6::SendRaw(OwnedPtr<Message> aMessagePtr) | |||
|
|
|||
| if (header.GetDestination().IsMulticast()) | |||
| { | |||
| #if OPENTHREAD_CONFIG_FILTER_LOCAL_ORIGINATED_MULTICAST_PACKETS | |||
There was a problem hiding this comment.
Should we add a new API to control the behavior at runtime? I think this is a security feature, and should be controlled by upper layer.
There was a problem hiding this comment.
I'll update this PR to add API control for it.
| VerifyOrExit(forwardThread, error = kErrorDrop); | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
With this feature we can decide not to forward this message over the thread mesh (to other nodes) which sounds reasonable. But the VerifyOrExit() check here, we also disallow it to be received by the device itself (since we skip calling HandleDatagram()).
What if this device itself is subscribed to the destination of the multicast message? Do we need to receive/process this multicast? Or should we drop it here as the desired behavior? (I don't have any answer/suggestion myself, just raising it so we consider and think about this situation)
Related to this, we have otMessageSetMulticastLoopEnabled() which allows caller to inidcate whether we want to allow multicast loops.
If the goal is to only disallow forwarding over thread mesh, we can consider moving this new code block to HandleDatagram() method instead?
There was a problem hiding this comment.
Letting otMessageSetMulticastLoopEnabled() decide if multicast loops are allowed for host originated packets sounds good to me, I'll update this PR to move the code to HandleDatagram().
This PR adds the possibility to filter multicast packets sent from host directly to its thread TUN interface with following rules:
This is to avoid potential multicast flooding into the low power and low data rate Thread network by malicious application on the device.
This feature can be enabled with
OPENTHREAD_CONFIG_FILTER_LOCAL_ORIGINATED_MULTICAST_PACKETS. It's disabled by default, and should be enabled on open environment, e.g. Android.Note that when this feature is enabled, ping command from host to Thread network link-local/realm-local multicast addresses will not work, to test connectivity the ot cli
pingcommand can be used to reach Thread multicast addresses.