URL: https://github.com/freeipa/freeipa/pull/317 Author: stlaz Title: #317: Unify password generation across FreeIPA Action: synchronized
To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/317/head:pr317 git checkout pr317
From 5398133a228d57e10d94268b73faad24ababe777 Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka <slazn...@redhat.com> Date: Tue, 6 Dec 2016 09:05:42 +0100 Subject: [PATCH 1/2] Unify password generation across FreeIPA Also had to recalculate entropy of the passwords as originally, probability of generating each character was 1/256, however the default probability of each character in the ipa_generate_password is 1/95 (1/94 for first and last character). https://fedorahosted.org/freeipa/ticket/5695 --- ipaserver/install/certs.py | 8 ++------ ipaserver/install/dogtaginstance.py | 3 +-- ipaserver/install/dsinstance.py | 5 +---- ipaserver/install/httpinstance.py | 5 ++--- ipaserver/install/server/replicainstall.py | 3 +-- ipaserver/secrets/store.py | 2 +- 6 files changed, 8 insertions(+), 18 deletions(-) diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py index 02b03d4..414a716 100644 --- a/ipaserver/install/certs.py +++ b/ipaserver/install/certs.py @@ -25,7 +25,6 @@ import xml.dom.minidom import pwd import base64 -from hashlib import sha1 import fcntl import time import datetime @@ -159,9 +158,6 @@ def set_perms(self, fname, write=False, uid=None): perms |= stat.S_IWUSR os.chmod(fname, perms) - def gen_password(self): - return sha1(ipautil.ipa_generate_password()).hexdigest() - def run_certutil(self, args, stdin=None, **kwargs): return self.nssdb.run_certutil(args, stdin, **kwargs) @@ -177,7 +173,7 @@ def create_noise_file(self): if ipautil.file_exists(self.noise_fname): os.remove(self.noise_fname) f = open(self.noise_fname, "w") - f.write(self.gen_password()) + f.write(ipautil.ipa_generate_password(pwd_len=25)) self.set_perms(self.noise_fname) def create_passwd_file(self, passwd=None): @@ -186,7 +182,7 @@ def create_passwd_file(self, passwd=None): if passwd is not None: f.write("%s\n" % passwd) else: - f.write(self.gen_password()) + f.write(ipautil.ipa_generate_password(pwd_len=25)) f.close() self.set_perms(self.passwd_fname) diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py index f4856c7..dc4b5b0 100644 --- a/ipaserver/install/dogtaginstance.py +++ b/ipaserver/install/dogtaginstance.py @@ -18,7 +18,6 @@ # import base64 -import binascii import ldap import os import shutil @@ -428,7 +427,7 @@ def __add_admin_to_group(self, group): def setup_admin(self): self.admin_user = "admin-%s" % self.fqdn - self.admin_password = binascii.hexlify(os.urandom(16)) + self.admin_password = ipautil.ipa_generate_password(pwd_len=20) self.admin_dn = DN(('uid', self.admin_user), ('ou', 'people'), ('o', 'ipaca')) diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index a0fdc4a..89315b6 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -508,7 +508,7 @@ def __setup_sub_dict(self): idrange_size = None self.sub_dict = dict(FQDN=self.fqdn, SERVERID=self.serverid, PASSWORD=self.dm_password, - RANDOM_PASSWORD=self.generate_random(), + RANDOM_PASSWORD=ipautil.ipa_generate_password(), SUFFIX=self.suffix, REALM=self.realm, USER=DS_USER, SERVER_ROOT=server_root, DOMAIN=self.domain, @@ -775,9 +775,6 @@ def __host_nis_groups(self): def __add_enrollment_module(self): self._ldap_mod("enrollment-conf.ldif", self.sub_dict) - def generate_random(self): - return ipautil.ipa_generate_password() - def __enable_ssl(self): dirname = config_dirname(self.serverid) dsdb = certs.CertDB(self.realm, nssdir=dirname, subject_base=self.subject_base) diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py index b7ce857..e8c706e 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -19,7 +19,6 @@ from __future__ import print_function -import binascii import os import os.path import pwd @@ -314,9 +313,9 @@ def create_cert_db(self): ipautil.backup_file(nss_path) # Create the password file for this db - hex_str = binascii.hexlify(os.urandom(10)) + password = ipautil.ipa_generate_password(pwd_len=15) f = os.open(pwd_file, os.O_CREAT | os.O_RDWR) - os.write(f, hex_str) + os.write(f, password) os.close(f) ipautil.run([paths.CERTUTIL, "-d", database, "-f", pwd_file, "-N"]) diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 6e986f7..2126169 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -45,7 +45,6 @@ ReplicationManager, replica_conn_check) import SSSDConfig from subprocess import CalledProcessError -from binascii import hexlify if six.PY3: unicode = str @@ -1303,7 +1302,7 @@ def install(installer): if conn.isconnected(): conn.disconnect() os.environ['KRB5CCNAME'] = ccache - config.dirman_password = hexlify(ipautil.ipa_generate_password()) + config.dirman_password = ipautil.ipa_generate_password() # FIXME: allow to use passed in certs instead if ca_enabled: diff --git a/ipaserver/secrets/store.py b/ipaserver/secrets/store.py index 1df7191..1c369d8 100644 --- a/ipaserver/secrets/store.py +++ b/ipaserver/secrets/store.py @@ -122,7 +122,7 @@ def export_key(self): with open(nsspwfile, 'w+') as f: f.write(self.nssdb_password) pk12pwfile = os.path.join(tdir, 'pk12pwfile') - password = b64encode(os.urandom(16)) + password = ipautil.ipa_generate_password(pwd_len=20) with open(pk12pwfile, 'w+') as f: f.write(password) pk12file = os.path.join(tdir, 'pk12file') From f7bd75a1b35262880b9803b6db00ca2d7ef3c53b Mon Sep 17 00:00:00 2001 From: Petr Spacek <pspa...@redhat.com> Date: Wed, 21 Dec 2016 15:07:34 +0100 Subject: [PATCH 2/2] ipa_generate_password algorithm change A change to the algorithm that generates random passwords for multiple purposes throught IPA. This spells out the need to assess password strength by the entropy it contains rather than its length. This new password generation should also be compatible with the NSS implementation of password requirements in FIPS environment so that newly created databases won't fail with wrong authentication. https://fedorahosted.org/freeipa/ticket/5695 --- ipaclient/install/client.py | 2 +- ipapython/ipautil.py | 116 +++++++++++++++++++++++--------- ipaserver/install/certs.py | 4 +- ipaserver/install/dnskeysyncinstance.py | 7 +- ipaserver/install/dogtaginstance.py | 2 +- ipaserver/install/httpinstance.py | 2 +- ipaserver/plugins/baseuser.py | 8 +-- ipaserver/plugins/host.py | 12 ++-- ipaserver/plugins/stageuser.py | 5 +- ipaserver/plugins/user.py | 5 +- ipaserver/secrets/store.py | 2 +- 11 files changed, 106 insertions(+), 59 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index 60a5c18..2ff6122 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -2296,7 +2296,7 @@ def create_ipa_nssdb(): ipautil.backup_file(os.path.join(db.secdir, 'secmod.db')) with open(pwdfile, 'w') as f: - f.write(ipautil.ipa_generate_password(pwd_len=40)) + f.write(ipautil.ipa_generate_password()) os.chmod(pwdfile, 0o600) db.create_db(pwdfile) diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py index f061e79..408ca3f 100644 --- a/ipapython/ipautil.py +++ b/ipapython/ipautil.py @@ -23,6 +23,7 @@ import tempfile import subprocess import random +import math import os import sys import copy @@ -51,8 +52,8 @@ from ipapython.ipa_log_manager import root_logger from ipapython.dn import DN -GEN_PWD_LEN = 22 -GEN_TMP_PWD_LEN = 12 # only for OTP password that is manually retyped by user +# only for OTP password that is manually retyped by user +TMP_PWD_ENTROPY_BITS = 128 PROTOCOL_NAMES = { @@ -789,34 +790,89 @@ def parse_generalized_time(timestr): except ValueError: return None -def ipa_generate_password(characters=None,pwd_len=None): - ''' Generates password. Password cannot start or end with a whitespace - character. It also cannot be formed by whitespace characters only. - Length of password as well as string of characters to be used by - generator could be optionaly specified by characters and pwd_len - parameters, otherwise default values will be used: characters string - will be formed by all printable non-whitespace characters and space, - pwd_len will be equal to value of GEN_PWD_LEN. - ''' - if not characters: - characters=string.digits + string.ascii_letters + string.punctuation + ' ' - else: - if characters.isspace(): - raise ValueError("password cannot be formed by whitespaces only") - if not pwd_len: - pwd_len = GEN_PWD_LEN - - upper_bound = len(characters) - 1 - rndpwd = '' - r = random.SystemRandom() - - for x in range(pwd_len): - rndchar = characters[r.randint(0,upper_bound)] - if (x == 0) or (x == pwd_len-1): - while rndchar.isspace(): - rndchar = characters[r.randint(0,upper_bound)] - rndpwd += rndchar - return rndpwd + +def ipa_generate_password(entropy_bits=256, uppercase=1, lowercase=1, digits=1, + special=1, min_len=0): + """ + Generate token containing at least `entropy_bits` bits and with the given + character restraints. + + :param entropy_bits: + The minimal number of entropy bits attacker has to guess: + 128 bits entropy: secure + 256 bits of entropy: secure enough if you care about quantum + computers + + Integer values specify minimal number of characters from given + character class and length. + Value None prevents given character from appearing in the token. + + Example: + TokenGenerator(uppercase=3, lowercase=3, digits=0, special=None) + + At least 3 upper and 3 lower case ASCII chars, may contain digits, + no special chars. + """ + special_chars = '!$%&()*+,-./:;<>?@[]^_{|}~' + pwd_charsets = { + 'uppercase': { + 'chars': string.ascii_uppercase, + 'entropy': math.log(len(string.ascii_uppercase), 2) + }, + 'lowercase': { + 'chars': string.ascii_lowercase, + 'entropy': math.log(len(string.ascii_lowercase), 2) + }, + 'digits': { + 'chars': string.digits, + 'entropy': math.log(len(string.digits), 2) + }, + 'special': { + 'chars': special_chars, + 'entropy': math.log(len(special_chars), 2) + }, + } + req_classes = dict( + uppercase=uppercase, + lowercase=lowercase, + digits=digits, + special=special + ) + # 'all' class is used when adding entropy to too-short tokens + # it contains characters from all allowed classes + pwd_charsets['all'] = { + 'chars': ''.join([ + charclass['chars'] for charclass_name, charclass + in pwd_charsets.items() + if req_classes[charclass_name] is not None + ]) + } + pwd_charsets['all']['entropy'] = math.log( + len(pwd_charsets['all']['chars']), 2) + rnd = random.SystemRandom() + + todo_entropy = entropy_bits + password = '' + # Generate required character classes: + # The order of generated characters is fixed to comply with check in + # NSS function sftk_newPinCheck() in nss/lib/softoken/fipstokn.c. + for charclass_name in ['digits', 'uppercase', 'lowercase', 'special']: + charclass = pwd_charsets[charclass_name] + todo_characters = req_classes[charclass_name] + while todo_characters > 0: + password += rnd.choice(charclass['chars']) + todo_entropy -= charclass['entropy'] + todo_characters -= 1 + + # required character classes do not provide sufficient entropy + # or does not fulfill minimal length constraint + allchars = pwd_charsets['all'] + while todo_entropy > 0 or len(password) < min_len: + password += rnd.choice(allchars['chars']) + todo_entropy -= allchars['entropy'] + + return password + def user_input(prompt, default = None, allow_empty = True): if default == None: diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py index 414a716..85c2d06 100644 --- a/ipaserver/install/certs.py +++ b/ipaserver/install/certs.py @@ -173,7 +173,7 @@ def create_noise_file(self): if ipautil.file_exists(self.noise_fname): os.remove(self.noise_fname) f = open(self.noise_fname, "w") - f.write(ipautil.ipa_generate_password(pwd_len=25)) + f.write(ipautil.ipa_generate_password()) self.set_perms(self.noise_fname) def create_passwd_file(self, passwd=None): @@ -182,7 +182,7 @@ def create_passwd_file(self, passwd=None): if passwd is not None: f.write("%s\n" % passwd) else: - f.write(ipautil.ipa_generate_password(pwd_len=25)) + f.write(ipautil.ipa_generate_password()) f.close() self.set_perms(self.passwd_fname) diff --git a/ipaserver/install/dnskeysyncinstance.py b/ipaserver/install/dnskeysyncinstance.py index 76a14f9..861a170 100644 --- a/ipaserver/install/dnskeysyncinstance.py +++ b/ipaserver/install/dnskeysyncinstance.py @@ -224,10 +224,11 @@ def __setup_softhsm(self): os.chown(paths.DNSSEC_TOKENS_DIR, self.ods_uid, self.named_gid) # generate PINs for softhsm - allowed_chars = u'123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ' pin_length = 30 # Bind allows max 32 bytes including ending '\0' - pin = ipautil.ipa_generate_password(allowed_chars, pin_length) - pin_so = ipautil.ipa_generate_password(allowed_chars, pin_length) + pin = ipautil.ipa_generate_password( + entropy_bits=0, special=None, min_len=pin_length) + pin_so = ipautil.ipa_generate_password( + entropy_bits=0, special=None, min_len=pin_length) self.logger.debug("Saving user PIN to %s", paths.DNSSEC_SOFTHSM_PIN) named_fd = open(paths.DNSSEC_SOFTHSM_PIN, 'w') diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py index dc4b5b0..c3c470d 100644 --- a/ipaserver/install/dogtaginstance.py +++ b/ipaserver/install/dogtaginstance.py @@ -427,7 +427,7 @@ def __add_admin_to_group(self, group): def setup_admin(self): self.admin_user = "admin-%s" % self.fqdn - self.admin_password = ipautil.ipa_generate_password(pwd_len=20) + self.admin_password = ipautil.ipa_generate_password() self.admin_dn = DN(('uid', self.admin_user), ('ou', 'people'), ('o', 'ipaca')) diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py index e8c706e..bacd5fc 100644 --- a/ipaserver/install/httpinstance.py +++ b/ipaserver/install/httpinstance.py @@ -313,7 +313,7 @@ def create_cert_db(self): ipautil.backup_file(nss_path) # Create the password file for this db - password = ipautil.ipa_generate_password(pwd_len=15) + password = ipautil.ipa_generate_password() f = os.open(pwd_file, os.O_CREAT | os.O_RDWR) os.write(f, password) os.close(f) diff --git a/ipaserver/plugins/baseuser.py b/ipaserver/plugins/baseuser.py index 4c7e9f0..85ad417 100644 --- a/ipaserver/plugins/baseuser.py +++ b/ipaserver/plugins/baseuser.py @@ -17,8 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -import string - import six from ipalib import api, errors @@ -35,7 +33,7 @@ from ipalib import _ from ipalib.constants import PATTERN_GROUPUSER_NAME from ipapython import kerberos -from ipapython.ipautil import ipa_generate_password, GEN_TMP_PWD_LEN +from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS from ipapython.ipavalidate import Email from ipalib.util import ( normalize_sshpubkey, @@ -75,8 +73,6 @@ ('cn', 'etc'), api.env.basedn) -# characters to be used for generating random user passwords -baseuser_pwdchars = string.digits + string.ascii_letters + '_,.@+-=' def validate_nsaccountlock(entry_attrs): if 'nsaccountlock' in entry_attrs: @@ -554,7 +550,7 @@ def check_manager(self, entry_attrs, container): def check_userpassword(self, entry_attrs, **options): if 'userpassword' not in entry_attrs and options.get('random'): entry_attrs['userpassword'] = ipa_generate_password( - baseuser_pwdchars, pwd_len=GEN_TMP_PWD_LEN) + entropy_bits=TMP_PWD_ENTROPY_BITS) # save the password so it can be displayed in post_callback setattr(context, 'randompassword', entry_attrs['userpassword']) diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py index 957a1ed..58e711f 100644 --- a/ipaserver/plugins/host.py +++ b/ipaserver/plugins/host.py @@ -21,7 +21,6 @@ from __future__ import absolute_import import dns.resolver -import string import six @@ -62,7 +61,7 @@ from ipapython.ipautil import ( ipa_generate_password, CheckedIPAddress, - GEN_TMP_PWD_LEN + TMP_PWD_ENTROPY_BITS ) from ipapython.dnsutil import DNSName from ipapython.ssh import SSHPublicKey @@ -136,10 +135,6 @@ register = Registry() -# Characters to be used by random password generator -# The set was chosen to avoid the need for escaping the characters by user -host_pwd_chars = string.digits + string.ascii_letters + '_,.@+-=' - def remove_ptr_rec(ipaddr, fqdn): """ @@ -688,7 +683,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): entry_attrs['objectclass'].remove('krbprincipal') if options.get('random'): entry_attrs['userpassword'] = ipa_generate_password( - characters=host_pwd_chars, pwd_len=GEN_TMP_PWD_LEN) + entropy_bits=TMP_PWD_ENTROPY_BITS) # save the password so it can be displayed in post_callback setattr(context, 'randompassword', entry_attrs['userpassword']) certs = options.get('usercertificate', []) @@ -915,7 +910,8 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): entry_attrs['usercertificate'] = certs_der if options.get('random'): - entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars) + entry_attrs['userpassword'] = ipa_generate_password( + entropy_bits=TMP_PWD_ENTROPY_BITS) setattr(context, 'randompassword', entry_attrs['userpassword']) if 'macaddress' in entry_attrs: diff --git a/ipaserver/plugins/stageuser.py b/ipaserver/plugins/stageuser.py index 1da43ec..afd402e 100644 --- a/ipaserver/plugins/stageuser.py +++ b/ipaserver/plugins/stageuser.py @@ -38,7 +38,6 @@ baseuser_find, baseuser_show, NO_UPG_MAGIC, - baseuser_pwdchars, baseuser_output_params, baseuser_add_manager, baseuser_remove_manager) @@ -47,7 +46,7 @@ from ipalib import _, ngettext from ipalib import output from ipaplatform.paths import paths -from ipapython.ipautil import ipa_generate_password, GEN_TMP_PWD_LEN +from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS from ipalib.capabilities import client_has_capability if six.PY3: @@ -340,7 +339,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): # If requested, generate a userpassword if 'userpassword' not in entry_attrs and options.get('random'): entry_attrs['userpassword'] = ipa_generate_password( - baseuser_pwdchars, pwd_len=GEN_TMP_PWD_LEN) + entropy_bits=TMP_PWD_ENTROPY_BITS) # save the password so it can be displayed in post_callback setattr(context, 'randompassword', entry_attrs['userpassword']) diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py index 5296093..6440548 100644 --- a/ipaserver/plugins/user.py +++ b/ipaserver/plugins/user.py @@ -38,7 +38,6 @@ NO_UPG_MAGIC, UPG_DEFINITION_DN, baseuser_output_params, - baseuser_pwdchars, validate_nsaccountlock, convert_nsaccountlock, fix_addressbook_permission_bindrule, @@ -63,7 +62,7 @@ from ipalib import output from ipaplatform.paths import paths from ipapython.dn import DN -from ipapython.ipautil import ipa_generate_password, GEN_TMP_PWD_LEN +from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS from ipalib.capabilities import client_has_capability if api.env.in_server: @@ -529,7 +528,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options): if 'userpassword' not in entry_attrs and options.get('random'): entry_attrs['userpassword'] = ipa_generate_password( - baseuser_pwdchars, pwd_len=GEN_TMP_PWD_LEN) + entropy_bits=TMP_PWD_ENTROPY_BITS) # save the password so it can be displayed in post_callback setattr(context, 'randompassword', entry_attrs['userpassword']) diff --git a/ipaserver/secrets/store.py b/ipaserver/secrets/store.py index 1c369d8..2c58eee 100644 --- a/ipaserver/secrets/store.py +++ b/ipaserver/secrets/store.py @@ -122,7 +122,7 @@ def export_key(self): with open(nsspwfile, 'w+') as f: f.write(self.nssdb_password) pk12pwfile = os.path.join(tdir, 'pk12pwfile') - password = ipautil.ipa_generate_password(pwd_len=20) + password = ipautil.ipa_generate_password() with open(pk12pwfile, 'w+') as f: f.write(password) pk12file = os.path.join(tdir, 'pk12file')
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
