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

@@ -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))
 

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

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:

cmdeploy/src/cmdeploy/remote/rdns.py

@@ -18,18 +18,19 @@
 def perform_initial_checks(mail_domain):
     """Collecting initial DNS settings."""
     assert mail_domain
+    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)
-    if not MTA_STS or (not A and not AAAA):
+    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)
-    if not shell("dig", fail_ok=True):
-        shell("apt-get install -y dnsutils")
-    shell(f"unbound-control flush_zone {mail_domain}", 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 = []

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 @@ def test_perform_initial_checks_with_one_of_A_AAAA(self, mockdns, drop):
         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 @@ def test_check_zonefile_output_required_fine(self, cm_data, mockdns_base, mockou
         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 @@ def test_check_zonefile_output_full(self, cm_data, mockdns_base, mockout):
         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