espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-08-10 20:57:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

[srp-client] update message ID assignment model (#11592)

Browse Source 
This commit updates the SRP client message ID assignment model.
Message IDs are now selected randomly, ensuring they differ from the
last ID used. The same message ID is reused when a message is
retried.

This replaces the earlier design where all messages, including
retries, used sequential message IDs.

The unit test is also updated to validate the new behavior.
This commit is contained in:
Abtin Keshavarzian
2025-06-24 12:10:45 -07:00
committed by GitHub
parent 84295be3f8
commit 3aab10a8fb
3 changed files with 124 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/net/srp_client.cpp
+75 -59
View File
@@ -355,8 +355,7 @@ Client::Client(Instance &aInstance)
, mServiceKeyRecordEnabled(false)
, mUseShortLeaseOption(false)
#endif
, mNextMessageId(0)
, mResponseMessageId(0)
, mCurMessageId(0)
, mAutoHostAddressCount(0)
, mRetryWaitInterval(kMinRetryWaitInterval)
, mTtl(0)
@@ -468,7 +467,6 @@ void Client::Stop(Requester aRequester, StopMode aMode)
mShouldRemoveKeyLease = false;
mTxFailureRetryCount = 0;
mResponseMessageId = mNextMessageId;
if (aMode == kResetRetryInterval)
{
@@ -1004,19 +1002,11 @@ void Client::SendUpdate(void)
{
LogInfo("Msg len %lu is larger than MTU, enabling single service mode", ToUlong(length));
mSingleServiceMode = true;
SelectNewMessageId();
IgnoreError(info.mMessage->SetLength(0));
SuccessOrExit(error = PrepareUpdateMessage(info));
}
SuccessOrExit(error = mSocket.SendTo(*info.mMessage, Ip6::MessageInfo()));
// Ownership of the message is transferred to the socket upon a
// successful `SendTo()` call.
info.mMessage.Release();
LogInfo("Send update, msg-id:0x%x", mNextMessageId);
// State changes:
// kToAdd -> kAdding
// kToRefresh -> kRefreshing
@@ -1024,26 +1014,21 @@ void Client::SendUpdate(void)
anyChanged = ChangeHostAndServiceStates(kNewStateOnMessageTx, kForServicesAppendedInMessage);
// `mNextMessageId` tracks the message ID used in the prepared
// update message. It is incremented after a successful
// `mSocket.SendTo()` call. If unsuccessful, the same ID can be
// reused for the next update.
//
// Acceptable response message IDs fall within the range starting
// at `mResponseMessageId ` and ending before `mNextMessageId`.
//
// `anyChanged` tracks if any host or service states have changed.
// If not, the prepared message is identical to the last one with
// the same hosts/services, allowing us to accept earlier message
// IDs. If changes occur, `mResponseMessageId ` is updated to
// ensure only responses to the latest message are accepted.
if (anyChanged)
{
mResponseMessageId = mNextMessageId;
SelectNewMessageId();
}
mNextMessageId++;
SuccessOrExit(error = UpdateIdAndSignatureInUpdateMessage(info));
SuccessOrExit(error = mSocket.SendTo(*info.mMessage, Ip6::MessageInfo()));
// Ownership of the message is transferred to the socket upon a
// successful `SendTo()` call.
info.mMessage.Release();
LogInfo("Send update, msg-id:0x%x", mCurMessageId);
// Remember the update message tx time to use later to determine the
// lease renew time.
@@ -1115,7 +1100,7 @@ Error Client::PrepareUpdateMessage(MsgInfo &aInfo)
SuccessOrExit(error = ReadOrGenerateKey(aInfo.mKeyInfo));
header.SetMessageId(mNextMessageId);
header.SetMessageId(mCurMessageId);
// SRP Update (DNS Update) message must have exactly one record in
// Zone section, no records in Prerequisite Section, can have
@@ -1148,7 +1133,24 @@ Error Client::PrepareUpdateMessage(MsgInfo &aInfo)
// Prepare Additional Data section
SuccessOrExit(error = AppendUpdateLeaseOptRecord(aInfo));
SuccessOrExit(error = AppendSignature(aInfo));
SuccessOrExit(error = AppendSignature(aInfo, kAppendEmptySignature));
exit:
return error;
}
Error Client::UpdateIdAndSignatureInUpdateMessage(MsgInfo &aInfo)
{
constexpr uint16_t kHeaderOffset = 0;
Error error;
Dns::UpdateHeader header;
IgnoreError(aInfo.mMessage->Read(kHeaderOffset, header));
header.SetMessageId(mCurMessageId);
aInfo.mMessage->Write(kHeaderOffset, header);
SuccessOrExit(error = AppendSignature(aInfo, kOverwriteWithNewSignature));
header.SetAdditionalRecordCount(2); // Lease OPT and SIG RRs
aInfo.mMessage->Write(kHeaderOffset, header);
@@ -1674,7 +1676,7 @@ exit:
return error;
}
Error Client::AppendSignature(MsgInfo &aInfo)
Error Client::AppendSignature(MsgInfo &aInfo, SignatureAppendMode aMode)
{
Error error;
Dns::SigRecord sig;
@@ -1694,37 +1696,49 @@ Error Client::AppendSignature(MsgInfo &aInfo)
sig.Init(Dns::ResourceRecord::kClassAny);
sig.SetAlgorithm(Dns::KeyRecord::kAlgorithmEcdsaP256Sha256);
// Append the SIG RR with full uncompressed form of the host name
// as the signer's name. This is used for SIG(0) calculation only.
// It will be overwritten with host name compressed.
switch (aMode)
{
case kAppendEmptySignature:
aInfo.mSigRecordOffset = aInfo.mMessage->GetLength();
break;
offset = aInfo.mMessage->GetLength();
SuccessOrExit(error = aInfo.mMessage->Append(sig));
SuccessOrExit(error = AppendHostName(aInfo, /* aDoNotCompress */ true));
case kOverwriteWithNewSignature:
// Revert back to the start of signature record.
IgnoreError(aInfo.mMessage->SetLength(aInfo.mSigRecordOffset));
// Calculate signature (RFC 2931): Calculated over "data" which is
// concatenation of (1) the SIG RR RDATA wire format (including
// the canonical form of the signer's name), entirely omitting the
// signature subfield, (2) DNS query message, including DNS header
// but not UDP/IP header before the header RR counts have been
// adjusted for the inclusion of SIG(0).
// Append the SIG RR with full uncompressed form of the host name
// as the signer's name. This is used for SIG(0) calculation only.
// It will be overwritten with host name compressed.
sha256.Start();
offset = aInfo.mMessage->GetLength();
SuccessOrExit(error = aInfo.mMessage->Append(sig));
SuccessOrExit(error = AppendHostName(aInfo, /* aDoNotCompress */ true));
// (1) SIG RR RDATA wire format
len = aInfo.mMessage->GetLength() - offset - sizeof(Dns::ResourceRecord);
sha256.Update(*aInfo.mMessage, offset + sizeof(Dns::ResourceRecord), len);
// Calculate signature (RFC 2931): Calculated over "data" which is
// concatenation of (1) the SIG RR RDATA wire format (including
// the canonical form of the signer's name), entirely omitting the
// signature subfield, (2) DNS query message, including DNS header
// but not UDP/IP header before the header RR counts have been
// adjusted for the inclusion of SIG(0).
// (2) Message from DNS header before SIG
sha256.Update(*aInfo.mMessage, 0, offset);
sha256.Start();
sha256.Finish(hash);
SuccessOrExit(error = aInfo.mKeyInfo.Sign(hash, signature));
// (1) SIG RR RDATA wire format
len = aInfo.mMessage->GetLength() - offset - sizeof(Dns::ResourceRecord);
sha256.Update(*aInfo.mMessage, offset + sizeof(Dns::ResourceRecord), len);
// Move back in message and append SIG RR now with compressed host
// name (as signer's name) along with the calculated signature.
// (2) Message from DNS header before SIG
sha256.Update(*aInfo.mMessage, 0, offset);
IgnoreError(aInfo.mMessage->SetLength(offset));
sha256.Finish(hash);
SuccessOrExit(error = aInfo.mKeyInfo.Sign(hash, signature));
// Move back in message and append SIG RR now with compressed host
// name (as signer's name) along with the calculated signature.
IgnoreError(aInfo.mMessage->SetLength(offset));
break;
}
// SIG(0) uses owner name of root (single zero byte).
SuccessOrExit(error = Dns::Name::AppendTerminator(*aInfo.mMessage));
@@ -1782,8 +1796,7 @@ void Client::ProcessResponse(Message &aMessage)
VerifyOrExit(header.GetType() == Dns::Header::kTypeResponse, error = kErrorParse);
VerifyOrExit(header.GetQueryType() == Dns::Header::kQueryTypeUpdate, error = kErrorParse);
VerifyOrExit(IsResponseMessageIdValid(header.GetMessageId()), error = kErrorDrop);
mResponseMessageId = header.GetMessageId() + 1;
VerifyOrExit(header.GetMessageId() == mCurMessageId, error = kErrorDrop);
if (!Get<Mle::Mle>().IsRxOnWhenIdle())
{
@@ -1921,11 +1934,14 @@ exit:
}
}
bool Client::IsResponseMessageIdValid(uint16_t aId) const
void Client::SelectNewMessageId(void)
{
// Semantically equivalent to `(aId >= mResponseMessageId) && (aId < mNextMessageId)`
uint16_t oldId = mCurMessageId;
return !SerialNumber::IsLess(aId, mResponseMessageId) && SerialNumber::IsLess(aId, mNextMessageId);
do
{
mCurMessageId = Random::NonCrypto::GetUint16();
} while (oldId == mCurMessageId);
}
void Client::HandleUpdateDone(void)
src/core/net/srp_client.hpp
+12 -13
View File
@@ -734,15 +734,6 @@ public:
*/
bool GetUseShortLeaseOption(void) const { return mUseShortLeaseOption; }
/**
* Set the next DNS message ID for client to use.
*
* This is intended for testing only.
*
* @pram[in] aMessageId A message ID.
*/
void SetNextMessageId(uint16_t aMessageId) { mNextMessageId = aMessageId; }
#endif // OPENTHREAD_CONFIG_REFERENCE_DEVICE_ENABLE
private:
@@ -903,6 +894,13 @@ private:
kForServicesAppendedInMessage,
};
// Used in `AppendSignature()`
enum SignatureAppendMode : uint8_t
{
kAppendEmptySignature,
kOverwriteWithNewSignature,
};
#if OPENTHREAD_CONFIG_PLATFORM_KEY_REFERENCES_ENABLE
typedef Crypto::Ecdsa::P256::KeyPairAsRef KeyInfo;
#else
@@ -995,6 +993,7 @@ private:
uint16_t mDomainNameOffset;
uint16_t mHostNameOffset;
uint16_t mRecordCount;
uint16_t mSigRecordOffset;
KeyInfo mKeyInfo;
};
@@ -1017,6 +1016,7 @@ private:
void HandleHostInfoOrServiceChange(void);
void SendUpdate(void);
Error PrepareUpdateMessage(MsgInfo &aInfo);
Error UpdateIdAndSignatureInUpdateMessage(MsgInfo &aInfo);
Error ReadOrGenerateKey(KeyInfo &aKeyInfo);
Error AppendServiceInstructions(MsgInfo &aInfo);
bool CanAppendService(const Service &aService);
@@ -1027,10 +1027,10 @@ private:
Error AppendHostName(MsgInfo &aInfo, bool aDoNotCompress = false) const;
Error AppendAaaaRecord(const Ip6::Address &aAddress, MsgInfo &aInfo) const;
Error AppendUpdateLeaseOptRecord(MsgInfo &aInfo);
Error AppendSignature(MsgInfo &aInfo);
Error AppendSignature(MsgInfo &aInfo, SignatureAppendMode aMode);
void HandleUdpReceive(Message &aMessage, const Ip6::MessageInfo &aMessageInfo);
void ProcessResponse(Message &aMessage);
bool IsResponseMessageIdValid(uint16_t aId) const;
void SelectNewMessageId(void);
void HandleUpdateDone(void);
void GetRemovedServices(LinkedList<Service> &aRemovedServices);
static Error ReadResourceRecord(const Message &aMessage, uint16_t &aOffset, Dns::ResourceRecord &aRecord);
@@ -1080,8 +1080,7 @@ private:
bool mUseShortLeaseOption : 1;
#endif
uint16_t mNextMessageId;
uint16_t mResponseMessageId;
uint16_t mCurMessageId;
uint16_t mAutoHostAddressCount;
uint32_t mRetryWaitInterval;
tests/unit/test_srp_server.cpp
+37 -47
View File
@@ -1063,7 +1063,7 @@ void TestSrpClientDelayedResponse(void)
srpClient = &sInstance->Get<Srp::Client>();
for (uint8_t testIter = 0; testIter < 3; testIter++)
for (uint8_t testIter = 0; testIter < 2; testIter++)
{
Log("- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -");
Log("testIter = %u", testIter);
@@ -1074,6 +1074,7 @@ void TestSrpClientDelayedResponse(void)
Ip6::Udp::Socket udpSocket(*sInstance, HandleServerUdpReceive, nullptr);
Ip6::SockAddr serverSockAddr;
uint16_t firstMsgId;
uint16_t secondMsgId;
Message *response;
Dns::UpdateHeader header;
@@ -1082,24 +1083,6 @@ void TestSrpClientDelayedResponse(void)
SuccessOrQuit(udpSocket.Open(Ip6::kNetifThreadInternal));
SuccessOrQuit(udpSocket.Bind(kServerPort));
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Manually start the client with a message ID based on `testIter`
// We use zero in the first iteration, `0xffff` in the second
// iteration to test wrapping of 16-bit message ID.
switch (testIter)
{
case 0:
srpClient->SetNextMessageId(0);
break;
case 1:
srpClient->SetNextMessageId(0xffff);
break;
case 2:
srpClient->SetNextMessageId(0xaaaa);
break;
}
serverSockAddr.SetAddress(sInstance->Get<Mle::Mle>().GetMeshLocalRloc());
serverSockAddr.SetPort(kServerPort);
SuccessOrQuit(srpClient->Start(serverSockAddr));
@@ -1120,36 +1103,44 @@ void TestSrpClientDelayedResponse(void)
AdvanceTime(1 * 1000);
VerifyOrQuit(sServerRxCount == 1);
firstMsgId = sServerLastMsgId;
firstMsgId = sServerLastMsgId;
sServerRxCount = 0;
if (testIter == 1)
{
// In the second test iteration, register a second
// service. Ensure that client uses a new ID for new
// updated SRP message (containing both services).
AdvanceTime(5 * 1000);
PrepareService2(service2);
SuccessOrQuit(srpClient->AddService(service2));
AdvanceTime(20 * 1000);
VerifyOrQuit(sServerRxCount > 1);
VerifyOrQuit(sServerLastMsgId != firstMsgId);
secondMsgId = sServerLastMsgId;
sServerRxCount = 0;
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Wait for longer to allow client to retry a bunch of times.
// Ensure the same ID is used for retries.
AdvanceTime(60 * 1000);
VerifyOrQuit(sServerRxCount > 1);
switch (testIter)
{
case 0:
VerifyOrQuit(firstMsgId == 0);
VerifyOrQuit(sServerLastMsgId == firstMsgId);
break;
case 1:
VerifyOrQuit(firstMsgId == 0xffff);
break;
case 2:
VerifyOrQuit(firstMsgId == 0xaaaa);
VerifyOrQuit(sServerLastMsgId == secondMsgId);
break;
}
if (testIter == 2)
{
AdvanceTime(2 * 1000);
PrepareService2(service2);
SuccessOrQuit(srpClient->AddService(service2));
}
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// Wait for longer to allow client to retry a bunch of times
AdvanceTime(20 * 1000);
VerifyOrQuit(sServerRxCount > 1);
VerifyOrQuit(sServerLastMsgId != firstMsgId);
VerifyOrQuit(srpClient->GetHostInfo().GetState() != Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() != Srp::Client::kRegistered);
@@ -1171,20 +1162,19 @@ void TestSrpClientDelayedResponse(void)
AdvanceTime(10);
//- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
// In the first two iterations, we ensure that client
// did successfully accept the response with older message ID.
// This should not be the case in the third iteration due to
// changes to client services after first UPdate message was
// sent by client.
// In the first test iteration, we ensure that client did
// successfully accept the response with first message ID.
// This should not be the case in the second iteration due to
// changes to client services after the first Update message
// was sent by client.
switch (testIter)
{
case 0:
case 1:
VerifyOrQuit(srpClient->GetHostInfo().GetState() == Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() == Srp::Client::kRegistered);
break;
case 2:
case 1:
VerifyOrQuit(srpClient->GetHostInfo().GetState() != Srp::Client::kRegistered);
VerifyOrQuit(service1.GetState() != Srp::Client::kRegistered);
break;