diff --git a/src/cli/README_DATASET.md b/src/cli/README_DATASET.md index a1882af40..cba2b9bc5 100644 --- a/src/cli/README_DATASET.md +++ b/src/cli/README_DATASET.md @@ -44,7 +44,7 @@ The Pending Operational Dataset is used to communicate changes to the Active Ope Network Name: OpenThread-5938 PAN ID: 0x5938 PSKc: 3ca67c969efb0d0c74a4d8ee923b576c - Security Policy: 672 onrc + Security Policy: 672 onrc 0 Done ``` @@ -103,7 +103,7 @@ After the device successfully attaches to a Thread network, the device will retr Network Name: OpenThread-5938 PAN ID: 0x5938 PSKc: 3ca67c969efb0d0c74a4d8ee923b576c - Security Policy: 672 onrc + Security Policy: 672 onrc 0 Done ``` @@ -183,7 +183,7 @@ Network Key: f366cec7a446bab978d90d27abe38f23 Network Name: OpenThread-5938 PAN ID: 0x5938 PSKc: 3ca67c969efb0d0c74a4d8ee923b576c -Security Policy: 672 onrc +Security Policy: 672 onrc 0 Done ``` @@ -362,7 +362,7 @@ Usage: `dataset mgmtsetcommand [TLV Type list] [-x]` Send MGMT_ACTIVE_SET or MGMT_PENDING_SET. ```bash -> dataset mgmtsetcommand active activetimestamp 123 securitypolicy 1 onrc +> dataset mgmtsetcommand active activetimestamp 123 securitypolicy 1 onrc 0 Done ``` @@ -444,7 +444,7 @@ Network Key: ed916e454d96fd00184f10a6f5c9e1d3 Network Name: OpenThread-bff8 PAN ID: 0xbff8 PSKc: 264f78414adc683191863d968f72d1b7 -Security Policy: 672 onrc +Security Policy: 672 onrc 0 Done ``` @@ -500,13 +500,13 @@ Done ### securitypolicy -Usage: `dataset securitypolicy [ [onrcCepR]]` +Usage: `dataset securitypolicy [ [onrcCepR] [versionthreshold]]` Get security policy. ```bash > dataset securitypolicy -672 onrc +672 onrc 0 Done ``` @@ -521,8 +521,10 @@ Set security policy. - p: Thread 1.2 Network Key Provisioning is enabled. - R: Non-CCM routers are allowed in Thread 1.2 CCM networks. +If the `versionthreshold` parameter is not provided, a default value of zero is assumed. + ```bash -> dataset securitypolicy 672 onrc +> dataset securitypolicy 672 onrc 0 Done ``` @@ -561,7 +563,7 @@ Network Key: 9929154dbc363218bcd22f907caf5c15 Network Name: OpenThread-de2b PAN ID: 0xde2b PSKc: 15b2c16f7ba92ed4bc7b1ee054f1553f -Security Policy: 672 onrc +Security Policy: 672 onrc 0 Done > dataset tlvs diff --git a/src/cli/cli_dataset.cpp b/src/cli/cli_dataset.cpp index 633fa5b12..e74ec635a 100644 --- a/src/cli/cli_dataset.cpp +++ b/src/cli/cli_dataset.cpp @@ -1093,7 +1093,7 @@ exit: void Dataset::OutputSecurityPolicy(const otSecurityPolicy &aSecurityPolicy) { - OutputFormat("%d ", aSecurityPolicy.mRotationTime); + OutputFormat("%u ", aSecurityPolicy.mRotationTime); if (aSecurityPolicy.mObtainNetworkKeyEnabled) { @@ -1135,13 +1135,16 @@ void Dataset::OutputSecurityPolicy(const otSecurityPolicy &aSecurityPolicy) OutputFormat("R"); } - OutputNewLine(); + OutputLine(" %u", aSecurityPolicy.mVersionThresholdForRouting); } otError Dataset::ParseSecurityPolicy(otSecurityPolicy &aSecurityPolicy, Arg *&aArgs) { + static constexpr uint8_t kMaxVersionThreshold = 7; + otError error; otSecurityPolicy policy; + uint8_t versionThreshold; memset(&policy, 0, sizeof(policy)); @@ -1192,6 +1195,11 @@ otError Dataset::ParseSecurityPolicy(otSecurityPolicy &aSecurityPolicy, Arg *&aA } aArgs++; + VerifyOrExit(!aArgs->IsEmpty()); + + SuccessOrExit(error = aArgs->ParseAsUint8(versionThreshold)); + VerifyOrExit(versionThreshold <= kMaxVersionThreshold, error = OT_ERROR_INVALID_ARGS); + policy.mVersionThresholdForRouting = versionThreshold; exit: if (error == OT_ERROR_NONE) diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 6ceed76b6..8ce0a35d4 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -1236,6 +1236,13 @@ void Mle::HandleNotifierEvents(Events aEvents) } } +#if OPENTHREAD_FTD + if (aEvents.Contains(kEventSecurityPolicyChanged)) + { + Get().HandleSecurityPolicyChanged(); + } +#endif + exit: return; } diff --git a/src/core/thread/mle_router.cpp b/src/core/thread/mle_router.cpp index 3e63ebe1b..cd2c3129e 100644 --- a/src/core/thread/mle_router.cpp +++ b/src/core/thread/mle_router.cpp @@ -188,6 +188,30 @@ exit: return error; } +void MleRouter::HandleSecurityPolicyChanged(void) +{ + // If we are currently router or leader and no longer eligible to + // be a router (due to security policy change), we start jitter + // timeout to downgrade. + + VerifyOrExit(IsRouterOrLeader() && !IsRouterEligible()); + + // `mRouterSelectionJitterTimeout` is non-zero if we are already + // waiting to downgrade. + + VerifyOrExit(mRouterSelectionJitterTimeout == 0); + + mRouterSelectionJitterTimeout = 1 + Random::NonCrypto::GetUint8InRange(0, mRouterSelectionJitter); + + if (IsLeader()) + { + mRouterSelectionJitterTimeout += kLeaderDowngradeExtraDelay; + } + +exit: + return; +} + #if (OPENTHREAD_CONFIG_THREAD_VERSION >= OT_THREAD_VERSION_1_3_1) void MleRouter::SetDeviceProperties(const DeviceProperties &aDeviceProperties) { @@ -1592,9 +1616,15 @@ void MleRouter::HandleTimeTick(void) Attach(kDowngradeToReed); } - break; + OT_FALL_THROUGH; case kRoleLeader: + if (routerStateUpdate && !IsRouterEligible()) + { + LogInfo("No longer router eligible"); + IgnoreError(BecomeDetached()); + } + break; case kRoleDisabled: diff --git a/src/core/thread/mle_router.hpp b/src/core/thread/mle_router.hpp index 00cbade6a..e7caafe2d 100644 --- a/src/core/thread/mle_router.hpp +++ b/src/core/thread/mle_router.hpp @@ -566,6 +566,7 @@ private: static constexpr uint16_t kDiscoveryMaxJitter = 250; // Max jitter delay Discovery Responses (in msec). static constexpr uint16_t kChallengeTimeout = 2; // Challenge timeout (in sec). static constexpr uint16_t kUnsolicitedDataResponseJitter = 500; // Max delay for unsol Data Response (in msec). + static constexpr uint8_t kLeaderDowngradeExtraDelay = 10; // Extra delay to downgrade leader (in sec). static constexpr uint8_t kDefaultLeaderWeight = 64; // Threshold to accept a router upgrade request with reason @@ -585,6 +586,7 @@ private: void HandleDetachStart(void); void HandleChildStart(AttachMode aMode); + void HandleSecurityPolicyChanged(void); void HandleLinkRequest(RxInfo &aRxInfo); void HandleLinkAccept(RxInfo &aRxInfo); Error HandleLinkAccept(RxInfo &aRxInfo, bool aRequest); diff --git a/tests/scripts/thread-cert/Cert_9_2_05_ActiveDataset.py b/tests/scripts/thread-cert/Cert_9_2_05_ActiveDataset.py index 3c54cf04d..f495e8203 100755 --- a/tests/scripts/thread-cert/Cert_9_2_05_ActiveDataset.py +++ b/tests/scripts/thread-cert/Cert_9_2_05_ActiveDataset.py @@ -90,7 +90,7 @@ class Cert_9_2_05_ActiveDataset(thread_cert.TestCase): # Step 2: new, valid Timestamp TLV # all valid Active Operational Dataset parameters, # with new values in the TLVs that don’t affect connectivity - # binary = new pskc and security policy [3600, 0b11100000] + # binary = new pskc and security policy [3600, 0b11101111] self.nodes[ROUTER].send_mgmt_active_set( active_timestamp=100, channel_mask=0x3fff800, @@ -100,14 +100,14 @@ class Cert_9_2_05_ActiveDataset(thread_cert.TestCase): network_key='00112233445566778899aabbccddeeff', panid=0xface, channel=11, - binary='0410d2aa9cd8dff7919122d77d37ec3c1b5f0c030e10e0', + binary='0410d2aa9cd8dff7919122d77d37ec3c1b5f0c030e10ef', ) self.simulator.go(5) # Step 7: old, invalid Active Timestamp TLV # all valid Active Operational Dataset parameters, with # new values in the TLVs that don’t affect connectivity - # binary = new pskc and security policy [3600, 0b11110000] + # binary = new pskc and security policy [3600, 0b11111111] self.nodes[ROUTER].send_mgmt_active_set( active_timestamp=100, channel_mask=0x1fff800, @@ -117,14 +117,14 @@ class Cert_9_2_05_ActiveDataset(thread_cert.TestCase): network_key='00112233445566778899aabbccddeeff', panid=0xface, channel=11, - binary='041017d672be32b0c24a2f8385f2fbaf1d970c030e10f0', + binary='041017d672be32b0c24a2f8385f2fbaf1d970c030e10ff', ) self.simulator.go(5) # Step 9: new, valid Active Timestamp TLV # all of valid Commissioner Dataset parameters plus one bogus TLV, and # new values in the TLVs that don’t affect connectivity - # binary = new pskc and security policy [3600, 0b11111000] and BogusTLV=0x400 + # binary = new pskc and security policy [3600, 0b11111111] and BogusTLV=0x400 self.nodes[ROUTER].send_mgmt_active_set( active_timestamp=101, channel_mask=0xfff800, @@ -134,7 +134,7 @@ class Cert_9_2_05_ActiveDataset(thread_cert.TestCase): network_key='00112233445566778899aabbccddeeff', panid=0xface, channel=11, - binary='041008f4e9531e8efa8e852d5f4fb951b13e0c030e10f88202aa55', + binary='041008f4e9531e8efa8e852d5f4fb951b13e0c030e10ff8202aa55', ) self.simulator.go(5) diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py index 4124cb10c..a5e78e372 100755 --- a/tests/scripts/thread-cert/node.py +++ b/tests/scripts/thread-cert/node.py @@ -2611,11 +2611,14 @@ class NodeImpl: self.send_command(cmd, go=False) self._expect_done() - if security_policy and len(security_policy) == 2: - cmd = 'dataset securitypolicy %s %s' % ( - str(security_policy[0]), - security_policy[1], - ) + if security_policy is not None: + if len(security_policy) >= 2: + cmd = 'dataset securitypolicy %s %s' % ( + str(security_policy[0]), + security_policy[1], + ) + if len(security_policy) >= 3: + cmd += ' %s' % (str(security_policy[2])) self.send_command(cmd, go=False) self._expect_done() @@ -2723,8 +2726,9 @@ class NodeImpl: cmd += 'networkname %s ' % self._escape_escapable(network_name) if security_policy is not None: - rotation, flags = security_policy - cmd += 'securitypolicy %d %s ' % (rotation, flags) + cmd += 'securitypolicy %d %s ' % (security_policy[0], security_policy[1]) + if (len(security_policy) >= 3): + cmd += '%d ' % (security_policy[2]) if binary is not None: cmd += '-x %s ' % binary diff --git a/tests/scripts/thread-cert/test_router_downgrade_on_sec_policy_change.py b/tests/scripts/thread-cert/test_router_downgrade_on_sec_policy_change.py new file mode 100755 index 000000000..5241ad17d --- /dev/null +++ b/tests/scripts/thread-cert/test_router_downgrade_on_sec_policy_change.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +# +# Copyright (c) 2023, 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 ipaddress +import unittest + +import command +import config +import thread_cert + +# Test description: +# This test verifies leader and router downgrade delay on security policy version threshold change. +# +# Topology: +# +# LEADER +# | +# | +# ROUTER +# + +LEADER = 1 +ROUTER = 2 + + +class DowngradeOnSecPolicyChange(thread_cert.TestCase): + USE_MESSAGE_FACTORY = False + SUPPORT_NCP = False + + TOPOLOGY = { + LEADER: { + 'name': 'LEADER', + 'mode': 'rdn', + }, + ROUTER: { + 'name': 'ROUTER', + 'mode': 'rdn', + }, + } + + def test(self): + leader = self.nodes[LEADER] + router = self.nodes[ROUTER] + + # Start the leader & router devices. + + leader.start() + self.simulator.go(config.LEADER_STARTUP_DELAY) + self.assertEqual(leader.get_state(), 'leader') + + router.start() + self.simulator.go(config.ROUTER_STARTUP_DELAY) + self.assertEqual(router.get_state(), 'router') + + self.assertTrue(leader.get_router_eligible()) + self.assertTrue(router.get_router_eligible()) + + # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + # Change security policy, disable `R` bit and set version threshold to max value (7). + + leader.send_mgmt_active_set( + active_timestamp=30, + security_policy=[3600, 'onc', 7], + ) + + self.assertEqual(leader.get_state(), 'leader') + self.assertEqual(router.get_state(), 'router') + + # Leader should take at least 10 seconds before downgrading. + + self.simulator.go(8) + self.assertEqual(leader.get_state(), 'leader') + self.assertFalse(leader.get_router_eligible()) + self.assertFalse(router.get_router_eligible()) + + # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + # Change back security policy. This should cancel the ongoing downgrade delay. + + leader.send_mgmt_active_set( + active_timestamp=60, + security_policy=[3600, 'ornc', 0], + ) + + self.simulator.go(1) + self.assertEqual(leader.get_state(), 'leader') + self.assertTrue(leader.get_router_eligible()) + + # Make sure both device stay as leader and router. + + self.simulator.go(600) + self.assertEqual(leader.get_state(), 'leader') + self.assertEqual(router.get_state(), 'router') + self.assertTrue(leader.get_router_eligible()) + self.assertTrue(router.get_router_eligible()) + + # - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + # Change security policy again, disable `R` bit and set version threshold to max value (7). + + leader.send_mgmt_active_set( + active_timestamp=90, + security_policy=[3600, 'onc', 7], + ) + + self.assertEqual(leader.get_state(), 'leader') + + # Leader should take at least 10 seconds before downgrading. + + self.simulator.go(8) + self.assertEqual(leader.get_state(), 'leader') + self.assertFalse(leader.get_router_eligible()) + + # Make sure both leader and router are downgraded and are now `detached`. + + self.simulator.go(80) + self.assertEqual(leader.get_state(), 'detached') + self.assertEqual(router.get_state(), 'detached') + + self.assertFalse(leader.get_router_eligible()) + self.assertFalse(router.get_router_eligible()) + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/otci/otci/otci.py b/tools/otci/otci/otci.py index d9d5c1f25..7df5930fd 100644 --- a/tools/otci/otci/otci.py +++ b/tools/otci/otci/otci.py @@ -1738,7 +1738,7 @@ class OTCI(object): # Network Name: OpenThread-7caa # PAN ID: 0x7caa # PSKc: 167d89fd169e439ca0b8266de248090f - # Security Policy: 0, onrc + # Security Policy: 672 onrc 0 dataset = {} @@ -1765,7 +1765,7 @@ class OTCI(object): elif key == 'PSKc': dataset['pskc'] = val elif key == 'Security Policy': - rotation_time, flags = val.split(', ') if ', ' in val else val.split(' ') + rotation_time, flags, version_threshold = val.split(' ') rotation_time = int(rotation_time) dataset['security_policy'] = SecurityPolicy(rotation_time, flags) else: