Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PASSWORD Improvements #421

Merged
merged 6 commits into from
Oct 25, 2023

Conversation

cleberb
Copy link
Contributor

@cleberb cleberb commented Oct 19, 2023

  • Add options:

    • faillock_enable
    • faillock
      • admin_group
      • audit
      • deny
      • dir
      • even_deny_root
      • fail_interval
      • local_users_only
      • no_log_info
      • nodelay
      • root_unlock_time
      • silent
      • unlock_time
    • login_defs
      • login_retries
      • login_timeout
      • pass_max_days
      • pass_min_days
      • pass_warn_age
    • password_remember
  • Change options:

    • pwquality
      • dcredit
      • dictcheck
      • dictpath
      • difok
      • enforce_for_root
      • enforcing
      • gecoscheck
      • lcredit
      • local_users_only
      • maxclassrepeat
      • maxrepeat
      • minclass
      • minlen
      • ocredit
      • retry
      • ucredit
      • usercheck
      • usersubstr
  • Improvements of password algorithm

  • Add e change templates

  • Remove pam_tally2

  • Verify hash of passwords with mkpasswd with yescrypt support

- Add options:
  - faillock_enable
  - faillock
    - admin_group
    - audit
    - deny
    - dir
    - even_deny_root
    - fail_interval
    - local_users_only
    - no_log_info
    - nodelay
    - root_unlock_time
    - silent
    - unlock_time
  - login_defs
    - login_retries
    - login_timeout
    - pass_max_days
    - pass_min_days
    - pass_warn_age
  - password_remember

- Change options:
  - pwquality
    - dcredit
    - dictcheck
    - dictpath
    - difok
    - enforce_for_root
    - enforcing
    - gecoscheck
    - lcredit
    - local_users_only
    - maxclassrepeat
    - maxrepeat
    - minclass
    - minlen
    - ocredit
    - retry
    - ucredit
    - usercheck
    - usersubstr

- Improvements of password algorithm
- Add e change templates
- Remove pam_tally2
- Verify hash of passwords with mkpasswd with yescrypt support
@cleberb
Copy link
Contributor Author

cleberb commented Oct 19, 2023

There are many changes and a more detailed test is necessary, especially in the molecule test, in terms of using mkpasswd. If anyone knows a simpler alternative, feel free to help, as unfortunately yescrypt is not yet supported by Ansible.

@konstruktoid
Copy link
Owner

Thanks yet again. It looks good when glancing over the changes, but I'll start testing as soon as possible.

@konstruktoid konstruktoid self-assigned this Oct 19, 2023
@konstruktoid
Copy link
Owner

konstruktoid commented Oct 20, 2023

TASK [Install mkpasswd] ********************************************************
fatal: [focal]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [bookworm]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [jammy]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [almalinux8]: FAILED! => {"changed": false, "failures": ["No package mkpasswd available."], "msg": "Failed to install some of the specified packages", "rc": 1, "results": []}
changed: [almalinux9]

it's oddly named whois on Ubuntu, don't know about AlmaLinux8.

@cleberb
Copy link
Contributor Author

cleberb commented Oct 20, 2023

TASK [Install mkpasswd] ********************************************************
fatal: [focal]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [bookworm]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [jammy]: FAILED! => {"changed": false, "msg": "No package matching 'mkpasswd' is available"}
fatal: [almalinux8]: FAILED! => {"changed": false, "failures": ["No package mkpasswd available."], "msg": "Failed to install some of the specified packages", "rc": 1, "results": []}
changed: [almalinux9]

it's oddly named whois on Ubuntu, don't know about AlmaLinux8.

As they are specific tools for this test, I believe that installing them only on the test machines is ideal.
In the redhat family, the package is only available in version 9 with the name "mkpasswd" and the encryption "yescrypt" is also only available in this version.

@konstruktoid
Copy link
Owner

As they are specific tools for this test, I believe that installing them only on the test machines is ideal. In the redhat family, the package is only available in version 9 with the name "mkpasswd" and the encryption "yescrypt" is also only available in this version.

Indeed, just install them when verifying

@konstruktoid
Copy link
Owner

The Set rounds task isn't idempotent on AlmaLinux.

Idempotence test failed because of the following tasks:
*  => ansible-role-hardening : Set rounds

@konstruktoid
Copy link
Owner

Will debug this...

Test machine:

~$ lsb_release -d
Description:	Ubuntu 22.04.3 LTS
~$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt --rounds=11
crypt: Invalid argument
~$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt
$y$j9T$fKUyYvKRu7J3pTcdOV0DG0$amcL1DWCQqGvhbnnU83bxQEK4pPphDBRKjz8CBNYdN.
~$ mkpasswd --version | head -n1
mkpasswd 5.5.13

My laptop:

$ lsb_release -d
Description:	Ubuntu 22.04.3 LTS
$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt --rounds=11
$y$jFT$TNDsck0dtL/n6LLXNmAeh0$wJycEcIihrHDz5raItc4egQmsVV7YcmM7cifEO7bwm6

@cleberb
Copy link
Contributor Author

cleberb commented Oct 20, 2023

Will debug this...

Test machine:

~$ lsb_release -d
Description:	Ubuntu 22.04.3 LTS
~$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt --rounds=11
crypt: Invalid argument
~$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt
$y$j9T$fKUyYvKRu7J3pTcdOV0DG0$amcL1DWCQqGvhbnnU83bxQEK4pPphDBRKjz8CBNYdN.
~$ mkpasswd --version | head -n1
mkpasswd 5.5.13

My laptop:

$ lsb_release -d
Description:	Ubuntu 22.04.3 LTS
$ echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt --rounds=11
$y$jFT$TNDsck0dtL/n6LLXNmAeh0$wJycEcIihrHDz5raItc4egQmsVV7YcmM7cifEO7bwm6

This is very strange. Functional on my machine and an Ubuntu VM. Is it limited to resource settings (CPU and RAM)?
For yescrypt, it is limited to 11, try reducing this number and see if the error persists, if it does, I will remove the option in the test.

https://medium.com/@slimm609/cost-based-password-hashing-b383bbb355df

@konstruktoid
Copy link
Owner

konstruktoid commented Oct 20, 2023

Yes, reducing the rounds fixed this, tested on VMs and on RPis.
Since the default is 6 rounds, https://github.com/openwall/yescrypt/blob/main/yescrypt.h#L118, I'll suggest we up it to 8 just to be safe.

@konstruktoid
Copy link
Owner

diff --git a/molecule/default/verify.yml b/molecule/default/verify.yml
index 23fadaa..e602e6c 100644
--- a/molecule/default/verify.yml
+++ b/molecule/default/verify.yml
@@ -933,9 +933,9 @@
             - ansible_os_family == "Debian"
 
         - name: Create yescrypt password hash
-          ansible.builtin.shell: |-
+          ansible.builtin.shell: |
             set -o pipefail
-            echo -n '{{ item }}' | mkpasswd --stdin --method=yescrypt --rounds=11 | tr -d '\n'
+            echo -n '{{ item }}' | mkpasswd --stdin --method=yescrypt --rounds=8
           args:
             executable: /bin/bash
           register: mkpasswd
@@ -949,7 +949,7 @@
         - name: Set passwords
           ansible.builtin.set_fact:
             passwords: "{{ mkpasswd.results | map(attribute='stdout') | list }}"
-          when: mkpasswd.rc != 0
+          when: mkpasswd and password_algorithm == "yescrypt"
 
         - name: Create sha512 password hash
           vars:
diff --git a/tasks/password.yml b/tasks/password.yml
index a6965c7..29d41e7 100644
--- a/tasks/password.yml
+++ b/tasks/password.yml
@@ -117,7 +117,7 @@
       ansible.builtin.replace:
         path: "{{ item }}"
         regexp: '(\s+{{ password_algorithm }}\s+(?!rounds=)\S*)(\s+rounds=[0-9]+)*'
-        replace: '\1 rounds={{ "65536" if (password_algorithm == "sha512") else "11" }}'
+        replace: '\1 rounds={{ "65536" if (password_algorithm == "sha512") else "8" }}'
         mode: "0644"
         owner: root
         group: root
diff --git a/templates/etc/pam.d/common-password.j2 b/templates/etc/pam.d/common-password.j2
index 5f85189..9d13203 100644
--- a/templates/etc/pam.d/common-password.j2
+++ b/templates/etc/pam.d/common-password.j2
@@ -2,6 +2,6 @@
 # Generated by Ansible role {{ ansible_role_name }}
 
 password requisite pam_pwquality.so retry=3
-password [success=1 default=ignore] pam_unix.so obscure use_authtok try_first_pass {{ password_algorithm }} rounds={{ '65536' if (password_algorithm == 'sha512') else '11' }} remember={{ password_remember }}
+password [success=1 default=ignore] pam_unix.so obscure use_authtok try_first_pass {{ password_algorithm }} rounds={{ '65536' if (password_algorithm == 'sha512') else '8' }} remember={{ password_remember }}
 password requisite pam_deny.so
 password required pam_permit.so

@cleberb
Copy link
Contributor Author

cleberb commented Oct 20, 2023

The Set rounds task isn't idempotent on AlmaLinux.

Idempotence test failed because of the following tasks:
*  => ansible-role-hardening : Set rounds

Do you have any suggestions for this issue? the old regex for the remember parameter was removing extra predefined parameters.

    - name: Set system-auth remember
      ansible.builtin.replace:
        regexp: use_authtok(\s+.*)
        replace: use_authtok remember=5

@cleberb
Copy link
Contributor Author

cleberb commented Oct 20, 2023

The Set rounds task isn't idempotent on AlmaLinux.

Idempotence test failed because of the following tasks:
*  => ansible-role-hardening : Set rounds

Do you have any suggestions for this issue? the old regex for the remember parameter was removing extra predefined parameters.

    - name: Set system-auth remember
      ansible.builtin.replace:
        regexp: use_authtok(\s+.*)
        replace: use_authtok remember=5

Anyway the regex is wrong:

--- before: /etc/pam.d/password-auth
+++ after: /etc/pam.d/password-auth
@@ -8,7 +8,7 @@
 account     required      pam_unix.so
 
 password    requisite     pam_pwquality.so try_first_pass local_users_only retry=3 authtok_type=
-password    sufficient    pam_unix.so try_first_pass use_authtok sha512 remember=10 shadow rounds=65536
+password    sufficient    pam_unix.so try_first_pass use_authtok sha512 remember=10 rounds=65536 shadow rounds=65536

@konstruktoid
Copy link
Owner

-password    sufficient    pam_unix.so try_first_pass use_authtok sha512 remember=10 shadow rounds=65536
+password    sufficient    pam_unix.so try_first_pass use_authtok sha512 remember=10 rounds=65536 shadow rounds=65536

yeah, this is why the role need to be idempotent but I haven't had the time to dig deeper

@cleberb
Copy link
Contributor Author

cleberb commented Oct 21, 2023

https://docs.ansible.com/ansible/latest/collections/community/general/pamd_module.html

Could using this module be an option? Do you have any observations?

@konstruktoid
Copy link
Owner

There are no issues with using that module. It just seems messy, but if you're okay with working with it

@cleberb
Copy link
Contributor Author

cleberb commented Oct 23, 2023

I made a change to the regex, I believe the replace solved it, but I need you to perform the molecule test to see if it will be identified as idempotent.

@konstruktoid
Copy link
Owner

"Idempotence completed successfully."

@konstruktoid
Copy link
Owner

konstruktoid commented Oct 24, 2023

update the testing.
related: ansible/ansible#82071

diff --git a/molecule/default/verify.yml b/molecule/default/verify.yml
index 9e4fb37..f545b19 100644
--- a/molecule/default/verify.yml
+++ b/molecule/default/verify.yml
@@ -938,49 +938,46 @@
         - name: Create yescrypt password hash
           ansible.builtin.shell: |
             set -o pipefail
-            echo -n '{{ item }}' | mkpasswd --stdin --method=yescrypt --rounds=8
+            echo -n 'Ansible Role Test User' | mkpasswd --stdin --method=yescrypt --rounds=8
           args:
             executable: /bin/bash
           register: mkpasswd
           changed_when: false
           failed_when: mkpasswd.rc != 0
-          loop:
-            - 'Ansible Role Test User'
-            - 'Change Ansible Role Test User'
           when: password_algorithm == "yescrypt"
 
         - name: Set passwords
           ansible.builtin.set_fact:
-            passwords: "{{ mkpasswd.results | map(attribute='stdout') | list }}"
-          when:
-            - password_algorithm == "yescrypt"
-            - mkpasswd.rc == 0
+            passwords: "{{ mkpasswd.stdout }}"
+          when: password_algorithm == "yescrypt"
 
         - name: Create sha512 password hash
           vars:
             salt: "{{ lookup('password', '/dev/null chars=ascii_lowercase,ascii_uppercase,digits length=16') }}"
           ansible.builtin.set_fact:
-            passwords: "{{ passwords | default([]) | union([item | password_hash('sha512', salt, rounds=65536)]) }}"
-          loop:
-            - 'Ansible Role Test User'
-            - 'Change Ansible Role Test User'
+            passwords: "{{ 'Ansible Role Test User' | password_hash('sha512', salt, rounds=656000) }}"
           when: password_algorithm == "sha512"
 
+        - name: Print passwords
+          ansible.builtin.debug:
+            msg: "{{ passwords }}"
+          when: passwords
+
     - name: Create test user
       become: true
       ansible.builtin.user:
         name: roletestuser
-        password: "{{ passwords[0] }}"
+        password: "{{ passwords }}"
         state: present
         shell: /bin/bash
 
-    - name: Change test user password
+    - name: Get test user password
       become: true
-      ansible.builtin.user:
-        name: roletestuser
-        password: "{{ passwords[1] }}"
-      register: test_user_pass
+      ansible.builtin.command:
+        cmd: grep roletestuser /etc/shadow
+      register: roletestuser_pass
+      changed_when: false
 
-    - name: Debug test user password
+    - name: Print test user
       ansible.builtin.debug:
-        msg: "{{ test_user_pass }}"
+        msg: "{{ roletestuser_pass.stdout }}"

molecule/default/verify.yml Outdated Show resolved Hide resolved
molecule/default/verify.yml Show resolved Hide resolved
@konstruktoid
Copy link
Owner

Thanks again!

@konstruktoid konstruktoid merged commit d1b2e5f into konstruktoid:master Oct 25, 2023
1 check passed
@cleberb cleberb deleted the password_improvements branch October 26, 2023 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants