[meshcop] fix commissioner's invalid call to SetState (#4494)

Commissioner should not call SetState(OT_COMMISSIONER_STATE_DISABLED)
directly, rather it should call Stop() so that joiners and resources
are cleaned up.

This commit:

- Fixes the above issue.

- Adds argument name /* aResign */ to Stop() calls.

- Ignores petition/keep-alive response if commissioner is not in
  PETITION/ACTIVE state.

- Calls CoapSecure::Stop when Start fails (suggested by @bukepo).

- Responds to Leader Petition Response (accept) by sending
  KeepAlive(reject) if commissioner is in disabled state.
This commit is contained in:
Simon Lin
2020-03-25 14:42:44 -07:00
committed by GitHub
parent db22529c27
commit 2ef2fb1717
3 changed files with 50 additions and 24 deletions
+1 -1
View File
@@ -64,7 +64,7 @@ otError otCommissionerStop(otInstance *aInstance)
otError error;
Instance &instance = *static_cast<Instance *>(aInstance);
SuccessOrExit(error = instance.Get<MeshCoP::Commissioner>().Stop());
SuccessOrExit(error = instance.Get<MeshCoP::Commissioner>().Stop(/* aResign */ true));
#if OPENTHREAD_CONFIG_BORDER_AGENT_ENABLE
SuccessOrExit(error = instance.Get<MeshCoP::BorderAgent>().Start());
#endif
+45 -22
View File
@@ -164,27 +164,42 @@ otError Commissioner::Start(otCommissionerStateCallback aStateCallback,
SetState(OT_COMMISSIONER_STATE_PETITION);
exit:
if (error != OT_ERROR_NONE)
{
Get<Coap::CoapSecure>().Stop();
}
return error;
}
otError Commissioner::Stop(void)
otError Commissioner::Stop(bool aResign)
{
otError error = OT_ERROR_NONE;
otError error = OT_ERROR_NONE;
bool needResign = false;
VerifyOrExit(mState != OT_COMMISSIONER_STATE_DISABLED, error = OT_ERROR_INVALID_STATE);
Get<Coap::CoapSecure>().Stop();
Get<ThreadNetif>().RemoveUnicastAddress(mCommissionerAloc);
RemoveCoapResources();
ClearJoiners();
mTransmitAttempts = 0;
if (mState == OT_COMMISSIONER_STATE_ACTIVE)
{
Get<ThreadNetif>().RemoveUnicastAddress(mCommissionerAloc);
RemoveCoapResources();
ClearJoiners();
needResign = true;
}
else if (mState == OT_COMMISSIONER_STATE_PETITION)
{
mTransmitAttempts = 0;
}
mTimer.Stop();
SetState(OT_COMMISSIONER_STATE_DISABLED);
SendKeepAlive();
if (needResign && aResign)
{
SendKeepAlive();
}
exit:
return error;
@@ -696,16 +711,25 @@ void Commissioner::HandleLeaderPetitionResponse(Coap::Message * aMessage
uint8_t state;
bool retransmit = false;
VerifyOrExit(mState == OT_COMMISSIONER_STATE_PETITION, SetState(OT_COMMISSIONER_STATE_DISABLED));
VerifyOrExit(aResult == OT_ERROR_NONE && aMessage->GetCode() == OT_COAP_CODE_CHANGED, retransmit = true);
VerifyOrExit(mState != OT_COMMISSIONER_STATE_ACTIVE);
VerifyOrExit(aResult == OT_ERROR_NONE && aMessage->GetCode() == OT_COAP_CODE_CHANGED,
retransmit = (mState == OT_COMMISSIONER_STATE_PETITION));
otLogInfoMeshCoP("received Leader Petition response");
SuccessOrExit(Tlv::ReadUint8Tlv(*aMessage, Tlv::kState, state));
VerifyOrExit(state == StateTlv::kAccept, SetState(OT_COMMISSIONER_STATE_DISABLED));
VerifyOrExit(state == StateTlv::kAccept, Stop(/* aResign */ false));
SuccessOrExit(Tlv::ReadUint16Tlv(*aMessage, Tlv::kCommissionerSessionId, mSessionId));
// reject this session by sending KeepAlive reject if commissioner is in disabled state
// this could happen if commissioner is stopped by API during petitioning
if (mState == OT_COMMISSIONER_STATE_DISABLED)
{
SendKeepAlive(mSessionId);
ExitNow();
}
Get<Mle::MleRouter>().GetCommissionerAloc(mCommissionerAloc.GetAddress(), mSessionId);
Get<ThreadNetif>().AddUnicastAddress(mCommissionerAloc);
@@ -721,7 +745,7 @@ exit:
{
if (mTransmitAttempts >= kPetitionRetryCount)
{
SetState(OT_COMMISSIONER_STATE_DISABLED);
Stop(/* aResign */ false);
}
else
{
@@ -731,6 +755,11 @@ exit:
}
otError Commissioner::SendKeepAlive(void)
{
return SendKeepAlive(mSessionId);
}
otError Commissioner::SendKeepAlive(uint16_t aSessionId)
{
otError error = OT_ERROR_NONE;
Coap::Message * message = NULL;
@@ -745,7 +774,7 @@ otError Commissioner::SendKeepAlive(void)
error = Tlv::AppendUint8Tlv(*message, Tlv::kState,
(mState == OT_COMMISSIONER_STATE_ACTIVE) ? StateTlv::kAccept : StateTlv::kReject));
SuccessOrExit(error = Tlv::AppendUint16Tlv(*message, Tlv::kCommissionerSessionId, mSessionId));
SuccessOrExit(error = Tlv::AppendUint16Tlv(*message, Tlv::kCommissionerSessionId, aSessionId));
messageInfo.SetSockAddr(Get<Mle::MleRouter>().GetMeshLocal16());
SuccessOrExit(error = Get<Mle::MleRouter>().GetLeaderAloc(messageInfo.GetPeerAddr()));
@@ -782,24 +811,18 @@ void Commissioner::HandleLeaderKeepAliveResponse(Coap::Message * aMessag
uint8_t state;
VerifyOrExit(mState == OT_COMMISSIONER_STATE_ACTIVE, SetState(OT_COMMISSIONER_STATE_DISABLED));
VerifyOrExit(aResult == OT_ERROR_NONE && aMessage->GetCode() == OT_COAP_CODE_CHANGED,
SetState(OT_COMMISSIONER_STATE_DISABLED));
VerifyOrExit(mState == OT_COMMISSIONER_STATE_ACTIVE);
VerifyOrExit(aResult == OT_ERROR_NONE && aMessage->GetCode() == OT_COAP_CODE_CHANGED, Stop(/* aResign */ false));
otLogInfoMeshCoP("received Leader keep-alive response");
SuccessOrExit(Tlv::ReadUint8Tlv(*aMessage, Tlv::kState, state));
VerifyOrExit(state == StateTlv::kAccept, SetState(OT_COMMISSIONER_STATE_DISABLED));
VerifyOrExit(state == StateTlv::kAccept, Stop(/* aResign */ false));
mTimer.Start(Time::SecToMsec(kKeepAliveTimeout) / 2);
exit:
if (mState != OT_COMMISSIONER_STATE_ACTIVE)
{
Get<ThreadNetif>().RemoveUnicastAddress(mCommissionerAloc);
RemoveCoapResources();
}
return;
}
void Commissioner::HandleRelayReceive(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo)
+4 -1
View File
@@ -84,11 +84,13 @@ public:
/**
* This method stops the Commissioner service.
*
* @param[in] aResign Whether send LEAD_KA.req to resign as Commissioner
*
* @retval OT_ERROR_NONE Successfully stopped the Commissioner service.
* @retval OT_ERROR_INVALID_STATE Commissioner is already stopped.
*
*/
otError Stop(void);
otError Stop(bool aResign);
/**
* This method clears all Joiner entries.
@@ -320,6 +322,7 @@ private:
otError SendCommissionerSet(void);
otError SendPetition(void);
otError SendKeepAlive(void);
otError SendKeepAlive(uint16_t aSessionId);
void SetState(otCommissionerState aState);
void SignalJoinerEvent(otCommissionerJoinerEvent aEvent, const Mac::ExtAddress &aJoinerId);