espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-07-26 13:59:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

[srp-server] allow service instance label with dot character (#9198)

Browse Source 
This commit updates the `Srp::Server` class to correctly handle the
case where a service is registered with a dot character in its
service instance name. The first label in a service instance name is
intended as a user-friendly name and can contain dot characters
(it has fewer restrictions than other labels in a DNS name).

In particular, this commit contains the following changes:
- The `PtrRecords::ReadPtrName()` method is used in `Srp::Server` to
  read and validate the first label and the rest of the labels
  separately. This also validates the format of the parsed PTR
  record.
- The `Service::Description` class now remembers the instance label in
  addition to the full instance name. This allows the instance label
  to be easily retrieved.
- The `Dns::Name::ReadName()` method is updated to only verify that
  the labels after the first label do not contain any dot characters.
  This allows it to be used to read instance service names.
- The tests are updated to validate the behavior of the SRP server
  when the instance label contains a dot character.
This commit is contained in:
Abtin Keshavarzian
2023-06-26 12:55:45 -07:00
committed by GitHub
parent 97e9f58026
commit 0cc1de7b5a
6 changed files with 66 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/net/dns_types.cpp
+1 -1
View File
@@ -336,7 +336,7 @@ Error Name::ReadName(const Message &aMessage, uint16_t &aOffset, char *aNameBuff
}
labelLength = static_cast<uint8_t>(Min(static_cast<uint16_t>(kMaxLabelSize), aNameBufferSize));
SuccessOrExit(error = iterator.ReadLabel(aNameBuffer, labelLength, /* aAllowDotCharInLabel */ false));
SuccessOrExit(error = iterator.ReadLabel(aNameBuffer, labelLength, /* aAllowDotCharInLabel */ firstLabel));
aNameBuffer += labelLength;
aNameBufferSize -= labelLength;
firstLabel = false;
src/core/net/dns_types.hpp
+2 -2
View File
@@ -853,8 +853,8 @@ public:
* On successful read, the read name follows "<label1>.<label2>.<label3>.", i.e., a sequence of labels separated by
* dot '.' character. The read name will ALWAYS end with a dot.
*
* Verifies that the read labels in message do not contain any dot character, otherwise it returns
* `kErrorParse`).
* Verifies that the labels after the first label in message do not contain any dot character. If they do,
* returns `kErrorParse`.
*
* @param[in] aMessage The message to read the name from. `aMessage.GetOffset()` MUST point to
* the start of DNS header (this is used to handle compressed names).
src/core/net/srp_server.cpp
+34 -18
View File
@@ -43,6 +43,7 @@
#include "common/new.hpp"
#include "common/num_utils.hpp"
#include "common/random.hpp"
#include "common/string.hpp"
#include "net/dns_types.hpp"
#include "thread/thread_netif.hpp"
@@ -915,12 +916,14 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost,
for (uint16_t numRecords = aMetadata.mDnsHeader.GetUpdateRecordCount(); numRecords > 0; numRecords--)
{
char serviceName[Dns::Name::kMaxNameSize];
char instanceName[Dns::Name::kMaxNameSize];
Dns::PtrRecord ptrRecord;
const char *subServiceName;
Service *service;
bool isSubType;
char serviceName[Dns::Name::kMaxNameSize];
char instanceLabel[Dns::Name::kMaxLabelSize];
char instanceServiceName[Dns::Name::kMaxNameSize];
String<Dns::Name::kMaxNameSize> instanceName;
Dns::PtrRecord ptrRecord;
const char *subServiceName;
Service *service;
bool isSubType;
SuccessOrExit(error = Dns::Name::ReadName(aMessage, offset, serviceName, sizeof(serviceName)));
VerifyOrExit(Dns::Name::IsSubDomainOf(serviceName, GetDomain()), error = kErrorSecurity);
@@ -937,7 +940,9 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost,
SuccessOrExit(error);
SuccessOrExit(error = Dns::Name::ReadName(aMessage, offset, instanceName, sizeof(instanceName)));
SuccessOrExit(error = ptrRecord.ReadPtrName(aMessage, offset, instanceLabel, sizeof(instanceLabel),
instanceServiceName, sizeof(instanceServiceName)));
instanceName.Append("%s.%s", instanceLabel, instanceServiceName);
VerifyOrExit(ptrRecord.GetClass() == Dns::ResourceRecord::kClassNone ||
ptrRecord.GetClass() == aMetadata.mDnsZone.GetClass(),
@@ -957,13 +962,14 @@ Error Server::ProcessServiceDiscoveryInstructions(Host &aHost,
}
// Verify that instance name and service name are related.
VerifyOrExit(Dns::Name::IsSubDomainOf(instanceName, isSubType ? subServiceName : serviceName),
VerifyOrExit(Dns::Name::IsSubDomainOf(instanceName.AsCString(), isSubType ? subServiceName : serviceName),
error = kErrorFailed);
// Ensure the same service does not exist already.
VerifyOrExit(aHost.FindService(serviceName, instanceName) == nullptr, error = kErrorFailed);
VerifyOrExit(aHost.FindService(serviceName, instanceName.AsCString()) == nullptr, error = kErrorFailed);
service = aHost.AddNewService(serviceName, instanceName, isSubType, aMetadata.mRxTime);
service =
aHost.AddNewService(serviceName, instanceName.AsCString(), instanceLabel, isSubType, aMetadata.mRxTime);
VerifyOrExit(service != nullptr, error = kErrorNoBufs);
// This RR is a "Delete an RR from an RRset" update when the CLASS is NONE.
@@ -1854,8 +1860,10 @@ void Server::Service::Log(Action) const {}
//---------------------------------------------------------------------------------------------------------------------
// Server::Service::Description
Error Server::Service::Description::Init(const char *aInstanceName, Host &aHost)
Error Server::Service::Description::Init(const char *aInstanceName, const char *aInstanceLabel, Host &aHost)
{
Error error;
mNext = nullptr;
mHost = &aHost;
mPriority = 0;
@@ -1867,7 +1875,11 @@ Error Server::Service::Description::Init(const char *aInstanceName, Host &aHost)
mUpdateTime = TimerMilli::GetNow().GetDistantPast();
mTxtData.Free();
return mInstanceName.Set(aInstanceName);
SuccessOrExit(error = mInstanceLabel.Set(aInstanceLabel));
error = mInstanceName.Set(aInstanceName);
exit:
return error;
}
bool Server::Service::Description::Matches(const char *aInstanceName) const
@@ -2042,6 +2054,7 @@ const Server::Service *Server::Host::FindNextService(const Service *aPrevService
Server::Service *Server::Host::AddNewService(const char *aServiceName,
const char *aInstanceName,
const char *aInstanceLabel,
bool aIsSubType,
TimeMilli aUpdateTime)
{
@@ -2050,7 +2063,7 @@ Server::Service *Server::Host::AddNewService(const char *aServiceName,
if (desc == nullptr)
{
desc.Reset(Service::Description::AllocateAndInit(aInstanceName, *this));
desc.Reset(Service::Description::AllocateAndInit(aInstanceName, aInstanceLabel, *this));
VerifyOrExit(desc != nullptr);
}
@@ -2063,6 +2076,12 @@ exit:
return service;
}
Server::Service *Server::Host::AddNewService(const Service &aService, TimeMilli aUpdateTime)
{
return AddNewService(aService.GetServiceName(), aService.GetInstanceName(), aService.GetInstanceLabel(),
aService.IsSubType(), aUpdateTime);
}
void Server::Host::RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler)
{
Server &server = Get<Server>();
@@ -2103,8 +2122,7 @@ Error Server::Host::AddCopyOfServiceAsDeletedIfNotPresent(const Service &aServic
VerifyOrExit(FindService(aService.GetServiceName(), aService.GetInstanceName()) == nullptr);
newService =
AddNewService(aService.GetServiceName(), aService.GetInstanceName(), aService.IsSubType(), aUpdateTime);
newService = AddNewService(aService, aUpdateTime);
VerifyOrExit(newService != nullptr, error = kErrorNoBufs);
@@ -2156,9 +2174,7 @@ Error Server::Host::MergeServicesAndResourcesFrom(Host &aHost)
// Add/Merge `service` into the existing service or a allocate a new one
newService = (existingService != nullptr) ? existingService
: AddNewService(service.GetServiceName(), service.GetInstanceName(),
service.IsSubType(), service.GetUpdateTime());
newService = (existingService != nullptr) ? existingService : AddNewService(service, service.GetUpdateTime());
VerifyOrExit(newService != nullptr, error = kErrorNoBufs);
src/core/net/srp_server.hpp
+14 -2
View File
@@ -245,7 +245,15 @@ public:
* @returns A pointer service instance name (as a null-terminated C string).
*
*/
const char *GetInstanceName(void) const { return mDescription->mInstanceName.AsCString(); }
const char *GetInstanceName(void) const { return mDescription->GetInstanceName(); }
/**
* Gets the service instance label of the service.
*
* @returns A pointer service instance label (as a null-terminated C string).
*
*/
const char *GetInstanceLabel(void) const { return mDescription->GetInstanceLabel(); }
/**
* Gets the full service name of the service.
@@ -399,8 +407,9 @@ public:
public RetainCountable,
private NonCopyable
{
Error Init(const char *aInstanceName, Host &aHost);
Error Init(const char *aInstanceName, const char *aInstanceLabel, Host &aHost);
const char *GetInstanceName(void) const { return mInstanceName.AsCString(); }
const char *GetInstanceLabel(void) const { return mInstanceLabel.AsCString(); }
bool Matches(const char *aInstanceName) const;
void ClearResources(void);
void TakeResourcesFrom(Description &aDescription);
@@ -408,6 +417,7 @@ public:
Description *mNext;
Heap::String mInstanceName;
Heap::String mInstanceLabel;
Host *mHost;
Heap::Data mTxtData;
uint16_t mPriority;
@@ -600,8 +610,10 @@ public:
LinkedList<Service> &GetServices(void) { return mServices; }
Service *AddNewService(const char *aServiceName,
const char *aInstanceName,
const char *aInstanceLabel,
bool aIsSubType,
TimeMilli aUpdateTime);
Service *AddNewService(const Service &aService, TimeMilli aUpdateTime);
void RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler);
Error AddCopyOfServiceAsDeletedIfNotPresent(const Service &aService, TimeMilli aUpdateTime);
void FreeAllServices(void);
tests/unit/test_dns.cpp
+14 -9
View File
@@ -478,6 +478,7 @@ void TestDnsCompressedName(void)
static const char kExpectedReadName1[] = "F.ISI.ARPA.";
static const char kExpectedReadName2[] = "FOO.F.ISI.ARPA.";
static const char kExpectedReadName3[] = "ISI.ARPA.";
static const char kExpectedReadName4[] = "Human.Readable.F.ISI.ARPA.";
static const char kBadName[] = "bad.name";
@@ -745,10 +746,12 @@ void TestDnsCompressedName(void)
VerifyOrQuit(labelLength == strlen(label), "Name::ReadLabel() returned incorrect label length");
}
// `ReadName()` for name-4 should fails due to first label containing dot char.
// `ReadName()` for name-4 should still succeed since only the first label contains dot char
offset = name4Offset;
VerifyOrQuit(Dns::Name::ReadName(*message, offset, name, sizeof(name)) == kErrorParse,
"Name::ReadName() did not fail with invalid label");
SuccessOrQuit(Dns::Name::ReadName(*message, offset, name, sizeof(name)));
printf("Read name =\"%s\"\n", name);
VerifyOrQuit(strcmp(name, kExpectedReadName4) == 0, "Name::ReadName() did not return expected name");
VerifyOrQuit(offset == name4Offset + kName4EncodedSize, "Name::ParseName() returned incorrect offset");
offset = name4Offset;
@@ -826,9 +829,9 @@ void TestHeaderAndResourceRecords(void)
const char kServiceLabels[] = "_service._udp";
const char kServiceName[] = "_service._udp.example.com.";
const char kInstance1Label[] = "inst1";
const char kInstance2Label[] = "instance2";
const char kInstance2Label[] = "instance.2"; // Instance label includes dot '.' character.
const char kInstance1Name[] = "inst1._service._udp.example.com.";
const char kInstance2Name[] = "instance2._service._udp.example.com.";
const char kInstance2Name[] = "instance.2._service._udp.example.com.";
const char kHostName[] = "host.example.com.";
const uint8_t kTxtData[] = {9, 'k', 'e', 'y', '=', 'v', 'a', 'l', 'u', 'e', 0};
const char kHostAddress[] = "fd00::abcd:";
@@ -988,16 +991,18 @@ void TestHeaderAndResourceRecords(void)
VerifyOrQuit(offset == answerSectionOffset, "answer section offset is incorrect");
for (const char *instanceName : kInstanceNames)
for (const char *instanceLabel : kInstanceLabels)
{
SuccessOrQuit(Dns::Name::CompareName(*message, offset, kServiceName));
SuccessOrQuit(Dns::ResourceRecord::ReadRecord(*message, offset, ptrRecord));
VerifyOrQuit(ptrRecord.GetTtl() == kTtl, "Read PTR is incorrect");
SuccessOrQuit(ptrRecord.ReadPtrName(*message, offset, name, sizeof(name)));
VerifyOrQuit(strcmp(name, instanceName) == 0, "Inst1 name is incorrect");
SuccessOrQuit(ptrRecord.ReadPtrName(*message, offset, label, sizeof(label), name, sizeof(name)));
VerifyOrQuit(strcmp(label, instanceLabel) == 0, "Inst label is incorrect");
VerifyOrQuit(strcmp(name, kServiceName) == 0);
printf(" \"%s\" PTR %u %d \"%s\"\n", kServiceName, ptrRecord.GetTtl(), ptrRecord.GetLength(), name);
printf(" \"%s\" PTR %u %d \"%s.%s\"\n", kServiceName, ptrRecord.GetTtl(), ptrRecord.GetLength(), label,
name);
}
VerifyOrQuit(offset == additionalSectionOffset, "offset is incorrect after answer section parse");
tests/unit/test_srp_server.cpp
+1 -1
View File
@@ -287,7 +287,7 @@ static const char kHostName[] = "myhost";
void PrepareService1(Srp::Client::Service &aService)
{
static const char kServiceName[] = "_srv._udp";
static const char kInstanceLabel[] = "srv-instance";
static const char kInstanceLabel[] = "srv.instance";
static const char kSub1[] = "_sub1";
static const char kSub2[] = "_V1234567";
static const char kSub3[] = "_XYZWS";