From b20d903cc408ad849184ddc82258d0f7f631cf02 Mon Sep 17 00:00:00 2001
From: Zhikang Zhang <zzhang63@ncsu.edu>
Date: Mon, 13 Aug 2018 12:45:37 -0400
Subject: [PATCH] Give warning if user inputs not encrypted password to user
 module (#43615)

* Check the password format

Check the password format and notify user if they
input unencrypted password.

* Fix sanity error

* Add integration test

* Missed a task name

* Hard code the testing password

Since some testing platfrom has no passlib installed

* Add changelog fragment

* Rework some English sentences

* Fix a grammar mistake
---
 .../fragments/password_sanity_check.yml       |  3 ++
 lib/ansible/modules/system/user.py            | 36 +++++++++++++++++
 test/integration/targets/user/tasks/main.yml  | 40 +++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 changelogs/fragments/password_sanity_check.yml

diff --git a/changelogs/fragments/password_sanity_check.yml b/changelogs/fragments/password_sanity_check.yml
new file mode 100644
index 0000000000..d0c1f6fcd3
--- /dev/null
+++ b/changelogs/fragments/password_sanity_check.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - user module - add a sanity check for the user's password and a more helpful warning message (https://github.com/ansible/ansible/pull/43615)
diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py
index bb66323e92..6b030c88c1 100644
--- a/lib/ansible/modules/system/user.py
+++ b/lib/ansible/modules/system/user.py
@@ -342,6 +342,7 @@ uid:
 import errno
 import grp
 import os
+import re
 import platform
 import pwd
 import shutil
@@ -358,6 +359,9 @@ except ImportError:
     HAVE_SPWD = False
 
 
+_HASH_RE = re.compile(r'[^a-zA-Z0-9./=]')
+
+
 class User(object):
     """
     This is a generic User manipulation class that is subclassed
@@ -429,6 +433,37 @@ class User(object):
         else:
             self.ssh_file = os.path.join('.ssh', 'id_%s' % self.ssh_type)
 
+    def check_password_encrypted(self):
+        # darwin need cleartext password, so no check
+        if self.module.params['password'] and self.platform != 'Darwin':
+            maybe_invalid = False
+            # : for delimiter, * for disable user, ! for lock user
+            # these characters are invalid in the password
+            if any(char in self.module.params['password'] for char in ':*!'):
+                maybe_invalid = True
+            if '$' not in self.module.params['password']:
+                maybe_invalid = True
+            else:
+                fields = self.module.params['password'].split("$")
+                if len(fields) >= 3:
+                    # contains character outside the crypto constraint
+                    if bool(_HASH_RE.search(fields[-1])):
+                        maybe_invalid = True
+                    # md5
+                    if fields[1] == '1' and len(fields[-1]) != 22:
+                        maybe_invalid = True
+                    # sha256
+                    if fields[1] == '5' and len(fields[-1]) != 43:
+                        maybe_invalid = True
+                    # sha512
+                    if fields[1] == '6' and len(fields[-1]) != 86:
+                        maybe_invalid = True
+                else:
+                    maybe_invalid = True
+            if maybe_invalid:
+                self.module.warn("The input password appears not to have been hashed. "
+                                 "The 'password' argument must be encrypted for this module to work properly.")
+
     def execute_command(self, cmd, use_unsafe_shell=False, data=None, obey_checkmode=True):
         if self.module.check_mode and obey_checkmode:
             self.module.debug('In check mode, would have run: "%s"' % cmd)
@@ -2392,6 +2427,7 @@ def main():
     )
 
     user = User(module)
+    user.check_password_encrypted()
 
     module.debug('User instantiated - platform %s' % user.platform)
     if user.distribution:
diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml
index 329710cb78..690c00b107 100644
--- a/test/integration/targets/user/tasks/main.yml
+++ b/test/integration/targets/user/tasks/main.yml
@@ -64,6 +64,46 @@
       - user_test0_1 is not changed
       - '"ansibulluser" in user_names.stdout_lines'
 
+# test user add with password
+- name: add an encrypted password for user
+  user:
+    name: ansibulluser
+    password: "$6$rounds=656000$TT4O7jz2M57npccl$33LF6FcUMSW11qrESXL1HX0BS.bsiT6aenFLLiVpsQh6hDtI9pJh5iY7x8J7ePkN4fP8hmElidHXaeD51pbGS."
+    state: present
+    update_password: always
+  register: test_user_encrypt0
+
+- name: there should not be warnings
+  assert:
+    that: "'warnings' not in test_user_encrypt0"
+
+- block:
+    - name: add an plaintext password for user
+      user:
+        name: ansibulluser
+        password: "plaintextpassword"
+        state: present
+        update_password: always
+      register: test_user_encrypt1
+
+    - name: there should be a warning complains that the password is plaintext
+      assert:
+        that: "'warnings' in test_user_encrypt1"
+
+    - name: add an invalid hashed password
+      user:
+        name: ansibulluser
+        password: "$6$rounds=656000$tgK3gYTyRLUmhyv2$lAFrYUQwn7E6VsjPOwQwoSx30lmpiU9r/E0Al7tzKrR9mkodcMEZGe9OXD0H/clOn6qdsUnaL4zefy5fG+++++"
+        state: present
+        update_password: always
+      register: test_user_encrypt2
+
+    - name: there should be a warning complains about the character set of password
+      assert:
+        that: "'warnings' in test_user_encrypt2"
+  when: ansible_system != 'Darwin'
+
+
 # https://github.com/ansible/ansible/issues/42484
 # Skipping macOS for now since there is a bug when changing home directory
 - block: