[passwordstore] Use builtin _random_password function instead of pwgen (#25843)

* [password] _random_password -> random_password and moved to util/encrypt.py
* [passwordstore] Use built-in random_password instead of pwgen utility
* [passwordstore] Add integration tests

lib/ansible/plugins/lookup/password.py

@@ -21,15 +21,12 @@ __metaclass__ = type
 
 import os
 import string
-import random
 
-from ansible import constants as C
 from ansible.errors import AnsibleError
-from ansible.module_utils.six import text_type
 from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.parsing.splitter import parse_kv
 from ansible.plugins.lookup import LookupBase
-from ansible.utils.encrypt import do_encrypt
+from ansible.utils.encrypt import do_encrypt, random_password
 from ansible.utils.path import makedirs_safe
 
 
@@ -136,35 +133,13 @@ def _gen_candidate_chars(characters):
     return chars
 
 
-def _random_password(length=DEFAULT_LENGTH, chars=C.DEFAULT_PASSWORD_CHARS):
-    '''Return a random password string of length containing only chars
-
-    :kwarg length: The number of characters in the new password.  Defaults to 20.
-    :kwarg chars: The characters to choose from.  The default is all ascii
-        letters, ascii digits, and these symbols ``.,:-_``
-
-    .. note: this was moved from the old ansible utils code, as nothing
-        else appeared to use it.
-    '''
-    assert isinstance(chars, text_type), '%s (%s) is not a text_type' % (chars, type(chars))
-
-    random_generator = random.SystemRandom()
-
-    password = []
-    while len(password) < length:
-        new_char = random_generator.choice(chars)
-        password.append(new_char)
-
-    return u''.join(password)
-
-
 def _random_salt():
     """Return a text string suitable for use as a salt for the hash functions we use to encrypt passwords.
     """
     # Note passlib salt values must be pure ascii so we can't let the user
     # configure this
     salt_chars = _gen_candidate_chars(['ascii_letters', 'digits', './'])
-    return _random_password(length=8, chars=salt_chars)
+    return random_password(length=8, chars=salt_chars)
 
 
 def _parse_content(content):
@@ -234,7 +209,7 @@ class LookupModule(LookupBase):
             content = _read_password_file(b_path)
 
             if content is None or b_path == to_bytes('/dev/null'):
-                plaintext_password = _random_password(params['length'], chars)
+                plaintext_password = random_password(params['length'], chars)
                 salt = None
                 changed = True
             else:

lib/ansible/plugins/lookup/passwordstore.py

@@ -20,8 +20,11 @@ __metaclass__ = type
 import os
 import subprocess
 import time
+
 from distutils import util
 from ansible.errors import AnsibleError
+from ansible.module_utils._text import to_bytes, to_native, to_text
+from ansible.utils.encrypt import random_password
 from ansible.plugins.lookup import LookupBase
 
 
@@ -35,25 +38,31 @@ def check_output2(*popenargs, **kwargs):
     if 'input' in kwargs:
         if 'stdin' in kwargs:
             raise ValueError('stdin and input arguments may not both be used.')
-        inputdata = kwargs['input']
+        b_inputdata = to_bytes(kwargs['input'], errors='surrogate_or_strict')
         del kwargs['input']
         kwargs['stdin'] = subprocess.PIPE
     else:
-        inputdata = None
+        b_inputdata = None
     process = subprocess.Popen(*popenargs, stdout=subprocess.PIPE, stderr=subprocess.PIPE, **kwargs)
     try:
-        out, err = process.communicate(inputdata)
+        b_out, b_err = process.communicate(b_inputdata)
     except:
         process.kill()
         process.wait()
         raise
     retcode = process.poll()
-    if retcode:
+    if retcode != 0 or \
+            b'encryption failed: Unusable public key' in b_out or \
+            b'encryption failed: Unusable public key' in b_err:
         cmd = kwargs.get("args")
         if cmd is None:
             cmd = popenargs[0]
-        raise subprocess.CalledProcessError(retcode, cmd, out + err)
+        raise subprocess.CalledProcessError(
-    return out
+            retcode,
+            cmd,
+            to_native(b_out + b_err, errors='surrogate_or_strict')
+        )
+    return b_out
 
 
 class LookupModule(LookupBase):
@@ -95,11 +104,14 @@ class LookupModule(LookupBase):
 
     def check_pass(self):
         try:
-            self.passoutput = check_output2(["pass", self.passname]).splitlines()
+            self.passoutput = to_text(
+                check_output2(["pass", self.passname]),
+                errors='surrogate_or_strict'
+            ).splitlines()
             self.password = self.passoutput[0]
             self.passdict = {}
             for line in self.passoutput[1:]:
-                if ":" in line:
+                if ':' in line:
                     name, value = line.split(':', 1)
                     self.passdict[name.strip()] = value.strip()
         except (subprocess.CalledProcessError) as e:
@@ -118,10 +130,7 @@ class LookupModule(LookupBase):
         if self.paramvals['userpass']:
             newpass = self.paramvals['userpass']
         else:
-            try:
+            newpass = random_password(length=self.paramvals['length'])
-                newpass = check_output2(['pwgen', '-cns', str(self.paramvals['length']), '1']).rstrip()
-            except (subprocess.CalledProcessError) as e:
-                raise AnsibleError(e)
         return newpass
 
     def update_password(self):
@@ -131,7 +140,7 @@ class LookupModule(LookupBase):
         msg = newpass + '\n' + '\n'.join(self.passoutput[1:])
         msg += "\nlookup_pass: old password was {} (Updated on {})\n".format(self.password, datetime)
         try:
-            generate = check_output2(['pass', 'insert', '-f', '-m', self.passname], input=msg)
+            check_output2(['pass', 'insert', '-f', '-m', self.passname], input=msg)
         except (subprocess.CalledProcessError) as e:
             raise AnsibleError(e)
         return newpass
@@ -143,7 +152,7 @@ class LookupModule(LookupBase):
         datetime = time.strftime("%d/%m/%Y %H:%M:%S")
         msg = newpass + '\n' + "lookup_pass: First generated by ansible on {}\n".format(datetime)
         try:
-            generate = check_output2(['pass', 'insert', '-f', '-m', self.passname], input=msg)
+            check_output2(['pass', 'insert', '-f', '-m', self.passname], input=msg)
         except (subprocess.CalledProcessError) as e:
             raise AnsibleError(e)
         return newpass

lib/ansible/utils/encrypt.py

@@ -23,11 +23,14 @@ import stat
 import tempfile
 import time
 import warnings
+import random
 
 from ansible import constants as C
 from ansible.errors import AnsibleError
+from ansible.module_utils.six import text_type
 from ansible.module_utils._text import to_text, to_bytes
 
+
 PASSLIB_AVAILABLE = False
 try:
     import passlib.hash
@@ -162,3 +165,19 @@ def keyczar_decrypt(key, msg):
         return key.Decrypt(msg)
     except key_errors.InvalidSignatureError:
         raise AnsibleError("decryption failed")
+
+
+DEFAULT_PASSWORD_LENGTH = 20
+
+
+def random_password(length=DEFAULT_PASSWORD_LENGTH, chars=C.DEFAULT_PASSWORD_CHARS):
+    '''Return a random password string of length containing only chars
+
+    :kwarg length: The number of characters in the new password.  Defaults to 20.
+    :kwarg chars: The characters to choose from.  The default is all ascii
+        letters, ascii digits, and these symbols ``.,:-_``
+    '''
+    assert isinstance(chars, text_type), '%s (%s) is not a text_type' % (chars, type(chars))
+
+    random_generator = random.SystemRandom()
+    return u''.join(random_generator.choice(chars) for dummy in range(length))

test/integration/targets/lookup_passwordstore/aliases

@@ -0,0 +1 @@
+posix/ci/group2

test/integration/targets/lookup_passwordstore/tasks/main.yml

@@ -0,0 +1,4 @@
+- include: "package.yml"
+  when: "ansible_distribution_version not in passwordstore_skip_os.get(ansible_distribution, [])"
+- include: "tests.yml"
+  when: "ansible_distribution_version not in passwordstore_skip_os.get(ansible_distribution, [])"

test/integration/targets/lookup_passwordstore/tasks/package.yml

@@ -0,0 +1,50 @@
+- name: "Install package"
+  apt:
+    name: pass
+    state: present
+  when: ansible_pkg_mgr == 'apt'
+- name: "Install package"
+  yum:
+    name: pass
+    state: present
+  when: ansible_pkg_mgr == 'yum'
+- name: "Install package"
+  dnf:
+    name: pass
+    state: present
+  when: ansible_pkg_mgr == 'dnf'
+- name: "Install package"
+  zypper:
+    name: password-store
+    state: present
+  when: ansible_pkg_mgr == 'zypper'
+- name: "Install package"
+  pkgng:
+    name: "{{ item }}"
+    state: present
+  with_items:
+    - "gnupg"
+    - "password-store"
+  when: ansible_pkg_mgr == 'pkgng'
+
+
+- name: Find brew binary
+  command: which brew
+  register: brew_which
+  when: ansible_distribution in ['MacOSX']
+- name: Get owner of brew binary
+  stat:
+    path: "{{ brew_which.stdout }}"
+  register: brew_stat
+  when: ansible_distribution in ['MacOSX']
+- name: "Install package"
+  homebrew:
+    name: "{{ item }}"
+    state: present
+    update_homebrew: no
+  with_items:
+    - "gnupg2"
+    - "pass"
+  become: yes
+  become_user: "{{ brew_stat.stat.pw_name }}"
+  when: ansible_pkg_mgr == 'homebrew'

test/integration/targets/lookup_passwordstore/tasks/tests.yml

@@ -0,0 +1,36 @@
+- name: "check name of gpg2 binary"
+  command: which gpg2
+  register: gpg2_check
+  ignore_errors: true
+
+- name: "set gpg2 binary name"
+  set_fact:
+    gpg2_bin: '{{ "gpg2" if gpg2_check|success else "gpg" }}'
+
+- name: "remove previous password files and directory"
+  file: dest={{item}} state=absent
+  with_items:
+  - "~/.gnupg"
+  - "~/.password-store"
+
+- name: "import gpg private key"
+  shell: echo "{{passwordstore_privkey}}" | {{ gpg2_bin }} --import --allow-secret-key-import -
+
+- name: "trust gpg key"
+  shell: echo "A2A6052A09617FFC935644F1059AA7454B2652D1:6:" | {{ gpg2_bin }} --import-ownertrust
+
+- name: initialise passwordstore
+  command: pass init passwordstore-lookup
+
+- name: create a password
+  set_fact:
+    newpass: "{{ lookup('passwordstore', 'test-pass length=8 create=yes') }}"
+
+- name: fetch password from an existing file
+  set_fact:
+    readpass: "{{ lookup('passwordstore', 'test-pass') }}"
+
+- name: verify password
+  assert:
+    that:
+        - "readpass == newpass"

test/integration/targets/lookup_passwordstore/vars/main.yml

@@ -0,0 +1,62 @@
+passwordstore_privkey: |
+  -----BEGIN PGP PRIVATE KEY BLOCK-----
+
+  lQOYBFlJF1MBCACwLW/YzhKpTLVnNk8JucBfOKGdZjOzD6EB77vuJZGNt8sUMFuV
+  g3VUkPZln4fZ9tN04tDgUkOdZEZqAHkOJNFUEnRRXlzSK6u7NJwuQOnDhNe3E9uM
+  hsvbaL7rcPNmpra12RhUiwnATSBit5SZf5L80Y60HJxrJchGDilGGdshoyNJ5LZf
+  9r6JfWkSXsQR4EvGatkzVNqNyLYn4sy/ToguH1Et8c61B6DmJ0Jzb+Txh8dl64QQ
+  NbWcXXL7H1CfCR/E1ZtM50d7hyD5N1t9qdgmq6Zm0RCaf/ijTM0wqW6jL9oZEKZ+
+  YA8xgl7jW252oelhINYJ4qb5ITYiEx9dadk9ABEBAAEAB/wPp3Xm+oaTdv6uZVe4
+  CkKC434OxZBF8pdQm/vjoQByKnjXuiVFH3lrMndGV9rDHgize9zp9b1OzKRqElEv
+  Vcuoz/v4Z+1Q+nLnrzjKblenGRRuzsulDKwr+n5uXqqt/hXBikD8cB9FcET2qI/C
+  ZODLaJZowBsQ9To6qVL3COCc+CNICmNvYt5bAya7bw8UMIaFbZClx+jOSyViLw/h
+  g0CQh97x4xoemVBEniYDR/FcZNtc4FM7Ll6vGwGuGmWaqwnKzc2YHpZwX6Sb54pr
+  pj7U8fPcNQRXadeksnrJ+6vY02INyV5szeh5c8iOEeAAIo8GnQnTcYax+H3/fWSo
+  MLVpBADBVPrVYAozmjMWUAyRJTB1hfH2avz5dfZZHKDdBWx4sJAxHET2x7/dA+uM
+  x/+kdGy1g0CwY0M4cdZZfMsQL+OEXL5ZZ2WEHosIldpZVdz4jmqC67LaA3x3CCdW
+  8lnrT+cFqoeK2EUUriphiBZWkdyMGExGysPToAS0gAXHVESqKQQA6UjzLYkz+xjm
+  V/b8Q6QczDg5wOfnwEnoP6QdY/JS82XBtEvsIESc+tPwOL6eZPvs+ci+XPTrSP7j
+  KjGwgdio47KUa72PR171nJcDAShxAVlVLsWmRcgfMEwQl3gWybJ5NDeFWdjYMxCg
+  E1gFEi2c5szVYCGjvNnP3xt9eQZ7APUEAJlbozABpV+jyPn071zsLhJ3nkuasU1c
+  xgr6x2WTH9WuZ5/vcNuHEBo4/6szMonPJtr1ND4MOj43LgB0lcMERdAukn62kvVb
+  u7BhPacVbXeSWEAAZO63C+imRm3eHr/SFmNSAGmY3cX4ZpMZY6VXBPGeDqKsDILj
+  n+cfpSK2Fnb8RJW0FHBhc3N3b3Jkc3RvcmUtbG9va3VwiQFUBBMBCAA+FiEEoqYF
+  Kglhf/yTVkTxBZqnRUsmUtEFAllJF1MCGwMFCQPCZwAFCwkIBwIGFQgJCgsCBBYC
+  AwECHgECF4AACgkQBZqnRUsmUtHfXAgAqQIt/Bn53VNvuJDhl7ItANKDbVxz+oeD
+  en/uRtybBzTsQoQwyN/lDmXuFWFzgryfgoik6997fNBBB/cjBpqD21FHbVrJj1Ms
+  7Uzd5iwvK0EnzWH2R8mfMDVmDFPYbuPAfolUBabMYTjR8+OBnLnh60RVhGYeAoWf
+  Lr/smC0D57Zh6DQ1x6B8S1e0iZu9Xo0tx+r2xcNc/9Td+nPQW4d/gMXVNyO0ACZ+
+  L9yfb/1CgI/WW31t/bQLobeiCuMKEGetmVXxfEutjr1UZkfbipABzb+WVutUDdm0
+  jYNv3MgDRUlHFMLl1tW4+llXhpERxrRBPEJ6QqGcQgzr8E+dbMfP350DmARZSRdT
+  AQgA1SSrYi6Fec9ZCcy94c43bqWhNIjpQWEzlKQ1xV5GwJaO+zogDx8exNisUb0W
+  NqUsQunfjo7ACaA9swe28nsm18ZPceJ+UzJz3V1NIWXANmdnMnegbVEohGnJYb77
+  jd67A19DJF5c4elS2VHJiyNbygEWonvU12VWSDgPFr2Efo8eEV8HUBitN76D5sVV
+  ESUnvfxr/1TalrXMFmOPdMjTK5rZCRRpBR6ZPhKDQfrZJKWjWWiti5hiTfJAmVY9
+  NQiofTQcsEmyuPnuwS5D6Q7f5EjqhV+hPWaeqdDX+2tXMsMaU5zuOzD1d0yvZaWh
+  3ThsuTfFr+0RSeTxZrbC3viapQARAQABAAf8D9wRJpaYlvY/RVPnQyCRjlmjs6GG
+  XbeKW4KWf6+iqxzo2be6//UMWJBYziI4P2ut7fKyEEz97BlwzdwCmGtiegbHDY3R
+  YYZtCakyHoyQL1wlWSN+m/PAhI3MjsnjtOxAVSFnARNGbQbsA8CqswA4CcFn+kIl
+  lbt0Hp6RPNtwOuxvd3DikpDLVR2QDQ+zYV2j05R7lJbA37Yk5SDMJKKKxjWlvubq
+  2KTt70gav5Kutlac7SICCQgWO+h2L/9TZtp7cZ3PjHLzJKb66sUq9hSCjNiKOf4J
+  C5GZ4lS4uKHOTjh4jx3PbkUi59Ji6K0/GQoSZqSjjMPuAyEZe02ntBY91wQA24tr
+  M7D3or3w/CLp7ynYqsHcb3pQ6iTvnrC483pAHktYhEqLipeNjO2u0gfXfMrzm169
+  ooLP8ebQMh+r/jMCLO+Lr1gwz+uEe1oZMswez15W8Iqa6J5zycqB5Qjbk3UCvkc0
+  XJf3rrJYN7WuXL3woHo+sTQxqROnCTuwsqP1GI8EAPiJIuOiu2moMGzhWNDPOl3l
+  MaSi68Q3w62bH8s6rNry+nBvBRbVGg9tLomwE1WTxBN9Qc4BPke33NWtDv4WHIQA
+  COhHFp+uI1elIP9onPtSd66Dj55rale5S5YkHfSCXPf8OFklnzc7h5PLx+4KnTaK
+  WyjvkSiwFKo6IrMxl+uLBACFQDm6LX0NqDVbIvvh+/nA8Uia2Sv4fSkAvBCIVez2
+  LOC1QXpbG0t2uACWso1AiseaRQbaV4jYx+23M/5xKkAhqrgaqw3/LSszChtRZqFe
+  Co08X3x0fDZfKL3A2d+BYJsCKcfi9msDe2YrxG57jLXk/LPebKH0Md0cJrLAlI5Q
+  xUbqiQE2BBgBCAAgFiEEoqYFKglhf/yTVkTxBZqnRUsmUtEFAllJF1MCGwwACgkQ
+  BZqnRUsmUtE9CAgAhaB2d99PGITB5PH8wHvbwb/tNqORwOCjcgjbBtHyNTpCYqiv
+  nB1X+vA0+xIdBW/ZZT8ghq4B1RMR1CT2aCobHP+LVmIn+9FRXF43V/9+ddRT9rF1
+  4wFvwcRSbS+3Ql9y9Fs3yUE2U7EwonanWUaq4j+XOM7nuXM/afBmjpzUiX5ZV2Ep
+  G1dIfWkMBLE3t1k6/nR/hIJDUkzsz7rGFaXKLRk/UkOWgDAEDhDaEsZD3K8Du1DQ
+  +ZAbputP36PiAcjSnlzAcfs3ZfXMncaGShewOHO1gMH0iTZWv6qHyLNW1oEoQg3y
+  SxHTvI2pKk+gx0FB8wWhd/CocAHJpx9oNUs/7A==
+  =ZF3O
+  -----END PGP PRIVATE KEY BLOCK-----
+passwordstore_skip_os:
+  Ubuntu: ['12.04']
+  RedHat: ['7.4']
+  CentOS: ['6.9']

test/units/plugins/lookup/test_password.py

@@ -245,31 +245,31 @@ class TestRandomPassword(unittest.TestCase):
             self.assertIn(res_char, chars)
 
     def test_default(self):
-        res = password._random_password()
+        res = password.random_password()
         self.assertEquals(len(res), password.DEFAULT_LENGTH)
         self.assertTrue(isinstance(res, text_type))
         self._assert_valid_chars(res, DEFAULT_CANDIDATE_CHARS)
 
     def test_zero_length(self):
-        res = password._random_password(length=0)
+        res = password.random_password(length=0)
         self.assertEquals(len(res), 0)
         self.assertTrue(isinstance(res, text_type))
         self._assert_valid_chars(res, u',')
 
     def test_just_a_common(self):
-        res = password._random_password(length=1, chars=u',')
+        res = password.random_password(length=1, chars=u',')
         self.assertEquals(len(res), 1)
         self.assertEquals(res, u',')
 
     def test_free_will(self):
         # A Rush and Spinal Tap reference twofer
-        res = password._random_password(length=11, chars=u'a')
+        res = password.random_password(length=11, chars=u'a')
         self.assertEquals(len(res), 11)
         self.assertEquals(res, 'aaaaaaaaaaa')
         self._assert_valid_chars(res, u'a')
 
     def test_unicode(self):
-        res = password._random_password(length=11, chars=u'くらとみ')
+        res = password.random_password(length=11, chars=u'くらとみ')
         self._assert_valid_chars(res, u'くらとみ')
         self.assertEquals(len(res), 11)
 
@@ -278,8 +278,8 @@ class TestRandomPassword(unittest.TestCase):
             params = testcase['params']
             candidate_chars = testcase['candidate_chars']
             params_chars_spec = password._gen_candidate_chars(params['chars'])
-            password_string = password._random_password(length=params['length'],
+            password_string = password.random_password(length=params['length'],
-                                                        chars=params_chars_spec)
+                                                       chars=params_chars_spec)
             self.assertEquals(len(password_string),
                               params['length'],
                               msg='generated password=%s has length (%s) instead of expected length (%s)' %

test/utils/docker/centos6/Dockerfile

@@ -20,6 +20,7 @@ RUN yum clean all && \
     openssh-server \
     openssl-devel \
     python-argparse \
+    pass \
     python-devel \
     python-httplib2 \
     python-jinja2 \

test/utils/docker/centos7/Dockerfile

@@ -30,6 +30,7 @@ RUN yum clean all && \
     openssh-server \
     openssl-devel \
     python-cryptography \
+    pass \
     python-devel \
     python-httplib2 \
     python-jinja2 \

test/utils/docker/fedora24/Dockerfile

@@ -34,6 +34,7 @@ RUN dnf clean all && \
     openssh-clients \
     openssh-server \
     openssl-devel \
+    pass \
     procps \
     python-cryptography \
     python-devel \

test/utils/docker/fedora25/Dockerfile

@@ -30,6 +30,7 @@ RUN dnf clean all && \
     openssh-clients \
     openssh-server \
     openssl-devel \
+    pass \
     procps \
     python-cryptography \
     python-devel \

test/utils/docker/fedora25py3/Dockerfile

@@ -26,6 +26,7 @@ RUN dnf clean all && \
     openssh-clients \
     openssh-server \
     openssl-devel \
+    pass \
     procps \
     python3-cryptography \
     python3-dbus \

test/utils/docker/fedora26py3/Dockerfile

@@ -26,6 +26,7 @@ RUN dnf clean all && \
     openssh-clients \
     openssh-server \
     openssl-devel \
+    pass \
     procps \
     python3-cryptography \
     python3-dbus \

test/utils/docker/opensuse42.1/Dockerfile

@@ -19,6 +19,7 @@ RUN zypper --non-interactive --gpg-auto-import-keys refresh && \
     mariadb \
     mercurial \
     openssh \
+    password-store \
     postgresql-server \
     python-cryptography \
     python-devel \

test/utils/docker/opensuse42.2/Dockerfile

@@ -19,6 +19,7 @@ RUN zypper --non-interactive --gpg-auto-import-keys refresh && \
     mariadb \
     mercurial \
     openssh \
+    password-store \
     postgresql-server \
     python-cryptography \
     python-devel \

test/utils/docker/opensuse42.3/Dockerfile

@@ -19,6 +19,7 @@ RUN zypper --non-interactive --gpg-auto-import-keys refresh && \
     mariadb \
     mercurial \
     openssh \
+    password-store \
     postgresql-server \
     python-cryptography \
     python-devel \

test/utils/docker/ubuntu1404/Dockerfile

@@ -27,6 +27,7 @@ RUN apt-get update -y && \
     openssh-client \
     openssh-server \
     python-dev \
+    pass \
     python-httplib2 \
     python-jinja2 \
     python-keyczar \

test/utils/docker/ubuntu1604/Dockerfile

@@ -30,6 +30,7 @@ RUN apt-get update -y && \
     openssh-server \
     python-cryptography \
     python-dev \
+    pass \
     python-dbus \
     python-httplib2 \
     python-jinja2 \

test/utils/docker/ubuntu1604py3/Dockerfile

@@ -23,6 +23,7 @@ RUN apt-get update -y && \
     lsb-release \
     make \
     mysql-server \
+    pass \
     openssh-client \
     openssh-server \
     python3-cryptography \