[dnssd-server] skip additional records on a PTR query with multiple answers (#9281)

This commit updates `Dns::ServiceDiscovery::Server` such that when
answering a PTR query with more than one answer, it does not include
additional records. This is to keep the size of the response small.

This commit also updates the test scripts validating browse (PTR
query) function to check the new behavior. In particular, a common
python function `_parse_dns_service_info()` is added to parse service
info in CLI output of "dns browse" or "dns service" commands and
handle if output of "dns browse" does not include service info.
This commit is contained in:
Abtin Keshavarzian
2023-07-17 13:09:06 -07:00
committed by GitHub
parent 55cb65de62
commit bec592084c
8 changed files with 72 additions and 66 deletions
+13 -5
View File
@@ -1236,6 +1236,19 @@ Send a browse (service instance enumeration) DNS query to get the list of servic
The parameters after `service-name` are optional. Any unspecified (or zero) value for these optional parameters is replaced by the value from the current default config (`dns config`).
```bash
> dns browse _service._udp.example.com
DNS browse response for _service._udp.example.com.
inst1
inst2
inst3
Done
```
The detailed service info (port number, weight, host name, TXT data, host addresses) is outputted only when provided by server/resolver in the browse response (in additional Data Section). This is a SHOULD and not a MUST requirement, and servers/resolvers are not required to provide this.
The recommended behavior, which is supported by the OpenThread DNS-SD resolver, is to only provide the additional data when there is a single instance in the response. However, users should assume that the browse response may only contain the list of matching service instances and not any detail service info. To resolve a service instance, users can use the `dns service` or `dns servicehost` commands.
```bash
> dns browse _service._udp.example.com
DNS browse response for _service._udp.example.com.
@@ -1244,11 +1257,6 @@ inst1
Host:host.example.com.
HostAddress:fd00:0:0:0:0:0:0:abcd TTL:7200
TXT:[a=6531, b=6c12] TTL:7300
instance2
Port:1234, Priority:1, Weight:2, TTL:7200
Host:host.example.com.
HostAddress:fd00:0:0:0:0:0:0:abcd TTL:7200
TXT:[a=1234] TTL:7300
Done
```
+9
View File
@@ -734,6 +734,15 @@ Header::Response Server::ResolveBySrp(Header &aResponseHeader,
IgnoreError(aResponseMessage.Read(readOffset, question));
readOffset += sizeof(question);
if ((question.GetType() == ResourceRecord::kTypePtr) && (aResponseHeader.GetAnswerCount() > 1))
{
// Skip adding additional records, when answering a
// PTR query with more than one answer. This is the
// recommended behavior to keep the size of the
// response small.
continue;
}
VerifyOrExit(Header::kResponseServerFailure != ResolveQuestionBySrp(name, question, aResponseHeader,
aResponseMessage, aCompressInfo,
/* aAdditional */ true),
@@ -114,6 +114,8 @@ class TestDnssdInstanceNameWithSpace(thread_cert.TestCase):
full_instance_name = f'{INSTANCE_NAME}.{SERVICE_FULL_NAME}'
EMPTY_TXT = {}
# In all cases, there is one match, so server should include
# service info in additional section of PTR query response.
self._verify_service_browse_result(client.dns_browse(SERVICE_FULL_NAME, server=br1.get_rloc()))
self._verify_service_browse_result(client.dns_browse(SERVICE_FULL_NAME, server=br2.get_rloc()))
self._verify_service_browse_result(client.dns_browse(SERVICE_FULL_NAME.lower(), server=br2.get_rloc()))
@@ -136,16 +136,6 @@ class TestDnssdServerOnBr(thread_cert.TestCase):
'QUESTION': [(SERVICE_FULL_NAME, 'IN', 'PTR')],
'ANSWER': [(SERVICE_FULL_NAME, 'IN', 'PTR', f'ins1.{SERVICE_FULL_NAME}'),
(SERVICE_FULL_NAME, 'IN', 'PTR', f'ins2.{SERVICE_FULL_NAME}')],
'ADDITIONAL': [
(ins1_full_name, 'IN', 'SRV', 1, 1, 11111, host1_full_name),
(ins1_full_name, 'IN', 'TXT', EMPTY_TXT),
(host1_full_name, 'IN', 'AAAA', client1_addrs[0]),
(host1_full_name, 'IN', 'AAAA', client1_addrs[1]),
(ins2_full_name, 'IN', 'SRV', 2, 2, 22222, host2_full_name),
(ins2_full_name, 'IN', 'TXT', EMPTY_TXT),
(host2_full_name, 'IN', 'AAAA', client2_addrs[0]),
(host2_full_name, 'IN', 'AAAA', client2_addrs[1]),
],
})
# check if SRV query works
@@ -152,6 +152,9 @@ class MdnsRestart(thread_cert.TestCase):
self.assertEqual(len(host.browse_mdns_services('_ed2._tcp')), 1)
ed1.dns_set_config(br1.get_ip6_address(config.ADDRESS_TYPE.ML_EID))
# Since there is only one match, server should include
# service info in additional section of its response.
browsed_services = ed1.dns_browse('_ed2._tcp.default.service.arpa')
self.assertEqual(len(browsed_services), 1)
self.assertEqual(browsed_services['ed2']['port'], 12345)
+35 -42
View File
@@ -3310,6 +3310,31 @@ class NodeImpl:
return list(zip(ip, ttl))
def _parse_dns_service_info(self, output):
# Example of `output`
# Port:22222, Priority:2, Weight:2, TTL:7155
# Host:host2.default.service.arpa.
# HostAddress:0:0:0:0:0:0:0:0 TTL:0
# TXT:[a=00, b=02bb] TTL:7155
m = re.match(
r'.*Port:(\d+), Priority:(\d+), Weight:(\d+), TTL:(\d+)\s+Host:(.*?)\s+HostAddress:(\S+) TTL:(\d+)\s+TXT:\[(.*?)\] TTL:(\d+)',
'\r'.join(output))
if not m:
return {}
port, priority, weight, srv_ttl, hostname, address, aaaa_ttl, txt_data, txt_ttl = m.groups()
return {
'port': int(port),
'priority': int(priority),
'weight': int(weight),
'host': hostname,
'address': address,
'txt_data': txt_data,
'srv_ttl': int(srv_ttl),
'txt_ttl': int(txt_ttl),
'aaaa_ttl': int(aaaa_ttl),
}
def dns_resolve_service(self, instance, service, server=None, port=53):
"""
Resolves the service instance and returns the instance information as a dict.
@@ -3335,33 +3360,10 @@ class NodeImpl:
self.send_command(cmd)
self.simulator.go(10)
output = self._expect_command_output()
# Example output:
# DNS service resolution response for ins2 for service _ipps._tcp.default.service.arpa.
# Port:22222, Priority:2, Weight:2, TTL:7155
# Host:host2.default.service.arpa.
# HostAddress:0:0:0:0:0:0:0:0 TTL:0
# TXT:[a=00, b=02bb] TTL:7155
# Done
m = re.match(
r'.*Port:(\d+), Priority:(\d+), Weight:(\d+), TTL:(\d+)\s+Host:(.*?)\s+HostAddress:(\S+) TTL:(\d+)\s+TXT:\[(.*?)\] TTL:(\d+)',
'\r'.join(output))
if m:
port, priority, weight, srv_ttl, hostname, address, aaaa_ttl, txt_data, txt_ttl = m.groups()
return {
'port': int(port),
'priority': int(priority),
'weight': int(weight),
'host': hostname,
'address': address,
'txt_data': txt_data,
'srv_ttl': int(srv_ttl),
'txt_ttl': int(txt_ttl),
'aaaa_ttl': int(aaaa_ttl),
}
else:
info = self._parse_dns_service_info(output)
if not info:
raise Exception('dns resolve service failed: %s.%s' % (instance, service))
return info
@staticmethod
def __parse_hex_string(hexstr: str) -> bytes:
@@ -3404,9 +3406,10 @@ class NodeImpl:
self.send_command(cmd)
self.simulator.go(10)
output = '\n'.join(self._expect_command_output())
output = self._expect_command_output()
# Example output:
# DNS browse response for _ipps._tcp.default.service.arpa.
# ins2
# Port:22222, Priority:2, Weight:2, TTL:7175
# Host:host2.default.service.arpa.
@@ -3420,21 +3423,11 @@ class NodeImpl:
# Done
result = {}
for ins, port, priority, weight, srv_ttl, hostname, address, aaaa_ttl, txt_data, txt_ttl in re.findall(
r'(.*?)\s+Port:(\d+), Priority:(\d+), Weight:(\d+), TTL:(\d+)\s*Host:(\S+)\s+HostAddress:(\S+) TTL:(\d+)\s+TXT:\[(.*?)\] TTL:(\d+)',
output):
result[ins] = {
'port': int(port),
'priority': int(priority),
'weight': int(weight),
'host': hostname,
'address': address,
'txt_data': txt_data,
'srv_ttl': int(srv_ttl),
'txt_ttl': int(txt_ttl),
'aaaa_ttl': int(aaaa_ttl),
}
index = 1 # skip first line
while index < len(output):
ins = output[index].strip()
result[ins] = self._parse_dns_service_info(output[index + 1:index + 6])
index = index + (5 if result[ins] else 1)
return result
def set_mliid(self, mliid: str):
+8 -8
View File
@@ -172,16 +172,14 @@ class TestDnssd(thread_cert.TestCase):
# Browse for main service
service_instances = client1.dns_browse(f'{SERVICE}.{DOMAIN}'.upper(), 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)
# Browse for service sub-type _s1.
service_instances = client1.dns_browse(f'_s1._sub.{SERVICE}.{DOMAIN}'.upper(), 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.
# Since there is only one matching instance, validate that
# server included the service info in additional section.
service_instances = client1.dns_browse(f'_s2._sub.{SERVICE}.{DOMAIN}'.upper(), server.get_mleid(), 53)
self.assertEqual({'ins1'}, set(service_instances.keys()))
self._assert_service_instance_equal(service_instances['ins1'], instance1_verify_info)
@@ -195,6 +193,9 @@ class TestDnssd(thread_cert.TestCase):
service_instance = client1.dns_resolve_service('ins2', f'{SERVICE}.{DOMAIN}'.upper(), server.get_mleid(), 53)
self._assert_service_instance_equal(service_instance, instance2_verify_info)
service_instance = client1.dns_resolve_service('ins3', f'{SERVICE}.{DOMAIN}'.upper(), server.get_mleid(), 53)
self._assert_service_instance_equal(service_instance, instance3_verify_info)
#---------------------------------------------------------------
# Add another service with TXT entries to the existing host and
# verify that it is properly merged.
@@ -204,10 +205,9 @@ class TestDnssd(thread_cert.TestCase):
service_instances = client1.dns_browse(f'{SERVICE}.{DOMAIN}', server.get_mleid(), 53)
self.assertEqual({'ins1', 'ins2', 'ins3', 'ins4'}, 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)
self._assert_service_instance_equal(service_instances['ins4'], instance4_verify_info)
service_instance = client1.dns_resolve_service('ins4', f'{SERVICE}.{DOMAIN}'.upper(), server.get_mleid(), 53)
self._assert_service_instance_equal(service_instance, instance4_verify_info)
def _assert_service_instance_equal(self, instance, info):
self.assertEqual(instance['host'].lower(), info['host'].lower(), instance)
@@ -177,7 +177,8 @@ class TestSrpServerAnycastMode(thread_cert.TestCase):
#---------------------------------------------------------------
# Browse for a matching service name and verify that the registered
# service is successfully found.
# service is successfully found. Since there is only one match the
# server should include the service info in additional section.
service_instances = browser.dns_browse(f'{SERVICE}.{DOMAIN}', server.get_mleid(), DNS_RESOLVER_PORT)
self.assertEqual({INSTANCE}, set(service_instances.keys()))