From 6e4f394b3d820bb7d97a1d81e507a6a05283f68c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:06:47 -0500 Subject: [PATCH 1/8] Include NSEC records for non-existant types when responding with addresses --- zeroconf/_handlers.py | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index 06ed54cd1..67ad32330 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -26,15 +26,17 @@ from typing import Dict, Iterable, List, NamedTuple, Optional, Set, TYPE_CHECKING, Tuple, Union, cast from ._cache import DNSCache, _UniqueRecordsType -from ._dns import DNSAddress, DNSPointer, DNSQuestion, DNSRRSet, DNSRecord +from ._dns import DNSAddress, DNSNsec, DNSPointer, DNSQuestion, DNSRRSet, DNSRecord from ._history import QuestionHistory from ._logger import log from ._protocol import DNSIncoming, DNSOutgoing +from ._services.info import ServiceInfo from ._services.registry import ServiceRegistry from ._updates import RecordUpdate, RecordUpdateListener from ._utils.time import current_time_millis, millis_to_seconds from .const import ( _CLASS_IN, + _CLASS_UNIQUE, _DNS_OTHER_TTL, _DNS_PTR_MIN_TTL, _FLAGS_AA, @@ -44,6 +46,7 @@ _TYPE_A, _TYPE_AAAA, _TYPE_ANY, + _TYPE_NSEC, _TYPE_PTR, _TYPE_SRV, _TYPE_TXT, @@ -56,7 +59,8 @@ _AnswerWithAdditionalsType = Dict[DNSRecord, Set[DNSRecord]] _MULTICAST_DELAY_RANDOM_INTERVAL = (20, 120) -_RESPOND_IMMEDIATE_TYPES = {_TYPE_SRV, _TYPE_A, _TYPE_AAAA} +_ADDRESS_RECORD_TYPES = {_TYPE_A, _TYPE_AAAA} +_RESPOND_IMMEDIATE_TYPES = {_TYPE_SRV, *_ADDRESS_RECORD_TYPES} class QuestionAnswers(NamedTuple): @@ -78,6 +82,11 @@ def _message_is_probe(msg: DNSIncoming) -> bool: return msg.num_authorities > 0 +def construct_nsec_record(name: str, types: list[int], now: float) -> DNSNsec: + """Construct an NSEC record for name and a list of dns types.""" + return DNSNsec(name, _TYPE_NSEC, _CLASS_IN | _CLASS_UNIQUE, _DNS_OTHER_TTL, name, types, created=now) + + def construct_outgoing_multicast_answers(answers: _AnswerWithAdditionalsType) -> DNSOutgoing: """Add answers and additionals to a DNSOutgoing.""" out = DNSOutgoing(_FLAGS_QR_RESPONSE | _FLAGS_AA, multicast=True) @@ -244,12 +253,23 @@ def _add_pointer_answers( # Add recommended additional answers according to # https://tools.ietf.org/html/rfc6763#section-12.1. dns_pointer = service.dns_pointer(created=now) - if not known_answers.suppresses(dns_pointer): - answer_set[dns_pointer] = { - service.dns_service(created=now), - service.dns_text(created=now), - *service.dns_addresses(created=now), - } + if known_answers.suppresses(dns_pointer): + continue + additionals: Set[DNSRecord] = {service.dns_service(created=now), service.dns_text(created=now)} + additionals |= self._get_address_and_nsec_records(service, now) + answer_set[dns_pointer] = additionals + + def _get_address_and_nsec_records(self, service: ServiceInfo, now: float): + """Build a set of address records and NSEC records for non-present record types.""" + seen_types: Set[int] = set() + records: Set[DNSRecord] = set() + for dns_address in service.dns_addresses(created=now): + seen_types.add(dns_address.type) + records.add(dns_address) + missing_types: Set[int] = _ADDRESS_RECORD_TYPES - seen_types + if missing_types: + records.add(construct_nsec_record(service.name, list(missing_types), now)) + return records def _add_address_answers( self, @@ -263,11 +283,16 @@ def _add_address_answers( for service in self.registry.async_get_infos_server(name): answers: List[DNSAddress] = [] additionals: Set[DNSRecord] = set() + seen_types: Set[int] = set() for dns_address in service.dns_addresses(created=now): + seen_types.add(dns_address.type) if dns_address.type != type_: additionals.add(dns_address) elif not known_answers.suppresses(dns_address): answers.append(dns_address) + missing_types: Set[int] = _ADDRESS_RECORD_TYPES - seen_types + if missing_types: + additionals.add(construct_nsec_record(service.name, list(missing_types), now)) for answer in answers: answer_set[answer] = additionals @@ -299,7 +324,7 @@ def _answer_question( # https://tools.ietf.org/html/rfc6763#section-12.2. dns_service = service.dns_service(created=now) if not known_answers.suppresses(dns_service): - answer_set[dns_service] = set(service.dns_addresses(created=now)) + answer_set[dns_service] = self._get_address_and_nsec_records(service, now) if type_ in (_TYPE_TXT, _TYPE_ANY): dns_text = service.dns_text(created=now) if not known_answers.suppresses(dns_text): From c2c07c8318fc27a46bcbe0b1f31e43194706d501 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:08:46 -0500 Subject: [PATCH 2/8] types --- zeroconf/_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index 67ad32330..d6256dfd6 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -82,7 +82,7 @@ def _message_is_probe(msg: DNSIncoming) -> bool: return msg.num_authorities > 0 -def construct_nsec_record(name: str, types: list[int], now: float) -> DNSNsec: +def construct_nsec_record(name: str, types: List[int], now: float) -> DNSNsec: """Construct an NSEC record for name and a list of dns types.""" return DNSNsec(name, _TYPE_NSEC, _CLASS_IN | _CLASS_UNIQUE, _DNS_OTHER_TTL, name, types, created=now) @@ -259,7 +259,7 @@ def _add_pointer_answers( additionals |= self._get_address_and_nsec_records(service, now) answer_set[dns_pointer] = additionals - def _get_address_and_nsec_records(self, service: ServiceInfo, now: float): + def _get_address_and_nsec_records(self, service: ServiceInfo, now: float) -> Set[DNSRecord]: """Build a set of address records and NSEC records for non-present record types.""" seen_types: Set[int] = set() records: Set[DNSRecord] = set() From 955c5b3af985810924d1fce631dd1c0eaf66e44c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:14:51 -0500 Subject: [PATCH 3/8] fix name --- zeroconf/_handlers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index d6256dfd6..c0be8200e 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -268,7 +268,7 @@ def _get_address_and_nsec_records(self, service: ServiceInfo, now: float) -> Set records.add(dns_address) missing_types: Set[int] = _ADDRESS_RECORD_TYPES - seen_types if missing_types: - records.add(construct_nsec_record(service.name, list(missing_types), now)) + records.add(construct_nsec_record(service.server, list(missing_types), now)) return records def _add_address_answers( @@ -292,7 +292,7 @@ def _add_address_answers( answers.append(dns_address) missing_types: Set[int] = _ADDRESS_RECORD_TYPES - seen_types if missing_types: - additionals.add(construct_nsec_record(service.name, list(missing_types), now)) + additionals.add(construct_nsec_record(service.server, list(missing_types), now)) for answer in answers: answer_set[answer] = additionals From 93b12c65efb427876ffd9a95c878fa96081302ee Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:24:52 -0500 Subject: [PATCH 4/8] explict negative response --- zeroconf/_handlers.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index c0be8200e..e908c5c1e 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -60,7 +60,7 @@ _MULTICAST_DELAY_RANDOM_INTERVAL = (20, 120) _ADDRESS_RECORD_TYPES = {_TYPE_A, _TYPE_AAAA} -_RESPOND_IMMEDIATE_TYPES = {_TYPE_SRV, *_ADDRESS_RECORD_TYPES} +_RESPOND_IMMEDIATE_TYPES = {_TYPE_NSEC, _TYPE_SRV, *_ADDRESS_RECORD_TYPES} class QuestionAnswers(NamedTuple): @@ -83,7 +83,11 @@ def _message_is_probe(msg: DNSIncoming) -> bool: def construct_nsec_record(name: str, types: List[int], now: float) -> DNSNsec: - """Construct an NSEC record for name and a list of dns types.""" + """Construct an NSEC record for name and a list of dns types. + + This function should only be used for SRV/A/AAAA records + which have a TTL of _DNS_OTHER_TTL + """ return DNSNsec(name, _TYPE_NSEC, _CLASS_IN | _CLASS_UNIQUE, _DNS_OTHER_TTL, name, types, created=now) @@ -280,7 +284,12 @@ def _add_address_answers( type_: int, ) -> None: """Answer A/AAAA/ANY question.""" - for service in self.registry.async_get_infos_server(name): + services = self.registry.async_get_infos_server(name) + if not services: + answer_set[construct_nsec_record(name, list(_ADDRESS_RECORD_TYPES), now)] = set() + return + + for service in services: answers: List[DNSAddress] = [] additionals: Set[DNSRecord] = set() seen_types: Set[int] = set() From 79962a359e6e2c492d9eb5348f81343ed51d6d02 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:29:06 -0500 Subject: [PATCH 5/8] revert --- zeroconf/_handlers.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index e908c5c1e..1a7fe31db 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -284,12 +284,7 @@ def _add_address_answers( type_: int, ) -> None: """Answer A/AAAA/ANY question.""" - services = self.registry.async_get_infos_server(name) - if not services: - answer_set[construct_nsec_record(name, list(_ADDRESS_RECORD_TYPES), now)] = set() - return - - for service in services: + for service in self.registry.async_get_infos_server(name): answers: List[DNSAddress] = [] additionals: Set[DNSRecord] = set() seen_types: Set[int] = set() From 10e65d2dac930d3cfd04a5bda04b73e8a896257c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 29 Aug 2021 18:30:40 -0500 Subject: [PATCH 6/8] revert --- zeroconf/_handlers.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index 1a7fe31db..a3a564919 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -295,10 +295,13 @@ def _add_address_answers( elif not known_answers.suppresses(dns_address): answers.append(dns_address) missing_types: Set[int] = _ADDRESS_RECORD_TYPES - seen_types - if missing_types: - additionals.add(construct_nsec_record(service.server, list(missing_types), now)) - for answer in answers: - answer_set[answer] = additionals + if answers: + if missing_types: + additionals.add(construct_nsec_record(service.server, list(missing_types), now)) + for answer in answers: + answer_set[answer] = additionals + elif missing_types: + answer_set[construct_nsec_record(service.server, list(missing_types), now)] = set() def _answer_question( self, From c7e41b10083923b0c1d7e2cbf6145b47330be66e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 30 Aug 2021 09:36:43 -0500 Subject: [PATCH 7/8] only respond with nsec to directly asked question --- tests/test_handlers.py | 49 ++++++++++++++++++++++++++++++++++-------- zeroconf/_handlers.py | 2 +- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/tests/test_handlers.py b/tests/test_handlers.py index a621f0378..1a21ae191 100644 --- a/tests/test_handlers.py +++ b/tests/test_handlers.py @@ -108,8 +108,9 @@ def _process_outgoing_packet(out): _process_outgoing_packet(construct_outgoing_multicast_answers(question_answers.mcast_aggregate)) # The additonals should all be suppresed since they are all in the answers section + # There will be one NSEC additional to indicate the lack of AAAA record # - assert nbr_answers == 4 and nbr_additionals == 0 and nbr_authorities == 0 + assert nbr_answers == 4 and nbr_additionals == 1 and nbr_authorities == 0 nbr_answers = nbr_additionals = nbr_authorities = 0 # unregister @@ -143,7 +144,9 @@ def _process_outgoing_packet(out): [r.DNSIncoming(packet) for packet in query.packets()], False ) _process_outgoing_packet(construct_outgoing_multicast_answers(question_answers.mcast_aggregate)) - assert nbr_answers == 4 and nbr_additionals == 0 and nbr_authorities == 0 + + # There will be one NSEC additional to indicate the lack of AAAA record + assert nbr_answers == 4 and nbr_additionals == 1 and nbr_authorities == 0 nbr_answers = nbr_additionals = nbr_authorities = 0 # unregister @@ -271,7 +274,9 @@ def test_ptr_optimization(): has_txt = True elif answer.type == const._TYPE_A: has_a = True - assert nbr_answers == 1 and nbr_additionals == 3 + assert nbr_answers == 1 and nbr_additionals == 4 + # There will be one NSEC additional to indicate the lack of AAAA record + assert has_srv and has_txt and has_a # unregister @@ -406,7 +411,7 @@ def test_unicast_response(): [r.DNSIncoming(packet) for packet in query.packets()], True ) for answers in (question_answers.ucast, question_answers.mcast_aggregate): - has_srv = has_txt = has_a = False + has_srv = has_txt = has_a = has_aaaa = has_nsec = False nbr_additionals = 0 nbr_answers = len(answers) additionals = set().union(*answers.values()) @@ -418,8 +423,14 @@ def test_unicast_response(): has_txt = True elif answer.type == const._TYPE_A: has_a = True - assert nbr_answers == 1 and nbr_additionals == 3 - assert has_srv and has_txt and has_a + elif answer.type == const._TYPE_AAAA: + has_aaaa = True + elif answer.type == const._TYPE_NSEC: + has_nsec = True + # There will be one NSEC additional to indicate the lack of AAAA record + assert nbr_answers == 1 and nbr_additionals == 4 + assert has_srv and has_txt and has_a and has_nsec + assert not has_aaaa # unregister zc.registry.async_remove(info) @@ -497,7 +508,7 @@ def test_qu_response(): zc.register_service(info) def _validate_complete_response(answers): - has_srv = has_txt = has_a = False + has_srv = has_txt = has_a = has_aaaa = has_nsec = False nbr_answers = len(answers.keys()) additionals = set().union(*answers.values()) nbr_additionals = len(additionals) @@ -509,8 +520,13 @@ def _validate_complete_response(answers): has_txt = True elif answer.type == const._TYPE_A: has_a = True - assert nbr_answers == 1 and nbr_additionals == 3 - assert has_srv and has_txt and has_a + elif answer.type == const._TYPE_AAAA: + has_aaaa = True + elif answer.type == const._TYPE_NSEC: + has_nsec = True + assert nbr_answers == 1 and nbr_additionals == 4 + assert has_srv and has_txt and has_a and has_nsec + assert not has_aaaa # With QU should respond to only unicast when the answer has been recently multicast query = r.DNSOutgoing(const._FLAGS_QR_QUERY) @@ -635,6 +651,21 @@ def test_known_answer_supression(): assert not question_answers.mcast_aggregate assert not question_answers.mcast_aggregate_last_second + # Test NSEC record returned when there is no AAAA record and we expectly ask + generated = r.DNSOutgoing(const._FLAGS_QR_QUERY) + question = r.DNSQuestion(server_name, const._TYPE_AAAA, const._CLASS_IN) + generated.add_question(question) + for dns_address in info.dns_addresses(): + generated.add_answer_at_time(dns_address, now) + packets = generated.packets() + question_answers = zc.query_handler.async_response([r.DNSIncoming(packet) for packet in packets], False) + assert not question_answers.ucast + expected_nsec_record: r.DNSNsec = list(question_answers.mcast_now)[0] + assert const._TYPE_A not in expected_nsec_record.rdtypes + assert const._TYPE_AAAA in expected_nsec_record.rdtypes + assert not question_answers.mcast_aggregate + assert not question_answers.mcast_aggregate_last_second + # Test SRV supression generated = r.DNSOutgoing(const._FLAGS_QR_QUERY) question = r.DNSQuestion(registration_name, const._TYPE_SRV, const._CLASS_IN) diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py index a3a564919..76ba6cc3a 100644 --- a/zeroconf/_handlers.py +++ b/zeroconf/_handlers.py @@ -300,7 +300,7 @@ def _add_address_answers( additionals.add(construct_nsec_record(service.server, list(missing_types), now)) for answer in answers: answer_set[answer] = additionals - elif missing_types: + elif type_ in missing_types: answer_set[construct_nsec_record(service.server, list(missing_types), now)] = set() def _answer_question( From 5e1be18fd0de822252eddef5c3b9b533f28abaa4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 30 Aug 2021 09:36:51 -0500 Subject: [PATCH 8/8] black --- tests/test_handlers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_handlers.py b/tests/test_handlers.py index 1a21ae191..44ee1d5af 100644 --- a/tests/test_handlers.py +++ b/tests/test_handlers.py @@ -424,9 +424,9 @@ def test_unicast_response(): elif answer.type == const._TYPE_A: has_a = True elif answer.type == const._TYPE_AAAA: - has_aaaa = True + has_aaaa = True elif answer.type == const._TYPE_NSEC: - has_nsec = True + has_nsec = True # There will be one NSEC additional to indicate the lack of AAAA record assert nbr_answers == 1 and nbr_additionals == 4 assert has_srv and has_txt and has_a and has_nsec @@ -521,9 +521,9 @@ def _validate_complete_response(answers): elif answer.type == const._TYPE_A: has_a = True elif answer.type == const._TYPE_AAAA: - has_aaaa = True + has_aaaa = True elif answer.type == const._TYPE_NSEC: - has_nsec = True + has_nsec = True assert nbr_answers == 1 and nbr_additionals == 4 assert has_srv and has_txt and has_a and has_nsec assert not has_aaaa