diff options
| -rw-r--r-- | include/openthread/thread.h | 4 | ||||
| -rw-r--r-- | src/cli/README.md | 6 | ||||
| -rw-r--r-- | src/core/api/thread_api.cpp | 6 | ||||
| -rw-r--r-- | src/core/mac/mac.cpp | 2 | ||||
| -rw-r--r-- | src/core/thread/key_manager.cpp | 77 | ||||
| -rw-r--r-- | src/core/thread/key_manager.hpp | 60 | ||||
| -rw-r--r-- | src/core/thread/mle.cpp | 6 | ||||
| -rw-r--r-- | src/ncp/ncp_base_mtd.cpp | 2 | ||||
| -rwxr-xr-x | tests/scripts/thread-cert/test_mle_msg_key_seq_jump.py | 12 |
9 files changed, 60 insertions, 115 deletions
diff --git a/include/openthread/thread.h b/include/openthread/thread.h index 4d814c119..b077001e4 100644 --- a/include/openthread/thread.h +++ b/include/openthread/thread.h @@ -709,7 +709,7 @@ void otThreadSetKeySequenceCounter(otInstance *aInstance, uint32_t aKeySequenceC * @sa otThreadSetKeySwitchGuardTime * */ -uint16_t otThreadGetKeySwitchGuardTime(otInstance *aInstance); +uint32_t otThreadGetKeySwitchGuardTime(otInstance *aInstance); /** * Sets the thrKeySwitchGuardTime (in hours). @@ -723,7 +723,7 @@ uint16_t otThreadGetKeySwitchGuardTime(otInstance *aInstance); * @sa otThreadGetKeySwitchGuardTime * */ -void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint16_t aKeySwitchGuardTime); +void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint32_t aKeySwitchGuardTime); /** * Detach from the Thread network. diff --git a/src/cli/README.md b/src/cli/README.md index c8dfe2744..e2b43c8cc 100644 --- a/src/cli/README.md +++ b/src/cli/README.md @@ -1826,8 +1826,6 @@ Done Set the Thread Key Sequence Counter. -This command is reserved for testing and demo purposes only. Changing Key Sequence Counter will render a production application non-compliant with the Thread Specification. - ```bash > keysequence counter 10 Done @@ -1845,9 +1843,7 @@ Done ### keysequence guardtime \<guardtime\> -Set Thread Key Switch Guard Time (in hours). - -This command is reserved for testing and demo purposes only. Changing Key Switch Guard Time will render a production application non-compliant with the Thread Specification. +Set Thread Key Switch Guard Time (in hours) 0 means Thread Key Switch immediately if key index match ```bash > keysequence guardtime 0 diff --git a/src/core/api/thread_api.cpp b/src/core/api/thread_api.cpp index 97e5508f0..6bf2ca39d 100644 --- a/src/core/api/thread_api.cpp +++ b/src/core/api/thread_api.cpp @@ -275,15 +275,15 @@ uint32_t otThreadGetKeySequenceCounter(otInstance *aInstance) void otThreadSetKeySequenceCounter(otInstance *aInstance, uint32_t aKeySequenceCounter) { - AsCoreType(aInstance).Get<KeyManager>().SetCurrentKeySequence(aKeySequenceCounter, KeyManager::kForceUpdate); + AsCoreType(aInstance).Get<KeyManager>().SetCurrentKeySequence(aKeySequenceCounter); } -uint16_t otThreadGetKeySwitchGuardTime(otInstance *aInstance) +uint32_t otThreadGetKeySwitchGuardTime(otInstance *aInstance) { return AsCoreType(aInstance).Get<KeyManager>().GetKeySwitchGuardTime(); } -void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint16_t aKeySwitchGuardTime) +void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint32_t aKeySwitchGuardTime) { AsCoreType(aInstance).Get<KeyManager>().SetKeySwitchGuardTime(aKeySwitchGuardTime); } diff --git a/src/core/mac/mac.cpp b/src/core/mac/mac.cpp index e70fc982b..8c4d4771a 100644 --- a/src/core/mac/mac.cpp +++ b/src/core/mac/mac.cpp @@ -1641,7 +1641,7 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig if (keySequence > keyManager.GetCurrentKeySequence()) { - keyManager.SetCurrentKeySequence(keySequence, KeyManager::kApplyKeySwitchGuard); + keyManager.SetCurrentKeySequence(keySequence); } } diff --git a/src/core/thread/key_manager.cpp b/src/core/thread/key_manager.cpp index d38fdb8b5..3d9337d93 100644 --- a/src/core/thread/key_manager.cpp +++ b/src/core/thread/key_manager.cpp @@ -60,9 +60,6 @@ const uint8_t KeyManager::kTrelInfoString[] = {'T', 'h', 'r', 'e', 'a', 'd', 'O' 'r', 'I', 'n', 'f', 'r', 'a', 'K', 'e', 'y'}; #endif -//--------------------------------------------------------------------------------------------------------------------- -// SecurityPolicy - void SecurityPolicy::SetToDefault(void) { mRotationTime = kDefaultKeyRotationTime; @@ -166,9 +163,6 @@ exit: return; } -//--------------------------------------------------------------------------------------------------------------------- -// KeyManager - KeyManager::KeyManager(Instance &aInstance) : InstanceLocator(aInstance) , mKeySequence(0) @@ -177,7 +171,7 @@ KeyManager::KeyManager(Instance &aInstance) , mStoredMleFrameCounter(0) , mHoursSinceKeyRotation(0) , mKeySwitchGuardTime(kDefaultKeySwitchGuardTime) - , mKeySwitchGuardTimer(0) + , mKeySwitchGuardEnabled(false) , mKeyRotationTimer(aInstance) , mKekFrameCounter(0) , mIsPskcSet(false) @@ -204,8 +198,8 @@ KeyManager::KeyManager(Instance &aInstance) void KeyManager::Start(void) { - mKeySwitchGuardTimer = 0; - ResetKeyRotationTimer(); + mKeySwitchGuardEnabled = false; + StartKeyRotationTimer(); } void KeyManager::Stop(void) { mKeyRotationTimer.Stop(); } @@ -368,13 +362,20 @@ void KeyManager::UpdateKeyMaterial(void) #endif } -void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence, KeySequenceUpdateMode aUpdateMode) +void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence) { VerifyOrExit(aKeySequence != mKeySequence, Get<Notifier>().SignalIfFirst(kEventThreadKeySeqCounterChanged)); - if (aUpdateMode == kApplyKeySwitchGuard) + if ((aKeySequence == (mKeySequence + 1)) && mKeyRotationTimer.IsRunning()) { - VerifyOrExit(mKeySwitchGuardTimer == 0); + if (mKeySwitchGuardEnabled) + { + // Check if the guard timer has expired if key rotation is requested. + VerifyOrExit(mHoursSinceKeyRotation >= mKeySwitchGuardTime); + StartKeyRotationTimer(); + } + + mKeySwitchGuardEnabled = true; } mKeySequence = aKeySequence; @@ -383,9 +384,6 @@ void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence, KeySequenceUpdateM SetAllMacFrameCounters(0, /* aSetIfLarger */ false); mMleFrameCounter = 0; - ResetKeyRotationTimer(); - mKeySwitchGuardTimer = mKeySwitchGuardTime; - Get<Notifier>().Signal(kEventThreadKeySeqCounterChanged); exit: @@ -478,57 +476,40 @@ void KeyManager::SetKek(const Kek &aKek) void KeyManager::SetSecurityPolicy(const SecurityPolicy &aSecurityPolicy) { - SecurityPolicy newPolicy = aSecurityPolicy; - - if (newPolicy.mRotationTime < SecurityPolicy::kMinKeyRotationTime) - { - newPolicy.mRotationTime = SecurityPolicy::kMinKeyRotationTime; - LogNote("Key Rotation Time in SecurityPolicy is set to min allowed value of %u", newPolicy.mRotationTime); - } - - if (newPolicy.mRotationTime != mSecurityPolicy.mRotationTime) + if (aSecurityPolicy.mRotationTime < SecurityPolicy::kMinKeyRotationTime) { - uint32_t newGuardTime = newPolicy.mRotationTime; - - // Calculations are done using a `uint32_t` variable to prevent - // potential overflow. - - newGuardTime *= kKeySwitchGuardTimePercentage; - newGuardTime /= 100; - - mKeySwitchGuardTime = static_cast<uint16_t>(newGuardTime); + LogNote("Key Rotation Time too small: %d", aSecurityPolicy.mRotationTime); + ExitNow(); } - IgnoreError(Get<Notifier>().Update(mSecurityPolicy, newPolicy, kEventSecurityPolicyChanged)); + IgnoreError(Get<Notifier>().Update(mSecurityPolicy, aSecurityPolicy, kEventSecurityPolicyChanged)); - CheckForKeyRotation(); +exit: + return; } -void KeyManager::ResetKeyRotationTimer(void) +void KeyManager::StartKeyRotationTimer(void) { mHoursSinceKeyRotation = 0; - mKeyRotationTimer.Start(Time::kOneHourInMsec); + mKeyRotationTimer.Start(kOneHourIntervalInMsec); } void KeyManager::HandleKeyRotationTimer(void) { - mKeyRotationTimer.Start(Time::kOneHourInMsec); - mHoursSinceKeyRotation++; - if (mKeySwitchGuardTimer > 0) - { - mKeySwitchGuardTimer--; - } + // Order of operations below is important. We should restart the timer (from + // last fire time for one hour interval) before potentially calling + // `SetCurrentKeySequence()`. `SetCurrentKeySequence()` uses the fact that + // timer is running to decide to check for the guard time and to reset the + // rotation timer (and the `mHoursSinceKeyRotation`) if it updates the key + // sequence. - CheckForKeyRotation(); -} + mKeyRotationTimer.StartAt(mKeyRotationTimer.GetFireTime(), kOneHourIntervalInMsec); -void KeyManager::CheckForKeyRotation(void) -{ if (mHoursSinceKeyRotation >= mSecurityPolicy.mRotationTime) { - SetCurrentKeySequence(mKeySequence + 1, kForceUpdate); + SetCurrentKeySequence(mKeySequence + 1); } } diff --git a/src/core/thread/key_manager.hpp b/src/core/thread/key_manager.hpp index 099854c45..18f11f20b 100644 --- a/src/core/thread/key_manager.hpp +++ b/src/core/thread/key_manager.hpp @@ -77,17 +77,8 @@ public: */ static constexpr uint8_t kVersionThresholdOffsetVersion = 3; - /** - * Default Key Rotation Time (in unit of hours). - * - */ - static constexpr uint16_t kDefaultKeyRotationTime = 672; - - /** - * Minimum Key Rotation Time (in unit of hours). - * - */ - static constexpr uint16_t kMinKeyRotationTime = 2; + static constexpr uint16_t kMinKeyRotationTime = 1; ///< The minimum Key Rotation Time in hours. + static constexpr uint16_t kDefaultKeyRotationTime = 672; ///< Default Key Rotation Time (in unit of hours). /** * Initializes the object with default Key Rotation Time @@ -221,18 +212,6 @@ class KeyManager : public InstanceLocator, private NonCopyable { public: /** - * Determines whether to apply or ignore key switch guard when updating the key sequence. - * - * Used as input by `SetCurrentKeySequence()`. - * - */ - enum KeySequenceUpdateMode : uint8_t - { - kApplyKeySwitchGuard, ///< Apply key switch guard check before setting the new key sequence. - kForceUpdate, ///< Ignore key switch guard check and forcibly update the key sequence to new value. - }; - - /** * Initializes the object. * * @param[in] aInstance A reference to the OpenThread instance. @@ -342,14 +321,10 @@ public: /** * Sets the current key sequence value. * - * If @p aMode is `kApplyKeySwitchGuard`, the current key switch guard timer is checked and only if it is zero, key - * sequence will be updated. - * - * @param[in] aKeySequence The key sequence value. - * @param[in] aUpdateMode Whether or not to apply the key switch guard. + * @param[in] aKeySequence The key sequence value. * */ - void SetCurrentKeySequence(uint32_t aKeySequence, KeySequenceUpdateMode aUpdateMode); + void SetCurrentKeySequence(uint32_t aKeySequence); #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE /** @@ -525,19 +500,17 @@ public: * @returns The KeySwitchGuardTime value in hours. * */ - uint16_t GetKeySwitchGuardTime(void) const { return mKeySwitchGuardTime; } + uint32_t GetKeySwitchGuardTime(void) const { return mKeySwitchGuardTime; } /** * Sets the KeySwitchGuardTime. * * The KeySwitchGuardTime is the time interval during which key rotation procedure is prevented. * - * Intended for testing only. Changing the guard time will render device non-compliant with the Thread spec. - * - * @param[in] aGuardTime The KeySwitchGuardTime value in hours. + * @param[in] aKeySwitchGuardTime The KeySwitchGuardTime value in hours. * */ - void SetKeySwitchGuardTime(uint16_t aGuardTime) { mKeySwitchGuardTime = aGuardTime; } + void SetKeySwitchGuardTime(uint32_t aKeySwitchGuardTime) { mKeySwitchGuardTime = aKeySwitchGuardTime; } /** * Returns the Security Policy. @@ -592,13 +565,9 @@ public: #endif private: - static constexpr uint16_t kDefaultKeySwitchGuardTime = 624; // ~ 93% of 672 (default key rotation time) - static constexpr uint32_t kKeySwitchGuardTimePercentage = 93; // Percentage of key rotation time. - static constexpr bool kExportableMacKeys = OPENTHREAD_CONFIG_PLATFORM_MAC_KEYS_EXPORTABLE_ENABLE; - - static_assert(kDefaultKeySwitchGuardTime == - SecurityPolicy::kDefaultKeyRotationTime * kKeySwitchGuardTimePercentage / 100, - "Default key switch guard time value is not correct"); + static constexpr uint32_t kDefaultKeySwitchGuardTime = 624; + static constexpr uint32_t kOneHourIntervalInMsec = 3600u * 1000u; + static constexpr bool kExportableMacKeys = OPENTHREAD_CONFIG_PLATFORM_MAC_KEYS_EXPORTABLE_ENABLE; OT_TOOL_PACKED_BEGIN struct Keys @@ -622,9 +591,8 @@ private: void ComputeTrelKey(uint32_t aKeySequence, Mac::Key &aKey) const; #endif - void ResetKeyRotationTimer(void); + void StartKeyRotationTimer(void); void HandleKeyRotationTimer(void); - void CheckForKeyRotation(void); #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE void StoreNetworkKey(const NetworkKey &aNetworkKey, bool aOverWriteExisting); @@ -662,9 +630,9 @@ private: uint32_t mStoredMacFrameCounter; uint32_t mStoredMleFrameCounter; - uint16_t mHoursSinceKeyRotation; - uint16_t mKeySwitchGuardTime; - uint16_t mKeySwitchGuardTimer; + uint32_t mHoursSinceKeyRotation; + uint32_t mKeySwitchGuardTime; + bool mKeySwitchGuardEnabled; RotationTimer mKeyRotationTimer; #if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 99e6809f8..9a32f77e8 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -378,7 +378,7 @@ void Mle::Restore(void) SuccessOrExit(Get<Settings>().Read(networkInfo)); - Get<KeyManager>().SetCurrentKeySequence(networkInfo.GetKeySequence(), KeyManager::kForceUpdate); + Get<KeyManager>().SetCurrentKeySequence(networkInfo.GetKeySequence()); Get<KeyManager>().SetMleFrameCounter(networkInfo.GetMleFrameCounter()); Get<KeyManager>().SetAllMacFrameCounters(networkInfo.GetMacFrameCounter(), /* aSetIfLarger */ false); @@ -2736,7 +2736,7 @@ void Mle::ProcessKeySequence(RxInfo &aRxInfo) switch (aRxInfo.mClass) { case RxInfo::kAuthoritativeMessage: - Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence, KeyManager::kForceUpdate); + Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence); break; case RxInfo::kPeerMessage: @@ -2744,7 +2744,7 @@ void Mle::ProcessKeySequence(RxInfo &aRxInfo) { if (aRxInfo.mKeySequence - Get<KeyManager>().GetCurrentKeySequence() == 1) { - Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence, KeyManager::kApplyKeySwitchGuard); + Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence); } else { diff --git a/src/ncp/ncp_base_mtd.cpp b/src/ncp/ncp_base_mtd.cpp index 227ce61ce..79bf53538 100644 --- a/src/ncp/ncp_base_mtd.cpp +++ b/src/ncp/ncp_base_mtd.cpp @@ -690,7 +690,7 @@ template <> otError NcpBase::HandlePropertySet<SPINEL_PROP_NET_KEY_SWITCH_GUARDT SuccessOrExit(error = mDecoder.ReadUint32(keyGuardTime)); - otThreadSetKeySwitchGuardTime(mInstance, static_cast<uint16_t>(keyGuardTime)); + otThreadSetKeySwitchGuardTime(mInstance, keyGuardTime); exit: return error; diff --git a/tests/scripts/thread-cert/test_mle_msg_key_seq_jump.py b/tests/scripts/thread-cert/test_mle_msg_key_seq_jump.py index 246343421..4fcb7fa49 100755 --- a/tests/scripts/thread-cert/test_mle_msg_key_seq_jump.py +++ b/tests/scripts/thread-cert/test_mle_msg_key_seq_jump.py @@ -221,20 +221,20 @@ class MleMsgKeySeqJump(thread_cert.TestCase): self.assertEqual(reed.get_key_sequence_counter(), 20) #------------------------------------------------------------------- - # Move forward the key seq counter by two on router. Wait for max + # Move forward the key seq counter by one on router. Wait for max # time between advertisements. Validate that leader adopts the higher # counter value. - router.set_key_sequence_counter(22) - self.assertEqual(router.get_key_sequence_counter(), 22) + router.set_key_sequence_counter(21) + self.assertEqual(router.get_key_sequence_counter(), 21) self.simulator.go(52) - self.assertEqual(leader.get_key_sequence_counter(), 22) - self.assertEqual(reed.get_key_sequence_counter(), 22) + self.assertEqual(leader.get_key_sequence_counter(), 21) + self.assertEqual(reed.get_key_sequence_counter(), 21) child.set_mode('r') self.simulator.go(2) - self.assertEqual(child.get_key_sequence_counter(), 22) + self.assertEqual(child.get_key_sequence_counter(), 21) #------------------------------------------------------------------- # Force a reattachment from the child with a higher key seq counter, |