names: Store domains in kvstore instead of /etc/hosts

As reported in discussion forum[1], when clients connected via 'shared' network
connection try to resolve the a static domain name configured in FreedomBox,
they resolve to 127.0.1.1. Since this refers to client's own IP address, they
fail to connect.

In the previous version, this was not a problem because the entry was stored as
<hostname>.<domainname>. To resolve this, store domain names in kvstore instead
of /etc/hosts.

Links:

1)
https://discuss.freedombox.org/t/freedombox-resolves-its-own-external-name-as-127-0-1-1/3660

Tests:

- Adding/removing static domains from Names app works. The order of added
domains is preserved in the stored configuration. When adding a existing domain,
a proper error message is shown.

- Without the patch, configure multiple domains. They show up in /etc/hosts.
Apply the patches and restart the service. Names app setup will run. Entries
from /etc/hosts are removed and will be added to kvstore. The list of domains
shows properly in Names app. After restarting the services, domains are show
properly.

- Without the patch on a version of FreedomBox without support for multiple
static domains, configure a static domain. Switch to latest version FreedomBox
with the patches. Restart the service. Names app setup will run. Entry from
/etc/hosts will be removed and will be added to kvstore. The list of domains
shows properly in Names app. After restarting the services, domains are show
properly.

Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/names/__init__.py

@@ -3,6 +3,7 @@
 FreedomBox app to configure name services.
 """
 
+import json
 import logging
 import pathlib
 import subprocess
@@ -12,7 +13,7 @@ from django.utils.translation import gettext_lazy as _
 from django.utils.translation import gettext_noop
 
 from plinth import app as app_module
-from plinth import cfg, glib, menu, network, setup
+from plinth import cfg, glib, kvstore, menu, network, setup
 from plinth.daemon import Daemon
 from plinth.diagnostic_check import (DiagnosticCheck,
                                      DiagnosticCheckParameters, Result)
@@ -43,7 +44,7 @@ class NamesApp(app_module.App):
 
     app_id = 'names'
 
-    _version = 3
+    _version = 4
 
     can_be_disabled = False
 
@@ -85,7 +86,7 @@ class NamesApp(app_module.App):
         domain_removed.connect(on_domain_removed)
 
         # Register domain with Name Services module.
-        for domain in privileged.get_domains():
+        for domain in domains_list() + privileged.get_old_domains():
             domain_added.send_robust(sender='names',
                                      domain_type='domain-type-static',
                                      name=domain, services='__all__')
@@ -115,8 +116,10 @@ class NamesApp(app_module.App):
             except Exception:
                 pass
 
-        if old_version < 3:
-            privileged.domains_migrate()
+        if old_version < 4:
+            domains = domains_list() + privileged.get_old_domains()
+            _domains_set(domains)
+            privileged.domain_delete_all()
 
         if is_resolved_installed():
             # Fresh install or upgrading to version 2
@@ -179,6 +182,36 @@ class ResolvedDaemon(Daemon):
         return super().diagnose()
 
 
+def domains_list() -> list[str]:
+    """Return a list of domains from configuration."""
+    return json.loads(kvstore.get_default('domains', '[]'))
+
+
+def domain_add(domain_name: str):
+    """Add a domain to configuration."""
+    domains = domains_list()
+    if domain_name in domains:
+        return
+
+    domains.append(domain_name)
+    _domains_set(domains)
+
+
+def domain_delete(domain_name: str):
+    """Remove a domain from configuration."""
+    domains = domains_list()
+    if domain_name not in domains:
+        return
+
+    domains.remove(domain_name)
+    _domains_set(domains)
+
+
+def _domains_set(domains: list[str]):
+    """Set the full list of domains in the configuration."""
+    kvstore.set('domains', json.dumps(domains))
+
+
 def install_systemd_resolved(_data):
     """Re-run setup on app to install systemd-resolved."""
     if not is_resolved_installed():

plinth/modules/names/forms.py

@@ -9,10 +9,9 @@ from django.core.exceptions import ValidationError
 from django.utils.translation import gettext_lazy as _
 
 from plinth import cfg
+from plinth.modules import names
 from plinth.utils import format_lazy
 
-from . import privileged
-
 HOSTNAME_REGEX = r'^[a-zA-Z0-9]([-a-zA-Z0-9]{,61}[a-zA-Z0-9])?$'
 
 
@@ -122,7 +121,7 @@ class DomainAddForm(forms.Form):
     def clean_domain_name(self):
         """Check if the name is valid."""
         domain_name = self.cleaned_data['domain_name']
-        if domain_name in privileged.get_domains():
+        if domain_name in names.domains_list():
             raise ValidationError(_('Domain already exists.'))
 
         return domain_name

plinth/modules/names/privileged.py

@@ -39,8 +39,8 @@ def _load_augeas_hosts():
     return aug
 
 
-def get_domains(aug=None) -> list[str]:
-    """Return the list of domains."""
+def get_old_domains(aug=None) -> list[str]:
+    """Return the list of domains store in old /etc/hosts format."""
     if not aug:
         aug = _load_augeas_hosts()
 
@@ -69,55 +69,13 @@ def get_domains(aug=None) -> list[str]:
 
 
 @privileged
-def domain_add(domain_name: str | None = None):
-    """Set system's static domain name in /etc/hosts."""
+def domain_delete_all():
+    """Remove all static domain names from /etc/hosts."""
     aug = _load_augeas_hosts()
-    domains = get_domains(aug)
-    if domain_name in domains:
-        return  # Domain already present in /etc/hosts
-
-    aug.set('./01/ipaddr', HOSTS_LOCAL_IP)
-    aug.set('./01/canonical', domain_name)
-    aug.save()
-
-
-@privileged
-def domain_delete(domain_name: str | None = None):
-    """Set system's static domain name in /etc/hosts."""
-    aug = _load_augeas_hosts()
-    domains = get_domains(aug)
-    if domain_name not in domains:
-        return  # Domain already not present in /etc/hosts
-
-    for match in aug.match('*'):
-        if aug.get(match + '/ipaddr') == HOSTS_LOCAL_IP and \
-           aug.get(match + '/canonical') == domain_name:
-            aug.remove(match)
-
-    aug.save()
-
-
-@privileged
-def domains_migrate() -> None:
-    """Convert old style of adding domain names to /etc/hosts to new.
-
-    Old format:
-        127.0.1.1 <hostname>.<domain> <hostname>
-
-    New format:
-        127.0.1.1 <domain1>
-        127.0.1.1 <domain2>
-    """
-    aug = _load_augeas_hosts()
-    domains = get_domains(aug)
     for match in aug.match('*'):
         if aug.get(match + '/ipaddr') == HOSTS_LOCAL_IP:
             aug.remove(match)
 
-    for number, domain in enumerate(domains):
-        aug.set(f'./0{number}/ipaddr', HOSTS_LOCAL_IP)
-        aug.set(f'./0{number}/canonical', domain)
-
     aug.save()
 
 

plinth/modules/names/views.py

@@ -163,7 +163,7 @@ def _domain_add(domain_name: str):
     domain_name = domain_name.lower()
 
     logger.info('Adding domain name - %s', domain_name)
-    privileged.domain_add(domain_name)
+    names.domain_add(domain_name)
 
     domain_added.send_robust(sender='names', domain_type='domain-type-static',
                              name=domain_name, services='__all__')
@@ -176,7 +176,7 @@ def _domain_delete(domain_name: str):
     domain_name = domain_name.lower()
 
     logger.info('Removing domain name - %s', domain_name)
-    privileged.domain_delete(domain_name)
+    names.domain_delete(domain_name)
 
     # Update domain registered with Name Services module.
     domain_removed.send_robust(sender='names',