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

Bugfix 5 3 4 against issue #176 #177

Merged
merged 4 commits into from
Feb 19, 2024

Conversation

RoboPickle
Copy link
Contributor

@RoboPickle RoboPickle commented Feb 15, 2024

Overall Review

tasks/main.yml was altered to handle the user not being found in /etc/shadow.
The grep has been changed.

(grep {{ ansible_env.SUDO_USER }} /etc/shadow || echo 'not found:not found') | awk -F: '{print $2}'

In the event that the user is not found in /etc/shadow then the response "not found:not found" is given. This is then picked up by the following block to skip the password checking. A debug message is used to alert the user that this is happening.

An additional check is now being made for local accounts to check that the account has not been locked. This is achieved by checking for a single "!" at the beginning of the password hash.

Also updated dictionary in 6.1.11 to convert vars to dictionary to avoid deprecation warning.

Issue Fixes:

Fixes #176: Using an AD account to connect to host incorrectly fails rule 5.3.4

Enhancements:

Fixes #168 DEPRECATION WARNING is generating when play task 6.1.11 | AUDIT | Ensure no ungrouped files or directories exist

How has this been tested?:

Tested against a minimally configured Alma Linux 9 VM.

  1. Created a new user with an unset password.
    • Local account check passes. Unset password is then detected and play fails.
  2. Add a password to new user.
    • Local account check passes. Password check then passes and play continues.
  3. Lock the account. Password hash now starts with "!"
    • Second password check is triggered and play fails.
  4. Updated VM to join AD realm. Use AD account.
    • The first local account check returns not found. Local password checks are skipped and play continues.

Addendum:
6.1.11 Deprecation warning during audit. Updated vars to dictionary and deprecation warning avoided.

More details on testing are available if required.

With an AD account there isn't an entry in the /etc/shadow file. This
caused the password length check to treat it as a zero length password.
Now local password check is skipped for AD account.
Also added an additional check for a locked local account for the sudo
user.

Signed-off-by: John Foster <robopickle@proton.me>
Signed-off-by: John Foster <robopickle@proton.me>
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

findings in issue ansible-lockdown#168. Changed vars on line 233 to use dictionary.

Signed-off-by: John Foster <robopickle@proton.me>
@uk-bolly
Copy link
Member

hi @RoboPickle

This is a great update, thank you for taking the time, could you possibly add the empty line before each named task just for consistency.
You will find the vars deprecation should have already nbeen picked up as well in and earlier PR.

Many thanks

uk-bolly

Signed-off-by: John Foster <robopickle@proton.me>
Copy link
Member

@uk-bolly uk-bolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent thank you for your time

@RoboPickle
Copy link
Contributor Author

RoboPickle commented Feb 19, 2024 via email

@uk-bolly uk-bolly merged commit 96536cc into ansible-lockdown:devel Feb 19, 2024
4 checks passed
@RoboPickle RoboPickle deleted the bugfix_5_3_4 branch February 19, 2024 12:20
@uk-bolly uk-bolly mentioned this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants