From d0666d86d6ecbf60cb078a46c8ae7ca7e5eb921d Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:47:16 +0200 Subject: [PATCH 01/19] [mod] we don't use those filter/offset/limit stuff --- src/yunohost/user.py | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index d35b082d7..3b783b3af 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -40,7 +40,7 @@ from yunohost.service import service_status logger = getActionLogger('yunohost.user') -def user_list(auth, fields=None, filter=None, limit=None, offset=None): +def user_list(auth, fields=None): """ List users @@ -59,13 +59,6 @@ def user_list(auth, fields=None, filter=None, limit=None, offset=None): attrs = ['uid'] users = {} - # Set default arguments values - if offset is None: - offset = 0 - if limit is None: - limit = 1000 - if filter is None: - filter = '(&(objectclass=person)(!(uid=root))(!(uid=nobody)))' if fields: keys = user_attrs.keys() for attr in fields: @@ -77,18 +70,18 @@ def user_list(auth, fields=None, filter=None, limit=None, offset=None): else: attrs = ['uid', 'cn', 'mail', 'mailuserquota'] - result = auth.search('ou=users,dc=yunohost,dc=org', filter, attrs) + result = auth.search('ou=users,dc=yunohost,dc=org', '(&(objectclass=person)(!(uid=root))(!(uid=nobody)))', attrs) + + for user in result: + entry = {} + for attr, values in user.items(): + try: + entry[user_attrs[attr]] = values[0] + except: + pass + uid = entry[user_attrs['uid']] + users[uid] = entry - if len(result) > offset and limit > 0: - for user in result[offset:offset + limit]: - entry = {} - for attr, values in user.items(): - try: - entry[user_attrs[attr]] = values[0] - except: - pass - uid = entry[user_attrs['uid']] - users[uid] = entry return {'users': users} From c8ce4f843a9d501f8ac67366d46f74cb40f13b8a Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:47:34 +0200 Subject: [PATCH 02/19] [mod] style --- src/yunohost/user.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 3b783b3af..d2834b7bf 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -51,11 +51,14 @@ def user_list(auth, fields=None): fields -- fields to fetch """ - user_attrs = {'uid': 'username', - 'cn': 'fullname', - 'mail': 'mail', - 'maildrop': 'mail-forward', - 'mailuserquota': 'mailbox-quota'} + user_attrs = { + 'uid': 'username', + 'cn': 'fullname', + 'mail': 'mail', + 'maildrop': 'mail-forward', + 'mailuserquota': 'mailbox-quota' + } + attrs = ['uid'] users = {} @@ -70,7 +73,9 @@ def user_list(auth, fields=None): else: attrs = ['uid', 'cn', 'mail', 'mailuserquota'] - result = auth.search('ou=users,dc=yunohost,dc=org', '(&(objectclass=person)(!(uid=root))(!(uid=nobody)))', attrs) + result = auth.search('ou=users,dc=yunohost,dc=org', + '(&(objectclass=person)(!(uid=root))(!(uid=nobody)))', + attrs) for user in result: entry = {} From ad285937f30ac98fdc59231ee8bc65f4a33dc522 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:50:35 +0200 Subject: [PATCH 03/19] [mod] to the real test --- src/yunohost/user.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index d2834b7bf..6b10af249 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -80,10 +80,9 @@ def user_list(auth, fields=None): for user in result: entry = {} for attr, values in user.items(): - try: + if values: entry[user_attrs[attr]] = values[0] - except: - pass + uid = entry[user_attrs['uid']] users[uid] = entry From d441290b2f550193215ad0640f1131e79c2d075b Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:53:13 +0200 Subject: [PATCH 04/19] [mod] lisibility --- src/yunohost/user.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 6b10af249..cb1105bfd 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -123,10 +123,10 @@ def user_create(auth, username, firstname, lastname, mail, password, raise MoulinetteError(errno.EEXIST, m18n.n('system_username_exists')) # Check that the mail domain exists - if mail[mail.find('@') + 1:] not in domain_list(auth)['domains']: + if mail.split("@")[1] not in domain_list(auth)['domains']: raise MoulinetteError(errno.EINVAL, m18n.n('mail_domain_unknown', - domain=mail[mail.find('@') + 1:])) + domain=mail.split("@")[1])) # Get random UID/GID uid_check = gid_check = 0 From 062ceff6127337decf699b15c3393b7d1587929a Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:53:46 +0200 Subject: [PATCH 05/19] [mod] style --- src/yunohost/user.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index cb1105bfd..7b422c11b 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -24,13 +24,13 @@ Manage users """ import os +import re +import json +import errno import crypt import random import string -import json -import errno import subprocess -import re from moulinette import m18n from moulinette.core import MoulinetteError From 32002d56a0477d5085455e2c77159a061d1ef540 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 00:54:33 +0200 Subject: [PATCH 06/19] [mod] inline for code lisibility --- src/yunohost/user.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 7b422c11b..bc65146c9 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -137,7 +137,6 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) - rdn = 'uid=%s,ou=users' % username char_set = string.ascii_uppercase + string.digits salt = ''.join(random.sample(char_set, 8)) salt = '$1$' + salt + '$' @@ -189,7 +188,7 @@ def user_create(auth, username, firstname, lastname, mail, password, raise MoulinetteError(errno.EPERM, m18n.n('ssowat_persistent_conf_write_error', error=e.strerror)) - if auth.add(rdn, attr_dict): + if auth.add('uid=%s,ou=users' % username, attr_dict): # Invalidate passwd to take user creation into account subprocess.call(['nscd', '-i', 'passwd']) From d14bc3177179b0573b56db4b64550f3287cd9ff3 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 10:37:05 +0200 Subject: [PATCH 07/19] [mod] directly call python instead of using os.system --- src/yunohost/user.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index bc65146c9..ee0655291 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -129,11 +129,13 @@ def user_create(auth, username, firstname, lastname, mail, password, domain=mail.split("@")[1])) # Get random UID/GID - uid_check = gid_check = 0 - while uid_check == 0 and gid_check == 0: + all_uid = {x.pw_uid for x in pwd.getpwall()} + all_gid = {x.pw_gid for x in pwd.getpwall()} + + uid_guid_found = False + while not uid_guid_found: uid = str(random.randint(200, 99999)) - uid_check = os.system("getent passwd %s" % uid) - gid_check = os.system("getent group %s" % uid) + uid_guid_found = uid not in all_uid and uid not in all_gid # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) From c14acc0fae8b7c19988dfe3352a297ec22d624e9 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 10:38:16 +0200 Subject: [PATCH 08/19] [mod] don't use exception when not needed --- src/yunohost/user.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index ee0655291..deb5de077 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -115,11 +115,8 @@ def user_create(auth, username, firstname, lastname, mail, password, }) # Validate uniqueness of username in system users - try: - pwd.getpwnam(username) - except KeyError: - pass - else: + all_existing_usernames = {x.pw_name for x in pwd.getpwall()} + if username in all_existing_usernames: raise MoulinetteError(errno.EEXIST, m18n.n('system_username_exists')) # Check that the mail domain exists From 8c6db3845dea1675f1d71f919e5c08765a1ef79b Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 15:23:51 +0200 Subject: [PATCH 09/19] [fix] use real random for hash selection --- src/yunohost/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index deb5de077..a5b659e04 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -137,7 +137,7 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) char_set = string.ascii_uppercase + string.digits - salt = ''.join(random.sample(char_set, 8)) + salt = ''.join([random.SystemRandom().choice(char_set) for x in range(8)]) salt = '$1$' + salt + '$' user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) attr_dict = { From 9d0e615bb46f1b9ab04b216776313f486908aba8 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 15:24:45 +0200 Subject: [PATCH 10/19] [enh] use the full length of available chars for salt generation --- src/yunohost/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index a5b659e04..85e890301 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -136,7 +136,7 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) - char_set = string.ascii_uppercase + string.digits + char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" salt = ''.join([random.SystemRandom().choice(char_set) for x in range(8)]) salt = '$1$' + salt + '$' user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) From 49d5c70fd64ace00ef51179afd833161d7493e77 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 15:44:06 +0200 Subject: [PATCH 11/19] [mod] add more salt because life is miserable --- src/yunohost/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 85e890301..07ae50ddd 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -137,7 +137,7 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" - salt = ''.join([random.SystemRandom().choice(char_set) for x in range(8)]) + salt = ''.join([random.SystemRandom().choice(char_set) for x in range(12)]) salt = '$1$' + salt + '$' user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) attr_dict = { From 4419ec10ce27e5ef6ecb957815211e0cf989e3b4 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 16:09:46 +0200 Subject: [PATCH 12/19] [fix] move to sh512 because it's fucking year 2017 --- src/yunohost/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 07ae50ddd..97466f7f2 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -138,7 +138,7 @@ def user_create(auth, username, firstname, lastname, mail, password, fullname = '%s %s' % (firstname, lastname) char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" salt = ''.join([random.SystemRandom().choice(char_set) for x in range(12)]) - salt = '$1$' + salt + '$' + salt = '$6$' + salt + '$' user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) attr_dict = { 'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount'], From 6c9156c3aee483facebd5473cbb8fada3149f2da Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 16:49:50 +0200 Subject: [PATCH 13/19] [enh] according to https://www.safaribooksonline.com/library/view/practical-unix-and/0596003234/ch04s03.html we can go up to 16 salt caracters --- src/yunohost/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 97466f7f2..51b7400de 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -137,7 +137,7 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" - salt = ''.join([random.SystemRandom().choice(char_set) for x in range(12)]) + salt = ''.join([random.SystemRandom().choice(char_set) for x in range(16)]) salt = '$6$' + salt + '$' user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) attr_dict = { From 685f3746ef2dfefcbab0a20d334cd85529239c05 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Mon, 14 Aug 2017 23:07:45 +0200 Subject: [PATCH 14/19] [fix] forgot to remove args from actionsmap --- data/actionsmap/yunohost.yml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/data/actionsmap/yunohost.yml b/data/actionsmap/yunohost.yml index 24b099451..df13e3bc9 100644 --- a/data/actionsmap/yunohost.yml +++ b/data/actionsmap/yunohost.yml @@ -78,17 +78,6 @@ user: --fields: help: fields to fetch nargs: "+" - -f: - full: --filter - help: LDAP filter used to search - -l: - full: --limit - help: Maximum number of user fetched - type: int - -o: - full: --offset - help: Starting number for user fetching - type: int ### user_create() create: From 147b305e8ac1fa8236ebeafd33896c1e97901b67 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Tue, 15 Aug 2017 00:34:07 +0200 Subject: [PATCH 15/19] [mod] extract password generation code to a function --- src/yunohost/user.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 51b7400de..106c113d9 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -136,10 +136,6 @@ def user_create(auth, username, firstname, lastname, mail, password, # Adapt values for LDAP fullname = '%s %s' % (firstname, lastname) - char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" - salt = ''.join([random.SystemRandom().choice(char_set) for x in range(16)]) - salt = '$6$' + salt + '$' - user_pwd = '{CRYPT}' + crypt.crypt(str(password), salt) attr_dict = { 'objectClass': ['mailAccount', 'inetOrgPerson', 'posixAccount'], 'givenName': firstname, @@ -150,7 +146,7 @@ def user_create(auth, username, firstname, lastname, mail, password, 'mail': mail, 'maildrop': username, 'mailuserquota': mailbox_quota, - 'userPassword': user_pwd, + 'userPassword': _hash_user_password(password), 'gidNumber': uid, 'uidNumber': uid, 'homeDirectory': '/home/' + username, @@ -448,3 +444,10 @@ def _convertSize(num, suffix=''): return "%3.1f%s%s" % (num, unit, suffix) num /= 1024.0 return "%.1f%s%s" % (num, 'Yi', suffix) + + +def _hash_user_password(password): + char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" + salt = ''.join([random.SystemRandom().choice(char_set) for x in range(16)]) + salt = '$6$' + salt + '$' + return '{CRYPT}' + crypt.crypt(str(password), salt) From c5a44b863883442efdf4f4d2a303a7eb7f7aa704 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Tue, 15 Aug 2017 00:34:33 +0200 Subject: [PATCH 16/19] [fix] also uses sha512 in user_update() --- src/yunohost/user.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 106c113d9..1bcb3d279 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -289,10 +289,7 @@ def user_update(auth, username, firstname=None, lastname=None, mail=None, new_attr_dict['cn'] = new_attr_dict['displayName'] = firstname + ' ' + lastname if change_password: - char_set = string.ascii_uppercase + string.digits - salt = ''.join(random.sample(char_set, 8)) - salt = '$1$' + salt + '$' - new_attr_dict['userPassword'] = '{CRYPT}' + crypt.crypt(str(change_password), salt) + new_attr_dict['userPassword'] = _hash_user_password(change_password) if mail: auth.validate_uniqueness({'mail': mail}) From 970d9b0207da923a33ca114b3fc2b10b3952a1c0 Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Tue, 15 Aug 2017 12:52:44 +0200 Subject: [PATCH 17/19] [fix] uses strong hash for admin password --- src/yunohost/tools.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/yunohost/tools.py b/src/yunohost/tools.py index 5b74883f1..70c867525 100644 --- a/src/yunohost/tools.py +++ b/src/yunohost/tools.py @@ -122,8 +122,11 @@ def tools_adminpw(auth, new_password): new_password """ + from yunohost.user import _hash_user_password try: - auth.con.passwd_s('cn=admin,dc=yunohost,dc=org', None, new_password) + auth.update("cn=admin", { + "userPassword": _hash_user_password(new_password), + }) except: logger.exception('unable to change admin password') raise MoulinetteError(errno.EPERM, From 1cd121e801e416e8c770d4f7dbe2cc7fb8e7d83e Mon Sep 17 00:00:00 2001 From: Laurent Peuch Date: Tue, 15 Aug 2017 20:53:50 +0200 Subject: [PATCH 18/19] [doc] add comment explaining choices in _hash_user_password --- src/yunohost/user.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 1bcb3d279..4e382c4d8 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -445,6 +445,16 @@ def _convertSize(num, suffix=''): def _hash_user_password(password): char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" + # This 16 number is chosen according to this documentation stating that + # this is the maximum number of salt possible + # https://www.safaribooksonline.com/library/view/practical-unix-and/0596003234/ch04s03.html + # + # SystemRandom is the cryptographically secure random method provided by python stl + # You can refer to this https://docs.python.org/2/library/random.html for + # confirmation (read the red square), it internally uses /dev/urandom salt = ''.join([random.SystemRandom().choice(char_set) for x in range(16)]) + + # Using "$6$" means that we uses sha-512 which is the strongest hash available on the system + # You can refer to this for more explainations https://www.redpill-linpro.com/techblog/2016/08/16/ldap-password-hash.html salt = '$6$' + salt + '$' return '{CRYPT}' + crypt.crypt(str(password), salt) From d010a288b27564b0c266a745a3f2b8ab3daedd2f Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Tue, 15 Aug 2017 21:06:00 +0200 Subject: [PATCH 19/19] [enh] Add a complete docstring with explanations and reference --- src/yunohost/user.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/yunohost/user.py b/src/yunohost/user.py index 4e382c4d8..11f61d807 100644 --- a/src/yunohost/user.py +++ b/src/yunohost/user.py @@ -444,17 +444,29 @@ def _convertSize(num, suffix=''): def _hash_user_password(password): + """ + This function computes and return a salted hash for the password in input. + This implementation is inspired from [1]. + + The hash follows SHA-512 scheme from Linux/glibc. + Hence the {CRYPT} and $6$ prefixes + - {CRYPT} means it relies on the OS' crypt lib + - $6$ corresponds to SHA-512, the strongest hash available on the system + + The salt is generated using random.SystemRandom(). It is the crypto-secure + pseudo-random number generator according to the python doc [2] (c.f. the + red square). It internally relies on /dev/urandom + + The salt is made of 16 characters from the set [./a-zA-Z0-9]. This is the + max sized allowed for salts according to [3] + + [1] https://www.redpill-linpro.com/techblog/2016/08/16/ldap-password-hash.html + [2] https://docs.python.org/2/library/random.html + [3] https://www.safaribooksonline.com/library/view/practical-unix-and/0596003234/ch04s03.html + """ + char_set = string.ascii_uppercase + string.ascii_lowercase + string.digits + "./" - # This 16 number is chosen according to this documentation stating that - # this is the maximum number of salt possible - # https://www.safaribooksonline.com/library/view/practical-unix-and/0596003234/ch04s03.html - # - # SystemRandom is the cryptographically secure random method provided by python stl - # You can refer to this https://docs.python.org/2/library/random.html for - # confirmation (read the red square), it internally uses /dev/urandom salt = ''.join([random.SystemRandom().choice(char_set) for x in range(16)]) - # Using "$6$" means that we uses sha-512 which is the strongest hash available on the system - # You can refer to this for more explainations https://www.redpill-linpro.com/techblog/2016/08/16/ldap-password-hash.html salt = '$6$' + salt + '$' return '{CRYPT}' + crypt.crypt(str(password), salt)