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

Add missing variable defaults for 'rhel9cis_pam_faillock' #12

Merged
merged 3 commits into from
May 6, 2022

Conversation

alewando
Copy link

@alewando alewando commented May 5, 2022

Some required variables in the 'rhel9cis_pam_faillock' dict were missing default values. I copied the defaults from the RHEL8-CIS playbook

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.

@uk-bolly
Copy link
Member

uk-bolly commented May 6, 2022

hi @alewando

Thank you so much for the feedback and taking the time to provide a solution. I have just double checked that config and some of it is there in error for rhel9 as this uses the /etc/security/faillock.conf configuration.
As well as it still mentioned RH8 and we have been moving to adopt more of the rh9 specific steps. It appears 'attempts' and 'fail_for_root' is no longer configured.
It also highlighted the fact they are hardcoded for the particular control, so we do need to move to use some of the variables you have added.

The only variable we should now required are
rhel9cis_pam_faillock:
unlock_time: 900
deny: 5

If you could align this I can merge and then add extra improvements that this has highlighted.

I do expect the pipeline test to fail until rh9 is GA and we have an image to use.

many thanks once again

uk-bolly

@uk-bolly
Copy link
Member

uk-bolly commented May 6, 2022

Feel free to join us on Discord https://discord.gg/JFxpSgPFEJ

Be great to get some more discussions on this yet unreleased benchmark.

Thanks

uk-bolly

@alewando
Copy link
Author

alewando commented May 6, 2022

Ah, I see now the ansible_distribution_version check!
I ran into this because I am actually trying to use this playbook against Amazon Linux 2022, since it's based on Fedora 34 and I figured RHEL9 was close enough for a try. On AL2022, ansible_distribution_version is 2022, and I guess python thinks that "2022" is less than "8.1" when they are strings:

>>> "2022" <= "8.1"
True
>>> 2022 <= 8.1
False
>>>

What you are saying makes sense. I've pushed the requested update. I'll need to be more careful about the version checks! Thanks for all of your efforts!

@uk-bolly
Copy link
Member

uk-bolly commented May 6, 2022

hi @alewando

Thank you for confirming and the rapid turn around. I did notice the signed-off is present but its failing DCO on the GPG sign off.
Any chance you can add that?

Good to know this works on Amazon Linux 2022, keen to find our if you come across any specific there also.

best

uk-bolly

Adam Lewandowski added 3 commits May 6, 2022 11:04
Signed-off-by: Adam Lewandowski <adam.lewandowski@plxis.com>
Signed-off-by: Adam Lewandowski <adam.lewandowski@plxis.com>
…and 5.5.4

Signed-off-by: Adam Lewandowski <adam.lewandowski@plxis.com>
@uk-bolly uk-bolly merged commit 0348777 into ansible-lockdown:devel May 6, 2022
@uk-bolly
Copy link
Member

uk-bolly commented May 6, 2022

Thanks again for the feedback and subsequent PR @alewando

Thats all merged now.

Cheers

uk-bolly

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.

2 participants