From ba811c2e1c51319b8e241348efcbcd142fe91fd6 Mon Sep 17 00:00:00 2001 From: missytake Date: Fri, 13 Sep 2024 21:55:54 +0200 Subject: [PATCH] DNS: fix checking for required DNS records (#412) * Improve README for first setup * DNS: fix flushing DNS when requesting records * DNS: actually check whether mta-sts record is set correctly * DNS: add changelog * DNS: also check for www CNAME record * DNS: fix tests * lint: update ruff to 0.6.5 locally --- CHANGELOG.md | 3 +++ README.md | 9 +++++---- cmdeploy/src/cmdeploy/dns.py | 10 +++++++--- cmdeploy/src/cmdeploy/remote/rdns.py | 22 ++++++++++++---------- cmdeploy/src/cmdeploy/tests/test_dns.py | 24 +++++++++++++++++------- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8bb2131..24c1fd9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## untagged +- fix checking for required DNS records + ([#412](https://github.com/deltachat/chatmail/pull/412)) + - add a paragraph about "account deletion" to info page ([#405](https://github.com/deltachat/chatmail/pull/405)) diff --git a/README.md b/README.md index c837761f..8fc4a3a5 100644 --- a/README.md +++ b/README.md @@ -34,8 +34,8 @@ Please substitute it with your own domain. scripts/cmdeploy init chat.example.org # <-- use your domain ``` -3. Setup first DNS records for your chatmail domain, - according to the hints provided by `cmdeploy init`. +3. Point your domain to the server's IP address, + if you haven't done so already. Verify that SSH root login works: ``` @@ -47,7 +47,8 @@ Please substitute it with your own domain. ``` scripts/cmdeploy run ``` - This script will also show you additional DNS records + This script will check that you have all necessary DNS records. + If DNS records are missing, it will recommend which you should configure at your DNS provider (it can take some time until they are public). @@ -59,7 +60,7 @@ To check the status of your remotely running chatmail service: scripts/cmdeploy status ``` -To check whether your DNS records are correct: +To display and check all recommended DNS records: ``` scripts/cmdeploy dns diff --git a/cmdeploy/src/cmdeploy/dns.py b/cmdeploy/src/cmdeploy/dns.py index e4b95f90..c672da33 100644 --- a/cmdeploy/src/cmdeploy/dns.py +++ b/cmdeploy/src/cmdeploy/dns.py @@ -16,9 +16,12 @@ def check_initial_remote_data(remote_data, print=print): mail_domain = remote_data["mail_domain"] if not remote_data["A"] and not remote_data["AAAA"]: print(f"Missing A and/or AAAA DNS records for {mail_domain}!") - elif not remote_data["MTA_STS"]: + elif remote_data["MTA_STS"] != f"{mail_domain}.": print("Missing MTA-STS CNAME record:") - print(f"mta-sts.{mail_domain}. CNAME {mail_domain}") + print(f"mta-sts.{mail_domain}. CNAME {mail_domain}.") + elif remote_data["WWW"] != f"{mail_domain}.": + print("Missing www CNAME record:") + print(f"www.{mail_domain}. CNAME {mail_domain}.") else: return remote_data @@ -42,7 +45,8 @@ def check_full_zone(sshexec, remote_data, out, zonefile) -> int: and return (exitcode, remote_data) tuple.""" required_diff, recommended_diff = sshexec.logged( - remote.rdns.check_zonefile, kwargs=dict(zonefile=zonefile) + remote.rdns.check_zonefile, + kwargs=dict(zonefile=zonefile, mail_domain=remote_data["mail_domain"]), ) if required_diff: diff --git a/cmdeploy/src/cmdeploy/remote/rdns.py b/cmdeploy/src/cmdeploy/remote/rdns.py index ea0ec3e9..3c9bdabb 100644 --- a/cmdeploy/src/cmdeploy/remote/rdns.py +++ b/cmdeploy/src/cmdeploy/remote/rdns.py @@ -18,18 +18,19 @@ from .rshell import CalledProcessError, shell def perform_initial_checks(mail_domain): """Collecting initial DNS settings.""" assert mail_domain - A = query_dns("A", mail_domain) - AAAA = query_dns("AAAA", mail_domain) - MTA_STS = query_dns("CNAME", f"mta-sts.{mail_domain}") - - res = dict(mail_domain=mail_domain, A=A, AAAA=AAAA, MTA_STS=MTA_STS) - if not MTA_STS or (not A and not AAAA): - return res - - res["acme_account_url"] = shell("acmetool account-url", fail_ok=True) if not shell("dig", fail_ok=True): shell("apt-get install -y dnsutils") shell(f"unbound-control flush_zone {mail_domain}", fail_ok=True) + A = query_dns("A", mail_domain) + AAAA = query_dns("AAAA", mail_domain) + MTA_STS = query_dns("CNAME", f"mta-sts.{mail_domain}") + WWW = query_dns("CNAME", f"www.{mail_domain}") + + res = dict(mail_domain=mail_domain, A=A, AAAA=AAAA, MTA_STS=MTA_STS, WWW=WWW) + if not MTA_STS or not WWW or (not A and not AAAA): + return res + + res["acme_account_url"] = shell("acmetool account-url", fail_ok=True) res["dkim_entry"] = get_dkim_entry(mail_domain, dkim_selector="opendkim") # parse out sts-id if exists, example: "v=STSv1; id=2090123" @@ -59,8 +60,9 @@ def query_dns(typ, domain): return "" -def check_zonefile(zonefile): +def check_zonefile(zonefile, mail_domain): """Check expected zone file entries.""" + shell(f"unbound-control flush_zone {mail_domain}", fail_ok=True) required = True required_diff = [] recommended_diff = [] diff --git a/cmdeploy/src/cmdeploy/tests/test_dns.py b/cmdeploy/src/cmdeploy/tests/test_dns.py index 71b1baa5..fd11095f 100644 --- a/cmdeploy/src/cmdeploy/tests/test_dns.py +++ b/cmdeploy/src/cmdeploy/tests/test_dns.py @@ -24,7 +24,10 @@ def mockdns(mockdns_base): { "A": {"some.domain": "1.1.1.1"}, "AAAA": {"some.domain": "fde5:cd7a:9e1c:3240:5a99:936f:cdac:53ae"}, - "CNAME": {"mta-sts.some.domain": "some.domain"}, + "CNAME": { + "mta-sts.some.domain": "some.domain.", + "www.some.domain": "some.domain.", + }, } ) return mockdns_base @@ -33,13 +36,15 @@ def mockdns(mockdns_base): class TestPerformInitialChecks: def test_perform_initial_checks_ok1(self, mockdns): remote_data = remote.rdns.perform_initial_checks("some.domain") - assert len(remote_data) == 7 + assert remote_data["A"] == mockdns["A"]["some.domain"] + assert remote_data["AAAA"] == mockdns["AAAA"]["some.domain"] + assert remote_data["MTA_STS"] == mockdns["CNAME"]["mta-sts.some.domain"] + assert remote_data["WWW"] == mockdns["CNAME"]["www.some.domain"] @pytest.mark.parametrize("drop", ["A", "AAAA"]) def test_perform_initial_checks_with_one_of_A_AAAA(self, mockdns, drop): del mockdns[drop] remote_data = remote.rdns.perform_initial_checks("some.domain") - assert len(remote_data) == 7 assert not remote_data[drop] l = [] @@ -48,9 +53,8 @@ class TestPerformInitialChecks: assert not l def test_perform_initial_checks_no_mta_sts(self, mockdns): - del mockdns["CNAME"] + del mockdns["CNAME"]["mta-sts.some.domain"] remote_data = remote.rdns.perform_initial_checks("some.domain") - assert len(remote_data) == 4 assert not remote_data["MTA_STS"] l = [] @@ -85,14 +89,18 @@ class TestZonefileChecks: def test_check_zonefile_all_ok(self, cm_data, mockdns_base): zonefile = cm_data.get("zftest.zone") parse_zonefile_into_dict(zonefile, mockdns_base) - required_diff, recommended_diff = remote.rdns.check_zonefile(zonefile) + required_diff, recommended_diff = remote.rdns.check_zonefile( + zonefile, "some.domain" + ) assert not required_diff and not recommended_diff def test_check_zonefile_recommended_not_set(self, cm_data, mockdns_base): zonefile = cm_data.get("zftest.zone") zonefile_mocked = zonefile.split("; Recommended")[0] parse_zonefile_into_dict(zonefile_mocked, mockdns_base) - required_diff, recommended_diff = remote.rdns.check_zonefile(zonefile) + required_diff, recommended_diff = remote.rdns.check_zonefile( + zonefile, "some.domain" + ) assert not required_diff assert len(recommended_diff) == 8 @@ -101,6 +109,7 @@ class TestZonefileChecks: zonefile_mocked = zonefile.split("; Recommended")[0] parse_zonefile_into_dict(zonefile_mocked, mockdns_base, only_required=True) mssh = MockSSHExec() + mockdns_base["mail_domain"] = "some.domain" res = check_full_zone(mssh, mockdns_base, out=mockout, zonefile=zonefile) assert res == 0 assert "WARNING" in mockout.captured_plain[0] @@ -110,6 +119,7 @@ class TestZonefileChecks: zonefile = cm_data.get("zftest.zone") parse_zonefile_into_dict(zonefile, mockdns_base) mssh = MockSSHExec() + mockdns_base["mail_domain"] = "some.domain" res = check_full_zone(mssh, mockdns_base, out=mockout, zonefile=zonefile) assert res == 0 assert not mockout.captured_red