From 9a9083f20e9de099d153ed96e06d368719a6cd26 Mon Sep 17 00:00:00 2001 From: Zhanglong Xia Date: Tue, 25 Jan 2022 01:47:46 +0800 Subject: [PATCH 01/18] [posix] add `break` to case statement (#7349) --- src/posix/platform/trel_udp6.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/posix/platform/trel_udp6.cpp b/src/posix/platform/trel_udp6.cpp index babf524b135..894f2c42003 100644 --- a/src/posix/platform/trel_udp6.cpp +++ b/src/posix/platform/trel_udp6.cpp @@ -284,6 +284,7 @@ static void ReceiveNetlinkMessage(void) { otLogWarnPlat("netlink NLMSG_ERROR response: seq=%u, error=%d", header->nlmsg_seq, errMsg->error); } + break; } default: break; From 22bb1346fa9cbb462ba40b57bd37290c45c69a40 Mon Sep 17 00:00:00 2001 From: Simon Lin Date: Tue, 25 Jan 2022 01:48:40 +0800 Subject: [PATCH 02/18] [github-actions] add BR `--radio-version` test (#7345) --- .../border_router/test_rcp_radio_version.py | 70 +++++++++++++++++++ tests/scripts/thread-cert/node.py | 6 +- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100755 tests/scripts/thread-cert/border_router/test_rcp_radio_version.py diff --git a/tests/scripts/thread-cert/border_router/test_rcp_radio_version.py b/tests/scripts/thread-cert/border_router/test_rcp_radio_version.py new file mode 100755 index 00000000000..a8042ba2a9c --- /dev/null +++ b/tests/scripts/thread-cert/border_router/test_rcp_radio_version.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2021, The OpenThread Authors. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the +# names of its contributors may be used to endorse or promote products +# derived from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 'AS IS' +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# +import logging +import unittest + +import thread_cert + +# Test description: +# This test verifies `otbr-agent --radio-version` works. +# +# Topology: +# ----------------(eth)-------------------- +# | +# BR +# + +BR = 1 + + +class SingleBorderRouter(thread_cert.TestCase): + USE_MESSAGE_FACTORY = False + + TOPOLOGY = { + BR: { + 'name': 'BR', + 'is_otbr': True, + 'version': '1.2', + }, + } + + def test(self): + br = self.nodes[BR] + rcp_version = br.get_rcp_version() + logging.info('RCP version: %s', rcp_version) + br.stop_otbr_service() + + radio_version_output = br.bash("/usr/sbin/otbr-agent -d7 -v spinel+hdlc+uart:///dev/ttyUSB0 --radio-version") + logging.info("--radio-version output: %r", radio_version_output) + self.assertEqual(1, len(radio_version_output)) + self.assertEqual(rcp_version, radio_version_output[0].strip()) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py index 547b31b4166..98194fccd05 100755 --- a/tests/scripts/thread-cert/node.py +++ b/tests/scripts/thread-cert/node.py @@ -33,7 +33,6 @@ import os import re import socket -import string import subprocess import sys import time @@ -883,6 +882,11 @@ def set_bbr_registration_jitter(self, jitter): self.send_command(cmd) self._expect_done() + def get_rcp_version(self) -> str: + self.send_command('rcp version') + rcp_version = self._expect_command_output()[0].strip() + return rcp_version + def srp_server_get_state(self): states = ['disabled', 'running', 'stopped'] self.send_command('srp server state') From 4af585400e1412034ab65f4ad7cf6d6f064f288f Mon Sep 17 00:00:00 2001 From: JaneFromSilabs <58004715+JaneFromSilabs@users.noreply.github.com> Date: Mon, 24 Jan 2022 12:49:59 -0500 Subject: [PATCH 03/18] [thci] reset and multicast changes (#7340) - THCI changes for bbr reset procedure to make a better effort to reset and to also make sure to restart the otbr-agent prior to attempting a factoryreset to cover the case where the agent cannot currently communicate to the rcp. - THCI change to move from sudo service restart otbr-agent to sudo systemctl restart otbr-agent, which is more comprehensive. - THCI changes for multicast to use ipmaddr in both soc and bbr cases. --- tools/harness-thci/OpenThread.py | 13 +++++++- tools/harness-thci/OpenThread_BR.py | 46 ++++++++++++++--------------- 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/tools/harness-thci/OpenThread.py b/tools/harness-thci/OpenThread.py index bb4a98b9a73..a78eb6cef7a 100644 --- a/tools/harness-thci/OpenThread.py +++ b/tools/harness-thci/OpenThread.py @@ -1511,7 +1511,7 @@ def reset(self): start_time = time.time() while time.time() < start_time + timeout: - time.sleep(0.3) + time.sleep(0.5) if not self.IsBorderRouter: self._disconnect() self._connect() @@ -1519,6 +1519,10 @@ def reset(self): self.__executeCommand('state', timeout=0.1) break except Exception: + self.__restartAgentService() + time.sleep(2) + self.__sendCommand('factoryreset', expectEcho=False) + time.sleep(0.5) continue else: raise AssertionError("Could not connect with OT device {} after reset.".format(self)) @@ -3489,6 +3493,7 @@ def registerMulticast(self, sAddr='ff04::1234:777a:1', timeout=MLR_TIMEOUT_MIN): Args: sAddr : str : Multicast address to be subscribed and notified OTA. """ + self._beforeRegisterMulticast(sAddr, timeout) cmd = 'ipmaddr add ' + str(sAddr) @@ -3653,6 +3658,12 @@ def _deviceBeforeReset(self): def _deviceAfterReset(self): pass + def __restartAgentService(self): + pass + + def _beforeRegisterMulticast(self, sAddr, timeout): + pass + def __socRead(self, size=512): if self._is_net: return self.__handle.recv(size) diff --git a/tools/harness-thci/OpenThread_BR.py b/tools/harness-thci/OpenThread_BR.py index 4a04259a2fc..7df6b4defc6 100644 --- a/tools/harness-thci/OpenThread_BR.py +++ b/tools/harness-thci/OpenThread_BR.py @@ -319,7 +319,23 @@ def _deviceAfterReset(self): self.__dumpSyslog() self.__truncateSyslog() if not self.IsHost: - self.bash('sudo service otbr-agent restart') + self.bash('sudo systemctl restart otbr-agent') + time.sleep(2) + + def _beforeRegisterMulticast(self, sAddr='ff04::1234:777a:1', timeout=300): + """subscribe to the given ipv6 address (sAddr) in interface and send MLR.req OTA + + Args: + sAddr : str : Multicast address to be subscribed and notified OTA. + """ + + if self.externalCommissioner is not None: + self.externalCommissioner.MLR([sAddr], timeout) + return True + + cmd = 'sudo nohup ~/repo/openthread/tests/scripts/thread-cert/mcast6.py wpan0 %s' % sAddr + cmd = cmd + ' > /dev/null 2>&1 &' + self.bash(cmd) @API def setupHost(self, setDua=False): @@ -604,7 +620,10 @@ def __afterConnect(self): def __checkServiceStatus(self): self.bash('sudo service radvd stop') - self.bash('sudo service otbr-agent restart') + self.bash('sudo systemctl restart otbr-agent') + + def __restartAgentService(self): + self.bash('sudo systemctl restart otbr-agent') def __truncateSyslog(self): self.bash('sudo truncate -s 0 /var/log/syslog') @@ -691,14 +710,14 @@ def parse_cache(cache): @API def powerDown(self): self.log('Powering down BBR') - self.bash('sudo service otbr-agent stop') + self.bash('sudo systemctl stop otbr-agent') super(OpenThread_BR, self).powerDown() # Override powerUp @API def powerUp(self): self.log('Powering up BBR') - self.bash('sudo service otbr-agent start') + self.bash('sudo systemctl start otbr-agent') super(OpenThread_BR, self).powerUp() # Override forceSetSlaac @@ -707,25 +726,6 @@ def forceSetSlaac(self, slaacAddress): print('forceSetSlaac %s' % slaacAddress) self.bash('sudo ip -6 addr add %s/64 dev wpan0' % slaacAddress) - # Override registerMulticast - @API - def registerMulticast(self, sAddr='ff04::1234:777a:1', timeout=300): - """subscribe to the given ipv6 address (sAddr) in interface and send MLR.req OTA - - Args: - sAddr : str : Multicast address to be subscribed and notified OTA. - """ - - if self.externalCommissioner is not None: - self.externalCommissioner.MLR([sAddr], timeout) - return True - - cmd = 'sudo nohup ~/repo/openthread/tests/scripts/thread-cert/mcast6.py wpan0 %s' % sAddr - cmd = cmd + ' > /dev/null 2>&1 &' - self.bash(cmd) - - return True - # Override stopListeningToAddr @API def stopListeningToAddr(self, sAddr): From 9c6dc87ab7035e3fb09576391aa5caf4040d9d42 Mon Sep 17 00:00:00 2001 From: Eduardo Montoya Date: Mon, 24 Jan 2022 18:55:24 +0100 Subject: [PATCH 04/18] [bbr] fix `bbr skipseqnuminc` command response (#7354) `bbr skipseqnuminc` should complete without error. --- src/cli/cli.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 4825fca92bb..5fbdfcd3e0b 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -577,7 +577,7 @@ otError Interpreter::ProcessBackboneRouter(Arg aArgs[]) else if (aArgs[0] == "skipseqnuminc") { otBackboneRouterConfigSkipSeqNumIncrease(GetInstancePtr(), true); - ExitNow(); + ExitNow(error = OT_ERROR_NONE); } #endif SuccessOrExit(error = ProcessBackboneRouterLocal(aArgs)); From ca3dd08f26466174c9fdf31a1b6cfb12eea9447e Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 24 Jan 2022 10:07:19 -0800 Subject: [PATCH 05/18] [mac] smaller style changes (#7341) This commit contain smaller style changes in `Mac` and `SubMac` --- src/core/mac/mac.cpp | 12 ++++-------- src/core/mac/mac.hpp | 2 +- src/core/mac/sub_mac.cpp | 20 ++++++++++---------- src/core/mac/sub_mac.hpp | 6 +++--- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index 4819f46dc39..50073ebc918 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -951,7 +951,6 @@ void Mac::ProcessTransmitSecurity(TxFrame &aFrame) default: OT_ASSERT(false); - OT_UNREACHABLE_CODE(break); } #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE @@ -1062,7 +1061,6 @@ void Mac::BeginTransmit(void) default: OT_ASSERT(false); - OT_UNREACHABLE_CODE(break); } #if OPENTHREAD_CONFIG_TIME_SYNC_ENABLE @@ -1402,7 +1400,7 @@ void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError) aError = mTxError; } -#endif +#endif // OPENTHREAD_CONFIG_MULTI_RADIO // Determine next action based on current operation. @@ -1494,14 +1492,14 @@ void Mac::HandleTransmitDone(TxFrame &aFrame, RxFrame *aAckFrame, Error aError) Get().HandleSentFrame(aFrame, aError); PerformNextOperation(); break; -#endif +#endif // OPENTHREAD_FTD default: OT_ASSERT(false); - OT_UNREACHABLE_CODE(ExitNow()); // Added to suppress "unused label exit" warning (in TREL radio only). - OT_UNREACHABLE_CODE(break); } + ExitNow(); // Added to suppress "unused label exit" warning (in TREL radio only). + exit: return; } @@ -1548,7 +1546,6 @@ void Mac::HandleTimer(void) default: OT_ASSERT(false); - OT_UNREACHABLE_CODE(break); } } @@ -1644,7 +1641,6 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig default: ExitNow(); - OT_UNREACHABLE_CODE(break); } SuccessOrExit(aFrame.ProcessReceiveAesCcm(*extAddress, *macKey)); diff --git a/src/core/mac/mac.hpp b/src/core/mac/mac.hpp index b345b823466..1276fb09600 100644 --- a/src/core/mac/mac.hpp +++ b/src/core/mac/mac.hpp @@ -936,7 +936,7 @@ class Mac : public InstanceLocator, private NonCopyable #if OPENTHREAD_CONFIG_MAC_FILTER_ENABLE Filter mFilter; -#endif // OPENTHREAD_CONFIG_MAC_FILTER_ENABLE +#endif KeyMaterial mMode2KeyMaterial; }; diff --git a/src/core/mac/sub_mac.cpp b/src/core/mac/sub_mac.cpp index aacef704be9..0b8fbe71da0 100644 --- a/src/core/mac/sub_mac.cpp +++ b/src/core/mac/sub_mac.cpp @@ -318,7 +318,7 @@ void SubMac::HandleReceiveDone(RxFrame *aFrame, Error aError) static_cast(aFrame->mInfo.mRxInfo.mTimestamp) - mCslSampleTime.GetValue()); #endif } -#endif +#endif // OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE #if OPENTHREAD_CONFIG_MAC_FILTER_ENABLE if (!mRadioFilterEnabled) @@ -413,7 +413,7 @@ void SubMac::ProcessTransmitSecurity(void) void SubMac::StartCsmaBackoff(void) { uint32_t backoff; - uint32_t backoffExponent = kMinBE + mTransmitRetries + mCsmaBackoffs; + uint32_t backoffExponent = kCsmaMinBe + mTransmitRetries + mCsmaBackoffs; #if !OPENTHREAD_MTD && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE if (mTransmitFrame.mInfo.mTxInfo.mTxDelay != 0) @@ -446,9 +446,9 @@ void SubMac::StartCsmaBackoff(void) VerifyOrExit(ShouldHandleCsmaBackOff(), BeginTransmit()); - if (backoffExponent > kMaxBE) + if (backoffExponent > kCsmaMaxBe) { - backoffExponent = kMaxBE; + backoffExponent = kCsmaMaxBe; } backoff = Random::NonCrypto::GetUint32InRange(0, static_cast(1UL << backoffExponent)); @@ -477,8 +477,6 @@ void SubMac::BeginTransmit(void) { Error error; - OT_UNUSED_VARIABLE(error); - #if !OPENTHREAD_MTD && OPENTHREAD_CONFIG_MAC_CSL_TRANSMITTER_ENABLE VerifyOrExit(mState == kStateCsmaBackoff || mState == kStateCslTransmit); #else @@ -498,12 +496,14 @@ void SubMac::BeginTransmit(void) } error = Get().Transmit(mTransmitFrame); + if (error == kErrorInvalidState && mTransmitFrame.mInfo.mTxInfo.mTxDelay > 0) { // Platform `transmit_at` fails and we send the frame directly. mTransmitFrame.mInfo.mTxInfo.mTxDelay = 0; mTransmitFrame.mInfo.mTxInfo.mTxDelayBaseTime = 0; - error = Get().Transmit(mTransmitFrame); + + error = Get().Transmit(mTransmitFrame); } SuccessOrAssert(error); @@ -1128,7 +1128,7 @@ void SubMac::HandleCslTimer(void) } } -void SubMac::GetCslWindowEdges(uint32_t &ahead, uint32_t &after) +void SubMac::GetCslWindowEdges(uint32_t &aAhead, uint32_t &aAfter) { uint32_t semiPeriod = mCslPeriod * kUsPerTenSymbols / 2; uint32_t curTime = static_cast(otPlatRadioGetNow(&GetInstance())); @@ -1140,8 +1140,8 @@ void SubMac::GetCslWindowEdges(uint32_t &ahead, uint32_t &after) semiWindow = elapsed * (Get().GetCslAccuracy() + mCslParentAccuracy) / 1000000; semiWindow += mCslParentUncert * kUsPerUncertUnit; - ahead = (semiWindow + kCslReceiveTimeAhead > semiPeriod) ? semiPeriod : semiWindow + kCslReceiveTimeAhead; - after = (semiWindow + kMinCslWindow > semiPeriod) ? semiPeriod : semiWindow + kMinCslWindow; + aAhead = (semiWindow + kCslReceiveTimeAhead > semiPeriod) ? semiPeriod : semiWindow + kCslReceiveTimeAhead; + aAfter = (semiWindow + kMinCslWindow > semiPeriod) ? semiPeriod : semiWindow + kMinCslWindow; } #endif // OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE diff --git a/src/core/mac/sub_mac.hpp b/src/core/mac/sub_mac.hpp index e761b8fb099..7155fa13ce4 100644 --- a/src/core/mac/sub_mac.hpp +++ b/src/core/mac/sub_mac.hpp @@ -575,11 +575,11 @@ class SubMac : public InstanceLocator, private NonCopyable #if OPENTHREAD_CONFIG_MAC_CSL_RECEIVER_ENABLE static void HandleCslTimer(Timer &aTimer); void HandleCslTimer(void); - void GetCslWindowEdges(uint32_t &ahead, uint32_t &after); + void GetCslWindowEdges(uint32_t &aAhead, uint32_t &aAfter); #endif - static constexpr uint8_t kMinBE = 3; // macMinBE (IEEE 802.15.4-2006). - static constexpr uint8_t kMaxBE = 5; // macMaxBE (IEEE 802.15.4-2006). + static constexpr uint8_t kCsmaMinBe = 3; // macMinBE (IEEE 802.15.4-2006). + static constexpr uint8_t kCsmaMaxBe = 5; // macMaxBE (IEEE 802.15.4-2006). static constexpr uint32_t kUnitBackoffPeriod = 20; // Number of symbols (IEEE 802.15.4-2006). static constexpr uint32_t kAckTimeout = 16; // Timeout for waiting on an ACK (in msec). static constexpr uint32_t kCcaSampleInterval = 128; // CCA sample interval, 128 usec. From 3e214bd17a6f98b3c5bd069de57609d7e7650182 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 24 Jan 2022 10:10:58 -0800 Subject: [PATCH 06/18] [cli] inline simple sub-command process methods (#7347) --- src/cli/cli.cpp | 387 +++++++++++++++++------------------------------- src/cli/cli.hpp | 17 --- 2 files changed, 136 insertions(+), 268 deletions(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 5fbdfcd3e0b..23e3638a22a 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -2029,35 +2029,6 @@ otError Interpreter::ProcessIfconfig(Arg aArgs[]) return error; } -otError Interpreter::ProcessIpAddrAdd(Arg aArgs[]) -{ - otError error; - otNetifAddress aAddress; - - SuccessOrExit(error = aArgs[0].ParseAsIp6Address(aAddress.mAddress)); - aAddress.mPrefixLength = 64; - aAddress.mPreferred = true; - aAddress.mValid = true; - aAddress.mAddressOrigin = OT_ADDRESS_ORIGIN_MANUAL; - - error = otIp6AddUnicastAddress(GetInstancePtr(), &aAddress); - -exit: - return error; -} - -otError Interpreter::ProcessIpAddrDel(Arg aArgs[]) -{ - otError error; - otIp6Address address; - - SuccessOrExit(error = aArgs[0].ParseAsIp6Address(address)); - error = otIp6RemoveUnicastAddress(GetInstancePtr(), &address); - -exit: - return error; -} - const char *Interpreter::AddressOriginToString(uint8_t aOrigin) { static const char *const kOriginStrings[4] = { @@ -2104,11 +2075,22 @@ otError Interpreter::ProcessIpAddr(Arg aArgs[]) } else if (aArgs[0] == "add") { - error = ProcessIpAddrAdd(aArgs + 1); + otNetifAddress address; + + SuccessOrExit(error = aArgs[1].ParseAsIp6Address(address.mAddress)); + address.mPrefixLength = 64; + address.mPreferred = true; + address.mValid = true; + address.mAddressOrigin = OT_ADDRESS_ORIGIN_MANUAL; + + error = otIp6AddUnicastAddress(GetInstancePtr(), &address); } else if (aArgs[0] == "del") { - error = ProcessIpAddrDel(aArgs + 1); + otIp6Address address; + + SuccessOrExit(error = aArgs[1].ParseAsIp6Address(address)); + error = otIp6RemoveUnicastAddress(GetInstancePtr(), &address); } else if (aArgs[0] == "linklocal") { @@ -2127,57 +2109,6 @@ otError Interpreter::ProcessIpAddr(Arg aArgs[]) error = OT_ERROR_INVALID_COMMAND; } - return error; -} - -otError Interpreter::ProcessIpMulticastAddrAdd(Arg aArgs[]) -{ - otIp6Address address; - -#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - otError error = OT_ERROR_INVALID_ARGS; - for (Arg *arg = &aArgs[0]; !arg->IsEmpty(); arg++) - { - SuccessOrExit(error = arg->ParseAsIp6Address(address)); - SuccessOrExit(error = otIp6SubscribeMulticastAddress(GetInstancePtr(), &address)); - } -#else - otError error; - SuccessOrExit(error = aArgs[0].ParseAsIp6Address(address)); - error = otIp6SubscribeMulticastAddress(GetInstancePtr(), &address); -#endif -exit: - return error; -} - -otError Interpreter::ProcessIpMulticastAddrDel(Arg aArgs[]) -{ - otError error; - otIp6Address address; - - SuccessOrExit(error = aArgs[0].ParseAsIp6Address(address)); - error = otIp6UnsubscribeMulticastAddress(GetInstancePtr(), &address); - -exit: - return error; -} - -otError Interpreter::ProcessMulticastPromiscuous(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; - - if (aArgs[0].IsEmpty()) - { - OutputEnabledDisabledStatus(otIp6IsMulticastPromiscuousEnabled(GetInstancePtr())); - } - else - { - bool enable; - - SuccessOrExit(error = ParseEnableOrDisable(aArgs[0], enable)); - otIp6SetMulticastPromiscuousEnabled(GetInstancePtr(), enable); - } - exit: return error; } @@ -2196,15 +2127,41 @@ otError Interpreter::ProcessIpMulticastAddr(Arg aArgs[]) } else if (aArgs[0] == "add") { - error = ProcessIpMulticastAddrAdd(aArgs + 1); + aArgs++; + + do + { + otIp6Address address; + + SuccessOrExit(error = aArgs->ParseAsIp6Address(address)); + SuccessOrExit(error = otIp6SubscribeMulticastAddress(GetInstancePtr(), &address)); + } +#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE + while (!(++aArgs)->IsEmpty()); +#else + while (false); +#endif } else if (aArgs[0] == "del") { - error = ProcessIpMulticastAddrDel(aArgs + 1); + otIp6Address address; + + SuccessOrExit(error = aArgs[1].ParseAsIp6Address(address)); + error = otIp6UnsubscribeMulticastAddress(GetInstancePtr(), &address); } else if (aArgs[0] == "promiscuous") { - error = ProcessMulticastPromiscuous(aArgs + 1); + if (aArgs[1].IsEmpty()) + { + OutputEnabledDisabledStatus(otIp6IsMulticastPromiscuousEnabled(GetInstancePtr())); + } + else + { + bool enable; + + SuccessOrExit(error = ParseEnableOrDisable(aArgs[1], enable)); + otIp6SetMulticastPromiscuousEnabled(GetInstancePtr(), enable); + } } else if (aArgs[0] == "llatn") { @@ -2219,6 +2176,7 @@ otError Interpreter::ProcessIpMulticastAddr(Arg aArgs[]) error = OT_ERROR_INVALID_COMMAND; } +exit: return error; } @@ -2750,44 +2708,38 @@ otError Interpreter::ProcessMlr(Arg aArgs[]) if (aArgs[0] == "reg") { - error = ProcessMlrReg(aArgs + 1); - } - - return error; -} + otIp6Address addresses[kIp6AddressesNumMax]; + uint32_t timeout; + bool hasTimeout = false; + uint8_t numAddresses = 0; -otError Interpreter::ProcessMlrReg(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; - otIp6Address addresses[kIp6AddressesNumMax]; - uint32_t timeout; - bool hasTimeout = false; - uint8_t numAddresses = 0; - - while (aArgs->ParseAsIp6Address(addresses[numAddresses]) == OT_ERROR_NONE) - { aArgs++; - numAddresses++; - if (numAddresses == OT_ARRAY_LENGTH(addresses)) + while (aArgs->ParseAsIp6Address(addresses[numAddresses]) == OT_ERROR_NONE) { - break; + aArgs++; + numAddresses++; + + if (numAddresses == OT_ARRAY_LENGTH(addresses)) + { + break; + } } - } - if (aArgs->ParseAsUint32(timeout) == OT_ERROR_NONE) - { - aArgs++; - hasTimeout = true; - } + if (aArgs->ParseAsUint32(timeout) == OT_ERROR_NONE) + { + aArgs++; + hasTimeout = true; + } - VerifyOrExit(aArgs->IsEmpty() && (numAddresses > 0), error = OT_ERROR_INVALID_ARGS); + VerifyOrExit(aArgs->IsEmpty() && (numAddresses > 0), error = OT_ERROR_INVALID_ARGS); - SuccessOrExit(error = otIp6RegisterMulticastListeners(GetInstancePtr(), addresses, numAddresses, - hasTimeout ? &timeout : nullptr, - Interpreter::HandleMlrRegResult, this)); + SuccessOrExit(error = otIp6RegisterMulticastListeners(GetInstancePtr(), addresses, numAddresses, + hasTimeout ? &timeout : nullptr, + Interpreter::HandleMlrRegResult, this)); - error = OT_ERROR_PENDING; + error = OT_ERROR_PENDING; + } exit: return error; @@ -3614,70 +3566,42 @@ otError Interpreter::ParsePrefix(Arg aArgs[], otBorderRouterConfig &aConfig) return error; } -otError Interpreter::ProcessPrefixAdd(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; - otBorderRouterConfig config; - - SuccessOrExit(error = ParsePrefix(aArgs, config)); - error = otBorderRouterAddOnMeshPrefix(GetInstancePtr(), &config); - -exit: - return error; -} - -otError Interpreter::ProcessPrefixRemove(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; - otIp6Prefix prefix; - - SuccessOrExit(error = aArgs[0].ParseAsIp6Prefix(prefix)); - - error = otBorderRouterRemoveOnMeshPrefix(GetInstancePtr(), &prefix); - -exit: - return error; -} - -otError Interpreter::ProcessPrefixList(void) -{ - otNetworkDataIterator iterator = OT_NETWORK_DATA_ITERATOR_INIT; - otBorderRouterConfig config; - - while (otBorderRouterGetNextOnMeshPrefix(GetInstancePtr(), &iterator, &config) == OT_ERROR_NONE) - { - mNetworkData.OutputPrefix(config); - } - -#if OPENTHREAD_FTD && OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE - if (otBackboneRouterGetState(GetInstancePtr()) == OT_BACKBONE_ROUTER_STATE_DISABLED) - { - SuccessOrExit(otBackboneRouterGetDomainPrefix(GetInstancePtr(), &config)); - OutputFormat("- "); - mNetworkData.OutputPrefix(config); - } - // Else already printed via above while loop. -exit: -#endif - - return OT_ERROR_NONE; -} - otError Interpreter::ProcessPrefix(Arg aArgs[]) { otError error = OT_ERROR_NONE; if (aArgs[0].IsEmpty()) { - error = ProcessPrefixList(); + otNetworkDataIterator iterator = OT_NETWORK_DATA_ITERATOR_INIT; + otBorderRouterConfig config; + + while (otBorderRouterGetNextOnMeshPrefix(GetInstancePtr(), &iterator, &config) == OT_ERROR_NONE) + { + mNetworkData.OutputPrefix(config); + } + +#if OPENTHREAD_FTD && OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE + if (otBackboneRouterGetState(GetInstancePtr()) == OT_BACKBONE_ROUTER_STATE_DISABLED) + { + SuccessOrExit(otBackboneRouterGetDomainPrefix(GetInstancePtr(), &config)); + OutputFormat("- "); + mNetworkData.OutputPrefix(config); + } +#endif } else if (aArgs[0] == "add") { - error = ProcessPrefixAdd(aArgs + 1); + otBorderRouterConfig config; + + SuccessOrExit(error = ParsePrefix(aArgs + 1, config)); + error = otBorderRouterAddOnMeshPrefix(GetInstancePtr(), &config); } else if (aArgs[0] == "remove") { - error = ProcessPrefixRemove(aArgs + 1); + otIp6Prefix prefix; + + SuccessOrExit(error = aArgs[1].ParseAsIp6Prefix(prefix)); + error = otBorderRouterRemoveOnMeshPrefix(GetInstancePtr(), &prefix); } else if (aArgs[0] == "meshlocal") { @@ -3688,6 +3612,7 @@ otError Interpreter::ProcessPrefix(Arg aArgs[]) error = OT_ERROR_INVALID_COMMAND; } +exit: return error; } #endif // OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE @@ -3822,65 +3747,40 @@ otError Interpreter::ParseRoute(Arg aArgs[], otExternalRouteConfig &aConfig) return error; } -otError Interpreter::ProcessRouteAdd(Arg aArgs[]) -{ - otError error; - otExternalRouteConfig config; - - SuccessOrExit(error = ParseRoute(aArgs, config)); - error = otBorderRouterAddRoute(GetInstancePtr(), &config); - -exit: - return error; -} - -otError Interpreter::ProcessRouteRemove(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; - otIp6Prefix prefix; - - SuccessOrExit(error = aArgs[0].ParseAsIp6Prefix(prefix)); - - error = otBorderRouterRemoveRoute(GetInstancePtr(), &prefix); - -exit: - return error; -} - -otError Interpreter::ProcessRouteList(void) -{ - otNetworkDataIterator iterator = OT_NETWORK_DATA_ITERATOR_INIT; - otExternalRouteConfig config; - - while (otBorderRouterGetNextRoute(GetInstancePtr(), &iterator, &config) == OT_ERROR_NONE) - { - mNetworkData.OutputRoute(config); - } - - return OT_ERROR_NONE; -} - otError Interpreter::ProcessRoute(Arg aArgs[]) { otError error = OT_ERROR_NONE; if (aArgs[0].IsEmpty()) { - error = ProcessRouteList(); + otNetworkDataIterator iterator = OT_NETWORK_DATA_ITERATOR_INIT; + otExternalRouteConfig config; + + while (otBorderRouterGetNextRoute(GetInstancePtr(), &iterator, &config) == OT_ERROR_NONE) + { + mNetworkData.OutputRoute(config); + } } else if (aArgs[0] == "add") { - error = ProcessRouteAdd(aArgs + 1); + otExternalRouteConfig config; + + SuccessOrExit(error = ParseRoute(aArgs + 1, config)); + error = otBorderRouterAddRoute(GetInstancePtr(), &config); } else if (aArgs[0] == "remove") { - error = ProcessRouteRemove(aArgs + 1); + otIp6Prefix prefix; + + SuccessOrExit(error = aArgs[1].ParseAsIp6Prefix(prefix)); + error = otBorderRouterRemoveRoute(GetInstancePtr(), &prefix); } else { error = OT_ERROR_INVALID_COMMAND; } +exit: return error; } #endif // OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE @@ -4690,64 +4590,49 @@ otError Interpreter::ProcessMac(Arg aArgs[]) if (aArgs[0] == "retries") { - error = ProcessMacRetries(aArgs + 1); + if (aArgs[1] == "direct") + { + error = ProcessGetSet(aArgs + 2, otLinkGetMaxFrameRetriesDirect, otLinkSetMaxFrameRetriesDirect); + } +#if OPENTHREAD_FTD + else if (aArgs[1] == "indirect") + { + error = ProcessGetSet(aArgs + 2, otLinkGetMaxFrameRetriesIndirect, otLinkSetMaxFrameRetriesIndirect); + } +#endif + else + { + error = OT_ERROR_INVALID_ARGS; + } } #if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE else if (aArgs[0] == "send") { - error = ProcessMacSend(aArgs + 1); - } -#endif - else - { - error = OT_ERROR_INVALID_COMMAND; - } - - return error; -} - -otError Interpreter::ProcessMacRetries(Arg aArgs[]) -{ - otError error = OT_ERROR_NONE; + VerifyOrExit(aArgs[2].IsEmpty(), error = OT_ERROR_INVALID_ARGS); - if (aArgs[0] == "direct") - { - error = ProcessGetSet(aArgs + 1, otLinkGetMaxFrameRetriesDirect, otLinkSetMaxFrameRetriesDirect); - } -#if OPENTHREAD_FTD - else if (aArgs[0] == "indirect") - { - error = ProcessGetSet(aArgs + 1, otLinkGetMaxFrameRetriesIndirect, otLinkSetMaxFrameRetriesIndirect); + if (aArgs[1] == "datarequest") + { + error = otLinkSendDataRequest(GetInstancePtr()); + } + else if (aArgs[1] == "emptydata") + { + error = otLinkSendEmptyData(GetInstancePtr()); + } + else + { + error = OT_ERROR_INVALID_ARGS; + } } #endif else { - error = OT_ERROR_INVALID_ARGS; - } - - return error; -} - -#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE -otError Interpreter::ProcessMacSend(Arg aArgs[]) -{ - otError error = OT_ERROR_INVALID_ARGS; - - VerifyOrExit(aArgs[1].IsEmpty()); - - if (aArgs[0] == "datarequest") - { - error = otLinkSendDataRequest(GetInstancePtr()); - } - else if (aArgs[0] == "emptydata") - { - error = otLinkSendEmptyData(GetInstancePtr()); + error = OT_ERROR_INVALID_COMMAND; + ExitNow(); // To silence unused `exit` label warning when `REFERENCE_DEVICE_ENABLE` is not enabled. } exit: return error; } -#endif #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE otError Interpreter::ProcessTrel(Arg aArgs[]) diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index ebd71c7f1c6..2611cbafcb5 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -407,12 +407,7 @@ class Interpreter : public Output otError ProcessFem(Arg aArgs[]); otError ProcessIfconfig(Arg aArgs[]); otError ProcessIpAddr(Arg aArgs[]); - otError ProcessIpAddrAdd(Arg aArgs[]); - otError ProcessIpAddrDel(Arg aArgs[]); otError ProcessIpMulticastAddr(Arg aArgs[]); - otError ProcessIpMulticastAddrAdd(Arg aArgs[]); - otError ProcessIpMulticastAddrDel(Arg aArgs[]); - otError ProcessMulticastPromiscuous(Arg aArgs[]); #if OPENTHREAD_CONFIG_JOINER_ENABLE otError ProcessJoiner(Arg aArgs[]); #endif @@ -446,8 +441,6 @@ class Interpreter : public Output #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_TMF_PROXY_MLR_ENABLE && OPENTHREAD_CONFIG_COMMISSIONER_ENABLE otError ProcessMlr(Arg aArgs[]); - otError ProcessMlrReg(Arg aArgs[]); - static void HandleMlrRegResult(void * aContext, otError aError, uint8_t aMlrStatus, @@ -497,9 +490,6 @@ class Interpreter : public Output otError ProcessPollPeriod(Arg aArgs[]); #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE otError ProcessPrefix(Arg aArgs[]); - otError ProcessPrefixAdd(Arg aArgs[]); - otError ProcessPrefixRemove(Arg aArgs[]); - otError ProcessPrefixList(void); #endif otError ProcessPromiscuous(Arg aArgs[]); #if OPENTHREAD_FTD @@ -519,9 +509,6 @@ class Interpreter : public Output #endif #if OPENTHREAD_CONFIG_BORDER_ROUTER_ENABLE otError ProcessRoute(Arg aArgs[]); - otError ProcessRouteAdd(Arg aArgs[]); - otError ProcessRouteRemove(Arg aArgs[]); - otError ProcessRouteList(void); #endif #if OPENTHREAD_FTD otError ProcessRouter(Arg aArgs[]); @@ -558,10 +545,6 @@ class Interpreter : public Output otError ProcessMacFilterRss(Arg aArgs[]); #endif otError ProcessMac(Arg aArgs[]); - otError ProcessMacRetries(Arg aArgs[]); -#if OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE - otError ProcessMacSend(Arg aArgs[]); -#endif #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE otError ProcessTrel(Arg aArgs[]); #endif From 3243897cd1af7788ecbcaf0326ed1f438bc9bb3c Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 24 Jan 2022 10:13:17 -0800 Subject: [PATCH 07/18] [cli] add helper method to output a `otMacFilterEntry` (#7348) This commit also adds `MacFilterAddressModeToString()`. --- src/cli/cli.cpp | 97 +++++++++++++++++++++---------------------------- src/cli/cli.hpp | 10 +++-- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 23e3638a22a..5370b7595ac 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -4347,34 +4347,14 @@ otError Interpreter::ProcessMacFilter(Arg aArgs[]) void Interpreter::PrintMacFilter(void) { - otMacFilterEntry entry; - otMacFilterIterator iterator = OT_MAC_FILTER_ITERATOR_INIT; - otMacFilterAddressMode mode = otLinkFilterGetAddressMode(GetInstancePtr()); + otMacFilterEntry entry; + otMacFilterIterator iterator = OT_MAC_FILTER_ITERATOR_INIT; - if (mode == OT_MAC_FILTER_ADDRESS_MODE_DISABLED) - { - OutputLine("Address Mode: Disabled"); - } - else if (mode == OT_MAC_FILTER_ADDRESS_MODE_ALLOWLIST) - { - OutputLine("Address Mode: Allowlist"); - } - else if (mode == OT_MAC_FILTER_ADDRESS_MODE_DENYLIST) - { - OutputLine("Address Mode: Denylist"); - } + OutputLine("Address Mode: %s", MacFilterAddressModeToString(otLinkFilterGetAddressMode(GetInstancePtr()))); while (otLinkFilterGetNextAddress(GetInstancePtr(), &iterator, &entry) == OT_ERROR_NONE) { - OutputExtAddress(entry.mExtAddress); - - if (entry.mRssIn != OT_MAC_FILTER_FIXED_RSS_DISABLED) - { - OutputFormat(" : rss %d (lqi %d)", entry.mRssIn, - otLinkConvertRssToLinkQuality(GetInstancePtr(), entry.mRssIn)); - } - - OutputLine(""); + OutputMacFilterEntry(entry); } iterator = OT_MAC_FILTER_ITERATOR_INIT; @@ -4399,47 +4379,26 @@ void Interpreter::PrintMacFilter(void) } else { - OutputExtAddress(entry.mExtAddress); - OutputLine(" : rss %d (lqi %d)", entry.mRssIn, - otLinkConvertRssToLinkQuality(GetInstancePtr(), entry.mRssIn)); + OutputMacFilterEntry(entry); } } } otError Interpreter::ProcessMacFilterAddress(Arg aArgs[]) { - otError error = OT_ERROR_NONE; - otExtAddress extAddr; - otMacFilterEntry entry; - otMacFilterIterator iterator = OT_MAC_FILTER_ITERATOR_INIT; - otMacFilterAddressMode mode = otLinkFilterGetAddressMode(GetInstancePtr()); + otError error = OT_ERROR_NONE; + otExtAddress extAddr; if (aArgs[0].IsEmpty()) { - if (mode == OT_MAC_FILTER_ADDRESS_MODE_DISABLED) - { - OutputLine("Disabled"); - } - else if (mode == OT_MAC_FILTER_ADDRESS_MODE_ALLOWLIST) - { - OutputLine("Allowlist"); - } - else if (mode == OT_MAC_FILTER_ADDRESS_MODE_DENYLIST) - { - OutputLine("Denylist"); - } + otMacFilterIterator iterator = OT_MAC_FILTER_ITERATOR_INIT; + otMacFilterEntry entry; + + OutputLine("%s", MacFilterAddressModeToString(otLinkFilterGetAddressMode(GetInstancePtr()))); while (otLinkFilterGetNextAddress(GetInstancePtr(), &iterator, &entry) == OT_ERROR_NONE) { - OutputExtAddress(entry.mExtAddress); - - if (entry.mRssIn != OT_MAC_FILTER_FIXED_RSS_DISABLED) - { - OutputFormat(" : rss %d (lqi %d)", entry.mRssIn, - otLinkConvertRssToLinkQuality(GetInstancePtr(), entry.mRssIn)); - } - - OutputLine(""); + OutputMacFilterEntry(entry); } } else if (aArgs[0] == "disable") @@ -4519,9 +4478,7 @@ otError Interpreter::ProcessMacFilterRss(Arg aArgs[]) } else { - OutputExtAddress(entry.mExtAddress); - OutputLine(" : rss %d (lqi %d)", entry.mRssIn, - otLinkConvertRssToLinkQuality(GetInstancePtr(), entry.mRssIn)); + OutputMacFilterEntry(entry); } } } @@ -4582,6 +4539,34 @@ otError Interpreter::ProcessMacFilterRss(Arg aArgs[]) return error; } +void Interpreter::OutputMacFilterEntry(const otMacFilterEntry &aEntry) +{ + OutputExtAddress(aEntry.mExtAddress); + + if (aEntry.mRssIn != OT_MAC_FILTER_FIXED_RSS_DISABLED) + { + OutputFormat(" : rss %d (lqi %d)", aEntry.mRssIn, + otLinkConvertRssToLinkQuality(GetInstancePtr(), aEntry.mRssIn)); + } + + OutputLine(""); +} + +const char *Interpreter::MacFilterAddressModeToString(otMacFilterAddressMode aMode) +{ + static const char *const kModeStrings[] = { + "Disabled", // (0) OT_MAC_FILTER_ADDRESS_MODE_DISABLED + "Allowlist", // (1) OT_MAC_FILTER_ADDRESS_MODE_ALLOWLIST + "Denylist", // (2) OT_MAC_FILTER_ADDRESS_MODE_DENYLIST + }; + + static_assert(0 == OT_MAC_FILTER_ADDRESS_MODE_DISABLED, "OT_MAC_FILTER_ADDRESS_MODE_DISABLED value is incorrect"); + static_assert(1 == OT_MAC_FILTER_ADDRESS_MODE_ALLOWLIST, "OT_MAC_FILTER_ADDRESS_MODE_ALLOWLIST value is incorrect"); + static_assert(2 == OT_MAC_FILTER_ADDRESS_MODE_DENYLIST, "OT_MAC_FILTER_ADDRESS_MODE_DENYLIST value is incorrect"); + + return Stringify(aMode, kModeStrings); +} + #endif // OPENTHREAD_CONFIG_MAC_FILTER_ENABLE otError Interpreter::ProcessMac(Arg aArgs[]) diff --git a/src/cli/cli.hpp b/src/cli/cli.hpp index 2611cbafcb5..0f578472537 100644 --- a/src/cli/cli.hpp +++ b/src/cli/cli.hpp @@ -539,10 +539,12 @@ class Interpreter : public Output otError ProcessUptime(Arg aArgs[]); #endif #if OPENTHREAD_CONFIG_MAC_FILTER_ENABLE - otError ProcessMacFilter(Arg aArgs[]); - void PrintMacFilter(void); - otError ProcessMacFilterAddress(Arg aArgs[]); - otError ProcessMacFilterRss(Arg aArgs[]); + otError ProcessMacFilter(Arg aArgs[]); + void PrintMacFilter(void); + otError ProcessMacFilterAddress(Arg aArgs[]); + otError ProcessMacFilterRss(Arg aArgs[]); + void OutputMacFilterEntry(const otMacFilterEntry &aEntry); + static const char *MacFilterAddressModeToString(otMacFilterAddressMode aMode); #endif otError ProcessMac(Arg aArgs[]); #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE From b943b250abe8fe15f9052054e165768110fe7381 Mon Sep 17 00:00:00 2001 From: Jiacheng Guo Date: Tue, 25 Jan 2022 14:13:07 +0800 Subject: [PATCH 08/18] [ip6] always enable ICMP6 filtering when forwarding to Thread (#7343) This will prevent unexpected multicast packets on the Thread network. --- src/core/net/ip6.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/net/ip6.cpp b/src/core/net/ip6.cpp index 0fa07de962d..1fe8b7faf97 100644 --- a/src/core/net/ip6.cpp +++ b/src/core/net/ip6.cpp @@ -1269,7 +1269,7 @@ Error Ip6::HandleDatagram(Message &aMessage, Netif *aNetif, const void *aLinkMes hopLimit = header.GetHopLimit(); aMessage.Write(Header::kHopLimitFieldOffset, hopLimit); - if (aFromNcpHost && nextHeader == kProtoIcmp6) + if (nextHeader == kProtoIcmp6) { uint8_t icmpType; bool isAllowedType = false; From 3ef867602eef08a9bf3f60fb38db62a03e665cc0 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 25 Jan 2022 10:55:55 -0800 Subject: [PATCH 09/18] [address-resolver] add `LookUp()` method to check cache entries (#7350) This commit adds `AddressResolver::LookUp()` method which resolves an EID to an RLOC17 by checking if an existing entry already exists in the address cache table. --- src/core/backbone_router/bbr_manager.cpp | 9 +++------ src/core/thread/address_resolver.cpp | 18 +++++++++++++++--- src/core/thread/address_resolver.hpp | 21 ++++++++++++++++----- src/core/thread/mesh_forwarder_ftd.cpp | 8 +++----- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/core/backbone_router/bbr_manager.cpp b/src/core/backbone_router/bbr_manager.cpp index 666d2ee2f6d..397592244f8 100644 --- a/src/core/backbone_router/bbr_manager.cpp +++ b/src/core/backbone_router/bbr_manager.cpp @@ -508,9 +508,7 @@ NdProxyTable &Manager::GetNdProxyTable(void) bool Manager::ShouldForwardDuaToBackbone(const Ip6::Address &aAddress) { - bool forwardToBackbone = false; - Mac::ShortAddress rloc16; - Error error; + bool forwardToBackbone = false; VerifyOrExit(Get().IsPrimary()); VerifyOrExit(Get().IsDomainUnicast(aAddress)); @@ -519,9 +517,8 @@ bool Manager::ShouldForwardDuaToBackbone(const Ip6::Address &aAddress) VerifyOrExit(!mNdProxyTable.IsRegistered(aAddress.GetIid())); // Do not forward to Backbone if the DUA belongs to a MTD Child (which may have failed in DUA registration) VerifyOrExit(Get().FindNeighbor(aAddress) == nullptr); - // Forawrd to Backbone only if the DUA is resolved to the PBBR's RLOC16 - error = Get().Resolve(aAddress, rloc16, /* aAllowAddressQuery */ false); - VerifyOrExit(error == kErrorNone && rloc16 == Get().GetRloc16()); + // Forward to Backbone only if the DUA is resolved to the PBBR's RLOC16 + VerifyOrExit(Get().LookUp(aAddress) == Get().GetRloc16()); forwardToBackbone = true; diff --git a/src/core/thread/address_resolver.cpp b/src/core/thread/address_resolver.cpp index ad353aafe37..a843e311cff 100644 --- a/src/core/thread/address_resolver.cpp +++ b/src/core/thread/address_resolver.cpp @@ -459,6 +459,14 @@ void AddressResolver::RestartAddressQueries(void) } } +Mac::ShortAddress AddressResolver::LookUp(const Ip6::Address &aEid) +{ + Mac::ShortAddress rloc16 = Mac::kShortAddrInvalid; + + IgnoreError(Resolve(aEid, rloc16, /* aAllowAddressQuery */ false)); + return rloc16; +} + Error AddressResolver::Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRloc16, bool aAllowAddressQuery) { Error error = kErrorNone; @@ -474,6 +482,7 @@ Error AddressResolver::Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRlo // allocate a new entry and perform address query. We do not // allow first-time address query entries to be evicted till // timeout. + VerifyOrExit(aAllowAddressQuery, error = kErrorNotFound); entry = NewCacheEntry(/* aSnoopedEntry */ false); @@ -503,9 +512,12 @@ Error AddressResolver::Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRlo ExitNow(); } - // Note that if `aAllowAddressQuery` is `false` then the `entry` is definitely already in a list, i.e., we cannot - // not get here with `aAllowAddressQuery` being `false` and `entry` being a newly allocated one, due to the - // `VerifyOrExit` check that `aAllowAddressQuery` is `true` before allocating a new cache entry. + // Note that if `aAllowAddressQuery` is `false` then the `entry` + // is definitely already in a list, i.e., we cannot not get here + // with `aAllowAddressQuery` being `false` and `entry` being a + // newly allocated one, due to the `VerifyOrExit` check that + // `aAllowAddressQuery` is `true` before allocating a new cache + // entry. VerifyOrExit(aAllowAddressQuery, error = kErrorNotFound); if (list == &mQueryList) diff --git a/src/core/thread/address_resolver.hpp b/src/core/thread/address_resolver.hpp index 625fb4d96d7..5ce68e67e2e 100644 --- a/src/core/thread/address_resolver.hpp +++ b/src/core/thread/address_resolver.hpp @@ -145,21 +145,31 @@ class AddressResolver : public InstanceLocator, private NonCopyable void UpdateSnoopedCacheEntry(const Ip6::Address &aEid, Mac::ShortAddress aRloc16, Mac::ShortAddress aDest); /** - * This method returns the RLOC16 for a given EID, initiates an Address Query if allowed and the mapping is not - * known. + * This method returns the RLOC16 for a given EID, initiates an Address Query if the mapping is not known. * * @param[in] aEid A reference to the EID. * @param[out] aRloc16 The RLOC16 corresponding to @p aEid. - * @param[in] aAllowAddressQuery Allow to initiate Address Query if the mapping is not known. * * @retval kErrorNone Successfully provided the RLOC16. * @retval kErrorAddressQuery Initiated an Address Query if allowed. * @retval kErrorDrop Earlier Address Query for the EID timed out. In retry timeout interval. * @retval kErrorNoBufs Insufficient buffer space available to send Address Query. - * @retval kErrorNotFound The mapping was not found and Address Query was not allowed. * */ - Error Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRloc16, bool aAllowAddressQuery = true); + Error Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRloc16) + { + return Resolve(aEid, aRloc16, /* aAllowAddressQuery */ true); + } + + /** + * This method looks up the RLOC16 for a given EID in the address cache. + * + * @param[in] aEid A reference to the EID. + * + * @returns The RLOC16 mapping to @p aEid or `Mac::kShortAddrInvalid` if it is not found in the address cache. + * + */ + Mac::ShortAddress LookUp(const Ip6::Address &aEid); /** * This method restarts any ongoing address queries. @@ -295,6 +305,7 @@ class AddressResolver : public InstanceLocator, private NonCopyable CacheEntryPool &GetCacheEntryPool(void) { return mCacheEntryPool; } + Error Resolve(const Ip6::Address &aEid, Mac::ShortAddress &aRloc16, bool aAllowAddressQuery); void Remove(Mac::ShortAddress aRloc16, bool aMatchRouterId); void Remove(const Ip6::Address &aEid, Reason aReason); CacheEntry *FindCacheEntry(const Ip6::Address &aEid, CacheEntryList *&aList, CacheEntry *&aPrevEntry); diff --git a/src/core/thread/mesh_forwarder_ftd.cpp b/src/core/thread/mesh_forwarder_ftd.cpp index 5991a8c432b..f67edfcd50f 100644 --- a/src/core/thread/mesh_forwarder_ftd.cpp +++ b/src/core/thread/mesh_forwarder_ftd.cpp @@ -188,15 +188,13 @@ void MeshForwarder::HandleResolved(const Ip6::Address &aEid, Error aError) #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_ENABLE Error MeshForwarder::ForwardDuaToBackboneLink(Message &aMessage, const Ip6::Address &aDst) { - Error error = kErrorNone; - uint16_t dstRloc16; - uint8_t ttl; + Error error = kErrorNone; + uint8_t ttl; VerifyOrExit(Get().IsPrimary() && Get().IsDomainUnicast(aDst), error = kErrorNoRoute); - IgnoreError(Get().Resolve(aDst, dstRloc16, /* aAllowAddressQuery */ false)); - VerifyOrExit(dstRloc16 == Get().GetRloc16(), error = kErrorNoRoute); + VerifyOrExit(Get().LookUp(aDst) == Get().GetRloc16(), error = kErrorNoRoute); // Avoid decreasing TTL twice IgnoreError(aMessage.Read(Ip6::Header::kHopLimitFieldOffset, ttl)); From d4e041096f99b66449e133aa277f9fe14f005b01 Mon Sep 17 00:00:00 2001 From: Eduardo Montoya Date: Wed, 26 Jan 2022 03:20:36 +0100 Subject: [PATCH 10/18] [bbr] implement new Sequence Number increase rules (#7342) "BBR Sequence Number Updating" section in Thread Specification requires special wraps when increasing BBR Dataset Sequence Number. --- src/core/backbone_router/bbr_local.cpp | 30 ++++++++++++++++++++++---- src/core/backbone_router/bbr_local.hpp | 1 + tools/harness-thci/OpenThread.py | 9 ++++++-- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/core/backbone_router/bbr_local.cpp b/src/core/backbone_router/bbr_local.cpp index 9952583b288..862860fefbb 100644 --- a/src/core/backbone_router/bbr_local.cpp +++ b/src/core/backbone_router/bbr_local.cpp @@ -52,7 +52,7 @@ Local::Local(Instance &aInstance) , mState(OT_BACKBONE_ROUTER_STATE_DISABLED) , mMlrTimeout(Mle::kMlrTimeoutDefault) , mReregistrationDelay(Mle::kRegistrationDelayDefault) - , mSequenceNumber(Random::NonCrypto::GetUint8()) + , mSequenceNumber(Random::NonCrypto::GetUint8() % 127) , mRegistrationJitter(Mle::kBackboneRouterRegistrationJitter) , mIsServiceAdded(false) , mDomainPrefixCallback(nullptr) @@ -112,7 +112,7 @@ void Local::Reset(void) if (mState == OT_BACKBONE_ROUTER_STATE_PRIMARY) { // Increase sequence number when changing from Primary to Secondary. - mSequenceNumber++; + SequenceNumberIncrease(); Get().Signal(kEventThreadBackboneRouterLocalChanged); SetState(OT_BACKBONE_ROUTER_STATE_SECONDARY); } @@ -270,7 +270,7 @@ void Local::HandleBackboneRouterPrimaryUpdate(Leader::State aState, const Backbo { // Here original PBBR restores its Backbone Router Service from Thread Network, // Intentionally skips the state update as PBBR will refresh its service. - mSequenceNumber = aConfig.mSequenceNumber + 1; + mSequenceNumber = aConfig.mSequenceNumber; mReregistrationDelay = aConfig.mReregistrationDelay; mMlrTimeout = aConfig.mMlrTimeout; @@ -278,9 +278,13 @@ void Local::HandleBackboneRouterPrimaryUpdate(Leader::State aState, const Backbo if (mSkipSeqNumIncrease) { // BBR-TC-02 forces Sequence Number for the reference device with raw UDP API - mSequenceNumber = aConfig.mSequenceNumber; + // Do not increase Sequence Number in that case. } + else #endif + { + SequenceNumberIncrease(); + } Get().Signal(kEventThreadBackboneRouterLocalChanged); if (AddService(true /* Force registration to refresh and restore Primary state */) == kErrorNone) @@ -424,6 +428,24 @@ void Local::RemoveDomainPrefixFromNetworkData(void) LogDomainPrefix("Remove", error); } +void Local::SequenceNumberIncrease(void) +{ + switch (mSequenceNumber) + { + case 126: + case 127: + mSequenceNumber = 0; + break; + case 254: + case 255: + mSequenceNumber = 128; + break; + default: + mSequenceNumber++; + break; + } +} + void Local::AddDomainPrefixToNetworkData(void) { Error error = kErrorNotFound; // only used for logging. diff --git a/src/core/backbone_router/bbr_local.hpp b/src/core/backbone_router/bbr_local.hpp index f4878eab190..23c337a351f 100644 --- a/src/core/backbone_router/bbr_local.hpp +++ b/src/core/backbone_router/bbr_local.hpp @@ -273,6 +273,7 @@ class Local : public InstanceLocator, private NonCopyable void RemoveService(void); void AddDomainPrefixToNetworkData(void); void RemoveDomainPrefixFromNetworkData(void); + void SequenceNumberIncrease(void); #if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_INFO) && (OPENTHREAD_CONFIG_LOG_BBR == 1) void LogBackboneRouterService(const char *aAction, Error aError); void LogDomainPrefix(const char *aAction, Error aError); diff --git a/tools/harness-thci/OpenThread.py b/tools/harness-thci/OpenThread.py index a78eb6cef7a..95915ee43dc 100644 --- a/tools/harness-thci/OpenThread.py +++ b/tools/harness-thci/OpenThread.py @@ -1607,7 +1607,7 @@ def setDefaultValues(self): self.stopListeningToAddrAll() # BBR dataset - self.bbrSeqNum = random.randint(0, 254) # random seqnum except 255, so that BBR-TC-02 never need re-run + self.bbrSeqNum = random.randint(0, 126) # 5.21.4.2 self.bbrMlrTimeout = 3600 self.bbrReRegDelay = 5 @@ -3182,7 +3182,12 @@ def setBbrDataset(self, SeqNumInc=False, SeqNum=None, MlrTimeout=None, ReRegDela """ assert not (SeqNumInc and SeqNum is not None), "Must not specify both SeqNumInc and SeqNum" if SeqNumInc: - SeqNum = (self.bbrSeqNum + 1) % 256 + if self.bbrSeqNum in (126, 127): + self.bbrSeqNum = 0 + elif self.bbrSeqNum in (254, 255): + self.bbrSeqNum = 128 + else: + self.bbrSeqNum = (self.bbrSeqNum + 1) % 256 return self.__configBbrDataset(SeqNum=SeqNum, MlrTimeout=MlrTimeout, ReRegDelay=ReRegDelay) From 137c40136cab33eaa32066223f0a1b4838ed225d Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 25 Jan 2022 18:26:42 -0800 Subject: [PATCH 11/18] [commissioner] add `Dataset` and `ResignMode` (#7352) This commit adds `Commissioner::Dataset` class which mirrors the public `otCommissioningDataset`. It also adds `ResignMode` enum which is used as input to `Stop()` method to indicate whether or not to resign from being the commissioner. --- src/core/api/commissioner_api.cpp | 5 +- src/core/meshcop/commissioner.cpp | 55 +++++----- src/core/meshcop/commissioner.hpp | 173 +++++++++++++++++++++++++++--- 3 files changed, 184 insertions(+), 49 deletions(-) diff --git a/src/core/api/commissioner_api.cpp b/src/core/api/commissioner_api.cpp index 83eaf4b89cc..4f5aa4bccf3 100644 --- a/src/core/api/commissioner_api.cpp +++ b/src/core/api/commissioner_api.cpp @@ -52,7 +52,7 @@ otError otCommissionerStart(otInstance * aInstance, otError otCommissionerStop(otInstance *aInstance) { - return AsCoreType(aInstance).Get().Stop(/* aResign */ true); + return AsCoreType(aInstance).Get().Stop(); } otError otCommissionerAddJoiner(otInstance *aInstance, const otExtAddress *aEui64, const char *aPskd, uint32_t aTimeout) @@ -161,7 +161,8 @@ otError otCommissionerSendMgmtSet(otInstance * aInstance, const uint8_t * aTlvs, uint8_t aLength) { - return AsCoreType(aInstance).Get().SendMgmtCommissionerSetRequest(*aDataset, aTlvs, aLength); + return AsCoreType(aInstance).Get().SendMgmtCommissionerSetRequest(AsCoreType(aDataset), + aTlvs, aLength); } uint16_t otCommissionerGetSessionId(otInstance *aInstance) diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index d52d5f7416d..81b2d759047 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -129,8 +129,7 @@ void Commissioner::SignalJoinerEvent(JoinerEvent aEvent, const Joiner *aJoiner) noJoinerId = true; } - mJoinerCallback(static_cast(aEvent), &joinerInfo, noJoinerId ? nullptr : &joinerId, - mCallbackContext); + mJoinerCallback(MapEnum(aEvent), &joinerInfo, noJoinerId ? nullptr : &joinerId, mCallbackContext); exit: return; @@ -295,9 +294,7 @@ void Commissioner::RemoveJoinerEntry(Commissioner::Joiner &aJoiner) SignalJoinerEvent(kJoinerEventRemoved, &joinerCopy); } -Error Commissioner::Start(otCommissionerStateCallback aStateCallback, - otCommissionerJoinerCallback aJoinerCallback, - void * aCallbackContext) +Error Commissioner::Start(StateCallback aStateCallback, JoinerCallback aJoinerCallback, void *aCallbackContext) { Error error = kErrorNone; @@ -329,7 +326,7 @@ Error Commissioner::Start(otCommissionerStateCallback aStateCallback, return error; } -Error Commissioner::Stop(bool aResign) +Error Commissioner::Stop(ResignMode aResignMode) { Error error = kErrorNone; bool needResign = false; @@ -354,7 +351,7 @@ Error Commissioner::Stop(bool aResign) SetState(kStateDisabled); - if (needResign && aResign) + if (needResign && (aResignMode == kSendKeepAliveToResign)) { SendKeepAlive(); } @@ -402,18 +399,15 @@ void Commissioner::ComputeBloomFilter(SteeringData &aSteeringData) const void Commissioner::SendCommissionerSet(void) { - Error error = kErrorNone; - otCommissioningDataset dataset; + Error error = kErrorNone; + Dataset dataset; VerifyOrExit(mState == kStateActive, error = kErrorInvalidState); - memset(&dataset, 0, sizeof(dataset)); - - dataset.mSessionId = mSessionId; - dataset.mIsSessionIdSet = true; + dataset.Clear(); - ComputeBloomFilter(AsCoreType(&dataset.mSteeringData)); - dataset.mIsSteeringDataSet = true; + dataset.SetSessionId(mSessionId); + ComputeBloomFilter(dataset.UpdateSteeringData()); error = SendMgmtCommissionerSetRequest(dataset, nullptr, 0); @@ -745,9 +739,7 @@ void Commissioner::HandleMgmtCommissionerGetResponse(Coap::Message * aMe return; } -Error Commissioner::SendMgmtCommissionerSetRequest(const otCommissioningDataset &aDataset, - const uint8_t * aTlvs, - uint8_t aLength) +Error Commissioner::SendMgmtCommissionerSetRequest(const Dataset &aDataset, const uint8_t *aTlvs, uint8_t aLength) { Error error = kErrorNone; Coap::Message * message; @@ -758,25 +750,26 @@ Error Commissioner::SendMgmtCommissionerSetRequest(const otCommissioningDataset SuccessOrExit(error = message->InitAsConfirmablePost(UriPath::kCommissionerSet)); SuccessOrExit(error = message->SetPayloadMarker()); - if (aDataset.mIsLocatorSet) + if (aDataset.IsLocatorSet()) { - SuccessOrExit(error = Tlv::Append(*message, aDataset.mLocator)); + SuccessOrExit(error = Tlv::Append(*message, aDataset.GetLocator())); } - if (aDataset.mIsSessionIdSet) + if (aDataset.IsSessionIdSet()) { - SuccessOrExit(error = Tlv::Append(*message, aDataset.mSessionId)); + SuccessOrExit(error = Tlv::Append(*message, aDataset.GetSessionId())); } - if (aDataset.mIsSteeringDataSet) + if (aDataset.IsSteeringDataSet()) { - SuccessOrExit( - error = Tlv::Append(*message, aDataset.mSteeringData.m8, aDataset.mSteeringData.mLength)); + const SteeringData &steeringData = aDataset.GetSteeringData(); + + SuccessOrExit(error = Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); } - if (aDataset.mIsJoinerUdpPortSet) + if (aDataset.IsJoinerUdpPortSet()) { - SuccessOrExit(error = Tlv::Append(*message, aDataset.mJoinerUdpPort)); + SuccessOrExit(error = Tlv::Append(*message, aDataset.GetJoinerUdpPort())); } if (aLength > 0) @@ -876,7 +869,7 @@ void Commissioner::HandleLeaderPetitionResponse(Coap::Message * aMessage otLogInfoMeshCoP("received Leader Petition response"); SuccessOrExit(Tlv::Find(*aMessage, state)); - VerifyOrExit(state == StateTlv::kAccept, IgnoreError(Stop(/* aResign */ false))); + VerifyOrExit(state == StateTlv::kAccept, IgnoreError(Stop(kDoNotSendKeepAlive))); SuccessOrExit(Tlv::Find(*aMessage, mSessionId)); @@ -903,7 +896,7 @@ void Commissioner::HandleLeaderPetitionResponse(Coap::Message * aMessage { if (mTransmitAttempts >= kPetitionRetryCount) { - IgnoreError(Stop(/* aResign */ false)); + IgnoreError(Stop(kDoNotSendKeepAlive)); } else { @@ -965,12 +958,12 @@ void Commissioner::HandleLeaderKeepAliveResponse(Coap::Message * aMessag VerifyOrExit(mState == kStateActive); VerifyOrExit(aResult == kErrorNone && aMessage->GetCode() == Coap::kCodeChanged, - IgnoreError(Stop(/* aResign */ false))); + IgnoreError(Stop(kDoNotSendKeepAlive))); otLogInfoMeshCoP("received Leader keep-alive response"); SuccessOrExit(Tlv::Find(*aMessage, state)); - VerifyOrExit(state == StateTlv::kAccept, IgnoreError(Stop(/* aResign */ false))); + VerifyOrExit(state == StateTlv::kAccept, IgnoreError(Stop(kDoNotSendKeepAlive))); mTimer.Start(Time::SecToMsec(kKeepAliveTimeout) / 2); diff --git a/src/core/meshcop/commissioner.hpp b/src/core/meshcop/commissioner.hpp index e5c910a21cf..ea910609d62 100644 --- a/src/core/meshcop/commissioner.hpp +++ b/src/core/meshcop/commissioner.hpp @@ -43,6 +43,7 @@ #include "coap/coap.hpp" #include "coap/coap_secure.hpp" #include "common/as_core_type.hpp" +#include "common/clearable.hpp" #include "common/locator.hpp" #include "common/non_copyable.hpp" #include "common/timer.hpp" @@ -74,6 +75,150 @@ class Commissioner : public InstanceLocator, private NonCopyable kStateActive = OT_COMMISSIONER_STATE_ACTIVE, ///< Active Commissioner. }; + /** + * This enumeration type represents Joiner Event. + * + */ + enum JoinerEvent : uint8_t + { + kJoinerEventStart = OT_COMMISSIONER_JOINER_START, + kJoinerEventConnected = OT_COMMISSIONER_JOINER_CONNECTED, + kJoinerEventFinalize = OT_COMMISSIONER_JOINER_FINALIZE, + kJoinerEventEnd = OT_COMMISSIONER_JOINER_END, + kJoinerEventRemoved = OT_COMMISSIONER_JOINER_REMOVED, + }; + + typedef otCommissionerStateCallback StateCallback; ///< State change callback function pointer type. + typedef otCommissionerJoinerCallback JoinerCallback; ///< Joiner state change callback function pointer type. + + /** + * This type represents a Commissioning Dataset. + * + */ + class Dataset : public otCommissioningDataset, public Clearable + { + public: + /** + * This method indicates whether or not the Border Router RLOC16 Locator is set in the Dataset. + * + * @returns TRUE if Border Router RLOC16 Locator is set, FALSE otherwise. + * + */ + bool IsLocatorSet(void) const { return mIsLocatorSet; } + + /** + * This method gets the Border Router RLOC16 Locator in the Dataset. + * + * This method MUST be used when Locator is set in the Dataset, otherwise its behavior is undefined. + * + * @returns The Border Router RLOC16 Locator in the Dataset. + * + */ + uint16_t GetLocator(void) const { return mLocator; } + + /** + * This method sets the Border Router RLOCG16 Locator in the Dataset. + * + * @param[in] aLocator A Locator. + * + */ + void SetLocator(uint16_t aLocator) + { + mIsLocatorSet = true; + mLocator = aLocator; + } + + /** + * This method indicates whether or not the Session ID is set in the Dataset. + * + * @returns TRUE if Session ID is set, FALSE otherwise. + * + */ + bool IsSessionIdSet(void) const { return mIsSessionIdSet; } + + /** + * This method gets the Session ID in the Dataset. + * + * This method MUST be used when Session ID is set in the Dataset, otherwise its behavior is undefined. + * + * @returns The Session ID in the Dataset. + * + */ + uint16_t GetSessionId(void) const { return mSessionId; } + + /** + * This method sets the Session ID in the Dataset. + * + * @param[in] aSessionId The Session ID. + * + */ + void SetSessionId(uint16_t aSessionId) + { + mIsSessionIdSet = true; + mSessionId = aSessionId; + } + + /** + * This method indicates whether or not the Steering Data is set in the Dataset. + * + * @returns TRUE if Steering Data is set, FALSE otherwise. + * + */ + bool IsSteeringDataSet(void) const { return mIsSteeringDataSet; } + + /** + * This method gets the Steering Data in the Dataset. + * + * This method MUST be used when Steering Data is set in the Dataset, otherwise its behavior is undefined. + * + * @returns The Steering Data in the Dataset. + * + */ + const SteeringData &GetSteeringData(void) const { return AsCoreType(&mSteeringData); } + + /** + * This method returns a reference to the Steering Data in the Dataset to be updated by caller. + * + * @returns A reference to the Steering Data in the Dataset. + * + */ + SteeringData &UpdateSteeringData(void) + { + mIsSteeringDataSet = true; + return AsCoreType(&mSteeringData); + } + + /** + * This method indicates whether or not the Joiner UDP port is set in the Dataset. + * + * @returns TRUE if Joiner UDP port is set, FALSE otherwise. + * + */ + bool IsJoinerUdpPortSet(void) const { return mIsJoinerUdpPortSet; } + + /** + * This method gets the Joiner UDP port in the Dataset. + * + * This method MUST be used when Joiner UDP port is set in the Dataset, otherwise its behavior is undefined. + * + * @returns The Joiner UDP port in the Dataset. + * + */ + uint16_t GetJoinerUdpPort(void) const { return mJoinerUdpPort; } + + /** + * This method sets the Joiner UDP Port in the Dataset. + * + * @param[in] aJoinerUdpPort The Joiner UDP Port. + * + */ + void SetJoinerUdpPort(uint16_t aJoinerUdpPort) + { + mIsJoinerUdpPortSet = true; + mJoinerUdpPort = aJoinerUdpPort; + } + }; + /** * This constructor initializes the Commissioner object. * @@ -94,20 +239,16 @@ class Commissioner : public InstanceLocator, private NonCopyable * @retval kErrorInvalidState Device is not currently attached to a network. * */ - Error Start(otCommissionerStateCallback aStateCallback, - otCommissionerJoinerCallback aJoinerCallback, - void * aCallbackContext); + Error Start(StateCallback aStateCallback, JoinerCallback aJoinerCallback, void *aCallbackContext); /** * This method stops the Commissioner service. * - * @param[in] aResign Whether send LEAD_KA.req to resign as Commissioner - * * @retval kErrorNone Successfully stopped the Commissioner service. * @retval kErrorAlready Commissioner is already stopped. * */ - Error Stop(bool aResign); + Error Stop(void) { return Stop(kSendKeepAliveToResign); } /** * This method clears all Joiner entries. @@ -294,7 +435,7 @@ class Commissioner : public InstanceLocator, private NonCopyable * @retval kErrorInvalidState Commissioner service is not started. * */ - Error SendMgmtCommissionerSetRequest(const otCommissioningDataset &aDataset, const uint8_t *aTlvs, uint8_t aLength); + Error SendMgmtCommissionerSetRequest(const Dataset &aDataset, const uint8_t *aTlvs, uint8_t aLength); /** * This method returns a reference to the AnnounceBeginClient instance. @@ -333,13 +474,10 @@ class Commissioner : public InstanceLocator, private NonCopyable static constexpr uint32_t kKeepAliveTimeout = 50; // TIMEOUT_COMM_PET (seconds) static constexpr uint32_t kRemoveJoinerDelay = 20; // Delay to remove successfully joined joiner - enum JoinerEvent : uint8_t + enum ResignMode : uint8_t { - kJoinerEventStart = OT_COMMISSIONER_JOINER_START, - kJoinerEventConnected = OT_COMMISSIONER_JOINER_CONNECTED, - kJoinerEventFinalize = OT_COMMISSIONER_JOINER_FINALIZE, - kJoinerEventEnd = OT_COMMISSIONER_JOINER_END, - kJoinerEventRemoved = OT_COMMISSIONER_JOINER_REMOVED, + kSendKeepAliveToResign, + kDoNotSendKeepAlive, }; struct Joiner @@ -366,6 +504,7 @@ class Commissioner : public InstanceLocator, private NonCopyable void CopyToJoinerInfo(otJoinerInfo &aJoiner) const; }; + Error Stop(ResignMode aResignMode); Joiner *GetUnusedJoinerEntry(void); Joiner *FindJoinerEntry(const Mac::ExtAddress *aEui64); Joiner *FindJoinerEntry(const JoinerDiscerner &aDiscerner); @@ -469,14 +608,16 @@ class Commissioner : public InstanceLocator, private NonCopyable State mState; - otCommissionerStateCallback mStateCallback; - otCommissionerJoinerCallback mJoinerCallback; - void * mCallbackContext; + StateCallback mStateCallback; + JoinerCallback mJoinerCallback; + void * mCallbackContext; }; } // namespace MeshCoP DefineMapEnum(otCommissionerState, MeshCoP::Commissioner::State); +DefineMapEnum(otCommissionerJoinerEvent, MeshCoP::Commissioner::JoinerEvent); +DefineCoreType(otCommissioningDataset, MeshCoP::Commissioner::Dataset); } // namespace ot From 5896eefc2119b7e4c01855b82f5baa41f8d05071 Mon Sep 17 00:00:00 2001 From: Yi Date: Thu, 27 Jan 2022 13:27:08 +0800 Subject: [PATCH 12/18] [nat64] withdraw greater NAT64 prefix in network data (#7337) This commit enables BR to withdraw its local NAT64 prefix if a smaller one is found in network data. It might happen when two BRs join the Thread network at merely the same time and thus both advertise its local NAT64 prefix. The BR with numerically greater prefix will withdraw its NAT64 prefix. --- src/core/border_router/routing_manager.cpp | 32 ++++++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index 0ea712ccaa6..f861681e900 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -641,26 +641,46 @@ void RoutingManager::EvaluateNat64Prefix(void) NetworkData::Iterator iterator = NetworkData::kIteratorInit; NetworkData::ExternalRouteConfig config; - bool hasAdvertisedNat64Prefix = false; + Ip6::Prefix smallestNat64Prefix; otLogInfoBr("Evaluating NAT64 prefix"); + smallestNat64Prefix.Clear(); while (Get().GetNextExternalRoute(iterator, config) == kErrorNone) { - if (config.mNat64 && config.GetPrefix().IsValidNat64()) + const Ip6::Prefix &prefix = config.GetPrefix(); + + if (config.mNat64 && prefix.IsValidNat64()) { - hasAdvertisedNat64Prefix = true; + if (smallestNat64Prefix.GetLength() == 0 || prefix < smallestNat64Prefix) + { + smallestNat64Prefix = prefix; + } } } - if (!hasAdvertisedNat64Prefix && !mIsAdvertisingLocalNat64Prefix) + if (smallestNat64Prefix.GetLength() == 0 || smallestNat64Prefix == mLocalNat64Prefix) { - // TODO: when multiple prefixes exits, remove the ones with lower preference or numerically larger. - if (AddExternalRoute(mLocalNat64Prefix, NetworkData::kRoutePreferenceLow, /* aNat64= */ true) == kErrorNone) + otLogInfoBr("No NAT64 prefix in Network Data is smaller than the local NAT64 prefix %s", + mLocalNat64Prefix.ToString().AsCString()); + + // Advertise local NAT64 prefix. + if (!mIsAdvertisingLocalNat64Prefix && + AddExternalRoute(mLocalNat64Prefix, NetworkData::kRoutePreferenceLow, /* aNat64= */ true) == kErrorNone) { mIsAdvertisingLocalNat64Prefix = true; } } + else if (mIsAdvertisingLocalNat64Prefix && smallestNat64Prefix < mLocalNat64Prefix) + { + // Withdraw local NAT64 prefix if it's not the smallest one in Network Data. + // TODO: remove the prefix with lower preference after discovering upstream NAT64 prefix is supported + otLogNoteBr("Withdrawing local NAT64 prefix since a smaller one %s exists.", + smallestNat64Prefix.ToString().AsCString()); + + RemoveExternalRoute(mLocalNat64Prefix); + mIsAdvertisingLocalNat64Prefix = false; + } } #endif From 69ed42f4910253592b2a827a7b04b658b16fc384 Mon Sep 17 00:00:00 2001 From: wohdo <98126482+wohdo@users.noreply.github.com> Date: Thu, 27 Jan 2022 06:28:18 +0100 Subject: [PATCH 13/18] [coap] add 'otCoapMessageSetCode' (#7339) (#7351) Provide a setter for the message code to change it after initialization of the message. This complements the existing getter function `otCoapMessageGetCode`. --- include/openthread/coap.h | 9 +++++++++ include/openthread/instance.h | 2 +- src/core/api/coap_api.cpp | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/include/openthread/coap.h b/include/openthread/coap.h index 7e2312040fe..396d53573fc 100644 --- a/include/openthread/coap.h +++ b/include/openthread/coap.h @@ -717,6 +717,15 @@ otCoapType otCoapMessageGetType(const otMessage *aMessage); */ otCoapCode otCoapMessageGetCode(const otMessage *aMessage); +/** + * This function sets the Code value. + * + * @param[inout] aMessage A pointer to the CoAP message to initialize. + * @param[in] aCode CoAP message code. + * + */ +void otCoapMessageSetCode(otMessage *aMessage, otCoapCode aCode); + /** * This method returns the CoAP Code as human readable string. * diff --git a/include/openthread/instance.h b/include/openthread/instance.h index e06b7754e4e..699941c5def 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (186) +#define OPENTHREAD_API_VERSION (187) /** * @addtogroup api-instance diff --git a/src/core/api/coap_api.cpp b/src/core/api/coap_api.cpp index 48353678e2a..4dfb35edfe2 100644 --- a/src/core/api/coap_api.cpp +++ b/src/core/api/coap_api.cpp @@ -144,6 +144,11 @@ otCoapCode otCoapMessageGetCode(const otMessage *aMessage) return static_cast(AsCoapMessage(aMessage).GetCode()); } +void otCoapMessageSetCode(otMessage *aMessage, otCoapCode aCode) +{ + AsCoapMessage(aMessage).SetCode(MapEnum(aCode)); +} + const char *otCoapMessageCodeToString(const otMessage *aMessage) { return AsCoapMessage(aMessage).CodeToString(); From d71742049f79965797ecab24a7038b8551966b7b Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 27 Jan 2022 10:29:09 -0800 Subject: [PATCH 14/18] [logging] remove `otLogResult` and move to module (#7360) This commit removes the `otLogResult{Bbr/Platform}()` macros from `logging.hpp` and instead have each module implement `LogError()` method. The main goal behind this change to simplify the logging module and reduce number of macros/definitions in it (to prepare for new logging model to be introduced). --- src/core/backbone_router/backbone_tmf.cpp | 25 ++++++++---- src/core/backbone_router/backbone_tmf.hpp | 1 + src/core/backbone_router/bbr_manager.cpp | 16 +++++++- src/core/backbone_router/bbr_manager.hpp | 2 + src/core/common/logging.hpp | 41 ------------------- src/posix/platform/multicast_routing.cpp | 48 +++++++++++++++-------- 6 files changed, 68 insertions(+), 65 deletions(-) diff --git a/src/core/backbone_router/backbone_tmf.cpp b/src/core/backbone_router/backbone_tmf.cpp index cde9d46962e..676906f811b 100644 --- a/src/core/backbone_router/backbone_tmf.cpp +++ b/src/core/backbone_router/backbone_tmf.cpp @@ -77,20 +77,31 @@ bool BackboneTmfAgent::IsBackboneTmfMessage(const Ip6::MessageInfo &aMessageInfo void BackboneTmfAgent::SubscribeMulticast(const Ip6::Address &aAddress) { - Error error; + Error error = mSocket.JoinNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - error = mSocket.JoinNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - - otLogResultBbr(error, "Backbone TMF subscribes %s", aAddress.ToString().AsCString()); + LogError("Backbone TMF subscribes", aAddress, error); } void BackboneTmfAgent::UnsubscribeMulticast(const Ip6::Address &aAddress) { - Error error; + Error error = mSocket.LeaveNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - error = mSocket.LeaveNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); + LogError("Backbone TMF unsubscribes", aAddress, error); +} - otLogResultBbr(error, "Backbone TMF unsubscribes %s", aAddress.ToString().AsCString()); +void BackboneTmfAgent::LogError(const char *aText, const Ip6::Address &aAddress, Error aError) const +{ + OT_UNUSED_VARIABLE(aText); + OT_UNUSED_VARIABLE(aAddress); + + if (aError == kErrorNone) + { + otLogInfoBbr("%s %s: %s", aText, aAddress.ToString().AsCString(), ErrorToString(aError)); + } + else + { + otLogWarnBbr("%s %s: %s", aText, aAddress.ToString().AsCString(), ErrorToString(aError)); + } } } // namespace BackboneRouter diff --git a/src/core/backbone_router/backbone_tmf.hpp b/src/core/backbone_router/backbone_tmf.hpp index 31d6ce6f99d..0720f05b574 100644 --- a/src/core/backbone_router/backbone_tmf.hpp +++ b/src/core/backbone_router/backbone_tmf.hpp @@ -99,6 +99,7 @@ class BackboneTmfAgent : public Coap::Coap void UnsubscribeMulticast(const Ip6::Address &aAddress); private: + void LogError(const char *aText, const Ip6::Address &aAddress, Error aError) const; static Error Filter(const ot::Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, void *aContext); }; diff --git a/src/core/backbone_router/bbr_manager.cpp b/src/core/backbone_router/bbr_manager.cpp index 397592244f8..405d12e7fbd 100644 --- a/src/core/backbone_router/bbr_manager.cpp +++ b/src/core/backbone_router/bbr_manager.cpp @@ -130,7 +130,7 @@ void Manager::HandleNotifierEvents(Events aEvents) error = mBackboneTmfAgent.Start(); - otLogResultBbr(error, "Start Backbone TMF agent"); + LogError("Start Backbone TMF agent", error); } } } @@ -793,6 +793,20 @@ void Manager::HandleProactiveBackboneNotification(const Ip6::Address & } #endif // OPENTHREAD_CONFIG_BACKBONE_ROUTER_DUA_NDPROXYING_ENABLE +void Manager::LogError(const char *aText, Error aError) const +{ + OT_UNUSED_VARIABLE(aText); + + if (aError == kErrorNone) + { + otLogInfoBbr("%s: %s", aText, ErrorToString(aError)); + } + else + { + otLogWarnBbr("%s: %s", aText, ErrorToString(aError)); + } +} + } // namespace BackboneRouter } // namespace ot diff --git a/src/core/backbone_router/bbr_manager.hpp b/src/core/backbone_router/bbr_manager.hpp index 2835f7b863e..e5f17056812 100644 --- a/src/core/backbone_router/bbr_manager.hpp +++ b/src/core/backbone_router/bbr_manager.hpp @@ -229,6 +229,8 @@ class Manager : public InstanceLocator, private NonCopyable static void HandleTimer(Timer &aTimer); void HandleTimer(void); + void LogError(const char *aText, Error aError) const; + #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_MULTICAST_ROUTING_ENABLE Coap::Resource mMulticastListenerRegistration; #endif diff --git a/src/core/common/logging.hpp b/src/core/common/logging.hpp index ea9b698818f..dc3167b28e8 100644 --- a/src/core/common/logging.hpp +++ b/src/core/common/logging.hpp @@ -2582,47 +2582,6 @@ void otDumpMacFrame(otLogLevel aLogLevel, const char *aId, const void *aBuf, siz */ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const void *aBuf, size_t aLength); -#define _otLogResult(aRegion, aError, ...) \ - do \ - { \ - otError _err = (aError); \ - \ - if (_err == OT_ERROR_NONE) \ - { \ - otLogInfo##aRegion(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), \ - otThreadErrorToString(_err)); \ - } \ - else \ - { \ - otLogWarn##aRegion(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), \ - otThreadErrorToString(_err)); \ - } \ - } while (false) - -/** - * @def otLogResultPlat - * - * This function generates a log for the Plat region according to the error result. If @p aError is `kErrorNone`, the - * log level is info. Otherwise the log level is warn. - * - * @param[in] aError The error result. - * @param[in] ... Arguments for the format specification. - * - */ -#define otLogResultPlat(aError, ...) _otLogResult(Plat, aError, __VA_ARGS__) - -/** - * @def otLogResultBbr - * - * This function generates a log for the BBR region according to the error result. If @p aError is `OT_ERROR_NONE`, the - * log level is info. Otherwise the log level is warn. - * - * @param[in] aError The error result. - * @param[in] ... Arguments for the format specification. - * - */ -#define otLogResultBbr(aError, ...) _otLogResult(Bbr, aError, __VA_ARGS__) - #ifdef __cplusplus } #endif diff --git a/src/posix/platform/multicast_routing.cpp b/src/posix/platform/multicast_routing.cpp index e637ddb6991..ab35d03abd8 100644 --- a/src/posix/platform/multicast_routing.cpp +++ b/src/posix/platform/multicast_routing.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,21 @@ namespace ot { namespace Posix { +#define LogResult(aError, ...) \ + do \ + { \ + otError _err = (aError); \ + \ + if (_err == OT_ERROR_NONE) \ + { \ + otLogInfoPlat(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), otThreadErrorToString(_err)); \ + } \ + else \ + { \ + otLogWarnPlat(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), otThreadErrorToString(_err)); \ + } \ + } while (false) + void MulticastRoutingManager::SetUp(void) { OT_ASSERT(gInstance != nullptr); @@ -97,7 +113,7 @@ void MulticastRoutingManager::Enable(void) InitMulticastRouterSock(); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); exit: return; } @@ -106,7 +122,7 @@ void MulticastRoutingManager::Disable(void) { FinalizeMulticastRouterSock(); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); } void MulticastRoutingManager::Add(const Ip6::Address &aAddress) @@ -116,7 +132,7 @@ void MulticastRoutingManager::Add(const Ip6::Address &aAddress) UnblockInboundMulticastForwardingCache(aAddress); UpdateMldReport(aAddress, true); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); exit: return; @@ -131,7 +147,7 @@ void MulticastRoutingManager::Remove(const Ip6::Address &aAddress) RemoveInboundMulticastForwardingCache(aAddress); UpdateMldReport(aAddress, false); - otLogResultPlat(error, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); + LogResult(error, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); exit: return; @@ -149,8 +165,8 @@ void MulticastRoutingManager::UpdateMldReport(const Ip6::Address &aAddress, bool ? OT_ERROR_FAILED : OT_ERROR_NONE); - otLogResultPlat(error, "MulticastRoutingManager: %s: address %s %s", __FUNCTION__, aAddress.ToString().AsCString(), - (isAdd ? "Added" : "Removed")); + LogResult(error, "MulticastRoutingManager: %s: address %s %s", __FUNCTION__, aAddress.ToString().AsCString(), + (isAdd ? "Added" : "Removed")); } bool MulticastRoutingManager::HasMulticastListener(const Ip6::Address &aAddress) const @@ -266,7 +282,7 @@ void MulticastRoutingManager::ProcessMulticastRouterMessages(void) error = AddMulticastForwardingCache(src, dst, static_cast(mrt6msg->im6_mif)); exit: - otLogResultPlat(error, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(error, "MulticastRoutingManager: %s", __FUNCTION__); } otError MulticastRoutingManager::AddMulticastForwardingCache(const Ip6::Address &aSrcAddr, @@ -321,9 +337,9 @@ otError MulticastRoutingManager::AddMulticastForwardingCache(const Ip6::Address SaveMulticastForwardingCache(aSrcAddr, aGroupAddr, aIif, forwardMif); exit: - otLogResultPlat(error, "MulticastRoutingManager: %s: add dynamic route: %s %s => %s %s", __FUNCTION__, - MifIndexToString(aIif), aSrcAddr.ToString().AsCString(), aGroupAddr.ToString().AsCString(), - MifIndexToString(forwardMif)); + LogResult(error, "MulticastRoutingManager: %s: add dynamic route: %s %s => %s %s", __FUNCTION__, + MifIndexToString(aIif), aSrcAddr.ToString().AsCString(), aGroupAddr.ToString().AsCString(), + MifIndexToString(forwardMif)); return error; } @@ -358,9 +374,9 @@ void MulticastRoutingManager::UnblockInboundMulticastForwardingCache(const Ip6:: mfc.Set(kMifIndexBackbone, kMifIndexThread); - otLogResultPlat(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(mfc.mIif), - mfc.mSrcAddr.ToString().AsCString(), mfc.mGroupAddr.ToString().AsCString(), - MifIndexToString(kMifIndexThread)); + LogResult(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(mfc.mIif), + mfc.mSrcAddr.ToString().AsCString(), mfc.mGroupAddr.ToString().AsCString(), + MifIndexToString(kMifIndexThread)); } } @@ -586,9 +602,9 @@ void MulticastRoutingManager::RemoveMulticastForwardingCache( ? OT_ERROR_NONE : OT_ERROR_FAILED; - otLogResultPlat(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(aMfc.mIif), - aMfc.mSrcAddr.ToString().AsCString(), aMfc.mGroupAddr.ToString().AsCString(), - MifIndexToString(aMfc.mOif)); + LogResult(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(aMfc.mIif), + aMfc.mSrcAddr.ToString().AsCString(), aMfc.mGroupAddr.ToString().AsCString(), + MifIndexToString(aMfc.mOif)); aMfc.Erase(); } From 019e5ab4fabfb476e39a21a80c4943d3d39f8bc5 Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Fri, 28 Jan 2022 02:35:08 +0800 Subject: [PATCH 15/18] [cli] do not update aError which is not read (#7365) This commit avoid setting aError which is not read. --- src/cli/cli.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/cli.cpp b/src/cli/cli.cpp index 5370b7595ac..f0fb0347226 100644 --- a/src/cli/cli.cpp +++ b/src/cli/cli.cpp @@ -4712,7 +4712,7 @@ void Interpreter::HandleDiagnosticGetResponse(otError aError, OutputLine(""); // Output Network Diagnostic TLV values in standard YAML format. - while ((aError = otThreadGetNextDiagnosticTlv(aMessage, &iterator, &diagTlv)) == OT_ERROR_NONE) + while (otThreadGetNextDiagnosticTlv(aMessage, &iterator, &diagTlv) == OT_ERROR_NONE) { switch (diagTlv.mType) { From 9192f0aa63870b9a3653a16ab389ba96c0921f17 Mon Sep 17 00:00:00 2001 From: Lukasz Duda Date: Fri, 21 Jan 2022 15:35:40 +0100 Subject: [PATCH 16/18] [crypto] allow platform to define its own CSPRNG This commit provides the following changes: * Introduce a new otPlatCryptoRandom API * Move usage of mbedtls_entropy_* and mbedtls_ctr_drbg_* out of ARM PSA enabled platforms * Stop exposing internal mbedtls contextes for entropy and CTR DRBG in OpenThread API Signed-off-by: Lukasz Duda --- Android.mk | 1 - doc/ot_api_doc.h | 1 - include/Makefile.am | 1 - include/openthread/BUILD.gn | 1 - include/openthread/entropy.h | 70 ---------------------- include/openthread/platform/crypto.h | 30 ++++++++++ include/openthread/random_crypto.h | 7 --- src/core/BUILD.gn | 1 - src/core/CMakeLists.txt | 1 - src/core/Makefile.am | 1 - src/core/api/entropy_api.cpp | 43 -------------- src/core/api/random_crypto_api.cpp | 5 -- src/core/common/random.hpp | 11 ---- src/core/common/random_manager.cpp | 87 +--------------------------- src/core/common/random_manager.hpp | 64 +------------------- src/core/crypto/crypto_platform.cpp | 66 +++++++++++++++++++++ src/core/crypto/ecdsa.cpp | 16 ++--- src/core/crypto/mbedtls.cpp | 13 +++++ src/core/crypto/mbedtls.hpp | 14 +++++ src/core/meshcop/dtls.cpp | 8 +-- 20 files changed, 137 insertions(+), 304 deletions(-) delete mode 100644 include/openthread/entropy.h delete mode 100644 src/core/api/entropy_api.cpp diff --git a/Android.mk b/Android.mk index 1de97361ef2..96fbf741a00 100644 --- a/Android.mk +++ b/Android.mk @@ -175,7 +175,6 @@ LOCAL_SRC_FILES := \ src/core/api/diags_api.cpp \ src/core/api/dns_api.cpp \ src/core/api/dns_server_api.cpp \ - src/core/api/entropy_api.cpp \ src/core/api/error_api.cpp \ src/core/api/heap_api.cpp \ src/core/api/history_tracker_api.cpp \ diff --git a/doc/ot_api_doc.h b/doc/ot_api_doc.h index 8843171e68d..c826e68e218 100644 --- a/doc/ot_api_doc.h +++ b/doc/ot_api_doc.h @@ -121,7 +121,6 @@ * * @defgroup api-cli Command Line Interface * @defgroup api-crypto Crypto - Thread Stack - * @defgroup api-entropy Entropy Source * @defgroup api-factory-diagnostics Factory Diagnostics - Thread Stack * @defgroup api-heap Heap * @defgroup api-history-tracker History Tracker diff --git a/include/Makefile.am b/include/Makefile.am index 6b4f16635c1..426e17c2a4b 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -55,7 +55,6 @@ openthread_headers = \ openthread/dns.h \ openthread/dns_client.h \ openthread/dnssd_server.h \ - openthread/entropy.h \ openthread/error.h \ openthread/heap.h \ openthread/history_tracker.h \ diff --git a/include/openthread/BUILD.gn b/include/openthread/BUILD.gn index 6307afbcca8..7a16e83e4be 100644 --- a/include/openthread/BUILD.gn +++ b/include/openthread/BUILD.gn @@ -76,7 +76,6 @@ source_set("openthread") { "dns.h", "dns_client.h", "dnssd_server.h", - "entropy.h", "error.h", "heap.h", "history_tracker.h", diff --git a/include/openthread/entropy.h b/include/openthread/entropy.h deleted file mode 100644 index bc07c300b9d..00000000000 --- a/include/openthread/entropy.h +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright (c) 2019, The OpenThread Authors. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the copyright holder nor the - * names of its contributors may be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -/** - * @file - * @brief - * This file defines the OpenThread entropy source API. - */ - -#ifndef OPENTHREAD_ENTROPY_H_ -#define OPENTHREAD_ENTROPY_H_ - -#include - -#ifdef __cplusplus -extern "C" { -#endif - -/** - * @addtogroup api-entropy - * - * @brief - * This module includes functions that manages entropy source. - * - * @{ - * - */ - -/** - * This function returns initialized mbedtls_entropy_context. - * - * @returns A pointer to initialized mbedtls_entropy_context. - */ -mbedtls_entropy_context *otEntropyMbedTlsContextGet(void); - -/** - * @} - * - */ - -#ifdef __cplusplus -} // extern "C" -#endif - -#endif // OPENTHREAD_ENTROPY_H_ diff --git a/include/openthread/platform/crypto.h b/include/openthread/platform/crypto.h index efe40ab7050..5373d8dac94 100644 --- a/include/openthread/platform/crypto.h +++ b/include/openthread/platform/crypto.h @@ -469,6 +469,36 @@ otError otPlatCryptoSha256Update(otCryptoContext *aContext, const void *aBuf, ui */ otError otPlatCryptoSha256Finish(otCryptoContext *aContext, uint8_t *aHash, uint16_t aHashSize); +/** + * Initialize cryptographically-secure pseudorandom number generator (CSPRNG). + * + * @retval OT_ERROR_NONE Successfully initialized CSPRNG module. + * @retval OT_ERROR_FAILED Failed to initialize CSPRNG module. + * + */ +otError otPlatCryptoRandomInit(void); + +/** + * Deinitialize cryptographically-secure pseudorandom number generator (CSPRNG). + * + * @retval OT_ERROR_NONE Successfully deinitialized CSPRNG module. + * @retval OT_ERROR_FAILED Failed to deinitialize CSPRNG module. + * + */ +otError otPlatCryptoRandomDeinit(void); + +/** + * Fills a given buffer with cryptographically secure random bytes. + * + * @param[out] aBuffer A pointer to a buffer to fill with the random bytes. + * @param[in] aSize Size of buffer (number of bytes to fill). + * + * @retval OT_ERROR_NONE Successfully filled buffer with random values. + * @retval OT_ERROR_FAILED Operation failed. + * + */ +otError otPlatCryptoRandomGet(uint8_t *aBuffer, uint16_t aSize); + /** * @} * diff --git a/include/openthread/random_crypto.h b/include/openthread/random_crypto.h index da4ba7b0106..937136a782b 100644 --- a/include/openthread/random_crypto.h +++ b/include/openthread/random_crypto.h @@ -63,13 +63,6 @@ extern "C" { */ otError otRandomCryptoFillBuffer(uint8_t *aBuffer, uint16_t aSize); -/** - * This function returns initialized mbedtls_ctr_drbg_context. - * - * @returns A pointer to initialized mbedtls_ctr_drbg_context. - */ -mbedtls_ctr_drbg_context *otRandomCryptoMbedTlsContextGet(void); - /** * @} * diff --git a/src/core/BUILD.gn b/src/core/BUILD.gn index d04ec4b9ef9..f2a3894b099 100644 --- a/src/core/BUILD.gn +++ b/src/core/BUILD.gn @@ -316,7 +316,6 @@ openthread_core_files = [ "api/diags_api.cpp", "api/dns_api.cpp", "api/dns_server_api.cpp", - "api/entropy_api.cpp", "api/error_api.cpp", "api/heap_api.cpp", "api/history_tracker_api.cpp", diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt index 2e1d2d6bae2..013f4722546 100644 --- a/src/core/CMakeLists.txt +++ b/src/core/CMakeLists.txt @@ -48,7 +48,6 @@ set(COMMON_SOURCES api/diags_api.cpp api/dns_api.cpp api/dns_server_api.cpp - api/entropy_api.cpp api/error_api.cpp api/heap_api.cpp api/history_tracker_api.cpp diff --git a/src/core/Makefile.am b/src/core/Makefile.am index e60cc29ed23..f40f8510a08 100644 --- a/src/core/Makefile.am +++ b/src/core/Makefile.am @@ -138,7 +138,6 @@ SOURCES_COMMON = \ api/diags_api.cpp \ api/dns_api.cpp \ api/dns_server_api.cpp \ - api/entropy_api.cpp \ api/error_api.cpp \ api/heap_api.cpp \ api/history_tracker_api.cpp \ diff --git a/src/core/api/entropy_api.cpp b/src/core/api/entropy_api.cpp deleted file mode 100644 index 469e55d319f..00000000000 --- a/src/core/api/entropy_api.cpp +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (c) 2019, The OpenThread Authors. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the copyright holder nor the - * names of its contributors may be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - */ - -/** - * @file - * This file implements the OpenThread entropy source management API. - */ - -#include - -#include "common/random_manager.hpp" - -using namespace ot; - -mbedtls_entropy_context *otEntropyMbedTlsContextGet(void) -{ - return RandomManager::GetMbedTlsEntropyContext(); -} diff --git a/src/core/api/random_crypto_api.cpp b/src/core/api/random_crypto_api.cpp index 08797e89338..906a4a02795 100644 --- a/src/core/api/random_crypto_api.cpp +++ b/src/core/api/random_crypto_api.cpp @@ -43,8 +43,3 @@ otError otRandomCryptoFillBuffer(uint8_t *aBuffer, uint16_t aSize) { return Random::Crypto::FillBuffer(aBuffer, aSize); } - -mbedtls_ctr_drbg_context *otRandomCryptoMbedTlsContextGet(void) -{ - return Random::Crypto::MbedTlsContextGet(); -} diff --git a/src/core/common/random.hpp b/src/core/common/random.hpp index da002813228..a7e269453bf 100644 --- a/src/core/common/random.hpp +++ b/src/core/common/random.hpp @@ -177,17 +177,6 @@ inline Error FillBuffer(uint8_t *aBuffer, uint16_t aSize) return RandomManager::CryptoFillBuffer(aBuffer, aSize); } -/** - * This function returns initialized mbedtls_ctr_drbg_context. - * - * @returns A pointer to initialized mbedtls_ctr_drbg_context. - * - */ -inline mbedtls_ctr_drbg_context *MbedTlsContextGet(void) -{ - return RandomManager::GetMbedTlsCtrDrbgContext(); -} - } // namespace Crypto #endif // !OPENTHREAD_RADIO diff --git a/src/core/common/random_manager.cpp b/src/core/common/random_manager.cpp index 4caf2269d90..f050a5514d6 100644 --- a/src/core/common/random_manager.cpp +++ b/src/core/common/random_manager.cpp @@ -46,11 +46,6 @@ namespace ot { uint16_t RandomManager::sInitCount = 0; RandomManager::NonCryptoPrng RandomManager::sPrng; -#if !OPENTHREAD_RADIO -RandomManager::Entropy RandomManager::sEntropy; -RandomManager::CryptoCtrDrbg RandomManager::sCtrDrbg; -#endif - RandomManager::RandomManager(void) { uint32_t seed; @@ -60,9 +55,7 @@ RandomManager::RandomManager(void) VerifyOrExit(sInitCount == 0); #if !OPENTHREAD_RADIO - sEntropy.Init(); - sCtrDrbg.Init(); - + SuccessOrAssert(otPlatCryptoRandomInit()); SuccessOrAssert(Random::Crypto::FillBuffer(reinterpret_cast(&seed), sizeof(seed))); #else SuccessOrAssert(otPlatEntropyGet(reinterpret_cast(&seed), sizeof(seed))); @@ -82,8 +75,7 @@ RandomManager::~RandomManager(void) VerifyOrExit(sInitCount == 0); #if !OPENTHREAD_RADIO - sCtrDrbg.Deinit(); - sEntropy.Deinit(); + SuccessOrAssert(otPlatCryptoRandomDeinit()); #endif exit: @@ -135,79 +127,4 @@ uint32_t RandomManager::NonCryptoPrng::GetNext(void) return mlcg; } -#if !OPENTHREAD_RADIO - -//------------------------------------------------------------------- -// Entropy - -void RandomManager::Entropy::Init(void) -{ - mbedtls_entropy_init(&mEntropyContext); - -#ifndef OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT - mbedtls_entropy_add_source(&mEntropyContext, &RandomManager::Entropy::HandleMbedtlsEntropyPoll, nullptr, - kEntropyMinThreshold, MBEDTLS_ENTROPY_SOURCE_STRONG); -#endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT -} - -void RandomManager::Entropy::Deinit(void) -{ - mbedtls_entropy_free(&mEntropyContext); -} - -#ifndef OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT - -int RandomManager::Entropy::HandleMbedtlsEntropyPoll(void * aData, - unsigned char *aOutput, - size_t aInLen, - size_t * aOutLen) -{ - int rval = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; - - SuccessOrExit(otPlatEntropyGet(reinterpret_cast(aOutput), static_cast(aInLen))); - rval = 0; - - VerifyOrExit(aOutLen != nullptr); - *aOutLen = aInLen; - -exit: - OT_UNUSED_VARIABLE(aData); - return rval; -} - -#endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT - -//------------------------------------------------------------------- -// CryptoCtrDrbg - -void RandomManager::CryptoCtrDrbg::Init(void) -{ - int rval; - - mbedtls_ctr_drbg_init(&mCtrDrbg); - - rval = - mbedtls_ctr_drbg_seed(&mCtrDrbg, mbedtls_entropy_func, RandomManager::GetMbedTlsEntropyContext(), nullptr, 0); - - if (rval != 0) - { - otLogCritMbedTls("Failed to seed the CTR DRBG"); - } - - OT_ASSERT(rval == 0); -} - -void RandomManager::CryptoCtrDrbg::Deinit(void) -{ - mbedtls_ctr_drbg_free(&mCtrDrbg); -} - -Error RandomManager::CryptoCtrDrbg::FillBuffer(uint8_t *aBuffer, uint16_t aSize) -{ - return ot::Crypto::MbedTls::MapError( - mbedtls_ctr_drbg_random(&mCtrDrbg, static_cast(aBuffer), static_cast(aSize))); -} - -#endif // #if !OPENTHREAD_RADIO - } // namespace ot diff --git a/src/core/common/random_manager.hpp b/src/core/common/random_manager.hpp index 42e7767eec6..adc4d09ef3c 100644 --- a/src/core/common/random_manager.hpp +++ b/src/core/common/random_manager.hpp @@ -38,19 +38,11 @@ #include -#if !OPENTHREAD_RADIO -#include -#include -#endif +#include #include "common/error.hpp" #include "common/non_copyable.hpp" -#if (!defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ - (!defined(MBEDTLS_NO_PLATFORM_ENTROPY) || defined(MBEDTLS_HAVEGE_C) || defined(MBEDTLS_ENTROPY_HARDWARE_ALT))) -#define OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT -#endif - namespace ot { /** @@ -81,13 +73,6 @@ class RandomManager : private NonCopyable static uint32_t NonCryptoGetUint32(void); #if !OPENTHREAD_RADIO - /** - * This static method returns the initialized mbedtls_entropy_context. - * - * @returns A pointer to initialized mbedtls_entropy_context. - */ - static mbedtls_entropy_context *GetMbedTlsEntropyContext(void) { return sEntropy.GetContext(); } - /** * This static method fills a given buffer with cryptographically secure random bytes. * @@ -97,15 +82,7 @@ class RandomManager : private NonCopyable * @retval kErrorNone Successfully filled buffer with random values. * */ - static Error CryptoFillBuffer(uint8_t *aBuffer, uint16_t aSize) { return sCtrDrbg.FillBuffer(aBuffer, aSize); } - - /** - * This static method returns the initialized mbedtls_ctr_drbg_context. - * - * @returns A pointer to the initialized mbedtls_ctr_drbg_context. - * - */ - static mbedtls_ctr_drbg_context *GetMbedTlsCtrDrbgContext(void) { return sCtrDrbg.GetContext(); } + static Error CryptoFillBuffer(uint8_t *aBuffer, uint16_t aSize) { return otPlatCryptoRandomGet(aBuffer, aSize); } #endif private: @@ -119,45 +96,8 @@ class RandomManager : private NonCopyable uint32_t mState; }; -#if !OPENTHREAD_RADIO - class Entropy - { - public: - void Init(void); - void Deinit(void); - - mbedtls_entropy_context *GetContext(void) { return &mEntropyContext; } - - private: -#ifndef OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT - static int HandleMbedtlsEntropyPoll(void *aData, unsigned char *aOutput, size_t aInLen, size_t *aOutLen); -#endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT - - static constexpr size_t kEntropyMinThreshold = 32; - - mbedtls_entropy_context mEntropyContext; - }; - - class CryptoCtrDrbg - { - public: - void Init(void); - void Deinit(void); - Error FillBuffer(uint8_t *aBuffer, uint16_t aSize); - - mbedtls_ctr_drbg_context *GetContext(void) { return &mCtrDrbg; } - - private: - mbedtls_ctr_drbg_context mCtrDrbg; - }; -#endif - static uint16_t sInitCount; static NonCryptoPrng sPrng; -#if !OPENTHREAD_RADIO - static Entropy sEntropy; - static CryptoCtrDrbg sCtrDrbg; -#endif }; } // namespace ot diff --git a/src/core/crypto/crypto_platform.cpp b/src/core/crypto/crypto_platform.cpp index d49f1a6352d..f9ca284b40c 100644 --- a/src/core/crypto/crypto_platform.cpp +++ b/src/core/crypto/crypto_platform.cpp @@ -33,11 +33,14 @@ #include "openthread-core-config.h" #include +#include +#include #include #include #include #include +#include #include #include "common/code_utils.hpp" @@ -51,9 +54,22 @@ using namespace ot; using namespace Crypto; +#if OPENTHREAD_CONFIG_CRYPTO_LIB != OPENTHREAD_CONFIG_CRYPTO_LIB_PSA + //--------------------------------------------------------------------------------------------------------------------- // Default/weak implementation of crypto platform APIs +#if (!defined(MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES) && \ + (!defined(MBEDTLS_NO_PLATFORM_ENTROPY) || defined(MBEDTLS_HAVEGE_C) || defined(MBEDTLS_ENTROPY_HARDWARE_ALT))) +#define OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT +#endif + +#if !OPENTHREAD_RADIO +static mbedtls_ctr_drbg_context sCtrDrbgContext; +static mbedtls_entropy_context sEntropyContext; +static constexpr uint16_t kEntropyMinThreshold = 16; +#endif + OT_TOOL_WEAK otError otPlatCryptoInit(void) { return kErrorNone; @@ -419,4 +435,54 @@ OT_TOOL_WEAK otError otPlatCryptoSha256Finish(otCryptoContext *aContext, uint8_t return error; } +#ifndef OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT + +static int handleMbedtlsEntropyPoll(void *aData, unsigned char *aOutput, size_t aInLen, size_t *aOutLen) +{ + int rval = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; + + SuccessOrExit(otPlatEntropyGet(reinterpret_cast(aOutput), static_cast(aInLen))); + rval = 0; + + VerifyOrExit(aOutLen != nullptr); + *aOutLen = aInLen; + +exit: + OT_UNUSED_VARIABLE(aData); + return rval; +} + +#endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT + +OT_TOOL_WEAK otError otPlatCryptoRandomInit(void) +{ + mbedtls_entropy_init(&sEntropyContext); + +#ifndef OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT + mbedtls_entropy_add_source(&sEntropyContext, handleMbedtlsEntropyPoll, nullptr, kEntropyMinThreshold, + MBEDTLS_ENTROPY_SOURCE_STRONG); +#endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT + + mbedtls_ctr_drbg_init(&sCtrDrbgContext); + + return ot::Crypto::MbedTls::MapError( + mbedtls_ctr_drbg_seed(&sCtrDrbgContext, mbedtls_entropy_func, &sEntropyContext, nullptr, 0)); +} + +OT_TOOL_WEAK otError otPlatCryptoRandomDeinit(void) +{ + mbedtls_entropy_free(&sEntropyContext); + mbedtls_ctr_drbg_free(&sCtrDrbgContext); + + return kErrorNone; +} + +OT_TOOL_WEAK otError otPlatCryptoRandomGet(uint8_t *aBuffer, uint16_t aSize) +{ + return ot::Crypto::MbedTls::MapError( + mbedtls_ctr_drbg_random(&sCtrDrbgContext, static_cast(aBuffer), static_cast(aSize))); +} + #endif // #if !OPENTHREAD_RADIO + +#endif // #if OPENTHREAD_CONFIG_CRYPTO_LIB != OPENTHREAD_CONFIG_CRYPTO_LIB_PSA diff --git a/src/core/crypto/ecdsa.cpp b/src/core/crypto/ecdsa.cpp index 373169bc074..f3ea0810058 100644 --- a/src/core/crypto/ecdsa.cpp +++ b/src/core/crypto/ecdsa.cpp @@ -61,8 +61,7 @@ Error P256::KeyPair::Generate(void) ret = mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY)); VerifyOrExit(ret == 0); - ret = mbedtls_ecp_gen_key(MBEDTLS_ECP_DP_SECP256R1, mbedtls_pk_ec(pk), mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()); + ret = mbedtls_ecp_gen_key(MBEDTLS_ECP_DP_SECP256R1, mbedtls_pk_ec(pk), MbedTls::CryptoSecurePrng, nullptr); VerifyOrExit(ret == 0); ret = mbedtls_pk_write_key_der(&pk, mDerBytes, sizeof(mDerBytes)); @@ -87,8 +86,7 @@ Error P256::KeyPair::Parse(void *aContext) const VerifyOrExit(mbedtls_pk_setup(pk, mbedtls_pk_info_from_type(MBEDTLS_PK_ECKEY)) == 0, error = kErrorFailed); #if (MBEDTLS_VERSION_NUMBER >= 0x03000000) - VerifyOrExit(mbedtls_pk_parse_key(pk, mDerBytes, mDerLength, nullptr, 0, mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()) == 0, + VerifyOrExit(mbedtls_pk_parse_key(pk, mDerBytes, mDerLength, nullptr, 0, MbedTls::CryptoSecurePrng, nullptr) == 0, error = kErrorParse); #else VerifyOrExit(mbedtls_pk_parse_key(pk, mDerBytes, mDerLength, nullptr, 0) == 0, error = kErrorParse); @@ -144,8 +142,7 @@ Error P256::KeyPair::Sign(const Sha256::Hash &aHash, Signature &aSignature) cons #if (MBEDTLS_VERSION_NUMBER >= 0x02130000) ret = mbedtls_ecdsa_sign_det_ext(&ecdsa.MBEDTLS_PRIVATE(grp), &r, &s, &ecdsa.MBEDTLS_PRIVATE(d), aHash.GetBytes(), - Sha256::Hash::kSize, MBEDTLS_MD_SHA256, mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()); + Sha256::Hash::kSize, MBEDTLS_MD_SHA256, MbedTls::CryptoSecurePrng, nullptr); #else ret = mbedtls_ecdsa_sign_det(&ecdsa.MBEDTLS_PRIVATE(grp), &r, &s, &ecdsa.MBEDTLS_PRIVATE(d), aHash.GetBytes(), Sha256::Hash::kSize, MBEDTLS_MD_SHA256); @@ -230,8 +227,8 @@ Error Sign(uint8_t * aOutput, // Parse a private key in PEM format. #if (MBEDTLS_VERSION_NUMBER >= 0x03000000) - VerifyOrExit(mbedtls_pk_parse_key(&pkCtx, aPrivateKey, aPrivateKeyLength, nullptr, 0, mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()) == 0, + VerifyOrExit(mbedtls_pk_parse_key(&pkCtx, aPrivateKey, aPrivateKeyLength, nullptr, 0, MbedTls::CryptoSecurePrng, + nullptr) == 0, error = kErrorInvalidArgs); #else VerifyOrExit(mbedtls_pk_parse_key(&pkCtx, aPrivateKey, aPrivateKeyLength, nullptr, 0) == 0, @@ -246,8 +243,7 @@ Error Sign(uint8_t * aOutput, // Sign using ECDSA. VerifyOrExit(mbedtls_ecdsa_sign(&ctx.MBEDTLS_PRIVATE(grp), &rMpi, &sMpi, &ctx.MBEDTLS_PRIVATE(d), aInputHash, - aInputHashLength, mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()) == 0, + aInputHashLength, MbedTls::CryptoSecurePrng, nullptr) == 0, error = kErrorFailed); VerifyOrExit(mbedtls_mpi_size(&rMpi) + mbedtls_mpi_size(&sMpi) <= aOutputLength, error = kErrorNoBufs); diff --git a/src/core/crypto/mbedtls.cpp b/src/core/crypto/mbedtls.cpp index 84c535cb216..f8c1c531948 100644 --- a/src/core/crypto/mbedtls.cpp +++ b/src/core/crypto/mbedtls.cpp @@ -43,8 +43,10 @@ #include #endif +#include "common/code_utils.hpp" #include "common/error.hpp" #include "common/heap.hpp" +#include "common/random.hpp" namespace ot { namespace Crypto { @@ -168,5 +170,16 @@ Error MbedTls::MapError(int aMbedTlsError) return error; } +#if !OPENTHREAD_RADIO + +int MbedTls::CryptoSecurePrng(void *, unsigned char *aBuffer, size_t aSize) +{ + IgnoreError(ot::Random::Crypto::FillBuffer(aBuffer, static_cast(aSize))); + + return 0; +} + +#endif // !OPENTHREAD_RADIO + } // namespace Crypto } // namespace ot diff --git a/src/core/crypto/mbedtls.hpp b/src/core/crypto/mbedtls.hpp index 2be5cfca839..a5fd869b604 100644 --- a/src/core/crypto/mbedtls.hpp +++ b/src/core/crypto/mbedtls.hpp @@ -89,6 +89,20 @@ class MbedTls : private NonCopyable * */ static Error MapError(int aMbedTlsError); + +#if !OPENTHREAD_RADIO + /** + * This function fills a given buffer with cryptographically secure random bytes. + * + * @param[in] aContext A pointer to arbitrary context. + * @param[out] aBuffer A pointer to a buffer to fill with the random bytes. + * @param[in] aSize Size of buffer (number of bytes to fill). + * + * @retval kErrorNone Successfully filled buffer with random values. + * + */ + static int CryptoSecurePrng(void *aContext, unsigned char *aBuffer, size_t aSize); +#endif }; /** diff --git a/src/core/meshcop/dtls.cpp b/src/core/meshcop/dtls.cpp index e90bbfcdc32..12a43032ce6 100644 --- a/src/core/meshcop/dtls.cpp +++ b/src/core/meshcop/dtls.cpp @@ -281,7 +281,7 @@ Error Dtls::Setup(bool aClient) OT_UNUSED_VARIABLE(mVerifyPeerCertificate); #endif - mbedtls_ssl_conf_rng(&mConf, mbedtls_ctr_drbg_random, Random::Crypto::MbedTlsContextGet()); + mbedtls_ssl_conf_rng(&mConf, Crypto::MbedTls::CryptoSecurePrng, nullptr); mbedtls_ssl_conf_min_version(&mConf, MBEDTLS_SSL_MAJOR_VERSION_3, MBEDTLS_SSL_MINOR_VERSION_3); mbedtls_ssl_conf_max_version(&mConf, MBEDTLS_SSL_MAJOR_VERSION_3, MBEDTLS_SSL_MINOR_VERSION_3); @@ -307,7 +307,7 @@ Error Dtls::Setup(bool aClient) #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_COOKIE_C) if (!aClient) { - rval = mbedtls_ssl_cookie_setup(&mCookieCtx, mbedtls_ctr_drbg_random, Random::Crypto::MbedTlsContextGet()); + rval = mbedtls_ssl_cookie_setup(&mCookieCtx, Crypto::MbedTls::CryptoSecurePrng, nullptr); VerifyOrExit(rval == 0); mbedtls_ssl_conf_dtls_cookies(&mConf, mbedtls_ssl_cookie_write, mbedtls_ssl_cookie_check, &mCookieCtx); @@ -386,8 +386,8 @@ int Dtls::SetApplicationCoapSecureKeys(void) #if (MBEDTLS_VERSION_NUMBER >= 0x03000000) rval = mbedtls_pk_parse_key(&mPrivateKey, static_cast(mPrivateKeySrc), - static_cast(mPrivateKeyLength), nullptr, 0, mbedtls_ctr_drbg_random, - Random::Crypto::MbedTlsContextGet()); + static_cast(mPrivateKeyLength), nullptr, 0, + Crypto::MbedTls::CryptoSecurePrng, nullptr); #else rval = mbedtls_pk_parse_key(&mPrivateKey, static_cast(mPrivateKeySrc), static_cast(mPrivateKeyLength), nullptr, 0); From 3606149e9242f693f3856c962c2bbbb4c881898a Mon Sep 17 00:00:00 2001 From: Lukasz Duda Date: Wed, 26 Jan 2022 14:39:17 +0100 Subject: [PATCH 17/18] Address review comment --- src/core/crypto/crypto_platform.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/crypto/crypto_platform.cpp b/src/core/crypto/crypto_platform.cpp index f9ca284b40c..8e8f414924f 100644 --- a/src/core/crypto/crypto_platform.cpp +++ b/src/core/crypto/crypto_platform.cpp @@ -54,7 +54,7 @@ using namespace ot; using namespace Crypto; -#if OPENTHREAD_CONFIG_CRYPTO_LIB != OPENTHREAD_CONFIG_CRYPTO_LIB_PSA +#if OPENTHREAD_CONFIG_CRYPTO_LIB == OPENTHREAD_CONFIG_CRYPTO_LIB_MBEDTLS //--------------------------------------------------------------------------------------------------------------------- // Default/weak implementation of crypto platform APIs @@ -485,4 +485,4 @@ OT_TOOL_WEAK otError otPlatCryptoRandomGet(uint8_t *aBuffer, uint16_t aSize) #endif // #if !OPENTHREAD_RADIO -#endif // #if OPENTHREAD_CONFIG_CRYPTO_LIB != OPENTHREAD_CONFIG_CRYPTO_LIB_PSA +#endif // #if OPENTHREAD_CONFIG_CRYPTO_LIB == OPENTHREAD_CONFIG_CRYPTO_LIB_MBEDTLS From 4fd25e35a88e457f66aa872ef3baf0263a213719 Mon Sep 17 00:00:00 2001 From: Lukasz Duda Date: Thu, 27 Jan 2022 18:02:00 +0100 Subject: [PATCH 18/18] Change return types --- include/openthread/instance.h | 2 +- include/openthread/platform/crypto.h | 15 +++------------ src/core/common/random_manager.cpp | 4 ++-- src/core/crypto/crypto_platform.cpp | 15 +++++++-------- src/core/thread/key_manager.cpp | 2 +- 5 files changed, 14 insertions(+), 24 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 699941c5def..4aec89d3b39 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (187) +#define OPENTHREAD_API_VERSION (188) /** * @addtogroup api-instance diff --git a/include/openthread/platform/crypto.h b/include/openthread/platform/crypto.h index 5373d8dac94..0d9bf5ca3ee 100644 --- a/include/openthread/platform/crypto.h +++ b/include/openthread/platform/crypto.h @@ -133,11 +133,8 @@ typedef struct otCryptoContext /** * Initialize the Crypto module. * - * @retval OT_ERROR_NONE Successfully initialized Crypto module. - * @retval OT_ERROR_FAILED Failed to initialize Crypto module. - * */ -otError otPlatCryptoInit(void); +void otPlatCryptoInit(void); /** * Import a key into PSA ITS. @@ -472,20 +469,14 @@ otError otPlatCryptoSha256Finish(otCryptoContext *aContext, uint8_t *aHash, uint /** * Initialize cryptographically-secure pseudorandom number generator (CSPRNG). * - * @retval OT_ERROR_NONE Successfully initialized CSPRNG module. - * @retval OT_ERROR_FAILED Failed to initialize CSPRNG module. - * */ -otError otPlatCryptoRandomInit(void); +void otPlatCryptoRandomInit(void); /** * Deinitialize cryptographically-secure pseudorandom number generator (CSPRNG). * - * @retval OT_ERROR_NONE Successfully deinitialized CSPRNG module. - * @retval OT_ERROR_FAILED Failed to deinitialize CSPRNG module. - * */ -otError otPlatCryptoRandomDeinit(void); +void otPlatCryptoRandomDeinit(void); /** * Fills a given buffer with cryptographically secure random bytes. diff --git a/src/core/common/random_manager.cpp b/src/core/common/random_manager.cpp index f050a5514d6..3ff7ffee356 100644 --- a/src/core/common/random_manager.cpp +++ b/src/core/common/random_manager.cpp @@ -55,7 +55,7 @@ RandomManager::RandomManager(void) VerifyOrExit(sInitCount == 0); #if !OPENTHREAD_RADIO - SuccessOrAssert(otPlatCryptoRandomInit()); + otPlatCryptoRandomInit(); SuccessOrAssert(Random::Crypto::FillBuffer(reinterpret_cast(&seed), sizeof(seed))); #else SuccessOrAssert(otPlatEntropyGet(reinterpret_cast(&seed), sizeof(seed))); @@ -75,7 +75,7 @@ RandomManager::~RandomManager(void) VerifyOrExit(sInitCount == 0); #if !OPENTHREAD_RADIO - SuccessOrAssert(otPlatCryptoRandomDeinit()); + otPlatCryptoRandomDeinit(); #endif exit: diff --git a/src/core/crypto/crypto_platform.cpp b/src/core/crypto/crypto_platform.cpp index 8e8f414924f..7146151bacc 100644 --- a/src/core/crypto/crypto_platform.cpp +++ b/src/core/crypto/crypto_platform.cpp @@ -70,9 +70,9 @@ static mbedtls_entropy_context sEntropyContext; static constexpr uint16_t kEntropyMinThreshold = 16; #endif -OT_TOOL_WEAK otError otPlatCryptoInit(void) +OT_TOOL_WEAK void otPlatCryptoInit(void) { - return kErrorNone; + // Intentionally empty. } // AES Implementation @@ -454,7 +454,7 @@ static int handleMbedtlsEntropyPoll(void *aData, unsigned char *aOutput, size_t #endif // OT_MBEDTLS_STRONG_DEFAULT_ENTROPY_PRESENT -OT_TOOL_WEAK otError otPlatCryptoRandomInit(void) +OT_TOOL_WEAK void otPlatCryptoRandomInit(void) { mbedtls_entropy_init(&sEntropyContext); @@ -465,16 +465,15 @@ OT_TOOL_WEAK otError otPlatCryptoRandomInit(void) mbedtls_ctr_drbg_init(&sCtrDrbgContext); - return ot::Crypto::MbedTls::MapError( - mbedtls_ctr_drbg_seed(&sCtrDrbgContext, mbedtls_entropy_func, &sEntropyContext, nullptr, 0)); + int rval = mbedtls_ctr_drbg_seed(&sCtrDrbgContext, mbedtls_entropy_func, &sEntropyContext, nullptr, 0); + OT_ASSERT(rval == 0); + OT_UNUSED_VARIABLE(rval); } -OT_TOOL_WEAK otError otPlatCryptoRandomDeinit(void) +OT_TOOL_WEAK void otPlatCryptoRandomDeinit(void) { mbedtls_entropy_free(&sEntropyContext); mbedtls_ctr_drbg_free(&sCtrDrbgContext); - - return kErrorNone; } OT_TOOL_WEAK otError otPlatCryptoRandomGet(uint8_t *aBuffer, uint16_t aSize) diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index 6757fb49e5d..08c8b3e2b2c 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -181,7 +181,7 @@ KeyManager::KeyManager(Instance &aInstance) , mKekFrameCounter(0) , mIsPskcSet(false) { - IgnoreError(otPlatCryptoInit()); + otPlatCryptoInit(); #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE {