[mle] delay router role downgrade on security policy change (#9187)

This commit adds a mechanism to delay the downgrade of routers or
leader when the security policy TLV changes in the Active Operational
Dataset such that the device is no longer eligible to act as a
router.

If the decision to become a child is made due to a security policy
change, the device first delays a random period up to the "router
selection jitter" before downgrading. If the device is the leader, an
additional fixed delay of 10 seconds is added to the random period.

If the security policy changes again while the device is waiting to
downgrade such that it becomes router-eligible again, the downgrade
is cancelled and the device remains in its current role.

This commit adds a `test_router_downgrade_on_sec_policy_change` to
validate the behavior of newly added mechanism.

This commit also updates the CLI `dataset` sub-commands to allow
getting and setting the "version threshold for routing" (VR) field in
security policy.
This commit is contained in:
Abtin Keshavarzian
2023-06-30 15:09:50 -07:00
committed by GitHub
parent 60db56f0c0
commit ef8deb506f
9 changed files with 229 additions and 27 deletions
+11 -9
View File
@@ -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 <active|pending> [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 [<rotationtime> [onrcCepR]]`
Usage: `dataset securitypolicy [<rotationtime> [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
+10 -2
View File
@@ -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)
+7
View File
@@ -1236,6 +1236,13 @@ void Mle::HandleNotifierEvents(Events aEvents)
}
}
#if OPENTHREAD_FTD
if (aEvents.Contains(kEventSecurityPolicyChanged))
{
Get<MleRouter>().HandleSecurityPolicyChanged();
}
#endif
exit:
return;
}
+31 -1
View File
@@ -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:
+2
View File
@@ -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);
@@ -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 dont 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 dont 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 dont 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)
+11 -7
View File
@@ -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
@@ -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()
+2 -2
View File
@@ -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: