From 48b129e74b19bef719cf8388906ef2e7903eae87 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 2 Jul 2021 11:42:55 -0700 Subject: [PATCH] [dnssd] add support for service subtypes in DNS-SD server and client (#6773) This commit updates `Dns::ServiceDiscovery::Server` to add support for browsing a service subtype. In particular, it updates the code which breaks a DNS name into its components such that it can correctly parse and accept a service subtype name. It also updates how the service name compression is done to handle the subtype names. This commit also updates `Dns::Client` to remove the extra check of service name (which is already verified) when trying to find a PTR record for a given service instance name. This change enables browse for service subtype (parsing the browse response). This commit also updates and enhances the `test_dnssd.py` script adding test-cases for browsing for service subtype. --- include/openthread/instance.h | 2 +- src/core/net/dns_client.cpp | 12 +-- src/core/net/dnssd_server.cpp | 43 ++++++-- src/core/net/dnssd_server.hpp | 4 + tests/scripts/thread-cert/test_dnssd.py | 133 +++++++++++++++++------- 5 files changed, 142 insertions(+), 52 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 89a3973fc..108506205 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (133) +#define OPENTHREAD_API_VERSION (134) /** * @addtogroup api-instance diff --git a/src/core/net/dns_client.cpp b/src/core/net/dns_client.cpp index 28e3d0e46..bd4f18f27 100644 --- a/src/core/net/dns_client.cpp +++ b/src/core/net/dns_client.cpp @@ -441,21 +441,15 @@ Error Client::BrowseResponse::FindPtrRecord(const char *aInstanceLabel, Name &aI SuccessOrExit(error); // It is a PTR record. Check the first label to match the - // instance label and the rest of the name to match the service - // name from `mQuery`. + // instance label. labelOffset = offset; error = Name::CompareLabel(*mMessage, labelOffset, aInstanceLabel); if (error == kErrorNone) { - error = Name::CompareName(*mMessage, labelOffset, serviceName); - - if (error == kErrorNone) - { - aInstanceName.SetFromMessage(*mMessage, offset); - ExitNow(); - } + aInstanceName.SetFromMessage(*mMessage, offset); + ExitNow(); } VerifyOrExit(error == kErrorNotFound); diff --git a/src/core/net/dnssd_server.cpp b/src/core/net/dnssd_server.cpp index 95ddb0329..8e06ee5e0 100644 --- a/src/core/net/dnssd_server.cpp +++ b/src/core/net/dnssd_server.cpp @@ -49,6 +49,7 @@ namespace ServiceDiscovery { const char Server::kDnssdProtocolUdp[4] = {'_', 'u', 'd', 'p'}; const char Server::kDnssdProtocolTcp[4] = {'_', 't', 'c', 'p'}; +const char Server::kDnssdSubTypeLabel[] = "._sub."; const char Server::kDefaultDomainName[] = "default.service.arpa."; Server::Server(Instance &aInstance) @@ -370,8 +371,26 @@ exit: Error Server::AppendServiceName(Message &aMessage, const char *aName, NameCompressInfo &aCompressInfo) { - Error error; - uint16_t serviceCompressOffset = aCompressInfo.GetServiceNameOffset(aMessage, aName); + Error error; + uint16_t serviceCompressOffset = aCompressInfo.GetServiceNameOffset(aMessage, aName); + const char *serviceName; + + // Check whether `aName` is a sub-type service name. + serviceName = StringFind(aName, kDnssdSubTypeLabel); + + if (serviceName != nullptr) + { + uint8_t subTypeLabelLength = static_cast(serviceName - aName) + sizeof(kDnssdSubTypeLabel) - 1; + + SuccessOrExit(error = Name::AppendMultipleLabels(aName, subTypeLabelLength, aMessage)); + + // Skip over the "._sub." label to get to the root service name. + serviceName += sizeof(kDnssdSubTypeLabel) - 1; + } + else + { + serviceName = aName; + } if (serviceCompressOffset != NameCompressInfo::kUnknownOffset) { @@ -379,7 +398,7 @@ Error Server::AppendServiceName(Message &aMessage, const char *aName, NameCompre } else { - uint8_t domainStart = static_cast(StringLength(aName, Name::kMaxNameSize - 1) - + uint8_t domainStart = static_cast(StringLength(serviceName, Name::kMaxNameSize - 1) - StringLength(aCompressInfo.GetDomainName(), Name::kMaxNameSize - 1)); uint16_t domainCompressOffset = aCompressInfo.GetDomainNameOffset(); @@ -389,11 +408,11 @@ Error Server::AppendServiceName(Message &aMessage, const char *aName, NameCompre if (domainCompressOffset == NameCompressInfo::kUnknownOffset) { aCompressInfo.SetDomainNameOffset(serviceCompressOffset + domainStart); - error = Name::AppendName(aName, aMessage); + error = Name::AppendName(serviceName, aMessage); } else { - SuccessOrExit(error = Name::AppendMultipleLabels(aName, domainStart, aMessage)); + SuccessOrExit(error = Name::AppendMultipleLabels(serviceName, domainStart, aMessage)); error = Name::AppendPointerLabel(domainCompressOffset, aMessage); } } @@ -547,10 +566,22 @@ Error Server::FindNameComponents(const char *aName, const char *aDomain, NameCom aInfo.mServiceOffset = labelBegin; - // Treat everything before as label + // Check for service subtype error = FindPreviousLabel(aName, labelBegin, labelEnd); VerifyOrExit(error == kErrorNone, error = (error == kErrorNotFound ? kErrorNone : error)); + // Note that `kDnssdSubTypeLabel` is "._sub.". Here we get the + // label only so we want to compare it with "_sub". + if ((labelEnd == labelBegin + kSubTypeLabelLength) && + (memcmp(&aName[labelBegin], kDnssdSubTypeLabel + 1, kSubTypeLabelLength) == 0)) + { + SuccessOrExit(error = FindPreviousLabel(aName, labelBegin, labelEnd)); + VerifyOrExit(labelBegin == 0, error = kErrorInvalidArgs); + aInfo.mSubTypeOffset = labelBegin; + ExitNow(); + } + + // Treat everything before as label aInfo.mInstanceOffset = 0; exit: diff --git a/src/core/net/dnssd_server.hpp b/src/core/net/dnssd_server.hpp index bcb61c233..008659cec 100644 --- a/src/core/net/dnssd_server.hpp +++ b/src/core/net/dnssd_server.hpp @@ -231,6 +231,7 @@ private: { kPort = OPENTHREAD_CONFIG_DNSSD_SERVER_PORT, kProtocolLabelLength = 4, + kSubTypeLabelLength = 4, kMaxConcurrentQueries = 32, }; @@ -246,6 +247,7 @@ private: : mDomainOffset(kNotPresent) , mProtocolOffset(kNotPresent) , mServiceOffset(kNotPresent) + , mSubTypeOffset(kNotPresent) , mInstanceOffset(kNotPresent) { } @@ -261,6 +263,7 @@ private: // the name is not a service or instance. uint8_t mServiceOffset; // The offset to the beginning of or `kNotPresent` if the name is not a // service or instance. + uint8_t mSubTypeOffset; // The offset to the beginning of sub-type label or `kNotPresent` is not a sub-type. uint8_t mInstanceOffset; // The offset to the beginning of or `kNotPresent` if the name is not a // instance. }; @@ -393,6 +396,7 @@ private: static const char kDnssdProtocolUdp[4]; static const char kDnssdProtocolTcp[4]; + static const char kDnssdSubTypeLabel[]; static const char kDefaultDomainName[]; Ip6::Udp::Socket mSocket; diff --git a/tests/scripts/thread-cert/test_dnssd.py b/tests/scripts/thread-cert/test_dnssd.py index 6a12efb0e..a508d47a0 100755 --- a/tests/scripts/thread-cert/test_dnssd.py +++ b/tests/scripts/thread-cert/test_dnssd.py @@ -32,20 +32,27 @@ import unittest import thread_cert +# Test description: +# +# This test verifies DNS-SD server and DNS client behavior +# (browsing for services and/or subtype services, resolving an +# address, or resolving a service). It also indirectly covers the SRP +# client and server behavior and the interactions of DNS-SD server +# with SRP server). +# +# Topology: +# Four nodes, leader acting as SRP and DNS-SD servers, with 3 router +# nodes acting as SRP and DNS clients. +# + SERVER = 1 CLIENT1 = 2 CLIENT2 = 3 +CLIENT3 = 4 DOMAIN = 'default.service.arpa.' SERVICE = '_ipps._tcp' -# -# Topology: -# LEADER -- CLIENT1 -# | -# CLIENT2 -# - class TestDnssd(thread_cert.TestCase): SUPPORT_NCP = False @@ -61,39 +68,57 @@ class TestDnssd(thread_cert.TestCase): CLIENT2: { 'mode': 'rdn', }, + CLIENT3: { + 'mode': 'rdn', + } } def test(self): - self.nodes[SERVER].start() + server = self.nodes[SERVER] + client1 = self.nodes[CLIENT1] + client2 = self.nodes[CLIENT2] + client3 = self.nodes[CLIENT3] + + #--------------------------------------------------------------- + # Start the server & client devices. + + server.start() self.simulator.go(5) - self.assertEqual(self.nodes[SERVER].get_state(), 'leader') - self.nodes[SERVER].srp_server_set_enabled(True) + self.assertEqual(server.get_state(), 'leader') + server.srp_server_set_enabled(True) - self.nodes[CLIENT1].start() + client1.start() + client2.start() + client3.start() self.simulator.go(5) - self.assertEqual(self.nodes[CLIENT1].get_state(), 'router') + self.assertEqual(client1.get_state(), 'router') + self.assertEqual(client2.get_state(), 'router') + self.assertEqual(client3.get_state(), 'router') - self.nodes[CLIENT2].start() - self.simulator.go(5) - self.assertEqual(self.nodes[CLIENT1].get_state(), 'router') + #--------------------------------------------------------------- + # Register services on clients - client1_addrs = [self.nodes[CLIENT1].get_mleid(), self.nodes[CLIENT1].get_rloc()] - self._config_srp_client_services(CLIENT1, 'ins1', 'host1', 11111, 1, 1, client1_addrs) + client1_addrs = [client1.get_mleid(), client1.get_rloc()] + client2_addrs = [client2.get_mleid(), client2.get_rloc()] + client3_addrs = [client3.get_mleid(), client2.get_rloc()] - client2_addrs = [self.nodes[CLIENT2].get_mleid(), self.nodes[CLIENT2].get_rloc()] - self._config_srp_client_services(CLIENT2, 'ins2', 'host2', 22222, 2, 2, client2_addrs) + self._config_srp_client_services(client1, server, 'ins1', 'host1', 11111, 1, 1, client1_addrs, ",_s1,_s2") + self._config_srp_client_services(client2, server, 'ins2', 'host2', 22222, 2, 2, client2_addrs) + self._config_srp_client_services(client3, server, 'ins3', 'host3', 33333, 3, 3, client3_addrs, ",_s1") - # Test AAAA query using DNS client - answers = self.nodes[CLIENT1].dns_resolve(f"host1.{DOMAIN}", self.nodes[SERVER].get_mleid(), 53) + #--------------------------------------------------------------- + # Resolve address (AAAA records) + + answers = client1.dns_resolve(f"host1.{DOMAIN}", server.get_mleid(), 53) self.assertEqual(set(ipaddress.IPv6Address(ip) for ip, _ in answers), set(map(ipaddress.IPv6Address, client1_addrs))) - answers = self.nodes[CLIENT1].dns_resolve(f"host2.{DOMAIN}", self.nodes[SERVER].get_mleid(), 53) + answers = client1.dns_resolve(f"host2.{DOMAIN}", server.get_mleid(), 53) self.assertEqual(set(ipaddress.IPv6Address(ip) for ip, _ in answers), set(map(ipaddress.IPv6Address, client2_addrs))) - service_instances = self.nodes[CLIENT1].dns_browse(f'{SERVICE}.{DOMAIN}', self.nodes[SERVER].get_mleid(), 53) - self.assertEqual({'ins1', 'ins2'}, set(service_instances.keys()), service_instances) + #--------------------------------------------------------------- + # Browsing for services instance1_verify_info = { 'port': 11111, @@ -119,15 +144,42 @@ class TestDnssd(thread_cert.TestCase): 'aaaa_ttl': lambda x: x > 0, } + instance3_verify_info = { + 'port': 33333, + 'priority': 3, + 'weight': 3, + 'host': 'host3.default.service.arpa.', + 'address': client3_addrs, + 'txt_data': '', + 'srv_ttl': lambda x: x > 0, + 'txt_ttl': lambda x: x > 0, + 'aaaa_ttl': lambda x: x > 0, + } + + # Browse for main service + service_instances = client1.dns_browse(f'{SERVICE}.{DOMAIN}', server.get_mleid(), 53) + self.assertEqual({'ins1', 'ins2', 'ins3'}, set(service_instances.keys())) self._assert_service_instance_equal(service_instances['ins1'], instance1_verify_info) self._assert_service_instance_equal(service_instances['ins2'], instance2_verify_info) + self._assert_service_instance_equal(service_instances['ins3'], instance3_verify_info) - service_instance = self.nodes[CLIENT1].dns_resolve_service('ins1', f'{SERVICE}.{DOMAIN}', - self.nodes[SERVER].get_mleid(), 53) + # Browse for service sub-type _s1. + service_instances = client1.dns_browse(f'_s1._sub.{SERVICE}.{DOMAIN}', server.get_mleid(), 53) + self.assertEqual({'ins1', 'ins3'}, set(service_instances.keys())) + self._assert_service_instance_equal(service_instances['ins1'], instance1_verify_info) + + # Browse for service sub-type _s2. + service_instances = client1.dns_browse(f'_s2._sub.{SERVICE}.{DOMAIN}', server.get_mleid(), 53) + self.assertEqual({'ins1'}, set(service_instances.keys())) + self._assert_service_instance_equal(service_instances['ins1'], instance1_verify_info) + + #--------------------------------------------------------------- + # Resolve service + + service_instance = client1.dns_resolve_service('ins1', f'{SERVICE}.{DOMAIN}', server.get_mleid(), 53) self._assert_service_instance_equal(service_instance, instance1_verify_info) - service_instance = self.nodes[CLIENT1].dns_resolve_service('ins2', f'{SERVICE}.{DOMAIN}', - self.nodes[SERVER].get_mleid(), 53) + service_instance = client1.dns_resolve_service('ins2', f'{SERVICE}.{DOMAIN}', server.get_mleid(), 53) self._assert_service_instance_equal(service_instance, instance2_verify_info) def _assert_service_instance_equal(self, instance, info): @@ -147,17 +199,26 @@ class TestDnssd(thread_cert.TestCase): self.assertTrue(check_ttl(instance[ttl_f]), instance) - def _config_srp_client_services(self, client, instancename, hostname, port, priority, weight, addrs): - self.nodes[client].netdata_show() - srp_server_port = self.nodes[client].get_srp_server_port() + def _config_srp_client_services(self, + client, + server, + instancename, + hostname, + port, + priority, + weight, + addrs, + subtypes=''): + client.netdata_show() + srp_server_port = client.get_srp_server_port() - self.nodes[client].srp_client_start(self.nodes[SERVER].get_mleid(), srp_server_port) - self.nodes[client].srp_client_set_host_name(hostname) - self.nodes[client].srp_client_set_host_address(*addrs) - self.nodes[client].srp_client_add_service(instancename, SERVICE, port, priority, weight) + client.srp_client_start(server.get_mleid(), srp_server_port) + client.srp_client_set_host_name(hostname) + client.srp_client_set_host_address(*addrs) + client.srp_client_add_service(instancename, SERVICE + subtypes, port, priority, weight) self.simulator.go(5) - self.assertEqual(self.nodes[client].srp_client_get_host_state(), 'Registered') + self.assertEqual(client.srp_client_get_host_state(), 'Registered') if __name__ == '__main__':