From 5a9d5730a75d045405dedf2250469c5561e453a6 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 12 Mar 2025 14:13:24 -0700 Subject: [PATCH] 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 .. 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 --- plinth/modules/names/__init__.py | 43 ++++++++++++++++++++++--- plinth/modules/names/forms.py | 5 ++- plinth/modules/names/privileged.py | 50 +++--------------------------- plinth/modules/names/views.py | 4 +-- 4 files changed, 46 insertions(+), 56 deletions(-) diff --git a/plinth/modules/names/__init__.py b/plinth/modules/names/__init__.py index a085d974b..2a80fa3b8 100644 --- a/plinth/modules/names/__init__.py +++ b/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(): diff --git a/plinth/modules/names/forms.py b/plinth/modules/names/forms.py index 611e6e0e5..64ede00ae 100644 --- a/plinth/modules/names/forms.py +++ b/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 diff --git a/plinth/modules/names/privileged.py b/plinth/modules/names/privileged.py index d889f4d15..37804eebc 100644 --- a/plinth/modules/names/privileged.py +++ b/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 . - - New format: - 127.0.1.1 - 127.0.1.1 - """ - 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() diff --git a/plinth/modules/names/views.py b/plinth/modules/names/views.py index bbcd9646d..51e0b9745 100644 --- a/plinth/modules/names/views.py +++ b/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',