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

Invalid Conditionals in user_accounts.yml #255

Closed
ljkimmel opened this issue Jan 17, 2020 · 2 comments · Fixed by #258
Closed

Invalid Conditionals in user_accounts.yml #255

ljkimmel opened this issue Jan 17, 2020 · 2 comments · Fixed by #258
Labels

Comments

@ljkimmel
Copy link
Contributor

I'm a bit new to Ansible but experienced with coding and CM tools in general so I could be wrong here...
It appears that the check for the UID_MIN value in login.defs assigns a data object (JSON) to the 'uid_min' variable regardless of whether the file exists or not. Therefore, the test for 'when: uid_min is defined' ALWAYS passes.

The other tests for 'when: not uid_min' NEVER pass since the variable always has some value.

Also, even if the two tasks to set 'uid_max' were able to run, they would likely always set the value for ANY system to 499. Even though Debian systems have more specific logic that task comes first so the value may get set to 999 but the very next task will still set it to 499 when login.defs does not exist.

@rndmh3ro
Copy link
Member

Hey @ljkimmel,

thanks your taking time to look at this!

it seems you're right. I just tested it with a non-existing login.defs file:

PLAY [localhost] *******************************************************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************************************************************
ok: [localhost]

TASK [get UID_MIN from login.defs] *************************************************************************************************************************************************************
ok: [localhost] => {"changed": false, "cmd": "awk '/^\\s*UID_MIN\\s*([0-9]*).*?$/ {print $2}' /etc/login.defsa", "rc": 0, "stdout": "skipped, since /etc/login.defsa does not exist", "stdout_lines": ["skipped, since /etc/login.defsa does not exist"]}

TASK [debug] ***********************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": {
        "changed": false,
        "cmd": "awk '/^\\s*UID_MIN\\s*([0-9]*).*?$/ {print $2}' /etc/login.defsa",
        "failed": false,
        "rc": 0,
        "stdout": "skipped, since /etc/login.defsa does not exist",
        "stdout_lines": [
            "skipped, since /etc/login.defsa does not exist"
        ]
    }
}

TASK [calculate UID_MAX from UID_MIN by substracting 1] ****************************************************************************************************************************************
ok: [localhost] => {"ansible_facts": {"uid_max": "-1"}, "changed": false}

TASK [debug] ***********************************************************************************************************************************************************************************
ok: [localhost] => {
    "msg": "-1"
}

PLAY RECAP *************************************************************************************************************************************************************************************
localhost                  : ok=5    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=

I guess the Ansible-logic changed here since writing this code.

Also, even if the two tasks to set 'uid_max' were able to run, they would likely always set the value for ANY system to 499. Even though Debian systems have more specific logic that task comes first so the value may get set to 999 but the very next task will still set it to 499 when login.defs does not exist.

Yes, that's the conclusion of the above bug. :(

We'll have to fix this. Do you want to try yourself at this?

@ljkimmel
Copy link
Contributor Author

I'll take a look at it.

ljkimmel added a commit to ljkimmel/ansible-os-hardening that referenced this issue Jan 20, 2020
- Added logic to pull uid_min from login.defs when it returns an
  integer value greater than 0.
- Add logic so that Debian systems use inherit uid_max=999 when
  a uid_max was not found in login.defs.
- Add logic so that all other systems inherit uid_max=499 only
  when the value was not already found in login.defs or set for
  Debian systems.
ljkimmel added a commit to ljkimmel/ansible-os-hardening that referenced this issue Feb 13, 2020
- Added logic to pull uid_min from login.defs when it returns an
  integer value greater than 0.
- Add logic so that Debian systems use inherit uid_max=999 when
  a uid_max was not found in login.defs.
- Add logic so that all other systems inherit uid_max=499 only
  when the value was not already found in login.defs or set for
  Debian systems.

Signed-off-by: Lesley Kimmel <lesley.j.kimmel@gmail.com>
rndmh3ro pushed a commit that referenced this issue Feb 13, 2020
- Added logic to pull uid_min from login.defs when it returns an
  integer value greater than 0.
- Add logic so that Debian systems use inherit uid_max=999 when
  a uid_max was not found in login.defs.
- Add logic so that all other systems inherit uid_max=499 only
  when the value was not already found in login.defs or set for
  Debian systems.

Signed-off-by: Lesley Kimmel <lesley.j.kimmel@gmail.com>
@rndmh3ro rndmh3ro added the bug label May 5, 2020
divialth pushed a commit to divialth/ansible-collection-hardening that referenced this issue Aug 3, 2022
- Added logic to pull uid_min from login.defs when it returns an
  integer value greater than 0.
- Add logic so that Debian systems use inherit uid_max=999 when
  a uid_max was not found in login.defs.
- Add logic so that all other systems inherit uid_max=499 only
  when the value was not already found in login.defs or set for
  Debian systems.

Signed-off-by: Lesley Kimmel <lesley.j.kimmel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants