From 6cae524910a508dc16ca67a735e72ea20d29906a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 11 Jan 2022 14:53:04 +0100 Subject: [PATCH 01/19] Drop the 'admin' user, have 'admins' be a group of Yunohost users instead --- conf/slapd/config.ldif | 4 -- conf/slapd/db_init.ldif | 37 ++++++------- conf/slapd/mailserver.ldif | 3 ++ hooks/conf_regen/01-yunohost | 11 ++-- hooks/conf_regen/06-slapd | 24 --------- locales/en.json | 4 +- share/actionsmap.yml | 31 ++++++++--- src/authenticators/ldap_admin.py | 33 +++++++----- src/backup.py | 12 ++--- src/ssh.py | 9 ---- src/tools.py | 90 ++++++++++---------------------- src/user.py | 31 +++++++---- src/utils/config.py | 2 + 13 files changed, 125 insertions(+), 166 deletions(-) diff --git a/conf/slapd/config.ldif b/conf/slapd/config.ldif index e1fe3b1b5..249422950 100644 --- a/conf/slapd/config.ldif +++ b/conf/slapd/config.ldif @@ -130,7 +130,6 @@ olcSuffix: dc=yunohost,dc=org # admin entry below # These access lines apply to database #1 only olcAccess: {0}to attrs=userPassword,shadowLastChange - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write by anonymous auth by self write @@ -140,7 +139,6 @@ olcAccess: {0}to attrs=userPassword,shadowLastChange # owning it if they are authenticated. # Others should be able to see it. olcAccess: {1}to attrs=cn,gecos,givenName,mail,maildrop,displayName,sn - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write by self write by * read @@ -160,9 +158,7 @@ olcAccess: {2}to dn.base="" # The admin dn has full write access, everyone else # can read everything. olcAccess: {3}to * - by dn.base="cn=admin,dc=yunohost,dc=org" write by dn.base="gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth" write - by group/groupOfNames/member.exact="cn=admin,ou=groups,dc=yunohost,dc=org" write by * read # olcAddContentAcl: FALSE diff --git a/conf/slapd/db_init.ldif b/conf/slapd/db_init.ldif index be0181dfe..adea0dd89 100644 --- a/conf/slapd/db_init.ldif +++ b/conf/slapd/db_init.ldif @@ -5,15 +5,6 @@ objectClass: organization o: yunohost.org dc: yunohost -dn: cn=admin,ou=sudo,dc=yunohost,dc=org -cn: admin -objectClass: sudoRole -objectClass: top -sudoCommand: ALL -sudoUser: admin -sudoOption: !authenticate -sudoHost: ALL - dn: ou=users,dc=yunohost,dc=org objectClass: organizationalUnit objectClass: top @@ -39,28 +30,30 @@ objectClass: organizationalUnit objectClass: top ou: groups +dn: cn=admins,ou=sudo,dc=yunohost,dc=org +cn: admins +objectClass: sudoRole +objectClass: top +sudoCommand: ALL +sudoUser: %admins +sudoHost: ALL + dn: ou=sudo,dc=yunohost,dc=org objectClass: organizationalUnit objectClass: top ou: sudo -dn: cn=admin,dc=yunohost,dc=org -objectClass: organizationalRole -objectClass: posixAccount -objectClass: simpleSecurityObject -cn: admin -uid: admin -uidNumber: 1007 -gidNumber: 1007 -homeDirectory: /home/admin -loginShell: /bin/bash -userPassword: yunohost - dn: cn=admins,ou=groups,dc=yunohost,dc=org objectClass: posixGroup objectClass: top -memberUid: admin +objectClass: groupOfNamesYnh +objectClass: mailGroup gidNumber: 4001 +mail: root +mail: admin +mail: webmaster +mail: postmaster +mail: abuse cn: admins dn: cn=all_users,ou=groups,dc=yunohost,dc=org diff --git a/conf/slapd/mailserver.ldif b/conf/slapd/mailserver.ldif index 849d1d9e1..09f5c64cc 100644 --- a/conf/slapd/mailserver.ldif +++ b/conf/slapd/mailserver.ldif @@ -89,4 +89,7 @@ olcObjectClasses: ( 1.3.6.1.4.1.40328.1.1.2.3 NAME 'mailGroup' SUP top AUXILIARY DESC 'Mail Group' MUST ( mail ) + MAY ( + mailalias $ maildrop + ) ) diff --git a/hooks/conf_regen/01-yunohost b/hooks/conf_regen/01-yunohost index 1f6c143a6..9f26e1eea 100755 --- a/hooks/conf_regen/01-yunohost +++ b/hooks/conf_regen/01-yunohost @@ -42,7 +42,7 @@ do_init_regen() { # Backup folders mkdir -p /home/yunohost.backup/archives chmod 750 /home/yunohost.backup/archives - chown root:root /home/yunohost.backup/archives # This is later changed to admin:root once admin user exists + chown root:root /home/yunohost.backup/archives # This is later changed to root:admins once the admins group exists # Empty ssowat json persistent conf echo "{}" >'/etc/ssowat/conf.json.persistent' @@ -173,12 +173,11 @@ do_post_regen() { # Enfore permissions # ###################### - chmod 750 /home/admin - chmod 750 /home/yunohost.backup - chmod 750 /home/yunohost.backup/archives + chmod 770 /home/yunohost.backup + chmod 770 /home/yunohost.backup/archives chmod 700 /var/cache/yunohost - chown admin:root /home/yunohost.backup - chown admin:root /home/yunohost.backup/archives + chown root:admins /home/yunohost.backup + chown root:admins /home/yunohost.backup/archives chown root:root /var/cache/yunohost # NB: x permission for 'others' is important for ssl-cert (and maybe mdns), otherwise slapd will fail to start because can't access the certs diff --git a/hooks/conf_regen/06-slapd b/hooks/conf_regen/06-slapd index 616b383ec..bb7c075c4 100755 --- a/hooks/conf_regen/06-slapd +++ b/hooks/conf_regen/06-slapd @@ -58,14 +58,6 @@ EOF nscd -i passwd || true systemctl restart slapd - - # We don't use mkhomedir_helper because 'admin' may not be recognized - # when this script is ran in a chroot (e.g. ISO install) - # We also refer to admin as uid 1007 for the same reason - if [ ! -d /home/admin ]; then - cp -r /etc/skel /home/admin - chown -R 1007:1007 /home/admin - fi } _regenerate_slapd_conf() { @@ -172,22 +164,6 @@ objectClass: top" echo "Reloading slapd" systemctl force-reload slapd - - # on slow hardware/vm this regen conf would exit before the admin user that - # is stored in ldap is available because ldap seems to slow to restart - # so we'll wait either until we are able to log as admin or until a timeout - # is reached - # we need to do this because the next hooks executed after this one during - # postinstall requires to run as admin thus breaking postinstall on slow - # hardware which mean yunohost can't be correctly installed on those hardware - # and this sucks - # wait a maximum time of 5 minutes - # yes, force-reload behave like a restart - number_of_wait=0 - while ! su admin -c '' && ((number_of_wait < 60)); do - sleep 5 - ((number_of_wait += 1)) - done } do_$1_regen ${@:2} diff --git a/locales/en.json b/locales/en.json index 91db42cb5..e1089684e 100644 --- a/locales/en.json +++ b/locales/en.json @@ -6,7 +6,6 @@ "admin_password": "Administration password", "admin_password_change_failed": "Unable to change password", "admin_password_changed": "The administration password was changed", - "admin_password_too_long": "Please choose a password shorter than 127 characters", "already_up_to_date": "Nothing to do. Everything is already up-to-date.", "app_action_broke_system": "This action seems to have broken these important services: {services}", "app_action_cannot_be_ran_because_required_services_down": "These required services should be running to run this action: {services}. Try restarting them to continue (and possibly investigate why they are down).", @@ -534,6 +533,7 @@ "password_too_simple_2": "The password needs to be at least 8 characters long and contain a digit, upper and lower characters", "password_too_simple_3": "The password needs to be at least 8 characters long and contain a digit, upper, lower and special characters", "password_too_simple_4": "The password needs to be at least 12 characters long and contain a digit, upper, lower and special characters", + "password_too_long": "Please choose a password shorter than 127 characters", "pattern_backup_archive_name": "Must be a valid filename with max 30 characters, alphanumeric and -_. characters only", "pattern_domain": "Must be a valid domain name (e.g. my-domain.org)", "pattern_email": "Must be a valid e-mail address, without '+' symbol (e.g. someone@example.com)", @@ -685,5 +685,5 @@ "yunohost_configured": "YunoHost is now configured", "yunohost_installing": "Installing YunoHost...", "yunohost_not_installed": "YunoHost is not correctly installed. Please run 'yunohost tools postinstall'", - "yunohost_postinstall_end_tip": "The post-install completed! To finalize your setup, please consider:\n - adding a first user through the 'Users' section of the webadmin (or 'yunohost user create ' in command-line);\n - diagnose potential issues through the 'Diagnosis' section of the webadmin (or 'yunohost diagnosis run' in command-line);\n - reading the 'Finalizing your setup' and 'Getting to know YunoHost' parts in the admin documentation: https://yunohost.org/admindoc." + "yunohost_postinstall_end_tip": "The post-install completed! To finalize your setup, please consider:\n - diagnose potential issues through the 'Diagnosis' section of the webadmin (or 'yunohost diagnosis run' in command-line);\n - reading the 'Finalizing your setup' and 'Getting to know YunoHost' parts in the admin documentation: https://yunohost.org/admindoc." } diff --git a/share/actionsmap.yml b/share/actionsmap.yml index cad0212b2..8214c8d7e 100644 --- a/share/actionsmap.yml +++ b/share/actionsmap.yml @@ -1438,10 +1438,10 @@ tools: category_help: Specific tools actions: - ### tools_adminpw() - adminpw: - action_help: Change password of admin and root users - api: PUT /adminpw + ### tools_rootpw() + rootpw: + action_help: Change root password + api: PUT /rootpw arguments: -n: full: --new-password @@ -1476,6 +1476,25 @@ tools: ask: ask_main_domain pattern: *pattern_domain required: True + -u: + full: --username + help: Username for the first (admin) user + extra: + ask: ask_username + pattern: *pattern_username + required: True + -f: + full: --firstname + extra: + ask: ask_firstname + required: True + pattern: *pattern_firstname + -l: + full: --lastname + extra: + ask: ask_lastname + required: True + pattern: *pattern_lastname -p: full: --password help: YunoHost admin password @@ -1487,14 +1506,10 @@ tools: --ignore-dyndns: help: Do not subscribe domain to a DynDNS service action: store_true - --force-password: - help: Use this if you really want to set a weak password - action: store_true --force-diskspace: help: Use this if you really want to install YunoHost on a setup with less than 10 GB on the root filesystem action: store_true - ### tools_update() update: action_help: YunoHost update diff --git a/src/authenticators/ldap_admin.py b/src/authenticators/ldap_admin.py index 94d68a8db..55359379d 100644 --- a/src/authenticators/ldap_admin.py +++ b/src/authenticators/ldap_admin.py @@ -9,35 +9,45 @@ import time from moulinette import m18n from moulinette.authentication import BaseAuthenticator from yunohost.utils.error import YunohostError +from yunohost.utils.ldap import _get_ldap_interface + logger = logging.getLogger("yunohost.authenticators.ldap_admin") +LDAP_URI = "ldap://localhost:389" +ADMIN_GROUP = "cn=admins,ou=groups,dc=yunohost,dc=org" +AUTH_DN = "uid={uid},ou=users,dc=yunohost,dc=org" + class Authenticator(BaseAuthenticator): name = "ldap_admin" def __init__(self, *args, **kwargs): - self.uri = "ldap://localhost:389" - self.basedn = "dc=yunohost,dc=org" - self.admindn = "cn=admin,dc=yunohost,dc=org" + pass def _authenticate_credentials(self, credentials=None): - # TODO : change authentication format - # to support another dn to support multi-admins + admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0]["memberUid"] + + uid, password = credentials.split(":", 1) + + if uid not in admins: + raise YunohostError("invalid_credentials") + + dn = AUTH_DN.format(uid=uid) def _reconnect(): con = ldap.ldapobject.ReconnectLDAPObject( - self.uri, retry_max=10, retry_delay=0.5 + LDAP_URI, retry_max=10, retry_delay=0.5 ) - con.simple_bind_s(self.admindn, credentials) + con.simple_bind_s(dn, password) return con try: con = _reconnect() except ldap.INVALID_CREDENTIALS: - raise YunohostError("invalid_password") + raise YunohostError("invalid_credentials") except ldap.SERVER_DOWN: # ldap is down, attempt to restart it before really failing logger.warning(m18n.n("ldap_server_is_down_restart_it")) @@ -57,11 +67,8 @@ class Authenticator(BaseAuthenticator): logger.warning("Error during ldap authentication process: %s", e) raise else: - if who != self.admindn: - raise YunohostError( - f"Not logged with the appropriate identity ? Found {who}, expected {self.admindn} !?", - raw_msg=True, - ) + if who != dn: + raise YunohostError(f"Not logged with the appropriate identity ? Found {who}, expected {dn} !?", raw_msg=True) finally: # Free the connection, we don't really need it to keep it open as the point is only to check authentication... if con: diff --git a/src/backup.py b/src/backup.py index 3dc2a31f5..1c454b7aa 100644 --- a/src/backup.py +++ b/src/backup.py @@ -342,7 +342,7 @@ class BackupManager: # FIXME replace isdir by exists ? manage better the case where the path # exists if not os.path.isdir(self.work_dir): - filesystem.mkdir(self.work_dir, 0o750, parents=True, uid="admin") + filesystem.mkdir(self.work_dir, 0o750, parents=True) elif self.is_tmp_work_dir: logger.debug( @@ -358,7 +358,7 @@ class BackupManager: # we're in /home/yunohost.backup/tmp so that should be okay... # c.f. method clean() which also does this) filesystem.rm(self.work_dir, recursive=True, force=True) - filesystem.mkdir(self.work_dir, 0o750, parents=True, uid="admin") + filesystem.mkdir(self.work_dir, 0o750, parents=True) # # Backup target management # @@ -1886,7 +1886,7 @@ class CopyBackupMethod(BackupMethod): dest_parent = os.path.dirname(dest) if not os.path.exists(dest_parent): - filesystem.mkdir(dest_parent, 0o700, True, uid="admin") + filesystem.mkdir(dest_parent, 0o700, True) if os.path.isdir(source): shutil.copytree(source, dest) @@ -1948,7 +1948,7 @@ class TarBackupMethod(BackupMethod): """ if not os.path.exists(self.repo): - filesystem.mkdir(self.repo, 0o750, parents=True, uid="admin") + filesystem.mkdir(self.repo, 0o750, parents=True) # Check free space in output self._check_is_enough_free_space() @@ -2632,9 +2632,9 @@ def _create_archive_dir(): if os.path.lexists(ARCHIVES_PATH): raise YunohostError("backup_output_symlink_dir_broken", path=ARCHIVES_PATH) - # Create the archive folder, with 'admin' as owner, such that + # Create the archive folder, with 'admins' as groupowner, such that # people can scp archives out of the server - mkdir(ARCHIVES_PATH, mode=0o750, parents=True, uid="admin", gid="root") + mkdir(ARCHIVES_PATH, mode=0o770, parents=True, gid="admins") def _call_for_each_path(self, callback, csv_path=None): diff --git a/src/ssh.py b/src/ssh.py index ecee39f4a..582fc39a1 100644 --- a/src/ssh.py +++ b/src/ssh.py @@ -158,15 +158,6 @@ def _get_user_for_ssh(username, attrs=None): "home_path": root_unix.pw_dir, } - if username == "admin": - admin_unix = pwd.getpwnam("admin") - return { - "username": "admin", - "fullname": "", - "mail": "", - "home_path": admin_unix.pw_dir, - } - # TODO escape input using https://www.python-ldap.org/doc/html/ldap-filter.html from yunohost.utils.ldap import _get_ldap_interface diff --git a/src/tools.py b/src/tools.py index 1a80d020f..ef811a3bf 100644 --- a/src/tools.py +++ b/src/tools.py @@ -19,10 +19,6 @@ """ -""" yunohost_tools.py - - Specific tools -""" import re import os import subprocess @@ -67,63 +63,40 @@ def tools_versions(): return ynh_packages_version() -def tools_adminpw(new_password, check_strength=True): - """ - Change admin password +def tools_rootpw(new_password): - Keyword argument: - new_password - - """ from yunohost.user import _hash_user_password from yunohost.utils.password import assert_password_is_strong_enough import spwd - if check_strength: - assert_password_is_strong_enough("admin", new_password) + assert_password_is_strong_enough("admin", new_password) + + new_hash = _hash_user_password(new_password) # UNIX seems to not like password longer than 127 chars ... # e.g. SSH login gets broken (or even 'su admin' when entering the password) if len(new_password) >= 127: - raise YunohostValidationError("admin_password_too_long") - - new_hash = _hash_user_password(new_password) - - from yunohost.utils.ldap import _get_ldap_interface - - ldap = _get_ldap_interface() + raise YunohostValidationError("password_too_long") + # Write as root password try: - ldap.update( - "cn=admin", - {"userPassword": [new_hash]}, - ) - except Exception as e: - logger.error("unable to change admin password : %s" % e) - raise YunohostError("admin_password_change_failed") - else: - # Write as root password - try: - hash_root = spwd.getspnam("root").sp_pwd + hash_root = spwd.getspnam("root").sp_pwd - with open("/etc/shadow", "r") as before_file: - before = before_file.read() + with open("/etc/shadow", "r") as before_file: + before = before_file.read() - with open("/etc/shadow", "w") as after_file: - after_file.write( - before.replace( - "root:" + hash_root, "root:" + new_hash.replace("{CRYPT}", "") - ) + with open("/etc/shadow", "w") as after_file: + after_file.write( + before.replace( + "root:" + hash_root, "root:" + new_hash.replace("{CRYPT}", "") ) - # An IOError may be thrown if for some reason we can't read/write /etc/passwd - # A KeyError could also be thrown if 'root' is not in /etc/passwd in the first place (for example because no password defined ?) - # (c.f. the line about getspnam) - except (IOError, KeyError): - logger.warning(m18n.n("root_password_desynchronized")) - return - - logger.info(m18n.n("root_password_replaced_by_admin_password")) - logger.success(m18n.n("admin_password_changed")) + ) + # An IOError may be thrown if for some reason we can't read/write /etc/passwd + # A KeyError could also be thrown if 'root' is not in /etc/passwd in the first place (for example because no password defined ?) + # (c.f. the line about getspnam) + except (IOError, KeyError): + logger.warning(m18n.n("root_password_desynchronized")) + return def tools_maindomain(new_main_domain=None): @@ -189,25 +162,18 @@ def _detect_virt(): def tools_postinstall( operation_logger, domain, + username, + firstname, + lastname, password, ignore_dyndns=False, - force_password=False, force_diskspace=False, ): - """ - YunoHost post-install - Keyword argument: - domain -- YunoHost main domain - ignore_dyndns -- Do not subscribe domain to a DynDNS service (only - needed for nohost.me, noho.st domains) - password -- YunoHost admin password - - """ from yunohost.dyndns import _dyndns_available from yunohost.utils.dns import is_yunohost_dyndns_domain - from yunohost.utils.password import assert_password_is_strong_enough from yunohost.domain import domain_main_domain + from yunohost.user import user_create import psutil # Do some checks at first @@ -230,10 +196,6 @@ def tools_postinstall( if not force_diskspace and main_space < 10 * GB: raise YunohostValidationError("postinstall_low_rootfsspace") - # Check password - if not force_password: - assert_password_is_strong_enough("admin", password) - # If this is a nohost.me/noho.st, actually check for availability if not ignore_dyndns and is_yunohost_dyndns_domain(domain): # Check if the domain is available... @@ -268,8 +230,10 @@ def tools_postinstall( domain_add(domain, dyndns) domain_main_domain(domain) + user_create(username, firstname, lastname, domain, password, admin=True) + # Update LDAP admin and create home dir - tools_adminpw(password, check_strength=not force_password) + tools_rootpw(password) # Enable UPnP silently and reload firewall firewall_upnp("enable", no_refresh=True) diff --git a/src/user.py b/src/user.py index be9b74641..fe2695a1e 100644 --- a/src/user.py +++ b/src/user.py @@ -55,7 +55,7 @@ FIELDS_FOR_IMPORT = { "groups": r"^|([a-z0-9_]+(,?[a-z0-9_]+)*)$", } -FIRST_ALIASES = ["root@", "admin@", "webmaster@", "postmaster@", "abuse@"] +ADMIN_ALIASES = ["root@", "admin@", "webmaster@", "postmaster@", "abuse@"] def user_list(fields=None): @@ -138,6 +138,7 @@ def user_create( domain, password, mailbox_quota="0", + admin=False, from_import=False, ): @@ -146,8 +147,13 @@ def user_create( from yunohost.utils.password import assert_password_is_strong_enough from yunohost.utils.ldap import _get_ldap_interface + # UNIX seems to not like password longer than 127 chars ... + # e.g. SSH login gets broken (or even 'su admin' when entering the password) + if len(password) >= 127: + raise YunohostValidationError("password_too_long") + # Ensure sufficiently complex password - assert_password_is_strong_enough("user", password) + assert_password_is_strong_enough("admin" if admin else "user", password) # Validate domain used for email address/xmpp account if domain is None: @@ -189,9 +195,10 @@ def user_create( raise YunohostValidationError("system_username_exists") main_domain = _get_maindomain() - aliases = [alias + main_domain for alias in FIRST_ALIASES] + # FIXME: should forbit root@any.domain, not just main domain? + admin_aliases = [alias + main_domain for alias in ADMIN_ALIASES] - if mail in aliases: + if mail in admin_aliases: raise YunohostValidationError("mail_unavailable") if not from_import: @@ -232,10 +239,6 @@ def user_create( "loginShell": ["/bin/bash"], } - # If it is the first user, add some aliases - if not ldap.search(base="ou=users,dc=yunohost,dc=org", filter="uid=*"): - attr_dict["mail"] = [attr_dict["mail"]] + aliases - try: ldap.add("uid=%s,ou=users" % username, attr_dict) except Exception as e: @@ -263,6 +266,8 @@ def user_create( # Create group for user and add to group 'all_users' user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False) user_group_update(groupname="all_users", add=username, force=True, sync_perm=True) + if admin: + user_group_update(groupname="admins", add=username, sync_perm=True) # Trigger post_user_create hooks env_dict = { @@ -416,6 +421,12 @@ def user_update( change_password = Moulinette.prompt( m18n.n("ask_password"), is_password=True, confirm=True ) + + # UNIX seems to not like password longer than 127 chars ... + # e.g. SSH login gets broken (or even 'su admin' when entering the password) + if len(change_password) >= 127: + raise YunohostValidationError("password_too_long") + # Ensure sufficiently complex password assert_password_is_strong_enough("user", change_password) @@ -424,7 +435,6 @@ def user_update( if mail: main_domain = _get_maindomain() - aliases = [alias + main_domain for alias in FIRST_ALIASES] # If the requested mail address is already as main address or as an alias by this user if mail in user["mail"]: @@ -439,6 +449,9 @@ def user_update( raise YunohostError( "mail_domain_unknown", domain=mail[mail.find("@") + 1 :] ) + + # FIXME: should also forbid root@any.domain and not just the main domain + aliases = [alias + main_domain for alias in ADMIN_ALIASES] if mail in aliases: raise YunohostValidationError("mail_unavailable") diff --git a/src/utils/config.py b/src/utils/config.py index 99a002404..be8c76e15 100644 --- a/src/utils/config.py +++ b/src/utils/config.py @@ -1144,6 +1144,8 @@ class UserQuestion(Question): ) if self.default is None: + # FIXME: this code is obsolete with the new admins group + # Should be replaced by something like "any first user we find in the admin group" root_mail = "root@%s" % _get_maindomain() for user in self.choices: if root_mail in user_info(user).get("mail-aliases", []): From 767b5c3d7ecb141d5ce3e80fea3227443e71d4cd Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 11 Jan 2022 15:09:01 +0100 Subject: [PATCH 02/19] mail: Add ldap-groups virtual aliases --- conf/postfix/main.cf | 2 +- conf/postfix/plain/ldap-groups.cf | 9 +++++++++ conf/slapd/db_init.ldif | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 conf/postfix/plain/ldap-groups.cf diff --git a/conf/postfix/main.cf b/conf/postfix/main.cf index 51e35c85c..6a4b9f0b4 100644 --- a/conf/postfix/main.cf +++ b/conf/postfix/main.cf @@ -99,7 +99,7 @@ message_size_limit = 35914708 virtual_mailbox_domains = ldap:/etc/postfix/ldap-domains.cf virtual_mailbox_maps = ldap:/etc/postfix/ldap-accounts.cf virtual_mailbox_base = -virtual_alias_maps = ldap:/etc/postfix/ldap-aliases.cf +virtual_alias_maps = ldap:/etc/postfix/ldap-aliases.cf,ldap:/etc/postfix/ldap-groups.cf virtual_alias_domains = virtual_minimum_uid = 100 virtual_uid_maps = static:vmail diff --git a/conf/postfix/plain/ldap-groups.cf b/conf/postfix/plain/ldap-groups.cf new file mode 100644 index 000000000..dbf768641 --- /dev/null +++ b/conf/postfix/plain/ldap-groups.cf @@ -0,0 +1,9 @@ +server_host = localhost +server_port = 389 +search_base = dc=yunohost,dc=org +query_filter = (&(objectClass=groupOfNamesYnh)(mail=%s)) +exclude_internal = yes +search_timeout = 30 +scope = sub +result_attribute = memberUid, mail +terminal_result_attribute = memberUid diff --git a/conf/slapd/db_init.ldif b/conf/slapd/db_init.ldif index adea0dd89..95b9dd936 100644 --- a/conf/slapd/db_init.ldif +++ b/conf/slapd/db_init.ldif @@ -51,6 +51,7 @@ objectClass: mailGroup gidNumber: 4001 mail: root mail: admin +mail: admins mail: webmaster mail: postmaster mail: abuse From 66cd35d30446c44cfdf502bfaf709c6722c5d63b Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 11 Jan 2022 15:24:26 +0100 Subject: [PATCH 03/19] ci: Propagate postinstall cli changes to install.gitlab-ci.yml --- .gitlab/ci/install.gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/ci/install.gitlab-ci.yml b/.gitlab/ci/install.gitlab-ci.yml index e2662e9e2..89360c8f8 100644 --- a/.gitlab/ci/install.gitlab-ci.yml +++ b/.gitlab/ci/install.gitlab-ci.yml @@ -26,4 +26,4 @@ install-postinstall: script: - apt-get update -o Acquire::Retries=3 - DEBIAN_FRONTEND=noninteractive SUDO_FORCE_REMOVE=yes apt --assume-yes -o Dpkg::Options::="--force-confold" --allow-downgrades install ./$YNH_BUILD_DIR/*.deb - - yunohost tools postinstall -d domain.tld -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l Mine -p the_password --ignore-dyndns --force-diskspace From 9126beffc252abf74c2ecd715c7d80a2213e9b59 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 11 Jan 2022 17:11:45 +0100 Subject: [PATCH 04/19] Prevent deletion of the new admins group --- src/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/user.py b/src/user.py index 7fda78a25..4ba3f4081 100644 --- a/src/user.py +++ b/src/user.py @@ -1081,7 +1081,7 @@ def user_group_delete(operation_logger, groupname, force=False, sync_perm=True): # # We also can't delete "all_users" because that's a special group... existing_users = list(user_list()["users"].keys()) - undeletable_groups = existing_users + ["all_users", "visitors"] + undeletable_groups = existing_users + ["all_users", "visitors", "admins"] if groupname in undeletable_groups and not force: raise YunohostValidationError("group_cannot_be_deleted", group=groupname) From 4cb8c914758967f0c3e57c109138f9f7c5e20cbf Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 11 Jan 2022 18:15:34 +0100 Subject: [PATCH 05/19] Draft migration for new admins group --- src/migrations/0024_new_admins_group.py | 86 +++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/migrations/0024_new_admins_group.py diff --git a/src/migrations/0024_new_admins_group.py b/src/migrations/0024_new_admins_group.py new file mode 100644 index 000000000..ca9b45d07 --- /dev/null +++ b/src/migrations/0024_new_admins_group.py @@ -0,0 +1,86 @@ +import os +from moulinette.utils.log import getActionLogger + +from yunohost.utils.error import YunohostError +from yunohost.tools import Migration + +logger = getActionLogger("yunohost.migration") + +################################################### +# Tools used also for restoration +################################################### + + +class MyMigration(Migration): + """ + Add new permissions around SSH/SFTP features + """ + + introduced_in_version = "11.1" # FIXME? + dependencies = [] + + @Migration.ldap_migration + def run(self, *args): + + from yunohost.user import user_list, user_info, user_group_update + from yunohost.utils.ldap import _get_ldap_interface + + ldap = _get_ldap_interface() + + all_users = user_list()["users"].keys() + new_admin_user = None + for user in all_users: + if any(alias.startswith("root@") for alias in user_info(user).get("mail-aliases", [])): + new_admin_user = user + break + + if not new_admin_user: + new_admin_user = os.environ.get("YNH_NEW_ADMIN_USER") + if new_admin_user: + assert new_admin_user in all_users, f"{new_admin_user} is not an existing yunohost user" + else: + raise YunohostError( + # FIXME: i18n + """The very first user created on this Yunohost instance could not be found, and therefore this migration can not be ran. You should re-run this migration as soon as possible from the command line with, after choosing which user should become the admin: + +export YNH_NEW_ADMIN_USER=some_existing_username +yunohost tools migrations run""", + raw_msg=True + ) + + stuff_to_delete = [ + "cn=admin,ou=sudo", + "cn=admins,ou=sudo" + "cn=admin", + "cn=admins,ou=groups", + ] + + for stuff in stuff_to_delete: + if ldap.search(stuff): + ldap.remove(stuff) + + ldap.add( + "cn=admins,ou=sudo", + { + "cn": ["admins"], + "objectClass": ["top", "sudoRole"], + "sudoCommand": ["ALL"], + "sudoUser": ["%admins"], + "sudoHost": ["ALL"], + } + ) + + ldap.add( + "cn=admins,ou=groups", + { + "cn": ["admins"], + "objectClass": ["top", "posixGroup", "groupOfNamesYnh", "mailGroup"], + "gidNumber": [4001], + "mail": ["root", "admin", "admins", "webmaster", "postmaster", "abuse"], + } + ) + + user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) + + def run_after_system_restore(self): + self.run() From 899342057d58cc2a0a557de67a6e3e82041de133 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 9 Aug 2022 18:33:38 +0200 Subject: [PATCH 06/19] Rename admin group migration from 24 to 26 (25 gonna be global settings) --- .../{0024_new_admins_group.py => 0026_new_admins_group.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/migrations/{0024_new_admins_group.py => 0026_new_admins_group.py} (100%) diff --git a/src/migrations/0024_new_admins_group.py b/src/migrations/0026_new_admins_group.py similarity index 100% rename from src/migrations/0024_new_admins_group.py rename to src/migrations/0026_new_admins_group.py From 888f1d8e55407bc0a7c8c7a02fc39b16e492caac Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 4 Sep 2022 20:33:41 +0200 Subject: [PATCH 07/19] admins: fix class decorator syntax? --- src/tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools.py b/src/tools.py index 080f3a074..045f3c0e4 100644 --- a/src/tools.py +++ b/src/tools.py @@ -937,7 +937,7 @@ class Migration: def description(self): return m18n.n(f"migration_description_{self.id}") - def ldap_migration(self, run): + def ldap_migration(run): def func(self): # Backup LDAP before the migration From 3758611d13ee77bcc13def4673b18dc204225c77 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 5 Sep 2022 16:40:07 +0200 Subject: [PATCH 08/19] admins: bunch of fixes --- src/migrations/0026_new_admins_group.py | 7 +++-- src/tests/test_ldapauth.py | 6 ++-- src/tests/test_user-group.py | 2 -- src/tools.py | 40 ++++++++++++++----------- 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index ca9b45d07..5601c8bf7 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -19,6 +19,8 @@ class MyMigration(Migration): introduced_in_version = "11.1" # FIXME? dependencies = [] + ldap_migration_started = False + @Migration.ldap_migration def run(self, *args): @@ -48,9 +50,10 @@ yunohost tools migrations run""", raw_msg=True ) + self.ldap_migration_started = True + stuff_to_delete = [ "cn=admin,ou=sudo", - "cn=admins,ou=sudo" "cn=admin", "cn=admins,ou=groups", ] @@ -75,7 +78,7 @@ yunohost tools migrations run""", { "cn": ["admins"], "objectClass": ["top", "posixGroup", "groupOfNamesYnh", "mailGroup"], - "gidNumber": [4001], + "gidNumber": ["4001"], "mail": ["root", "admin", "admins", "webmaster", "postmaster", "abuse"], } ) diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index a95dea443..7a09fff40 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -2,7 +2,7 @@ import pytest import os from yunohost.authenticators.ldap_admin import Authenticator as LDAPAuth -from yunohost.tools import tools_adminpw +from yunohost.tools import tools_rootpw from moulinette import m18n from moulinette.core import MoulinetteError @@ -13,7 +13,7 @@ def setup_function(function): if os.system("systemctl is-active slapd") != 0: os.system("systemctl start slapd && sleep 3") - tools_adminpw("yunohost", check_strength=False) + tools_rootpw("yunohost", check_strength=False) def test_authenticate(): @@ -47,7 +47,7 @@ def test_authenticate_change_password(): LDAPAuth().authenticate_credentials(credentials="yunohost") - tools_adminpw("plopette", check_strength=False) + tools_rootpw("plopette", check_strength=False) with pytest.raises(MoulinetteError) as exception: LDAPAuth().authenticate_credentials(credentials="yunohost") diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index e561118e0..8ef732d61 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -11,7 +11,6 @@ from yunohost.user import ( user_import, user_export, FIELDS_FOR_IMPORT, - FIRST_ALIASES, user_group_list, user_group_create, user_group_delete, @@ -175,7 +174,6 @@ def test_import_user(mocker): def test_export_user(mocker): result = user_export() - aliases = ",".join([alias + maindomain for alias in FIRST_ALIASES]) should_be = ( "username;firstname;lastname;password;mail;mail-alias;mail-forward;mailbox-quota;groups\r\n" f"alice;Alice;White;;alice@{maindomain};{aliases};;0;dev\r\n" diff --git a/src/tools.py b/src/tools.py index 045f3c0e4..e21dd585d 100644 --- a/src/tools.py +++ b/src/tools.py @@ -30,7 +30,7 @@ from typing import List from moulinette import Moulinette, m18n from moulinette.utils.log import getActionLogger from moulinette.utils.process import call_async_output -from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm +from moulinette.utils.filesystem import read_yaml, write_to_yaml, cp, mkdir, rm, chown from yunohost.app import app_upgrade, app_list from yunohost.app_catalog import ( @@ -965,22 +965,28 @@ class Migration: try: run(self, backup_folder) except Exception: - logger.warning( - m18n.n("migration_ldap_migration_failed_trying_to_rollback") - ) - os.system("systemctl stop slapd") - # To be sure that we don't keep some part of the old config - rm("/etc/ldap/slapd.d", force=True, recursive=True) - cp(f"{backup_folder}/ldap_config", "/etc/ldap", recursive=True) - cp(f"{backup_folder}/ldap_db", "/var/lib/ldap", recursive=True) - cp( - f"{backup_folder}/apps_settings", - "/etc/yunohost/apps", - recursive=True, - ) - os.system("systemctl start slapd") - rm(backup_folder, force=True, recursive=True) - logger.info(m18n.n("migration_ldap_rollback_success")) + if self.ldap_migration_started: + logger.warning( + m18n.n("migration_ldap_migration_failed_trying_to_rollback") + ) + os.system("systemctl stop slapd") + # To be sure that we don't keep some part of the old config + rm("/etc/ldap", force=True, recursive=True) + cp(f"{backup_folder}/ldap_config", "/etc/ldap", recursive=True) + chown("/etc/ldap/schema/", "openldap", "openldap", recursive=True) + chown("/etc/ldap/slapd.d/", "openldap", "openldap", recursive=True) + rm("/var/lib/ldap", force=True, recursive=True) + cp(f"{backup_folder}/ldap_db", "/var/lib/ldap", recursive=True) + rm("/etc/yunohost/apps", force=True, recursive=True) + chown("/var/lib/ldap/", "openldap", recursive=True) + cp( + f"{backup_folder}/apps_settings", + "/etc/yunohost/apps", + recursive=True, + ) + os.system("systemctl start slapd") + rm(backup_folder, force=True, recursive=True) + logger.info(m18n.n("migration_ldap_rollback_success")) raise else: rm(backup_folder, force=True, recursive=True) From 1d98604e8826cf256dd05fb878fdab92f8a65b3c Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 5 Sep 2022 17:39:08 +0200 Subject: [PATCH 09/19] admins: moar fixes --- .gitlab/ci/test.gitlab-ci.yml | 2 +- src/migrations/0026_new_admins_group.py | 5 +++++ src/tests/test_user-group.py | 5 +++-- src/tools.py | 5 +++-- src/user.py | 5 +++-- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.gitlab/ci/test.gitlab-ci.yml b/.gitlab/ci/test.gitlab-ci.yml index 519ae427a..d7ccbc807 100644 --- a/.gitlab/ci/test.gitlab-ci.yml +++ b/.gitlab/ci/test.gitlab-ci.yml @@ -34,7 +34,7 @@ full-tests: PYTEST_ADDOPTS: "--color=yes" before_script: - *install_debs - - yunohost tools postinstall -d domain.tld -p the_password --ignore-dyndns --force-diskspace + - yunohost tools postinstall -d domain.tld -u syssa -f Syssa -l Mine -p the_password --ignore-dyndns --force-diskspace script: - python3 -m pytest --cov=yunohost tests/ src/tests/ src/diagnosers/ --junitxml=report.xml - cd tests diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index 5601c8bf7..227a30730 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -52,6 +52,11 @@ yunohost tools migrations run""", self.ldap_migration_started = True + aliases = user_info(new_admin_user).get("mail-aliases", []) + old_admin_aliases_to_remove = [alias for alias in aliases if any(alias.startswith(a) for a in ["root@", "admin@", "admins@", "webmaster@", "postmaster@", "abuse@"])] + + user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) + stuff_to_delete = [ "cn=admin,ou=sudo", "cn=admin", diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index 8ef732d61..30bb89162 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -38,7 +38,7 @@ def setup_function(function): global maindomain maindomain = _get_maindomain() - user_create("alice", "Alice", "White", maindomain, "test123Ynh") + user_create("alice", "Alice", "White", maindomain, "test123Ynh", admin=True) user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") user_create("jack", "Jack", "Black", maindomain, "test123Ynh") @@ -79,6 +79,7 @@ def test_list_groups(): assert "alice" in res assert "bob" in res assert "jack" in res + assert "alice" in res["admins"]["members"] for u in ["alice", "bob", "jack"]: assert u in res assert u in res[u]["members"] @@ -176,7 +177,7 @@ def test_export_user(mocker): result = user_export() should_be = ( "username;firstname;lastname;password;mail;mail-alias;mail-forward;mailbox-quota;groups\r\n" - f"alice;Alice;White;;alice@{maindomain};{aliases};;0;dev\r\n" + f"alice;Alice;White;;alice@{maindomain};;;0;admins,dev\r\n" f"bob;Bob;Snow;;bob@{maindomain};;;0;apps\r\n" f"jack;Jack;Black;;jack@{maindomain};;;0;" ) diff --git a/src/tools.py b/src/tools.py index e21dd585d..ccc2b4a32 100644 --- a/src/tools.py +++ b/src/tools.py @@ -60,7 +60,7 @@ def tools_versions(): return ynh_packages_version() -def tools_rootpw(new_password): +def tools_rootpw(new_password, check_strength=True): from yunohost.user import _hash_user_password from yunohost.utils.password import ( @@ -70,7 +70,8 @@ def tools_rootpw(new_password): import spwd assert_password_is_compatible(new_password) - assert_password_is_strong_enough("admin", new_password) + if check_strength: + assert_password_is_strong_enough("admin", new_password) new_hash = _hash_user_password(new_password) diff --git a/src/user.py b/src/user.py index 3fabc78c5..3b980e89e 100644 --- a/src/user.py +++ b/src/user.py @@ -381,7 +381,7 @@ def user_update( # Populate user informations ldap = _get_ldap_interface() - attrs_to_fetch = ["givenName", "sn", "mail", "maildrop"] + attrs_to_fetch = ["givenName", "sn", "mail", "maildrop", "memberOf"] result = ldap.search( base="ou=users", filter="uid=" + username, @@ -425,7 +425,8 @@ def user_update( # Ensure compatibility and sufficiently complex password assert_password_is_compatible(change_password) - assert_password_is_strong_enough("user", change_password) # FIXME FIXME FIXME : gotta use admin profile if user is admin + is_admin = "cn=admins,ou=groups,dc=yunohost,dc=org" in result["memberOf"] + assert_password_is_strong_enough("admin" if is_admin else "user", change_password) new_attr_dict["userPassword"] = [_hash_user_password(change_password)] env_dict["YNH_USER_PASSWORD"] = change_password From 98bd15ebf288fbe89eb77ec38873408be930bb94 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 5 Sep 2022 18:37:22 +0200 Subject: [PATCH 10/19] admins: moaaar fixes, moaaar --- src/migrations/0026_new_admins_group.py | 16 ++++++++++------ src/tests/test_user-group.py | 2 +- src/user.py | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index 227a30730..c1ba5b638 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -24,7 +24,7 @@ class MyMigration(Migration): @Migration.ldap_migration def run(self, *args): - from yunohost.user import user_list, user_info, user_group_update + from yunohost.user import user_list, user_info, user_group_update, user_update from yunohost.utils.ldap import _get_ldap_interface ldap = _get_ldap_interface() @@ -36,7 +36,9 @@ class MyMigration(Migration): new_admin_user = user break - if not new_admin_user: + # NB: we handle the edge-case where no user exist at all + # which is useful for the CI etc. + if all_users and not new_admin_user: new_admin_user = os.environ.get("YNH_NEW_ADMIN_USER") if new_admin_user: assert new_admin_user in all_users, f"{new_admin_user} is not an existing yunohost user" @@ -52,10 +54,11 @@ yunohost tools migrations run""", self.ldap_migration_started = True - aliases = user_info(new_admin_user).get("mail-aliases", []) - old_admin_aliases_to_remove = [alias for alias in aliases if any(alias.startswith(a) for a in ["root@", "admin@", "admins@", "webmaster@", "postmaster@", "abuse@"])] + if new_admin_user: + aliases = user_info(new_admin_user).get("mail-aliases", []) + old_admin_aliases_to_remove = [alias for alias in aliases if any(alias.startswith(a) for a in ["root@", "admin@", "admins@", "webmaster@", "postmaster@", "abuse@"])] - user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) + user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) stuff_to_delete = [ "cn=admin,ou=sudo", @@ -88,7 +91,8 @@ yunohost tools migrations run""", } ) - user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) + if new_admin_user: + user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) def run_after_system_restore(self): self.run() diff --git a/src/tests/test_user-group.py b/src/tests/test_user-group.py index 30bb89162..1a368ceac 100644 --- a/src/tests/test_user-group.py +++ b/src/tests/test_user-group.py @@ -28,7 +28,7 @@ def clean_user_groups(): user_delete(u, purge=True) for g in user_group_list()["groups"]: - if g not in ["all_users", "visitors"]: + if g not in ["all_users", "visitors", "admins"]: user_group_delete(g) diff --git a/src/user.py b/src/user.py index 3b980e89e..1f6cbc5c8 100644 --- a/src/user.py +++ b/src/user.py @@ -425,7 +425,7 @@ def user_update( # Ensure compatibility and sufficiently complex password assert_password_is_compatible(change_password) - is_admin = "cn=admins,ou=groups,dc=yunohost,dc=org" in result["memberOf"] + is_admin = "cn=admins,ou=groups,dc=yunohost,dc=org" in user["memberOf"] assert_password_is_strong_enough("admin" if is_admin else "user", change_password) new_attr_dict["userPassword"] = [_hash_user_password(change_password)] From fc14f64821bece00a25d7ac2e09c43e05e42757d Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 6 Sep 2022 00:35:10 +0200 Subject: [PATCH 11/19] admins: moar friskies? --- locales/en.json | 1 + src/authenticators/ldap_admin.py | 24 +++++++++++--- src/tests/test_ldapauth.py | 55 ++++++++++++++++++++++++++------ src/utils/ldap.py | 2 ++ 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/locales/en.json b/locales/en.json index c7f3d0085..74b62408e 100644 --- a/locales/en.json +++ b/locales/en.json @@ -429,6 +429,7 @@ "invalid_number_min": "Must be greater than {min}", "invalid_password": "Invalid password", "invalid_regex": "Invalid regex:'{regex}'", + "invalid_credentials": "Invalid password or username", "ip6tables_unavailable": "You cannot play with ip6tables here. You are either in a container or your kernel does not support it", "iptables_unavailable": "You cannot play with iptables here. You are either in a container or your kernel does not support it", "ldap_attribute_already_exists": "LDAP attribute '{attribute}' already exists with value '{value}'", diff --git a/src/authenticators/ldap_admin.py b/src/authenticators/ldap_admin.py index 31b2a7363..704816460 100644 --- a/src/authenticators/ldap_admin.py +++ b/src/authenticators/ldap_admin.py @@ -17,7 +17,7 @@ session_secret = random_ascii() logger = logging.getLogger("yunohost.authenticators.ldap_admin") LDAP_URI = "ldap://localhost:389" -ADMIN_GROUP = "cn=admins,ou=groups,dc=yunohost,dc=org" +ADMIN_GROUP = "cn=admins,ou=groups" AUTH_DN = "uid={uid},ou=users,dc=yunohost,dc=org" class Authenticator(BaseAuthenticator): @@ -29,11 +29,27 @@ class Authenticator(BaseAuthenticator): def _authenticate_credentials(self, credentials=None): - admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0]["memberUid"] + try: + admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", []) + except ldap.SERVER_DOWN: + # ldap is down, attempt to restart it before really failing + logger.warning(m18n.n("ldap_server_is_down_restart_it")) + os.system("systemctl restart slapd") + time.sleep(10) # waits 10 secondes so we are sure that slapd has restarted - uid, password = credentials.split(":", 1) + try: + admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", []) + except ldap.SERVER_DOWN: + raise YunohostError("ldap_server_down") - if uid not in admins: + try: + uid, password = credentials.split(":", 1) + except ValueError: + raise YunohostError("invalid_credentials") + + # Here we're explicitly using set() which are handled as hash tables + # and should prevent timing attacks to find out the admin usernames? + if uid not in set(admins): raise YunohostError("invalid_credentials") dn = AUTH_DN.format(uid=uid) diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index 7a09fff40..5e741fe0f 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -3,6 +3,8 @@ import os from yunohost.authenticators.ldap_admin import Authenticator as LDAPAuth from yunohost.tools import tools_rootpw +from yunohost.user import user_create, user_list, user_update, user_delete +from yunohost.domain import _get_maindomain from moulinette import m18n from moulinette.core import MoulinetteError @@ -10,19 +12,52 @@ from moulinette.core import MoulinetteError def setup_function(function): + for u in user_list()["users"]: + user_delete(u, purge=True) + + maindomain = _get_maindomain() + if os.system("systemctl is-active slapd") != 0: os.system("systemctl start slapd && sleep 3") - tools_rootpw("yunohost", check_strength=False) + user_create("alice", "Alice", "White", maindomain, "Yunohost", admin=True) + user_create("bob", "Bob", "Snow", maindomain, "test123Ynh") + + +def teardown_function(): + + os.system("systemctl is-active slapd || systemctl start slapd && sleep 5") + + for u in user_list()["users"]: + user_delete(u, purge=True) def test_authenticate(): - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") + + +def test_authenticate_with_no_user(): + + with pytest.raises(MoulinetteError): + LDAPAuth().authenticate_credentials(credentials="Yunohost") + + with pytest.raises(MoulinetteError): + LDAPAuth().authenticate_credentials(credentials=":Yunohost") + + +def test_authenticate_with_user_who_is_not_admin(): + + with pytest.raises(MoulinetteError) as exception: + LDAPAuth().authenticate_credentials(credentials="bob:test123Ynh") + + translation = m18n.n("invalid_password") + expected_msg = translation.format() + assert expected_msg in str(exception) def test_authenticate_with_wrong_password(): with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="bad_password_lul") + LDAPAuth().authenticate_credentials(credentials="alice:bad_password_lul") translation = m18n.n("invalid_password") expected_msg = translation.format() @@ -30,17 +65,15 @@ def test_authenticate_with_wrong_password(): def test_authenticate_server_down(mocker): - os.system("systemctl stop slapd && sleep 3") + os.system("systemctl stop slapd && sleep 5") # Now if slapd is down, moulinette tries to restart it mocker.patch("os.system") mocker.patch("time.sleep") with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") - translation = m18n.n("ldap_server_down") - expected_msg = translation.format() - assert expected_msg in str(exception) + assert "Unable to reach LDAP server" in str(exception) def test_authenticate_change_password(): @@ -50,10 +83,12 @@ def test_authenticate_change_password(): tools_rootpw("plopette", check_strength=False) with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") translation = m18n.n("invalid_password") expected_msg = translation.format() assert expected_msg in str(exception) - LDAPAuth().authenticate_credentials(credentials="plopette") + user_update("alice", password="plopette") + + LDAPAuth().authenticate_credentials(credentials="alice:plopette") diff --git a/src/utils/ldap.py b/src/utils/ldap.py index 98c0fecf7..28ff8eebe 100644 --- a/src/utils/ldap.py +++ b/src/utils/ldap.py @@ -145,6 +145,8 @@ class LDAPInterface: try: result = self.con.search_s(base, ldap.SCOPE_SUBTREE, filter, attrs) + except ldap.SERVER_DOWN as e: + raise e except Exception as e: raise MoulinetteError( "error during LDAP search operation with: base='%s', " From 2199d60732296d34d4cc1dfb4af0fabfb5863f84 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 30 Sep 2022 19:22:29 +0200 Subject: [PATCH 12/19] admins: change migration strategy, recreate 'admin' as a legacy regular user that will be encouraged to manually get rid of --- src/migrations/0026_new_admins_group.py | 34 ++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index c1ba5b638..ada92d2a2 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -1,4 +1,5 @@ import os +import subprocess from moulinette.utils.log import getActionLogger from yunohost.utils.error import YunohostError @@ -24,7 +25,7 @@ class MyMigration(Migration): @Migration.ldap_migration def run(self, *args): - from yunohost.user import user_list, user_info, user_group_update, user_update + from yunohost.user import user_list, user_info, user_group_update, user_update, user_create from yunohost.utils.ldap import _get_ldap_interface ldap = _get_ldap_interface() @@ -60,6 +61,8 @@ yunohost tools migrations run""", user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) + admin_hashs = ldap.search("cn=admin", "", {"userPassword"})[0]["userPassword"] + stuff_to_delete = [ "cn=admin,ou=sudo", "cn=admin", @@ -94,5 +97,34 @@ yunohost tools migrations run""", if new_admin_user: user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) + # Re-add admin as a regular user + attr_dict = { + "objectClass": [ + "mailAccount", + "inetOrgPerson", + "posixAccount", + "userPermissionYnh", + ], + "givenName": ["Admin"], + "sn": ["Admin"], + "displayName": ["Admin"], + "cn": ["Admin"], + "uid": ["admin"], + "mail": "", + "maildrop": ["admin"], + "mailuserquota": ["0"], + "userPassword": admin_hashs, + "gidNumber": ["1007"], + "uidNumber": ["1007"], + "homeDirectory": ["/home/admin"], + "loginShell": ["/bin/bash"], + } + ldap.add("uid=admin,ou=users", attr_dict) + user_group_update(groupname="admins", add="admin", sync_perm=True) + + subprocess.call(["nscd", "-i", "passwd"]) + subprocess.call(["nscd", "-i", "group"]) + + def run_after_system_restore(self): self.run() From 87447def383ae6742c8b0525d2829f98099039af Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 30 Sep 2022 19:31:04 +0200 Subject: [PATCH 13/19] admins/ldapauth: attempt to fix tests, root auth gets broken during slapd restart test --- src/tests/test_ldapauth.py | 2 +- src/utils/ldap.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index 5e741fe0f..0ec0346da 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -78,7 +78,7 @@ def test_authenticate_server_down(mocker): def test_authenticate_change_password(): - LDAPAuth().authenticate_credentials(credentials="yunohost") + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") tools_rootpw("plopette", check_strength=False) diff --git a/src/utils/ldap.py b/src/utils/ldap.py index 28ff8eebe..627ab4e7a 100644 --- a/src/utils/ldap.py +++ b/src/utils/ldap.py @@ -84,7 +84,7 @@ class LDAPInterface: def connect(self): def _reconnect(): con = ldap.ldapobject.ReconnectLDAPObject( - self.uri, retry_max=10, retry_delay=0.5 + self.uri, retry_max=10, retry_delay=2 ) con.sasl_non_interactive_bind_s("EXTERNAL") return con From acb0993bc95aee78ebfae56de62d1c9b3ac9c761 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Fri, 30 Sep 2022 19:35:15 +0200 Subject: [PATCH 14/19] Unhappy linter gods are unhappy --- src/migrations/0026_new_admins_group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index ada92d2a2..23bbf213d 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -25,7 +25,7 @@ class MyMigration(Migration): @Migration.ldap_migration def run(self, *args): - from yunohost.user import user_list, user_info, user_group_update, user_update, user_create + from yunohost.user import user_list, user_info, user_group_update, user_update from yunohost.utils.ldap import _get_ldap_interface ldap = _get_ldap_interface() From 18f64ce80b2c20f5248158546103b2a06de62ac8 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 3 Oct 2022 17:03:13 +0200 Subject: [PATCH 15/19] Moar friskies --- src/migrations/0026_new_admins_group.py | 7 +++---- src/user.py | 5 +++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index 23bbf213d..afe299cfe 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -1,5 +1,4 @@ import os -import subprocess from moulinette.utils.log import getActionLogger from yunohost.utils.error import YunohostError @@ -27,6 +26,7 @@ class MyMigration(Migration): from yunohost.user import user_list, user_info, user_group_update, user_update from yunohost.utils.ldap import _get_ldap_interface + from yunohost.permission import permission_sync_to_user ldap = _get_ldap_interface() @@ -94,6 +94,8 @@ yunohost tools migrations run""", } ) + permission_sync_to_user() + if new_admin_user: user_group_update(groupname="admins", add=new_admin_user, sync_perm=True) @@ -122,9 +124,6 @@ yunohost tools migrations run""", ldap.add("uid=admin,ou=users", attr_dict) user_group_update(groupname="admins", add="admin", sync_perm=True) - subprocess.call(["nscd", "-i", "passwd"]) - subprocess.call(["nscd", "-i", "group"]) - def run_after_system_restore(self): self.run() diff --git a/src/user.py b/src/user.py index 1f6cbc5c8..efffbdc7e 100644 --- a/src/user.py +++ b/src/user.py @@ -208,6 +208,11 @@ def user_create( all_uid = {str(x.pw_uid) for x in pwd.getpwall()} all_gid = {str(x.gr_gid) for x in grp.getgrall()} + # Prevent users from obtaining uid 1007 which is the uid of the legacy admin, + # and there could be a edge case where a new user becomes owner of an old, removed admin user + all_uid.add("1007") + all_gid.add("1007") + uid_guid_found = False while not uid_guid_found: # LXC uid number is limited to 65536 by default From 6512bbf70cc76b711df910acc5d47e13208dd6ed Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Mon, 3 Oct 2022 18:03:00 +0200 Subject: [PATCH 16/19] Moaaar friskies --- src/migrations/0026_new_admins_group.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index afe299cfe..87ea26907 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -61,7 +61,7 @@ yunohost tools migrations run""", user_update(new_admin_user, remove_mailalias=old_admin_aliases_to_remove) - admin_hashs = ldap.search("cn=admin", "", {"userPassword"})[0]["userPassword"] + admin_hashs = ldap.search("cn=admin", attrs={"userPassword"})[0]["userPassword"] stuff_to_delete = [ "cn=admin,ou=sudo", @@ -112,7 +112,7 @@ yunohost tools migrations run""", "displayName": ["Admin"], "cn": ["Admin"], "uid": ["admin"], - "mail": "", + "mail": "admin_legacy", "maildrop": ["admin"], "mailuserquota": ["0"], "userPassword": admin_hashs, From ae73e94c3e839e634e14def965c77265fde1a57a Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 4 Oct 2022 02:00:40 +0200 Subject: [PATCH 17/19] Friskies pl0x? --- src/authenticators/ldap_admin.py | 4 ++++ src/tests/test_ldapauth.py | 17 ++++------------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/authenticators/ldap_admin.py b/src/authenticators/ldap_admin.py index 704816460..a7fc18da6 100644 --- a/src/authenticators/ldap_admin.py +++ b/src/authenticators/ldap_admin.py @@ -37,6 +37,10 @@ class Authenticator(BaseAuthenticator): os.system("systemctl restart slapd") time.sleep(10) # waits 10 secondes so we are sure that slapd has restarted + # Force-reset existing LDAP interface + from yunohost.utils import ldap as ldaputils + ldaputils._ldap_interface = None + try: admins = _get_ldap_interface().search(ADMIN_GROUP, attrs=["memberUid"])[0].get("memberUid", []) except ldap.SERVER_DOWN: diff --git a/src/tests/test_ldapauth.py b/src/tests/test_ldapauth.py index 0ec0346da..db5229342 100644 --- a/src/tests/test_ldapauth.py +++ b/src/tests/test_ldapauth.py @@ -2,7 +2,6 @@ import pytest import os from yunohost.authenticators.ldap_admin import Authenticator as LDAPAuth -from yunohost.tools import tools_rootpw from yunohost.user import user_create, user_list, user_update, user_delete from yunohost.domain import _get_maindomain @@ -17,7 +16,7 @@ def setup_function(function): maindomain = _get_maindomain() - if os.system("systemctl is-active slapd") != 0: + if os.system("systemctl is-active slapd >/dev/null") != 0: os.system("systemctl start slapd && sleep 3") user_create("alice", "Alice", "White", maindomain, "Yunohost", admin=True) @@ -26,7 +25,7 @@ def setup_function(function): def teardown_function(): - os.system("systemctl is-active slapd || systemctl start slapd && sleep 5") + os.system("systemctl is-active slapd >/dev/null || systemctl start slapd; sleep 5") for u in user_list()["users"]: user_delete(u, purge=True) @@ -67,20 +66,14 @@ def test_authenticate_with_wrong_password(): def test_authenticate_server_down(mocker): os.system("systemctl stop slapd && sleep 5") - # Now if slapd is down, moulinette tries to restart it - mocker.patch("os.system") - mocker.patch("time.sleep") - with pytest.raises(MoulinetteError) as exception: - LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") - - assert "Unable to reach LDAP server" in str(exception) + LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") def test_authenticate_change_password(): LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") - tools_rootpw("plopette", check_strength=False) + user_update("alice", change_password="plopette") with pytest.raises(MoulinetteError) as exception: LDAPAuth().authenticate_credentials(credentials="alice:Yunohost") @@ -89,6 +82,4 @@ def test_authenticate_change_password(): expected_msg = translation.format() assert expected_msg in str(exception) - user_update("alice", password="plopette") - LDAPAuth().authenticate_credentials(credentials="alice:plopette") From d7067c0b2264a01a5910bc0d161016a2efb8aa07 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 4 Oct 2022 12:38:33 +0200 Subject: [PATCH 18/19] Friskies m0ar --- src/migrations/0026_new_admins_group.py | 16 ---------------- src/tests/test_permission.py | 2 +- src/user.py | 2 +- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index 87ea26907..e44f1d716 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -37,22 +37,6 @@ class MyMigration(Migration): new_admin_user = user break - # NB: we handle the edge-case where no user exist at all - # which is useful for the CI etc. - if all_users and not new_admin_user: - new_admin_user = os.environ.get("YNH_NEW_ADMIN_USER") - if new_admin_user: - assert new_admin_user in all_users, f"{new_admin_user} is not an existing yunohost user" - else: - raise YunohostError( - # FIXME: i18n - """The very first user created on this Yunohost instance could not be found, and therefore this migration can not be ran. You should re-run this migration as soon as possible from the command line with, after choosing which user should become the admin: - -export YNH_NEW_ADMIN_USER=some_existing_username -yunohost tools migrations run""", - raw_msg=True - ) - self.ldap_migration_started = True if new_admin_user: diff --git a/src/tests/test_permission.py b/src/tests/test_permission.py index f2bff5507..379f1cf39 100644 --- a/src/tests/test_permission.py +++ b/src/tests/test_permission.py @@ -109,7 +109,7 @@ def clean_user_groups_permission(): user_delete(u) for g in user_group_list()["groups"]: - if g not in ["all_users", "visitors"]: + if g not in ["all_users", "visitors", "admins"]: user_group_delete(g) for p in user_permission_list()["permissions"]: diff --git a/src/user.py b/src/user.py index efffbdc7e..e00fa3685 100644 --- a/src/user.py +++ b/src/user.py @@ -56,7 +56,7 @@ FIELDS_FOR_IMPORT = { "groups": r"^|([a-z0-9_]+(,?[a-z0-9_]+)*)$", } -ADMIN_ALIASES = ["root@", "admin@", "webmaster@", "postmaster@", "abuse@"] +ADMIN_ALIASES = ["root@", "admin@", "admins", "webmaster@", "postmaster@", "abuse@"] def user_list(fields=None): From 35ab8a7c987d3a5511b48ec933aaf2fe7eae6d87 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 4 Oct 2022 16:05:45 +0200 Subject: [PATCH 19/19] Unused imports --- src/migrations/0026_new_admins_group.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/migrations/0026_new_admins_group.py b/src/migrations/0026_new_admins_group.py index e44f1d716..3fa9a2325 100644 --- a/src/migrations/0026_new_admins_group.py +++ b/src/migrations/0026_new_admins_group.py @@ -1,7 +1,5 @@ -import os from moulinette.utils.log import getActionLogger -from yunohost.utils.error import YunohostError from yunohost.tools import Migration logger = getActionLogger("yunohost.migration")