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

Fix Ubuntu remediation for pam_faillock rules #11488

Merged
merged 3 commits into from
Jan 29, 2024

Conversation

mpurg
Copy link
Contributor

@mpurg mpurg commented Jan 26, 2024

Description:

  • Fix Ubuntu remediations for pam_faillock rules to correctly position pam_faillock authfail module in modified pam files.
  • Fix Ubuntu remediations for pam_faillock rules to add pam_faillock preauth above the Primary block instead of above pam_unix.
  • Clean up and add a few more tests.
  • This affects rules in Ubuntu 22.04 CIS and Ubuntu 20.04 STIG

Rationale:

  • The remediation wrongly positions the pam_faillock authfail module when the pam file contains commented or empty lines, potentially locking the system.
  • Placing the pam_faillock preauth module directly above pam_unix interferes with modules from other rules, specifically, pam_pkcs11 in smartcard_pam_enabled.

Example /etc/pam.d/common-auth on Ubuntu 22.04 after remediation:

# here are the per-package modules (the "Primary" block)
auth        required      pam_faillock.so preauth
auth    [success=2 default=ignore]      pam_unix.so nullok
# random user comment 
auth        [default=die]      pam_faillock.so authfail
auth        sufficient      pam_faillock.so authsucc
auth    [success=1 default=ignore]      pam_sss.so use_first_pass
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so

After fix:

auth        required      pam_faillock.so preauth
# here are the per-package modules (the "Primary" block)
auth    [success=2 default=ignore]      pam_unix.so nullok
# random user comment 
auth    [success=1 default=ignore]      pam_sss.so use_first_pass
auth        [default=die]      pam_faillock.so authfail
auth        sufficient      pam_faillock.so authsucc
# here's the fallback if no module succeeds
auth    requisite                       pam_deny.so

The line `auth required pam_faillock.so preauth` is now added
above the Primary block instead of above pam_unix.so. This
prevents it from interfering with modules from other rules,
specifically, pam_pkcs11.so in smartcard_pam_enabled.
The macro now ignores commented and empty lines when
determining the position for pam_faillock authfail
in the pam stack.
Cleaned up the comments in pam template and
added missing tests for remediation.
@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Jan 26, 2024
Copy link

openshift-ci bot commented Jan 26, 2024

Hi @mpurg. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

Copy link

codeclimate bot commented Jan 26, 2024

Code Climate has analyzed commit c538c20 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 58.5% (0.0% change).

View more on Code Climate.

@marcusburghardt marcusburghardt added Ubuntu Ubuntu product related. Bash Bash remediation update. labels Jan 29, 2024
@dodys dodys requested a review from a team January 29, 2024 10:55
@dodys dodys self-assigned this Jan 29, 2024
@dodys dodys added ok-to-test Used by openshift-ci bot. and removed needs-ok-to-test Used by openshift-ci bot. labels Jan 29, 2024
Copy link
Contributor

@dodys dodys left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@dodys dodys merged commit 587aa83 into ComplianceAsCode:master Jan 29, 2024
41 of 43 checks passed
@Mab879 Mab879 added this to the 0.1.73 milestone Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bash Bash remediation update. ok-to-test Used by openshift-ci bot. Ubuntu Ubuntu product related.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants