From fc0f4ab7a9f1863846a97a117658c1e3c7b3ce91 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Sun, 28 Feb 2016 14:18:23 +0100 Subject: [PATCH 1/8] Use the authoritative DNS server for PTR lookups --- management/status_checks.py | 41 ++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index 1feea68a..9fc7843c 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -369,8 +369,8 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # Check reverse DNS matches the PRIMARY_HOSTNAME. Note that it might not be # a DNS zone if it is a subdomain of another domain we have a zone for. - existing_rdns_v4 = query_dns(dns.reversename.from_address(env['PUBLIC_IP']), "PTR") - existing_rdns_v6 = query_dns(dns.reversename.from_address(env['PUBLIC_IPV6']), "PTR") if env.get("PUBLIC_IPV6") else None + existing_rdns_v4 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IP'])) + existing_rdns_v6 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IPV6'])) if env.get("PUBLIC_IPV6") else None if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: @@ -394,11 +394,38 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): else: output.print_error("""The DANE TLSA record for incoming mail (%s) is not correct. It is '%s' but it should be '%s'. It may take several hours for public DNS to update after a change.""" - % (tlsa_qname, tlsa25, tlsa25_expected)) + % (tlsa_qname, tlsa25, tlsa25_expected)) # Check that the hostmaster@ email address exists. check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) +def query_dns_ptr(qname): + # Find the authoritative name server for the address using the default nameservers + resolver = dns.resolver.get_default_resolver() + nameserver = resolver.nameservers[0] + query = dns.message.make_query(qname, dns.rdatatype.PTR) + timeout = 5 + response = dns.query.udp(query, nameserver, timeout) + returnCode = response.rcode() + + # Check that we were able to query the dns for the authoritative server + if returnCode != dns.rcode.NOERROR: + return "[%s]" % dns.rcode.to_text(returnCode) + + # If the current DNS server isn't the authority for this address use the one we find in the response + if len(response.authority) > 0: + rrset = response.authority[0] + else: + rrset = response.answer[0] + + rr = rrset[0] + if rr.rdtype != dns.rdatatype.SOA: + authority = rr.target + nameserver = resolver.query(authority).rrset[0].to_text() + + # Resolve the PTR record using the proper name server + return query_dns(qname, "PTR", at=nameserver) + def check_alias_exists(alias_name, alias, env, output): mail_aliases = dict([(address, receivers) for address, receivers, *_ in get_mail_aliases(env)]) if alias in mail_aliases: @@ -918,6 +945,14 @@ if __name__ == "__main__": elif sys.argv[1] == "--show-changes": run_and_output_changes(env, pool) + elif sys.argv[1] == "--check-ptr": + out = ConsoleOutput() + # Get the list of domains we serve DNS zones for (i.e. does not include subdomains). + dns_zonefiles = dict(get_dns_zones(env)) + dns_domains = set(dns_zonefiles) + check_primary_hostname_dns(env["PRIMARY_HOSTNAME"], env, out, dns_domains, dns_zonefiles) + + elif sys.argv[1] == "--check-primary-hostname": # See if the primary hostname appears resolvable and has a signed certificate. domain = env['PRIMARY_HOSTNAME'] From 14f7ef6b207a7440c991bababc6366424877039b Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Sun, 28 Feb 2016 21:40:47 +0100 Subject: [PATCH 2/8] Resuse internal dns query method to wrap errors --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 9fc7843c..fc2e876c 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -421,7 +421,7 @@ def query_dns_ptr(qname): rr = rrset[0] if rr.rdtype != dns.rdatatype.SOA: authority = rr.target - nameserver = resolver.query(authority).rrset[0].to_text() + nameserver = query_dns(authority, "A") # Resolve the PTR record using the proper name server return query_dns(qname, "PTR", at=nameserver) From cb94897e68c6d98bb53862cf2186e06a69f470e5 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Mon, 29 Feb 2016 18:43:36 +0100 Subject: [PATCH 3/8] When authoritative dns servers don't respond properly, retry 3 times --- management/status_checks.py | 69 +++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index fc2e876c..17df21d9 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -4,7 +4,7 @@ # TLS certificates have been signed, etc., and if not tells the user # what to do next. -import sys, os, os.path, re, subprocess, datetime, multiprocessing.pool +import sys, os, os.path, re, subprocess, datetime, multiprocessing.pool, time import dns.reversename, dns.resolver import dateutil.parser, dateutil.tz @@ -394,37 +394,27 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): else: output.print_error("""The DANE TLSA record for incoming mail (%s) is not correct. It is '%s' but it should be '%s'. It may take several hours for public DNS to update after a change.""" - % (tlsa_qname, tlsa25, tlsa25_expected)) + % (tlsa_qname, tlsa25, tlsa25_expected)) # Check that the hostmaster@ email address exists. check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) def query_dns_ptr(qname): - # Find the authoritative name server for the address using the default nameservers - resolver = dns.resolver.get_default_resolver() - nameserver = resolver.nameservers[0] - query = dns.message.make_query(qname, dns.rdatatype.PTR) - timeout = 5 - response = dns.query.udp(query, nameserver, timeout) - returnCode = response.rcode() + # When looking up PTR records bind will contact the authoritative servers for a response. + # Sometimes these servers don't respond properly, we will give these servers 3 chances + # with a 2 second pause in between. + for attempt in range(3): + result=query_dns(qname, "PTR") - # Check that we were able to query the dns for the authoritative server - if returnCode != dns.rcode.NOERROR: - return "[%s]" % dns.rcode.to_text(returnCode) + # Check if the authoritative servers respond properly + if result == "[nonameservers]": + time.sleep(2) + else: + # There might still be an error, like a timeout, but we will continue + # chances of recovering from those are slim. + break - # If the current DNS server isn't the authority for this address use the one we find in the response - if len(response.authority) > 0: - rrset = response.authority[0] - else: - rrset = response.answer[0] - - rr = rrset[0] - if rr.rdtype != dns.rdatatype.SOA: - authority = rr.target - nameserver = query_dns(authority, "A") - - # Resolve the PTR record using the proper name server - return query_dns(qname, "PTR", at=nameserver) + return result def check_alias_exists(alias_name, alias, env, output): mail_aliases = dict([(address, receivers) for address, receivers, *_ in get_mail_aliases(env)]) @@ -672,10 +662,12 @@ def query_dns(qname, rtype, nxdomain='[Not Set]', at=None): # Do the query. try: response = resolver.query(qname, rtype) - except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): - # Host did not have an answer for this query; not sure what the - # difference is between the two exceptions. + except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer): + # No response was received return nxdomain + except dns.resolver.NoNameservers: + # No non-broken nameservers were available to respond to the request + return "[nonameservers]" except dns.exception.Timeout: return "[timeout]" @@ -946,12 +938,21 @@ if __name__ == "__main__": run_and_output_changes(env, pool) elif sys.argv[1] == "--check-ptr": - out = ConsoleOutput() - # Get the list of domains we serve DNS zones for (i.e. does not include subdomains). - dns_zonefiles = dict(get_dns_zones(env)) - dns_domains = set(dns_zonefiles) - check_primary_hostname_dns(env["PRIMARY_HOSTNAME"], env, out, dns_domains, dns_zonefiles) - + # Run only the primary hostname checks, specifically the ptr check + shell('check_call', ["/usr/sbin/rndc", "flush"], trap=True) + output = ConsoleOutput() + domain=env["PRIMARY_HOSTNAME"] + my_ips = env['PUBLIC_IP'] + ((" / "+env['PUBLIC_IPV6']) if env.get("PUBLIC_IPV6") else "") + existing_rdns_v4 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IP'])) + existing_rdns_v6 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IPV6'])) if env.get("PUBLIC_IPV6") else None + if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): + output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) + elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: + output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" % (existing_rdns_v4, domain) ) + else: + output.print_error("""Your box's reverse DNS is currently %s (IPv4) and %s (IPv6), but it should be %s. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" % (existing_rdns_v4, existing_rdns_v6, domain) ) elif sys.argv[1] == "--check-primary-hostname": # See if the primary hostname appears resolvable and has a signed certificate. From 4fb0f7182530a475692b6f8a26e70e51db41f35e Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Mon, 29 Feb 2016 18:54:02 +0100 Subject: [PATCH 4/8] Refactor reverse dns checks so there is no duplication between the commandline option and the normal status checks --- management/status_checks.py | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index 17df21d9..323de29d 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -366,19 +366,9 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): issues listed above.""" % (my_ips, ip + ((" / " + ipv6) if ipv6 is not None else ""))) - # Check reverse DNS matches the PRIMARY_HOSTNAME. Note that it might not be # a DNS zone if it is a subdomain of another domain we have a zone for. - existing_rdns_v4 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IP'])) - existing_rdns_v6 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IPV6'])) if env.get("PUBLIC_IPV6") else None - if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): - output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) - elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: - output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, domain) ) - else: - output.print_error("""Your box's reverse DNS is currently %s (IPv4) and %s (IPv6), but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, existing_rdns_v6, domain) ) + check_reverse_dns(domain, my_ips, output, env) # Check the TLSA record. tlsa_qname = "_25._tcp." + domain @@ -399,6 +389,20 @@ def check_primary_hostname_dns(domain, env, output, dns_domains, dns_zonefiles): # Check that the hostmaster@ email address exists. check_alias_exists("Hostmaster contact address", "hostmaster@" + domain, env, output) +def check_reverse_dns(domain, my_ips, output, env): + existing_rdns_v4 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IP'])) + existing_rdns_v6 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IPV6'])) if env.get( + "PUBLIC_IPV6") else None + if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): + output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) + elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: + output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" % (existing_rdns_v4, domain)) + else: + output.print_error("""Your box's reverse DNS is currently %s (IPv4) and %s (IPv6), but it should be %s. Your ISP or cloud provider will have instructions + on setting up reverse DNS for your box.""" % (existing_rdns_v4, existing_rdns_v6, domain)) + + def query_dns_ptr(qname): # When looking up PTR records bind will contact the authoritative servers for a response. # Sometimes these servers don't respond properly, we will give these servers 3 chances @@ -943,16 +947,7 @@ if __name__ == "__main__": output = ConsoleOutput() domain=env["PRIMARY_HOSTNAME"] my_ips = env['PUBLIC_IP'] + ((" / "+env['PUBLIC_IPV6']) if env.get("PUBLIC_IPV6") else "") - existing_rdns_v4 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IP'])) - existing_rdns_v6 = query_dns_ptr(dns.reversename.from_address(env['PUBLIC_IPV6'])) if env.get("PUBLIC_IPV6") else None - if existing_rdns_v4 == domain and existing_rdns_v6 in (None, domain): - output.print_ok("Reverse DNS is set correctly at ISP. [%s ↦ %s]" % (my_ips, env['PRIMARY_HOSTNAME'])) - elif existing_rdns_v4 == existing_rdns_v6 or existing_rdns_v6 is None: - output.print_error("""Your box's reverse DNS is currently %s, but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, domain) ) - else: - output.print_error("""Your box's reverse DNS is currently %s (IPv4) and %s (IPv6), but it should be %s. Your ISP or cloud provider will have instructions - on setting up reverse DNS for your box.""" % (existing_rdns_v4, existing_rdns_v6, domain) ) + check_reverse_dns(domain, my_ips, output, env) elif sys.argv[1] == "--check-primary-hostname": # See if the primary hostname appears resolvable and has a signed certificate. From ad8cf1319f1edcaa185402c0916f1fd320069eb0 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Mon, 29 Feb 2016 18:56:05 +0100 Subject: [PATCH 5/8] Small refactoring in commandline call to check ptr --- management/status_checks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/management/status_checks.py b/management/status_checks.py index 323de29d..1702ca8c 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -945,9 +945,8 @@ if __name__ == "__main__": # Run only the primary hostname checks, specifically the ptr check shell('check_call', ["/usr/sbin/rndc", "flush"], trap=True) output = ConsoleOutput() - domain=env["PRIMARY_HOSTNAME"] my_ips = env['PUBLIC_IP'] + ((" / "+env['PUBLIC_IPV6']) if env.get("PUBLIC_IPV6") else "") - check_reverse_dns(domain, my_ips, output, env) + check_reverse_dns(env["PRIMARY_HOSTNAME"], my_ips, output, env) elif sys.argv[1] == "--check-primary-hostname": # See if the primary hostname appears resolvable and has a signed certificate. From f93fcd26dc6d20b821ad4dc5cfcc7604f2ae3166 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Mon, 29 Feb 2016 21:46:55 +0100 Subject: [PATCH 6/8] Rewrite a comment --- management/status_checks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 1702ca8c..c6355289 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -942,7 +942,7 @@ if __name__ == "__main__": run_and_output_changes(env, pool) elif sys.argv[1] == "--check-ptr": - # Run only the primary hostname checks, specifically the ptr check + # See if the reverse dns of the box is configured properly shell('check_call', ["/usr/sbin/rndc", "flush"], trap=True) output = ConsoleOutput() my_ips = env['PUBLIC_IP'] + ((" / "+env['PUBLIC_IPV6']) if env.get("PUBLIC_IPV6") else "") From 423cbe668f80e10d41c7ac4b126c9733db7761a7 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Mon, 29 Feb 2016 21:51:05 +0100 Subject: [PATCH 7/8] Display a slightly more meaningful message to the end-user --- management/status_checks.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/management/status_checks.py b/management/status_checks.py index c6355289..83d972de 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -418,6 +418,10 @@ def query_dns_ptr(qname): # chances of recovering from those are slim. break + # Display a slighty more meaningful message to the end users + if result == "[nonameservers]": + return "[bad response from authoritative nameservers]" + return result def check_alias_exists(alias_name, alias, env, output): From 0799b21dfafdbea6671d838be6da5d5e9889e681 Mon Sep 17 00:00:00 2001 From: Michael Kroes Date: Tue, 1 Mar 2016 05:29:08 +0100 Subject: [PATCH 8/8] Update comment --- management/status_checks.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/management/status_checks.py b/management/status_checks.py index 83d972de..70a016a0 100755 --- a/management/status_checks.py +++ b/management/status_checks.py @@ -406,7 +406,8 @@ def check_reverse_dns(domain, my_ips, output, env): def query_dns_ptr(qname): # When looking up PTR records bind will contact the authoritative servers for a response. # Sometimes these servers don't respond properly, we will give these servers 3 chances - # with a 2 second pause in between. + # with a 2 second pause in between. These servers don't take a long time to fail, so it + # shouldn't slow the checks down to much. for attempt in range(3): result=query_dns(qname, "PTR")