RHEL8: Fix rules breaking sudo command by yuumasato · Pull Request #9024 · ComplianceAsCode/content

yuumasato · 2022-06-23T14:40:52Z

Description:

Rule dir_permissions_library_dirs is about preventing group-writable or world-writable directories in the library dirs.
The Suid bits and the stick bit don't need to be stripped.
Rule sssd_enable_smartcards was breaking the PAM stack in a way that non-root users were not able to execute commands with sudo. The commands are failing with Sorry, try again 3 times.

Rationale:

Stripin suid bit from /usr/bin/sudo breaks it:
sudo: /usr/bin/sudo must be owned by uid 0 and have the setuid bit set
Related to File permissions library dirs shall act on files when 'file_regex' is set #8420
Related to Update rule to allow special bits in library dirs #8768

Rule dir_permissions_library_dirs is about preventing group-writable or world-writable directories in the library dirs. The Suid bits and the stick bit don't need to be stripped.

github-actions · 2022-06-23T15:07:57Z

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

github-actions · 2022-06-23T15:09:50Z

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff

bash remediation for rule 'xccdf_org.ssgproject.content_rule_dir_permissions_library_dirs' differs:
--- old datastream
+++ new datastream
@@ -2,10 +2,10 @@
 
 
 
-find -H /lib/ -perm /u+s,g+ws,o+wt -type d -exec chmod u-s,g-ws,o-wt {} \;
+find -H /lib/ -perm /g+w,o+w -type d -exec chmod g-w,o-w {} \;
 
-find -H /lib64/ -perm /u+s,g+ws,o+wt -type d -exec chmod u-s,g-ws,o-wt {} \;
+find -H /lib64/ -perm /g+w,o+w -type d -exec chmod g-w,o-w {} \;
 
-find -H /usr/lib/ -perm /u+s,g+ws,o+wt -type d -exec chmod u-s,g-ws,o-wt {} \;
+find -H /usr/lib/ -perm /g+w,o+w -type d -exec chmod g-w,o-w {} \;
 
-find -H /usr/lib64/ -perm /u+s,g+ws,o+wt -type d -exec chmod u-s,g-ws,o-wt {} \;
+find -H /usr/lib64/ -perm /g+w,o+w -type d -exec chmod g-w,o-w {} \;

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_dir_permissions_library_dirs' differs:
--- old datastream
+++ new datastream
@@ -3,7 +3,7 @@
 path: /lib/
 state: directory
 recurse: true
- mode: u-s,g-ws,o-wt
+ mode: g-w,o-w
 tags:
 - CCE-88692-9
 - DISA-STIG-RHEL-08-010331
@@ -22,7 +22,7 @@
 path: /lib64/
 state: directory
 recurse: true
- mode: u-s,g-ws,o-wt
+ mode: g-w,o-w
 tags:
 - CCE-88692-9
 - DISA-STIG-RHEL-08-010331
@@ -41,7 +41,7 @@
 path: /usr/lib/
 state: directory
 recurse: true
- mode: u-s,g-ws,o-wt
+ mode: g-w,o-w
 tags:
 - CCE-88692-9
 - DISA-STIG-RHEL-08-010331
@@ -60,7 +60,7 @@
 path: /usr/lib64/
 state: directory
 recurse: true
- mode: u-s,g-ws,o-wt
+ mode: g-w,o-w
 tags:
 - CCE-88692-9
 - DISA-STIG-RHEL-08-010331

OVAL definition oval:ssg-sssd_enable_smartcards:def:1 differs:
--- old datastream
+++ new datastream
- criterion oval:ssg-test_sssd_enable_smartcards_cert_auth_smartcard_auth:tst:1
bash remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_smartcards' differs:
--- old datastream
+++ new datastream
@@ -31,90 +31,9 @@
 
 if [ -f /usr/bin/authselect ]; then
 if authselect check; then
- CURRENT_PROFILE=$(authselect current -r | awk '{ print $1 }')
- # Standard profiles delivered with authselect should not be modified.
- # If not already in use, a custom profile is created preserving the enabled features.
- if [[ ! $CURRENT_PROFILE == custom/* ]]; then
- ENABLED_FEATURES=$(authselect current | tail -n+3 | awk '{ print $2 }')
- authselect create-profile hardening -b $CURRENT_PROFILE
- CURRENT_PROFILE="custom/hardening"
- # Ensure a backup before changing the profile
- authselect apply-changes -b --backup=before-pwhistory-hardening.backup
- authselect select $CURRENT_PROFILE
- for feature in $ENABLED_FEATURES; do
- authselect enable-feature $feature;
- done
- fi
- # Include the desired configuration in the custom profile
- CUSTOM_PROFILE_DIR="/etc/authselect/$CURRENT_PROFILE"
- # The line should be included on the top of postlogin file
 
- if [ -e "$CUSTOM_PROFILE_DIR/smartcard-auth" ] ; then
- valueRegex="" defaultValue=""
- # non-empty values need to be preceded by an equals sign
- [ -n "${valueRegex}" ] && valueRegex="=${valueRegex}"
- # add an equals sign to non-empty values
- [ -n "${defaultValue}" ] && defaultValue="=${defaultValue}"
-
- # fix 'type' if it's wrong
- if grep -q -P "^\\s*(?"'!'"auth\\s)[[:alnum:]]+\\s+[[:alnum:]]+\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/smartcard-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*)[[:alnum:]]+(\\s+[[:alnum:]]+\\s+pam_sss.so)/\\1auth\\2/" "$CUSTOM_PROFILE_DIR/smartcard-auth"
- fi
-
- # fix 'control' if it's wrong
- if grep -q -P "^\\s*auth\\s+(?"'!'"sufficient)[[:alnum:]]+\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/smartcard-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+)[[:alnum:]]+(\\s+pam_sss.so)/\\1sufficient\\2/" "$CUSTOM_PROFILE_DIR/smartcard-auth"
- fi
-
- # fix the value for 'option' if one exists but does not match 'valueRegex'
- if grep -q -P "^\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s+try_cert_auth(?"'!'"${valueRegex}(\\s|\$))" < "$CUSTOM_PROFILE_DIR/smartcard-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s)try_cert_auth=[^[:space:]]*/\\1try_cert_auth${defaultValue}/" "$CUSTOM_PROFILE_DIR/smartcard-auth"
-
- # add 'option=default' if option is not set
- elif grep -q -E "^\\s*auth\\s+sufficient\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/smartcard-auth" &&
- grep -E "^\\s*auth\\s+sufficient\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/smartcard-auth" | grep -q -E -v "\\stry_cert_auth(=|\\s|\$)" ; then
-
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+sufficient\\s+pam_sss.so[^\\n]*)/\\1 try_cert_auth${defaultValue}/" "$CUSTOM_PROFILE_DIR/smartcard-auth"
- # add a new entry if none exists
- elif ! grep -q -P "^\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s+try_cert_auth${valueRegex}(\\s|\$)" < "$CUSTOM_PROFILE_DIR/smartcard-auth" ; then
- echo "auth sufficient pam_sss.so try_cert_auth${defaultValue}" >> "$CUSTOM_PROFILE_DIR/smartcard-auth"
- fi
-else
- echo "$CUSTOM_PROFILE_DIR/smartcard-auth doesn't exist" >&2
-fi
- if [ -e "$CUSTOM_PROFILE_DIR/system-auth" ] ; then
- valueRegex="" defaultValue=""
- # non-empty values need to be preceded by an equals sign
- [ -n "${valueRegex}" ] && valueRegex="=${valueRegex}"
- # add an equals sign to non-empty values
- [ -n "${defaultValue}" ] && defaultValue="=${defaultValue}"
-
- # fix 'type' if it's wrong
- if grep -q -P "^\\s*(?"'!'"auth\\s)[[:alnum:]]+\\s+[[:alnum:]]+\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/system-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*)[[:alnum:]]+(\\s+[[:alnum:]]+\\s+pam_sss.so)/\\1auth\\2/" "$CUSTOM_PROFILE_DIR/system-auth"
- fi
-
- # fix 'control' if it's wrong
- if grep -q -P "^\\s*auth\\s+(?"'!'"sufficient)[[:alnum:]]+\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/system-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+)[[:alnum:]]+(\\s+pam_sss.so)/\\1sufficient\\2/" "$CUSTOM_PROFILE_DIR/system-auth"
- fi
-
- # fix the value for 'option' if one exists but does not match 'valueRegex'
- if grep -q -P "^\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s+try_cert_auth(?"'!'"${valueRegex}(\\s|\$))" < "$CUSTOM_PROFILE_DIR/system-auth" ; then
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s)try_cert_auth=[^[:space:]]*/\\1try_cert_auth${defaultValue}/" "$CUSTOM_PROFILE_DIR/system-auth"
-
- # add 'option=default' if option is not set
- elif grep -q -E "^\\s*auth\\s+sufficient\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/system-auth" &&
- grep -E "^\\s*auth\\s+sufficient\\s+pam_sss.so" < "$CUSTOM_PROFILE_DIR/system-auth" | grep -q -E -v "\\stry_cert_auth(=|\\s|\$)" ; then
-
- sed --follow-symlinks -i -E -e "s/^(\\s*auth\\s+sufficient\\s+pam_sss.so[^\\n]*)/\\1 try_cert_auth${defaultValue}/" "$CUSTOM_PROFILE_DIR/system-auth"
- # add a new entry if none exists
- elif ! grep -q -P "^\\s*auth\\s+sufficient\\s+pam_sss.so(\\s.+)?\\s+try_cert_auth${valueRegex}(\\s|\$)" < "$CUSTOM_PROFILE_DIR/system-auth" ; then
- echo "auth sufficient pam_sss.so try_cert_auth${defaultValue}" >> "$CUSTOM_PROFILE_DIR/system-auth"
- fi
-else
- echo "$CUSTOM_PROFILE_DIR/system-auth doesn't exist" >&2
-fi
+ authselect enable-feature with-smartcard
+ 
 authselect apply-changes -b --backup=after-pwhistory-hardening.backup
 else
 echo "

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_sssd_enable_smartcards' differs:
--- old datastream
+++ new datastream
@@ -174,117 +174,22 @@
 when:
 - result_authselect_check_cmd is success
 
- - name: Define the current authselect profile as a local fact
- ansible.builtin.set_fact:
- authselect_current_profile: '{{ result_authselect_profile.stdout }}'
- authselect_custom_profile: '{{ result_authselect_profile.stdout }}'
- when:
- - result_authselect_profile is not skipped
- - result_authselect_profile.stdout is match("custom/")
-
- - name: Define the new authselect custom profile as a local fact
- ansible.builtin.set_fact:
- authselect_current_profile: '{{ result_authselect_profile.stdout }}'
- authselect_custom_profile: custom/hardening
- when:
- - result_authselect_profile is not skipped
- - result_authselect_profile.stdout is not match("custom/")
-
- - name: Get authselect current features to also enable them in the custom profile
+ - name: Get authselect current features
 ansible.builtin.shell:
 cmd: authselect current | tail -n+3 | awk '{ print $2 }'
 register: result_authselect_features
 changed_when: false
 when:
+ - result_authselect_check_cmd is success
 - result_authselect_profile is not skipped
- - authselect_current_profile is not match("custom/")
-
- - name: Check if any custom profile with the same name was already created in the
- past
- ansible.builtin.stat:
- path: /etc/authselect/{{ authselect_custom_profile }}
- register: result_authselect_custom_profile_present
- changed_when: false
- when:
- - authselect_current_profile is not match("custom/")
-
- - name: Create a custom profile based on the current profile
+
+ - name: Ensure smartcards are enabled via authselect tool
 ansible.builtin.command:
- cmd: authselect create-profile hardening -b sssd
- when:
- - result_authselect_check_cmd is success
- - authselect_current_profile is not match("custom/")
- - not result_authselect_custom_profile_present.stat.exists
-
- - name: Ensure the desired configuration is present in the custom profile smartcard-auth
- ansible.builtin.lineinfile:
- create: true
- dest: /etc/authselect/{{ authselect_custom_profile }}/smartcard-auth
- insertbefore: ^session.*
- firstmatch: true
- regexp: ^(\s*auth.*pam_sss\.so)$
- line: auth sufficient pam_sss.so try_cert_auth
- when:
+ cmd: authselect enable-feature with-smartcard
+ when:
+ - result_authselect_check_cmd is success
+ - result_authselect_features.stdout is not search("with-smartcard")
 - result_authselect_profile is not skipped
-
- - name: Ensure the desired configuration is present in the custom profile system-auth
- ansible.builtin.lineinfile:
- create: true
- dest: /etc/authselect/{{ authselect_custom_profile }}/system-auth
- insertbefore: ^session.*
- firstmatch: true
- regexp: ^(\s*auth.*pam_sss\.so)$
- line: auth sufficient pam_sss.so try_cert_auth
- when:
- - result_authselect_profile is not skipped
-
- - name: Ensure the try_cert_auth option is in smartcard-auth
- ansible.builtin.replace:
- dest: /etc/authselect/{{ authselect_custom_profile }}/smartcard-auth
- regexp: ^(\s*auth.*pam_sss\.so)((?!try_cert_auth).)*$
- replace: \g<1> try_cert_auth \g<2>
- when:
- - result_authselect_profile is not skipped
- - result_pam_try_cert_auth_present_smartcard_auth.found == 0
-
- - name: Ensure the try_cert_auth option is in system-auth
- ansible.builtin.replace:
- dest: /etc/authselect/{{ authselect_custom_profile }}/system-auth
- regexp: ^(\s*auth.*pam_sss\.so)((?!try_cert_auth).)*$
- replace: \g<1> try_cert_auth \g<2>
- when:
- - result_authselect_profile is not skipped
- - result_pam_try_cert_auth_present_system_auth.found == 0
-
- - name: Ensure a backup of current authselect profile before selecting the custom
- profile
- ansible.builtin.command:
- cmd: authselect apply-changes -b --backup=before-pwhistory-hardening.backup
- register: result_authselect_backup
- when:
- - result_authselect_check_cmd is success
- - result_authselect_profile is not skipped
- - authselect_current_profile is not match("custom/")
- - authselect_custom_profile is not match(authselect_current_profile)
-
- - name: Ensure the custom profile is selected
- ansible.builtin.command:
- cmd: authselect select {{ authselect_custom_profile }} --force
- register: result_pam_authselect_select_profile
- when:
- - result_authselect_check_cmd is success
- - result_authselect_profile is not skipped
- - authselect_current_profile is not match("custom/")
- - authselect_custom_profile is not match(authselect_current_profile)
-
- - name: Restore the authselect features in the custom profile
- ansible.builtin.command:
- cmd: authselect enable-feature {{ item }}
- loop: '{{ result_authselect_features.stdout_lines }}'
- when:
- - result_authselect_profile is not skipped
- - result_authselect_features is not skipped
- - result_pam_authselect_select_profile is not skipped
 
 - name: Ensure the custom profile changes are applied
 ansible.builtin.command:

Use authselect enable-feature with-smartcard to remediate rule sssd_enable_smartcards. The Ansible remediation for this rule is breaking the pam stack. 'sudo' stops working after Ansible task is applied. This commit switches the RHEL8 to use authselect with-smartcard. I'm aware this moves the rule out of alignment with DISA, but it is better than having broken 'sudo'. The alignment needs to be discussed with DISA and fixed later.

yuumasato · 2022-06-24T16:50:41Z

So fixing the 'sudo' issue caused by dir_permission_library_dirs revealed another issue caused by sssd_enable_smartcards.
I've added a temporary fix for the rule for RHEL8.
The commit switches the remediation of the rule to use authselect enable-feature with-smartcard instead of creating a custom profile. This unfortunately moves the rule out of alignment with DISA Benchmark.

The misalignment needs to be further discussed, maybe we add the check for smartcard-auth or we convice DISA to not require change to it.

qlty-cloud-legacy · 2022-06-24T17:17:30Z

Code Climate has analyzed commit 07483af 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 42.7% (0.0% change).

View more on Code Climate.

openshift-ci · 2022-06-24T19:21:38Z

@yuumasato: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-rhcos4-moderate	`07483af`	link	true	`/test e2e-aws-rhcos4-moderate`

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

mildas · 2022-06-27T08:12:19Z

LGTM

mildas · 2022-06-27T08:19:10Z

sssd_enable_smartcards errors in CI because it expects running sssd service. That's not possible as it runs in container.

Tests passed on my local VM, merging.

marcusburghardt · 2022-06-27T09:53:59Z

Also related to #8912 issue.

jseiser · 2022-07-26T15:01:46Z

@yuumasato

Im not following what the fix is here?

We use a downstream Ansible role, built from this repo.

They just updated to release 0.1.62 and the RHEL 8 roles now fail to run.

sudo: /usr/bin/sudo must be owned by uid 0 and have the setuid bit set

Which is this code block:

- name: Set permissions for /lib/ recursively
  file:
    path: /lib/
    state: directory
    recurse: true
    mode: u-s,g-ws,o-wt
  tags:
  - CCE-88692-9
  - DISA-STIG-RHEL-08-010331
  - NIST-800-53-CM-5
  - NIST-800-53-CM-5(6)
  - NIST-800-53-CM-5(6).1
  - configure_strategy
  - dir_permissions_library_dirs
  - low_complexity
  - low_disruption
  - medium_severity
  - no_reboot_needed
  when:
  - configure_strategy | bool
  - dir_permissions_library_dirs | bool
  - low_complexity | bool
  - low_disruption | bool
  - medium_severity | bool
  - no_reboot_needed | bool

I have no problem shimming around this issue, until 0.1.63, but im not understanding what was the change done on this MR to fix the issue?

yuumasato · 2022-07-26T16:22:09Z

@jseiser The fix relevant for rule dir_permissions_library_dirs is dbdbb40.
It changes mode applied in the remediation from mode: u-s,g-ws,o-wt to mode: g-ws,o-wt, so it doesn't remove the setuid bits anymore.

In /lib/.build/ there are symlinks to binaries in the system, and the rule was going through them and breaking sudo command.

Similar change was done in #8768.
And #8420 may help you understand the context even more.

Update rule to allow special bits in library dirs

dbdbb40

Rule dir_permissions_library_dirs is about preventing group-writable or world-writable directories in the library dirs. The Suid bits and the stick bit don't need to be stripped.

yuumasato requested a review from mildas June 23, 2022 14:40

mildas self-assigned this Jun 23, 2022

yuumasato changed the title ~~Update dir_permisssions_library_dirs to allow special bits in library dirs~~ RHEL8: Fix rules breaking sudo command Jun 27, 2022

mildas approved these changes Jun 27, 2022

View reviewed changes

mildas merged commit 14f10f5 into ComplianceAsCode:master Jun 27, 2022

yuumasato deleted the allow_special_bits_dir_permissions_library_dirs branch June 30, 2022 11:56

yuumasato added this to the 0.1.63 milestone Jun 30, 2022

jseiser mentioned this pull request Aug 9, 2022

Compliance As Code 0.1.63 Released RedHatOfficial/ansible-role-rhel8-stig#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RHEL8: Fix rules breaking sudo command#9024

RHEL8: Fix rules breaking sudo command#9024
mildas merged 2 commits intoComplianceAsCode:masterfrom
yuumasato:allow_special_bits_dir_permissions_library_dirs

yuumasato commented Jun 23, 2022 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 23, 2022

Uh oh!

github-actions Bot commented Jun 23, 2022 •

edited

Loading

Uh oh!

yuumasato commented Jun 24, 2022

Uh oh!

qlty-cloud-legacy Bot commented Jun 24, 2022

Uh oh!

openshift-ci Bot commented Jun 24, 2022

Uh oh!

mildas commented Jun 27, 2022

Uh oh!

mildas commented Jun 27, 2022

Uh oh!

marcusburghardt commented Jun 27, 2022

Uh oh!

jseiser commented Jul 26, 2022

Uh oh!

yuumasato commented Jul 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

yuumasato commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Rationale:

Uh oh!

github-actions Bot commented Jun 23, 2022

Uh oh!

github-actions Bot commented Jun 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuumasato commented Jun 24, 2022

Uh oh!

qlty-cloud-legacy Bot commented Jun 24, 2022

Uh oh!

openshift-ci Bot commented Jun 24, 2022

Uh oh!

mildas commented Jun 27, 2022

Uh oh!

mildas commented Jun 27, 2022

Uh oh!

marcusburghardt commented Jun 27, 2022

Uh oh!

jseiser commented Jul 26, 2022

Uh oh!

yuumasato commented Jul 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yuumasato commented Jun 23, 2022 •

edited

Loading

github-actions Bot commented Jun 23, 2022 •

edited

Loading