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

[srp-server] identify name conflicts by comparing only ECDSA public keys (#7281)

Browse Source 
Current `Srp::Server` is comparing the entire KEY RRs to determine if
the two SRP messages are using the same signing key. But the TTL in
the KEY RR can change even between two SRP messages of the same client
and this will result in false name conflicts.

This commit fixes this issue by changing to compare only the rdata of
the KEY RR.
This commit is contained in:
Kangping
2022-01-06 21:47:04 -08:00
committed by GitHub
parent 053af39479
commit 442b7e82f6
3 changed files with 32 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/crypto/ecdsa.hpp
+1 -1
View File
@@ -228,7 +228,7 @@ public:
*
*/
OT_TOOL_PACKED_BEGIN
class PublicKey
class PublicKey : public Equatable<PublicKey>
{
friend class KeyPair;
src/core/net/srp_server.cpp
+17 -17
View File
@@ -308,7 +308,7 @@ bool Server::HasNameConflictsWith(Host &aHost) const
bool hasConflicts = false;
const Host *existingHost = mHosts.FindMatching(aHost.GetFullName());
if (existingHost != nullptr && *aHost.GetKey() != *existingHost->GetKey())
if (existingHost != nullptr && aHost.GetKeyRecord()->GetKey() != existingHost->GetKeyRecord()->GetKey())
{
ExitNow(hasConflicts = true);
}
@@ -323,7 +323,7 @@ bool Server::HasNameConflictsWith(Host &aHost) const
{
if (host.HasServiceInstance(service.GetInstanceName()))
{
VerifyOrExit(*aHost.GetKey() == *host.GetKey(), hasConflicts = true);
VerifyOrExit(aHost.GetKeyRecord()->GetKey() == host.GetKeyRecord()->GetKey(), hasConflicts = true);
}
}
}
@@ -799,14 +799,14 @@ Error Server::ProcessHostDescriptionInstruction(Host & aHost,
else if (record.GetType() == Dns::ResourceRecord::kTypeKey)
{
// We currently support only ECDSA P-256.
Dns::Ecdsa256KeyRecord key;
Dns::Ecdsa256KeyRecord keyRecord;
VerifyOrExit(record.GetClass() == aMetadata.mDnsZone.GetClass(), error = kErrorFailed);
SuccessOrExit(error = aMessage.Read(offset, key));
VerifyOrExit(key.IsValid(), error = kErrorParse);
SuccessOrExit(error = aMessage.Read(offset, keyRecord));
VerifyOrExit(keyRecord.IsValid(), error = kErrorParse);
VerifyOrExit(aHost.GetKey() == nullptr || *aHost.GetKey() == key, error = kErrorSecurity);
aHost.SetKey(key);
VerifyOrExit(aHost.GetKeyRecord() == nullptr || *aHost.GetKeyRecord() == keyRecord, error = kErrorSecurity);
aHost.SetKeyRecord(keyRecord);
}
offset += record.GetSize();
@@ -815,7 +815,7 @@ Error Server::ProcessHostDescriptionInstruction(Host & aHost,
// Verify that we have a complete Host Description Instruction.
VerifyOrExit(aHost.GetFullName() != nullptr, error = kErrorFailed);
VerifyOrExit(aHost.GetKey() != nullptr, error = kErrorFailed);
VerifyOrExit(aHost.GetKeyRecord() != nullptr, error = kErrorFailed);
// We check the number of host addresses after processing of the
// Lease Option in the Addition Section and determining whether
@@ -1060,8 +1060,8 @@ Error Server::ProcessAdditionalSection(Host *aHost, const Message &aMessage, Mes
VerifyOrExit(sigRecord.GetTypeCovered() == 0, error = kErrorFailed);
VerifyOrExit(signatureLength == Crypto::Ecdsa::P256::Signature::kSize, error = kErrorParse);
SuccessOrExit(error = VerifySignature(*aHost->GetKey(), aMessage, aMetadata.mDnsHeader, sigOffset, sigRdataOffset,
sigRecord.GetLength(), signerName));
SuccessOrExit(error = VerifySignature(*aHost->GetKeyRecord(), aMessage, aMetadata.mDnsHeader, sigOffset,
sigRdataOffset, sigRecord.GetLength(), signerName));
aMetadata.mOffset = offset;
@@ -1069,7 +1069,7 @@ exit:
return error;
}
Error Server::VerifySignature(const Dns::Ecdsa256KeyRecord &aKey,
Error Server::VerifySignature(const Dns::Ecdsa256KeyRecord &aKeyRecord,
const Message & aMessage,
Dns::UpdateHeader aDnsHeader,
uint16_t aSigOffset,
@@ -1109,7 +1109,7 @@ Error Server::VerifySignature(const Dns::Ecdsa256KeyRecord &aKey,
signatureOffset = aSigRdataOffset + aSigRdataLength - Crypto::Ecdsa::P256::Signature::kSize;
SuccessOrExit(error = aMessage.Read(signatureOffset, signature));
error = aKey.GetKey().Verify(hash, signature);
error = aKeyRecord.GetKey().Verify(hash, signature);
exit:
FreeMessage(signerNameMessage);
@@ -1649,7 +1649,7 @@ Server::Host::Host(Instance &aInstance, TimeMilli aUpdateTime)
, mKeyLease(0)
, mUpdateTime(aUpdateTime)
{
mKey.Clear();
mKeyRecord.Clear();
}
Server::Host::~Host(void)
@@ -1682,11 +1682,11 @@ bool Server::Host::Matches(const char *aFullName) const
return StringMatch(mFullName.AsCString(), aFullName, kStringCaseInsensitiveMatch);
}
void Server::Host::SetKey(Dns::Ecdsa256KeyRecord &aKey)
void Server::Host::SetKeyRecord(Dns::Ecdsa256KeyRecord &aKeyRecord)
{
OT_ASSERT(aKey.IsValid());
OT_ASSERT(aKeyRecord.IsValid());
mKey = aKey;
mKeyRecord = aKeyRecord;
}
TimeMilli Server::Host::GetExpireTime(void) const
@@ -1811,7 +1811,7 @@ Error Server::Host::MergeServicesAndResourcesFrom(Host &aHost)
otLogInfoSrp("[server] update host %s", GetFullName());
mAddresses = aHost.mAddresses;
mKey = aHost.mKey;
mKeyRecord = aHost.mKeyRecord;
mLease = aHost.mLease;
mKeyLease = aHost.mKeyLease;
mUpdateTime = TimerMilli::GetNow();
src/core/net/srp_server.hpp
+14 -11
View File
@@ -453,13 +453,13 @@ public:
uint32_t GetKeyLease(void) const { return mKeyLease; }
/**
* This method returns the KEY resource of the host.
* This method returns the KEY resource record of the host.
*
* @returns A pointer to the ECDSA P 256 public key if there is valid one.
* `nullptr` if no valid key exists.
* @returns A pointer to the ECDSA P 256 public key resource record
* if there is valid one. `nullptr` if no valid key exists.
*
*/
const Dns::Ecdsa256KeyRecord *GetKey(void) const { return mKey.IsValid() ? &mKey : nullptr; }
const Dns::Ecdsa256KeyRecord *GetKeyRecord(void) const { return mKeyRecord.IsValid() ? &mKeyRecord : nullptr; }
/**
* This method returns the expire time (in milliseconds) of the host.
@@ -518,7 +518,7 @@ public:
~Host(void);
Error SetFullName(const char *aFullName);
void SetKey(Dns::Ecdsa256KeyRecord &aKey);
void SetKeyRecord(Dns::Ecdsa256KeyRecord &aKeyRecord);
void SetLease(uint32_t aLease) { mLease = aLease; }
void SetKeyLease(uint32_t aKeyLease) { mKeyLease = aKeyLease; }
LinkedList<Service> &GetServices(void) { return mServices; }
@@ -540,11 +540,14 @@ public:
Host * mNext;
Heap::String mFullName;
Array<Ip6::Address, kMaxAddresses> mAddresses;
Dns::Ecdsa256KeyRecord mKey;
uint32_t mLease; // The LEASE time in seconds.
uint32_t mKeyLease; // The KEY-LEASE time in seconds.
TimeMilli mUpdateTime;
LinkedList<Service> mServices;
// TODO(wgtdkp): there is no necessary to save the entire resource
// record, saving only the ECDSA-256 public key should be enough.
Dns::Ecdsa256KeyRecord mKeyRecord;
uint32_t mLease; // The LEASE time in seconds.
uint32_t mKeyLease; // The KEY-LEASE time in seconds.
TimeMilli mUpdateTime;
LinkedList<Service> mServices;
};
/**
@@ -845,7 +848,7 @@ private:
void ProcessDnsUpdate(Message &aMessage, MessageMetadata &aMetadata);
Error ProcessUpdateSection(Host &aHost, const Message &aMessage, MessageMetadata &aMetadata) const;
Error ProcessAdditionalSection(Host *aHost, const Message &aMessage, MessageMetadata &aMetadata) const;
Error VerifySignature(const Dns::Ecdsa256KeyRecord &aKey,
Error VerifySignature(const Dns::Ecdsa256KeyRecord &aKeyRecord,
const Message & aMessage,
Dns::UpdateHeader aDnsHeader,
uint16_t aSigOffset,