From 499f6680a3afd600fdd8f2d003ea533334b4741f Mon Sep 17 00:00:00 2001 From: whd <7058128+superwhd@users.noreply.github.com> Date: Fri, 4 Aug 2023 01:04:40 +0800 Subject: [PATCH] [netdata] fix the source address check in `LeaderBase::RouteLookup()` (#9335) In `LeaderBase::RouteLookup()`, OT currently checks the source address of the packet to ensure it matches any of the Prefix TLV in leader's netdata. Actually it should also verify that the Prefix TLV has a Border Router sub-TLV. In the current implementation, OT may wrongly send/forward a packet to BR when its source address matches with a Prefix TLV which only contains an External Route sub-TLV. This lets BR accidentally forward packets from Thread to infra network. For example, a packet from Mesh-Local address to On-Link address will be wrongly forwarded to infra network. I recently noticed this problem because we're now using either `fc00:/7` or `::/0` for external routes in netdata, which always matches Mesh-Local source addresses and makes the issue more obvious. --- src/core/thread/network_data_leader.cpp | 5 +++++ tests/scripts/thread-cert/border_router/test_firewall.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/core/thread/network_data_leader.cpp b/src/core/thread/network_data_leader.cpp index 351ea69a7..5171aa664 100644 --- a/src/core/thread/network_data_leader.cpp +++ b/src/core/thread/network_data_leader.cpp @@ -244,6 +244,11 @@ Error LeaderBase::RouteLookup(const Ip6::Address &aSource, const Ip6::Address &a while ((prefixTlv = FindNextMatchingPrefixTlv(aSource, prefixTlv)) != nullptr) { + if (prefixTlv->FindSubTlv() == nullptr) + { + continue; + } + if (ExternalRouteLookup(prefixTlv->GetDomainId(), aDestination, aRloc16) == kErrorNone) { ExitNow(error = kErrorNone); diff --git a/tests/scripts/thread-cert/border_router/test_firewall.py b/tests/scripts/thread-cert/border_router/test_firewall.py index c1bd9ad67..7201035dd 100755 --- a/tests/scripts/thread-cert/border_router/test_firewall.py +++ b/tests/scripts/thread-cert/border_router/test_firewall.py @@ -178,6 +178,10 @@ class Firewall(thread_cert.TestCase): # 12. Host pings MA1 from router1's MLE-ID. self.assertFalse(host_ping_ether(MA1, ttl=10, interface=router1.get_mleid(), add_interface=True)) + # 13. Router1 pings Host from router1's MLE-ID. + self.assertFalse( + router1.ping(host.get_ip6_address(config.ADDRESS_TYPE.ONLINK_ULA)[0], interface=router1.get_mleid())) + self.collect_ipaddrs() self.collect_rlocs() self.collect_rloc16s() @@ -265,6 +269,10 @@ class Firewall(thread_cert.TestCase): pkts.filter_wpan_src64( vars['BR_1']).filter_AMPLFMA().filter_ping_request(identifier=_pkt.icmpv6.echo.identifier).must_not_next() + # 13. Router1 pings Host from router1's MLE-ID. + pkts.filter_eth_src(vars['BR_1_ETH']).filter_ipv6_src_dst( + vars['Router_1_MLEID'], vars['Host_BGUA']).filter_ping_request().must_not_next() + if __name__ == '__main__': unittest.main()