From 02acc480dd1b64133ced5a2757fd1a3a8ea71bdd Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 14 May 2024 14:28:02 -0700 Subject: [PATCH] [routing-manager] ignore checksum in RA hash calculation (#10230) This commit modifies the `CalculateHash()` method to use a zero checksum value for RA (ICMPv6) header for both received and emitted RA message. In prepared RA messages, the checksum is always set to zero, and the platform layer is responsible for calculating and updating it. For a received RA, while platforms typically zero out the checksum after validation, this behavior isn't explicitly required by `otPlatInfraIf` APIs. By ignoring the checksum(setting it to zero) during hash calculation, this change ensures correct calculation regardless of platform behavior. This commit also updates `test_routing_manager` to intentionally modify the checksum field in an emitted RA message before passing it back to the OT stack. This validates the updated hash calculation behavior. --- src/core/border_router/routing_manager.cpp | 17 +++++++++++------ src/core/border_router/routing_manager.hpp | 4 ++-- src/core/net/nd6.hpp | 17 ++++++++++++++++- tests/unit/test_routing_manager.cpp | 10 ++++++++-- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 75a3be9ca..c0750c774 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -3503,10 +3503,10 @@ void RoutingManager::RaInfo::IncrementTxCountAndSaveHash(const InfraIf::Icmp6Pac mLastHashIndex = 0; } - CalculateHash(aRaMessage, mHashes[mLastHashIndex]); + CalculateHash(RouterAdvert::RxMessage(aRaMessage), mHashes[mLastHashIndex]); } -bool RoutingManager::RaInfo::IsRaFromManager(const Ip6::Nd::RouterAdvert::RxMessage &aRaMessage) const +bool RoutingManager::RaInfo::IsRaFromManager(const RouterAdvert::RxMessage &aRaMessage) const { // Determines whether or not a received RA message was prepared by // by `RoutingManager` itself (is present in the saved `mHashes`). @@ -3516,7 +3516,7 @@ bool RoutingManager::RaInfo::IsRaFromManager(const Ip6::Nd::RouterAdvert::RxMess uint32_t count = Min(mTxCount, kNumHashEntries); Hash hash; - CalculateHash(aRaMessage.GetAsPacket(), hash); + CalculateHash(aRaMessage, hash); for (; count > 0; count--) { @@ -3541,12 +3541,17 @@ bool RoutingManager::RaInfo::IsRaFromManager(const Ip6::Nd::RouterAdvert::RxMess return isFromManager; } -void RoutingManager::RaInfo::CalculateHash(const InfraIf::Icmp6Packet &aRaMessage, Hash &aHash) +void RoutingManager::RaInfo::CalculateHash(const RouterAdvert::RxMessage &aRaMessage, Hash &aHash) { - Crypto::Sha256 sha256; + RouterAdvert::Header header; + Crypto::Sha256 sha256; + + header = aRaMessage.GetHeader(); + header.SetChecksum(0); sha256.Start(); - sha256.Update(aRaMessage.GetBytes(), aRaMessage.GetLength()); + sha256.Update(header); + sha256.Update(aRaMessage.GetOptionStart(), aRaMessage.GetOptionLength()); sha256.Finish(aHash); } diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index 16f4601d4..a814b5452 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -1259,8 +1259,8 @@ private: } void IncrementTxCountAndSaveHash(const InfraIf::Icmp6Packet &aRaMessage); - bool IsRaFromManager(const Ip6::Nd::RouterAdvert::RxMessage &aRaMessage) const; - static void CalculateHash(const InfraIf::Icmp6Packet &aRaMessage, Hash &aHash); + bool IsRaFromManager(const RouterAdvert::RxMessage &aRaMessage) const; + static void CalculateHash(const RouterAdvert::RxMessage &aRaMessage, Hash &aHash); RouterAdvert::Header mHeader; TimeMilli mHeaderUpdateTime; diff --git a/src/core/net/nd6.hpp b/src/core/net/nd6.hpp index 4178f181f..c4753b660 100644 --- a/src/core/net/nd6.hpp +++ b/src/core/net/nd6.hpp @@ -729,6 +729,22 @@ public: */ bool ContainsAnyOptions(void) const { return (mData.GetLength() > sizeof(Header)); } + /** + * Returns pointer to the start of option bytes (after header). + * + * @returns Pointer to start of options. + * + */ + const uint8_t *GetOptionStart(void) const { return (mData.GetBytes() + sizeof(Header)); } + + /** + * Gets the length (number of bytes) of options. + * + * @returns Number of bytes after header specifying RA options. + * + */ + uint16_t GetOptionLength(void) const { return ContainsAnyOptions() ? mData.GetLength() - sizeof(Header) : 0; } + // The following methods are intended to support range-based `for` // loop iteration over `Option`s in the RA message. @@ -736,7 +752,6 @@ public: Option::Iterator end(void) const { return Option::Iterator(); } private: - const uint8_t *GetOptionStart(void) const { return (mData.GetBytes() + sizeof(Header)); } const uint8_t *GetDataEnd(void) const { return mData.GetBytes() + mData.GetLength(); } Data mData; diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index 928a52b5c..74413e43d 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -295,7 +295,8 @@ otError otPlatInfraIfSendIcmp6Nd(uint32_t aInfraIfIndex, const uint8_t *aBuffer, uint16_t aBufferLength) { - Icmp6Packet packet; + Icmp6Packet packet; + Ip6::Icmp::Header *header; Log("otPlatInfraIfSendIcmp6Nd(aDestAddr: %s, aBufferLength:%u)", AsCoreType(aDestAddress).ToString().AsCString(), aBufferLength); @@ -306,7 +307,9 @@ otError otPlatInfraIfSendIcmp6Nd(uint32_t aInfraIfIndex, VerifyOrQuit(aBufferLength >= sizeof(Ip6::Icmp::Header)); - switch (reinterpret_cast(aBuffer)->GetType()) + header = reinterpret_cast(const_cast(aBuffer)); + + switch (header->GetType()) { case Ip6::Icmp::Header::kTypeRouterSolicit: Log(" Router Solicit message"); @@ -317,6 +320,9 @@ otError otPlatInfraIfSendIcmp6Nd(uint32_t aInfraIfIndex, Log(" Router Advertisement message"); LogRouterAdvert(packet); ValidateRouterAdvert(packet); + // Intentionally modify the checksum field in RA Header + // before passing it back to the OT stack. + header->SetChecksum(0x1234); otPlatInfraIfRecvIcmp6Nd(sInstance, kInfraIfIndex, &sInfraIfAddress, aBuffer, aBufferLength); break;