[key-manager] update how key guard time is determined and applied (#9871)

This commit makes changes/fixes to `KeyManager` regarding key switch
guard time.

Key Rotation Time updates:
- When the Key Rotation Time changes (due to security policy updates),
  the key switch guard time (`mKeySwitchGuardTime`) is also adjusted.
  It's set to 93% of the Rotation Time (rounded down).
- Immediately checks if the new rotation time indicates a rotation is
  due and keys are rotated.

New variable `mKeySwitchGuardTimer`:
- This is reset to the current guard time whenever the key sequence is
  updated.
- It decrements hourly until reaching zero.
- Key switch guard comparison is made with this value, aligning the
  implementation with the Thread specification.

`SetCurrentKeySequence()` modification:
- Now accepts a new input parameter that determines whether to apply
  or ignore the key switch guard when updating the key sequence.
- During a key rotation check (when the rotation time has passed), the
  key switch guard is ignored and we always move to the next key
  sequence number.

Other changes:
- Variables handling guard and rotation time now use `uint16_t`
  instead of `uint32_t` to align with security policy definitions.
- API and CLI command documentation for setting the "key switch guard
  time" emphasize that they are intended for testing purposes.
This commit is contained in:
Abtin Keshavarzian
2024-03-07 21:39:52 -08:00
committed by GitHub
parent 5b88759da9
commit c66d91bdd7
10 changed files with 116 additions and 61 deletions
+1 -1
View File
@@ -53,7 +53,7 @@ extern "C" {
* @note This number versions both OpenThread platform and user APIs.
*
*/
#define OPENTHREAD_API_VERSION (399)
#define OPENTHREAD_API_VERSION (400)
/**
* @addtogroup api-instance
+2 -2
View File
@@ -709,7 +709,7 @@ void otThreadSetKeySequenceCounter(otInstance *aInstance, uint32_t aKeySequenceC
* @sa otThreadSetKeySwitchGuardTime
*
*/
uint32_t otThreadGetKeySwitchGuardTime(otInstance *aInstance);
uint16_t otThreadGetKeySwitchGuardTime(otInstance *aInstance);
/**
* Sets the thrKeySwitchGuardTime (in hours).
@@ -723,7 +723,7 @@ uint32_t otThreadGetKeySwitchGuardTime(otInstance *aInstance);
* @sa otThreadGetKeySwitchGuardTime
*
*/
void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint32_t aKeySwitchGuardTime);
void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint16_t aKeySwitchGuardTime);
/**
* Detach from the Thread network.
+5 -1
View File
@@ -1826,6 +1826,8 @@ 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
@@ -1843,7 +1845,9 @@ Done
### keysequence guardtime \<guardtime\>
Set Thread Key Switch Guard Time (in hours) 0 means Thread Key Switch immediately if key index match
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.
```bash
> keysequence guardtime 0
+3 -3
View File
@@ -275,15 +275,15 @@ uint32_t otThreadGetKeySequenceCounter(otInstance *aInstance)
void otThreadSetKeySequenceCounter(otInstance *aInstance, uint32_t aKeySequenceCounter)
{
AsCoreType(aInstance).Get<KeyManager>().SetCurrentKeySequence(aKeySequenceCounter);
AsCoreType(aInstance).Get<KeyManager>().SetCurrentKeySequence(aKeySequenceCounter, KeyManager::kForceUpdate);
}
uint32_t otThreadGetKeySwitchGuardTime(otInstance *aInstance)
uint16_t otThreadGetKeySwitchGuardTime(otInstance *aInstance)
{
return AsCoreType(aInstance).Get<KeyManager>().GetKeySwitchGuardTime();
}
void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint32_t aKeySwitchGuardTime)
void otThreadSetKeySwitchGuardTime(otInstance *aInstance, uint16_t aKeySwitchGuardTime)
{
AsCoreType(aInstance).Get<KeyManager>().SetKeySwitchGuardTime(aKeySwitchGuardTime);
}
+1 -1
View File
@@ -1637,7 +1637,7 @@ Error Mac::ProcessReceiveSecurity(RxFrame &aFrame, const Address &aSrcAddr, Neig
if (keySequence > keyManager.GetCurrentKeySequence())
{
keyManager.SetCurrentKeySequence(keySequence);
keyManager.SetCurrentKeySequence(keySequence, KeyManager::kApplyKeySwitchGuard);
}
}
+48 -29
View File
@@ -60,6 +60,9 @@ 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;
@@ -163,6 +166,9 @@ exit:
return;
}
//---------------------------------------------------------------------------------------------------------------------
// KeyManager
KeyManager::KeyManager(Instance &aInstance)
: InstanceLocator(aInstance)
, mKeySequence(0)
@@ -171,7 +177,7 @@ KeyManager::KeyManager(Instance &aInstance)
, mStoredMleFrameCounter(0)
, mHoursSinceKeyRotation(0)
, mKeySwitchGuardTime(kDefaultKeySwitchGuardTime)
, mKeySwitchGuardEnabled(false)
, mKeySwitchGuardTimer(0)
, mKeyRotationTimer(aInstance)
, mKekFrameCounter(0)
, mIsPskcSet(false)
@@ -198,8 +204,8 @@ KeyManager::KeyManager(Instance &aInstance)
void KeyManager::Start(void)
{
mKeySwitchGuardEnabled = false;
StartKeyRotationTimer();
mKeySwitchGuardTimer = 0;
ResetKeyRotationTimer();
}
void KeyManager::Stop(void) { mKeyRotationTimer.Stop(); }
@@ -362,20 +368,13 @@ void KeyManager::UpdateKeyMaterial(void)
#endif
}
void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence)
void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence, KeySequenceUpdateMode aUpdateMode)
{
VerifyOrExit(aKeySequence != mKeySequence, Get<Notifier>().SignalIfFirst(kEventThreadKeySeqCounterChanged));
if ((aKeySequence == (mKeySequence + 1)) && mKeyRotationTimer.IsRunning())
if (aUpdateMode == kApplyKeySwitchGuard)
{
if (mKeySwitchGuardEnabled)
{
// Check if the guard timer has expired if key rotation is requested.
VerifyOrExit(mHoursSinceKeyRotation >= mKeySwitchGuardTime);
StartKeyRotationTimer();
}
mKeySwitchGuardEnabled = true;
VerifyOrExit(mKeySwitchGuardTimer == 0);
}
mKeySequence = aKeySequence;
@@ -384,6 +383,9 @@ void KeyManager::SetCurrentKeySequence(uint32_t aKeySequence)
SetAllMacFrameCounters(0, /* aSetIfLarger */ false);
mMleFrameCounter = 0;
ResetKeyRotationTimer();
mKeySwitchGuardTimer = mKeySwitchGuardTime;
Get<Notifier>().Signal(kEventThreadKeySeqCounterChanged);
exit:
@@ -476,40 +478,57 @@ void KeyManager::SetKek(const Kek &aKek)
void KeyManager::SetSecurityPolicy(const SecurityPolicy &aSecurityPolicy)
{
if (aSecurityPolicy.mRotationTime < SecurityPolicy::kMinKeyRotationTime)
SecurityPolicy newPolicy = aSecurityPolicy;
if (newPolicy.mRotationTime < SecurityPolicy::kMinKeyRotationTime)
{
LogNote("Key Rotation Time too small: %d", aSecurityPolicy.mRotationTime);
ExitNow();
newPolicy.mRotationTime = SecurityPolicy::kMinKeyRotationTime;
LogNote("Key Rotation Time in SecurityPolicy is set to min allowed value of %u", newPolicy.mRotationTime);
}
IgnoreError(Get<Notifier>().Update(mSecurityPolicy, aSecurityPolicy, kEventSecurityPolicyChanged));
if (newPolicy.mRotationTime != mSecurityPolicy.mRotationTime)
{
uint32_t newGuardTime = newPolicy.mRotationTime;
exit:
return;
// Calculations are done using a `uint32_t` variable to prevent
// potential overflow.
newGuardTime *= kKeySwitchGuardTimePercentage;
newGuardTime /= 100;
mKeySwitchGuardTime = static_cast<uint16_t>(newGuardTime);
}
IgnoreError(Get<Notifier>().Update(mSecurityPolicy, newPolicy, kEventSecurityPolicyChanged));
CheckForKeyRotation();
}
void KeyManager::StartKeyRotationTimer(void)
void KeyManager::ResetKeyRotationTimer(void)
{
mHoursSinceKeyRotation = 0;
mKeyRotationTimer.Start(kOneHourIntervalInMsec);
mKeyRotationTimer.Start(Time::kOneHourInMsec);
}
void KeyManager::HandleKeyRotationTimer(void)
{
mKeyRotationTimer.Start(Time::kOneHourInMsec);
mHoursSinceKeyRotation++;
// 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.
if (mKeySwitchGuardTimer > 0)
{
mKeySwitchGuardTimer--;
}
mKeyRotationTimer.StartAt(mKeyRotationTimer.GetFireTime(), kOneHourIntervalInMsec);
CheckForKeyRotation();
}
void KeyManager::CheckForKeyRotation(void)
{
if (mHoursSinceKeyRotation >= mSecurityPolicy.mRotationTime)
{
SetCurrentKeySequence(mKeySequence + 1);
SetCurrentKeySequence(mKeySequence + 1, kForceUpdate);
}
}
+46 -14
View File
@@ -77,8 +77,17 @@ public:
*/
static constexpr uint8_t kVersionThresholdOffsetVersion = 3;
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).
/**
* 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;
/**
* Initializes the object with default Key Rotation Time
@@ -211,6 +220,18 @@ typedef Mac::KeyMaterial KekKeyMaterial;
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.
*
@@ -321,10 +342,14 @@ public:
/**
* Sets the current key sequence value.
*
* @param[in] aKeySequence The 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.
*
*/
void SetCurrentKeySequence(uint32_t aKeySequence);
void SetCurrentKeySequence(uint32_t aKeySequence, KeySequenceUpdateMode aUpdateMode);
#if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE
/**
@@ -500,17 +525,19 @@ public:
* @returns The KeySwitchGuardTime value in hours.
*
*/
uint32_t GetKeySwitchGuardTime(void) const { return mKeySwitchGuardTime; }
uint16_t GetKeySwitchGuardTime(void) const { return mKeySwitchGuardTime; }
/**
* Sets the KeySwitchGuardTime.
*
* The KeySwitchGuardTime is the time interval during which key rotation procedure is prevented.
*
* @param[in] aKeySwitchGuardTime The KeySwitchGuardTime value in hours.
* 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.
*
*/
void SetKeySwitchGuardTime(uint32_t aKeySwitchGuardTime) { mKeySwitchGuardTime = aKeySwitchGuardTime; }
void SetKeySwitchGuardTime(uint16_t aGuardTime) { mKeySwitchGuardTime = aGuardTime; }
/**
* Returns the Security Policy.
@@ -565,9 +592,13 @@ public:
#endif
private:
static constexpr uint32_t kDefaultKeySwitchGuardTime = 624;
static constexpr uint32_t kOneHourIntervalInMsec = 3600u * 1000u;
static constexpr bool kExportableMacKeys = OPENTHREAD_CONFIG_PLATFORM_MAC_KEYS_EXPORTABLE_ENABLE;
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");
OT_TOOL_PACKED_BEGIN
struct Keys
@@ -591,8 +622,9 @@ private:
void ComputeTrelKey(uint32_t aKeySequence, Mac::Key &aKey) const;
#endif
void StartKeyRotationTimer(void);
void ResetKeyRotationTimer(void);
void HandleKeyRotationTimer(void);
void CheckForKeyRotation(void);
#if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
void StoreNetworkKey(const NetworkKey &aNetworkKey, bool aOverWriteExisting);
@@ -630,9 +662,9 @@ private:
uint32_t mStoredMacFrameCounter;
uint32_t mStoredMleFrameCounter;
uint32_t mHoursSinceKeyRotation;
uint32_t mKeySwitchGuardTime;
bool mKeySwitchGuardEnabled;
uint16_t mHoursSinceKeyRotation;
uint16_t mKeySwitchGuardTime;
uint16_t mKeySwitchGuardTimer;
RotationTimer mKeyRotationTimer;
#if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
+3 -3
View File
@@ -378,7 +378,7 @@ void Mle::Restore(void)
SuccessOrExit(Get<Settings>().Read(networkInfo));
Get<KeyManager>().SetCurrentKeySequence(networkInfo.GetKeySequence());
Get<KeyManager>().SetCurrentKeySequence(networkInfo.GetKeySequence(), KeyManager::kForceUpdate);
Get<KeyManager>().SetMleFrameCounter(networkInfo.GetMleFrameCounter());
Get<KeyManager>().SetAllMacFrameCounters(networkInfo.GetMacFrameCounter(), /* aSetIfLarger */ false);
@@ -2726,7 +2726,7 @@ void Mle::ProcessKeySequence(RxInfo &aRxInfo)
switch (aRxInfo.mClass)
{
case RxInfo::kAuthoritativeMessage:
Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence);
Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence, KeyManager::kForceUpdate);
break;
case RxInfo::kPeerMessage:
@@ -2734,7 +2734,7 @@ void Mle::ProcessKeySequence(RxInfo &aRxInfo)
{
if (aRxInfo.mKeySequence - Get<KeyManager>().GetCurrentKeySequence() == 1)
{
Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence);
Get<KeyManager>().SetCurrentKeySequence(aRxInfo.mKeySequence, KeyManager::kApplyKeySwitchGuard);
}
else
{
+1 -1
View File
@@ -690,7 +690,7 @@ template <> otError NcpBase::HandlePropertySet<SPINEL_PROP_NET_KEY_SWITCH_GUARDT
SuccessOrExit(error = mDecoder.ReadUint32(keyGuardTime));
otThreadSetKeySwitchGuardTime(mInstance, keyGuardTime);
otThreadSetKeySwitchGuardTime(mInstance, static_cast<uint16_t>(keyGuardTime));
exit:
return error;
@@ -221,20 +221,20 @@ class MleMsgKeySeqJump(thread_cert.TestCase):
self.assertEqual(reed.get_key_sequence_counter(), 20)
#-------------------------------------------------------------------
# Move forward the key seq counter by one on router. Wait for max
# Move forward the key seq counter by two on router. Wait for max
# time between advertisements. Validate that leader adopts the higher
# counter value.
router.set_key_sequence_counter(21)
self.assertEqual(router.get_key_sequence_counter(), 21)
router.set_key_sequence_counter(22)
self.assertEqual(router.get_key_sequence_counter(), 22)
self.simulator.go(52)
self.assertEqual(leader.get_key_sequence_counter(), 21)
self.assertEqual(reed.get_key_sequence_counter(), 21)
self.assertEqual(leader.get_key_sequence_counter(), 22)
self.assertEqual(reed.get_key_sequence_counter(), 22)
child.set_mode('r')
self.simulator.go(2)
self.assertEqual(child.get_key_sequence_counter(), 21)
self.assertEqual(child.get_key_sequence_counter(), 22)
#-------------------------------------------------------------------
# Force a reattachment from the child with a higher key seq counter,