From 19f8033b78c050586ccdbb337e7fb0adeef647cf Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 13 Jun 2022 10:56:42 -0700 Subject: [PATCH] [srp-server] validate sub-types and treat as atomic (#7795) This commit updates `Srp::Server` to treat the update instructions in a received SRP message for a service and all its sub-types as atomic. It adds a new method `ValidateServiceSubTypes()` which performs two checks. First, it verifies that there is a matching base service for all sub-type services in a received SRP Update message. Second, it treats the sub-types in the message as atomic, i.e., whatever information appears in the message is considered the entirety of information about the service and its sub-types. In particular, any previously registered service sub-type that does not appear in a new SRP Update is removed (marked as deleted). This commit updates `test_srp_sub_type.py` test-case to validate the behavior of newly added mechanism (i.e. removal of older sub-types when not present in the new SRP update message). --- src/core/net/srp_server.cpp | 104 ++++++++++++++++++++++++++++++++---- src/core/net/srp_server.hpp | 4 ++ 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index dd18a4070..cdcee311f 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -718,6 +718,8 @@ void Server::ProcessDnsUpdate(Message &aMessage, MessageMetadata &aMetadata) // Parse lease time and validate signature. SuccessOrExit(error = ProcessAdditionalSection(host, aMessage, aMetadata)); + SuccessOrExit(error = ValidateServiceSubTypes(*host, aMetadata)); + HandleUpdate(*host, aMetadata); exit: @@ -1199,6 +1201,68 @@ exit: return error; } +Error Server::ValidateServiceSubTypes(Host &aHost, const MessageMetadata &aMetadata) +{ + Error error = kErrorNone; + Host *existingHost; + + // Verify that there is a matching base type service for all + // sub-type services in `aHost` (which is from the received + // and parsed SRP Update message). + + for (const Service &service : aHost.GetServices()) + { + if (service.IsSubType() && (aHost.FindBaseService(service.GetInstanceName()) == nullptr)) + { +#if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_WARN) + char subLabel[Dns::Name::kMaxLabelSize]; + + IgnoreError(service.GetServiceSubTypeLabel(subLabel, sizeof(subLabel))); + LogWarn("Message contains instance %s with subtype %s without base type", service.GetInstanceName(), + subLabel); +#endif + + ExitNow(error = kErrorParse); + } + } + + // SRP server must treat the update instructions for a service type + // and all its sub-types as atomic, i.e., when a service and its + // sub-types are being updated, whatever information appears in the + // SRP Update is the entirety of information about that service and + // its sub-types. Any previously registered sub-type that does not + // appear in a new SRP Update, must be removed. + // + // We go though the list of registered services for the same host + // and if the base service is included in the new SRP Update + // message, we add any previously registered service sub-type that + // does not appear in new Update message as "deleted". + + existingHost = mHosts.FindMatching(aHost.GetFullName()); + VerifyOrExit(existingHost != nullptr); + + for (const Service &baseService : existingHost->GetServices()) + { + if (baseService.IsSubType() || (aHost.FindBaseService(baseService.GetInstanceName()) == nullptr)) + { + continue; + } + + for (const Service &subService : existingHost->GetServices()) + { + if (!subService.IsSubType() || !subService.MatchesInstanceName(baseService.GetInstanceName())) + { + continue; + } + + SuccessOrExit(error = aHost.AddCopyOfServiceAsDeletedIfNotPresent(subService, aMetadata.mRxTime)); + } + } + +exit: + return error; +} + void Server::HandleUpdate(Host &aHost, const MessageMetadata &aMetadata) { Error error = kErrorNone; @@ -1224,15 +1288,7 @@ void Server::HandleUpdate(Host &aHost, const MessageMetadata &aMetadata) continue; } - if (aHost.FindService(service.GetServiceName(), service.GetInstanceName()) == nullptr) - { - Service *newService = aHost.AddNewService(service.GetServiceName(), service.GetInstanceName(), - service.IsSubType(), aMetadata.mRxTime); - - VerifyOrExit(newService != nullptr, error = kErrorNoBufs); - newService->mDescription->mUpdateTime = aMetadata.mRxTime; - newService->mIsDeleted = true; - } + SuccessOrExit(error = aHost.AddCopyOfServiceAsDeletedIfNotPresent(service, aMetadata.mRxTime)); } exit: @@ -1959,6 +2015,25 @@ exit: return; } +Error Server::Host::AddCopyOfServiceAsDeletedIfNotPresent(const Service &aService, TimeMilli aUpdateTime) +{ + Error error = kErrorNone; + Service *newService; + + VerifyOrExit(FindService(aService.GetServiceName(), aService.GetInstanceName()) == nullptr); + + newService = + AddNewService(aService.GetServiceName(), aService.GetInstanceName(), aService.IsSubType(), aUpdateTime); + + VerifyOrExit(newService != nullptr, error = kErrorNoBufs); + + newService->mDescription->mUpdateTime = aUpdateTime; + newService->mIsDeleted = true; + +exit: + return error; +} + void Server::Host::FreeAllServices(void) { while (!mServices.IsEmpty()) @@ -2064,6 +2139,17 @@ Server::Service *Server::Host::FindService(const char *aServiceName, const char return AsNonConst(AsConst(this)->FindService(aServiceName, aInstanceName)); } +const Server::Service *Server::Host::FindBaseService(const char *aInstanceName) const +{ + return FindNextService(/*a PrevService */ nullptr, kFlagsBaseTypeServiceOnly, /* aServiceName */ nullptr, + aInstanceName); +} + +Server::Service *Server::Host::FindBaseService(const char *aInstanceName) +{ + return AsNonConst(AsConst(this)->FindBaseService(aInstanceName)); +} + Error Server::Host::AddIp6Address(const Ip6::Address &aIp6Address) { Error error = kErrorNone; diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 6fcc1d818..b2c371688 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -585,6 +585,7 @@ public: bool aIsSubType, TimeMilli aUpdateTime); void RemoveService(Service *aService, RetainName aRetainName, NotifyMode aNotifyServiceHandler); + Error AddCopyOfServiceAsDeletedIfNotPresent(const Service &aService, TimeMilli aUpdateTime); void FreeAllServices(void); void ClearResources(void); Error MergeServicesAndResourcesFrom(Host &aHost); @@ -594,6 +595,8 @@ public: const RetainPtr FindServiceDescription(const char *aInstanceName) const; Service * FindService(const char *aServiceName, const char *aInstanceName); const Service * FindService(const char *aServiceName, const char *aInstanceName) const; + Service * FindBaseService(const char *aInstanceName); + const Service * FindBaseService(const char *aInstanceName) const; Host * mNext; Heap::String mFullName; @@ -977,6 +980,7 @@ private: uint16_t aSigRdataOffset, uint16_t aSigRdataLength, const char * aSignerName) const; + Error ValidateServiceSubTypes(Host &aHost, const MessageMetadata &aMetadata); Error ProcessZoneSection(const Message &aMessage, MessageMetadata &aMetadata) const; Error ProcessHostDescriptionInstruction(Host & aHost, const Message & aMessage,