Merge pull request #1014 from YunoHost/fix-regen-conf-for-ssh

Fix regen-conf mechanism for SSH

locales/en.json

@@ -549,6 +549,7 @@
     "regenconf_dry_pending_applying": "Checking pending configuration which would have been applied for category '{category}'…",
     "regenconf_failed": "Could not regenerate the configuration for category(s): {categories}",
     "regenconf_pending_applying": "Applying pending configuration for category '{category}'…",
+    "regenconf_need_to_explicitly_specify_ssh": "The ssh configuration has been manually modified, but you need to explicitly specify category 'ssh' with --force to actually apply the changes.",
     "restore_already_installed_app": "An app with the ID '{app:s}' is already installed",
     "restore_already_installed_apps": "The following apps can't be restored because they are already installed: {apps}",
     "restore_app_failed": "Could not restore the app '{app:s}'",

src/yunohost/regenconf.py

@@ -125,11 +125,12 @@ def regen_conf(operation_logger, names=[], with_diff=False, force=False, dry_run
         # return the arguments to pass to the script
         return pre_args + [category_pending_path, ]
 
-    # Don't regen SSH if not specifically specified
+    ssh_explicitly_specified = isinstance(names, list) and "ssh" in names
+
+    # By default, we regen everything
     if not names:
         names = hook_list('conf_regen', list_by='name',
                           show_info=False)['hooks']
-        names.remove('ssh')
 
     # Dirty hack for legacy code : avoid attempting to regen the conf for
     # glances because it got removed ...  This is only needed *once*
@@ -185,6 +186,20 @@ def regen_conf(operation_logger, names=[], with_diff=False, force=False, dry_run
         succeed_regen = {}
         failed_regen = {}
 
+        # Here we are doing some weird legacy shit
+        # The thing is, on some very old or specific setup, the sshd_config file
+        # was absolutely not managed by the regenconf ...
+        # But we now want to make sure that this file is managed.
+        # However, we don't want to overwrite a specific custom sshd_config
+        # which may make the admin unhappy ...
+        # So : if the hash for this file does not exists, we set the hash as the
+        # hash of the pending configuration ...
+        # That way, the file will later appear as manually modified.
+        sshd_config = "/etc/ssh/sshd_config"
+        if category == "ssh" and sshd_config not in conf_hashes and sshd_config in conf_files:
+            conf_hashes[sshd_config] = _calculate_hash(conf_files[sshd_config])
+            _update_conf_hashes(category, conf_hashes)
+
         # Consider the following scenario:
         # - you add a domain foo.bar
         # - the regen-conf creates file /etc/dnsmasq.d/foo.bar
@@ -328,6 +343,9 @@ def regen_conf(operation_logger, names=[], with_diff=False, force=False, dry_run
                     logger.debug("> new conf is as current system conf")
                     conf_status = 'managed'
                     regenerated = True
+                elif force and system_path == sshd_config and not ssh_explicitly_specified:
+                    logger.warning(m18n.n('regenconf_need_to_explicitly_specify_ssh'))
+                    conf_status = 'modified'
                 elif force:
                     regenerated = _regen(system_path, pending_path)
                     conf_status = 'force-updated'

src/yunohost/tests/test_regenconf.py

@@ -1,5 +1,6 @@
 import os
 
+from conftest import message
 from yunohost.domain import domain_add, domain_remove, domain_list
 from yunohost.regenconf import regen_conf, manually_modified_files, _get_conf_hashes, _force_clear_hashes
 
@@ -7,6 +8,7 @@ TEST_DOMAIN = "secondarydomain.test"
 TEST_DOMAIN_NGINX_CONFIG = "/etc/nginx/conf.d/%s.conf" % TEST_DOMAIN
 TEST_DOMAIN_DNSMASQ_CONFIG = "/etc/dnsmasq.d/%s" % TEST_DOMAIN
 
+SSHD_CONFIG = "/etc/ssh/sshd_config"
 
 def setup_function(function):
 
@@ -37,6 +39,8 @@ def clean():
     assert TEST_DOMAIN_NGINX_CONFIG not in _get_conf_hashes("nginx")
     assert TEST_DOMAIN_NGINX_CONFIG not in manually_modified_files()
 
+    regen_conf(['ssh'], force=True)
+
 
 def test_add_domain():
 
@@ -74,6 +78,41 @@ def test_add_domain_conf_already_exists():
     assert TEST_DOMAIN_NGINX_CONFIG not in manually_modified_files()
 
 
+def test_ssh_conf_unmanaged():
+
+    _force_clear_hashes([SSHD_CONFIG])
+
+    assert SSHD_CONFIG not in _get_conf_hashes("ssh")
+
+    regen_conf()
+
+    assert SSHD_CONFIG in _get_conf_hashes("ssh")
+
+
+def test_ssh_conf_unmanaged_and_manually_modified(mocker):
+
+    _force_clear_hashes([SSHD_CONFIG])
+    os.system("echo ' ' >> %s" % SSHD_CONFIG)
+
+    assert SSHD_CONFIG not in _get_conf_hashes("ssh")
+
+    regen_conf()
+
+    assert SSHD_CONFIG in _get_conf_hashes("ssh")
+    assert SSHD_CONFIG in manually_modified_files()
+
+    with message(mocker, "regenconf_need_to_explicitly_specify_ssh"):
+        regen_conf(force=True)
+
+    assert SSHD_CONFIG in _get_conf_hashes("ssh")
+    assert SSHD_CONFIG in manually_modified_files()
+
+    regen_conf(['ssh'], force=True)
+
+    assert SSHD_CONFIG in _get_conf_hashes("ssh")
+    assert SSHD_CONFIG not in manually_modified_files()
+
+
 def test_stale_hashes_get_removed_if_empty():
     """
     This is intended to test that if a file gets removed and is indeed removed,
@@ -162,4 +201,4 @@ def test_stale_hashes_if_file_manually_deleted():
 #    regen_conf(names=["dnsmasq"], force=True)
 #
 #    assert not os.path.exists(TEST_DOMAIN_DNSMASQ_CONFIG)
-#    assert TEST_DOMAIN_DNSMASQ_CONFIG not in _get_conf_hashes("dnsmasq")
+#    assert TEST_DOMAIN_DNSMASQ_CONFIG not in _get_conf_hashes("dnsmasq")

src/yunohost/tools.py

@@ -369,6 +369,7 @@ def tools_postinstall(operation_logger, domain, password, ignore_dyndns=False,
     service_start("yunohost-firewall")
 
     regen_conf(force=True)
+    regen_conf(names=["ssh"], force=True)
 
     # Restore original ssh conf, as chosen by the
     # admin during the initial install
@@ -382,10 +383,6 @@ def tools_postinstall(operation_logger, domain, password, ignore_dyndns=False,
     original_sshd_conf = '/etc/ssh/sshd_config.before_yunohost'
     if os.path.exists(original_sshd_conf):
         os.rename(original_sshd_conf, '/etc/ssh/sshd_config')
-    else:
-        # We need to explicitly ask the regen conf to regen ssh
-        # (by default, i.e. first argument = None, it won't because it's too touchy)
-        regen_conf(names=["ssh"], force=True)
 
     logger.success(m18n.n('yunohost_configured'))