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.

Sign up for GitHub

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

Fixing issue https://github.com/ansible-lockdown/AMAZON2023-CIS/issues/26 #27

Closed
wants to merge 42 commits into from
Closed

Fixing issue https://github.com/ansible-lockdown/AMAZON2023-CIS/issues/26 #27

Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
830d42c
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 6, 2023
269b56b
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 6, 2023
9e7ef28
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 6, 2023
e3ffb8b
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 6, 2023
592fbbd
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 7, 2023
7bc0c12
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 20, 2023
9a4d9fb
Merge pull request #21 from siemens/siemens/feat/r_2.1.2_chrony_config
uk-bolly Jan 26, 2024
758bb04
Merge pull request #25 from siemens/siemens/feat/r_4.2.20_clientalive…
uk-bolly Jan 26, 2024
53b254a
Merge pull request #29 from siemens/siemens/feat/r_1.6.1.x_r_1.9_impo…
uk-bolly Jan 26, 2024
710425b
Removing trailing whitespaces
DianaMariaDDM Jan 30, 2024
95857f7
Removing trailing whitespaces
DianaMariaDDM Jan 30, 2024
9488e19
Removing trailing whitespaces and fixing an end-of-file
DianaMariaDDM Jan 30, 2024
a95bdb1
Merge pull request #23 from siemens/siemens/feat/r_2.2.17_masking_ser…
uk-bolly Jan 30, 2024
75ea3ec
Merge pull request #31 from siemens/siemens/feat/r_4.2.x_ssh_conf_files
uk-bolly Jan 30, 2024
6a3c7ec
Refactoring docs
DianaMariaDDM Feb 1, 2024
4a7ce35
Small fixings for https://code.siemens.com/infosec-pss-gov/security-c…
DianaMariaDDM Feb 14, 2024
c28b8a4
Removing trailing whitespace
DianaMariaDDM Feb 14, 2024
8bf9197
Fixing fail message so that is states the correct number of the rule …
DianaMariaDDM Feb 14, 2024
f5ec60c
Fixing inconsistencies for issue https://code.siemens.com/infosec-pss…
DianaMariaDDM Feb 15, 2024
5593023
Fixing minor syntax issues by adding missing "PATCH" keywords or miss…
DianaMariaDDM Feb 15, 2024
9ee76ca
Fixing PRELIM task "PRELIM | 4.3.3 | Find all sudoers files" mentione…
DianaMariaDDM Feb 15, 2024
3bec70e
Removing 1.1.2.1 from multiline task 1.1.2.2 ,1.1.2.3, 1.1.2.4 becaus…
DianaMariaDDM Feb 15, 2024
e8f766f
Removing prelim for installing authconfig, as it is not used.
DianaMariaDDM Feb 16, 2024
e14d248
[pre-commit.ci] pre-commit autoupdate
pre-commit-ci[bot] Feb 19, 2024
a480622
Removing the 6.1.12 duplicate task and adding it to the 6.1.10 task a…
DianaMariaDDM Feb 21, 2024
f6e12ab
De-commenting allow and deny variables for sshd.
DianaMariaDDM Feb 21, 2024
5b2165d
Removing double import of cis_5.3.yml.
DianaMariaDDM Feb 22, 2024
fdd3c87
Merge pull request #18 from ansible-lockdown/pre-commit-ci-update-config
uk-bolly Feb 22, 2024
19a64e3
Merge pull request #35 from siemens/siemens/feat/new_docs
uk-bolly Feb 22, 2024
95c7f19
Merge pull request #39 from siemens/siemens/feat/ensure_root_psswd_fix
uk-bolly Feb 22, 2024
1c3bc34
Merge pull request #43 from siemens/siemens/feat/fixing_prelim_find_a…
uk-bolly Feb 22, 2024
283366c
Merge pull request #45 from siemens/siemens/feat/r_1.1.2.1
uk-bolly Feb 22, 2024
c9ce3e1
Merge pull request #47 from siemens/siemens/feat/fixing_inconsistencies
uk-bolly Feb 22, 2024
27f69f8
Merge pull request #49 from siemens/siemens/feat/minor_syntax_fixes
uk-bolly Feb 22, 2024
fb93017
Merge pull request #53 from siemens/siemens/feat/remove_6.1.12_duplicate
uk-bolly Feb 22, 2024
a452618
Merge pull request #55 from siemens/siemens/feat/fixing_double_import…
uk-bolly Feb 22, 2024
46b8d7d
Merge branch 'devel' into siemens/feat/removing_prelim_install_authco…
DianaMariaDDM Feb 23, 2024
66f73f5
Merge pull request #51 from siemens/siemens/feat/removing_prelim_inst…
uk-bolly Feb 23, 2024
08d3be9
Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafte…
DianaMariaDDM Dec 6, 2023
d39be02
Removing trailing whitespaces
DianaMariaDDM Jan 30, 2024
8205dfd
Updating regexps as suggested!
DianaMariaDDM Feb 23, 2024
bf2cffd
Merge branch 'siemens/feat/r_4.6.5_regexp_and_module_fixings' of gith…
DianaMariaDDM Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions tasks/section_4/cis_4.6.x.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,22 @@

- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive"
block:
- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive | Set umask for /etc/login.defs pam_umask settings"
- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive | Set umask for /etc/login.defs and /etc/profile"
ansible.builtin.lineinfile:
path: "{{ item.path }}"
regexp: '(?i)(umask\s*)'
regexp: '(?i)(umask\s*\d\d\d)'
Copy link

Choose a reason for hiding this comment

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

umask can be 0027 or 027
should check for lines without comments
no need for capture group

Suggested change
regexp: '(?i)(umask\s*\d\d\d)'
regexp: '(?i)^\s*umask\s*\d{3,4}'

Copy link
Member

Choose a reason for hiding this comment

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

Agreed this should be able to capture both types 3 & 4

Copy link

Choose a reason for hiding this comment

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

And the ^\s* ensure the line starts with umask, and the line is not commented out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, I will add these modifications!

line: '{{ item.line }} 027'
with_items:
- { path: '/etc/bashrc', line: 'umask' }
- { path: '/etc/profile', line: 'umask' }
- { path: '/etc/login.defs', line: 'UMASK' }

- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive | Set umask for /etc/bashrc"
- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive | Set umask for /etc/bashrc"
ansible.builtin.replace:
path: /etc/bashrc
regexp: '\s+umask\s*\d\d\d'
Copy link

Choose a reason for hiding this comment

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

Suggested change
regexp: '\s+umask\s*\d\d\d'
regexp: '^\s*umask\s*\d{3,4}'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will modify this as well, thank you for your input!

replace: '\numask 027'
Copy link

Choose a reason for hiding this comment

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

why add a new line ?

Suggested change
replace: '\numask 027'
replace: 'umask 027'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I presented in the issue responsible for this PR:
"Actual Behavior
Firstly, [...]
Secondly, the way the task is written, it does not edit the /etc/bashrc file in a proper manner, only modifying one of
the
umask lines, not all of them."

The only way it worked for my tests was to add 'umask 027' with \n in order to edit properly both umask lines from /etc/bashrc.

Copy link

Choose a reason for hiding this comment

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

I am sure this would get you where you want too.

ansible.builtin.replace:
            path: /etc/bashrc
            regexp: '^(\s+umask\s*)\d\d\d'
            replace: '\1 027'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried you approach but unfortunately, it did not work. This is the content of the /etc/bashrc file after executing the task with replace: '\1 027':

image

Using \numask 027 still works fine and modifies all the lines with umask.

Copy link
Member

Choose a reason for hiding this comment

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

Does this work?

This should only change those not 027 or more restrictive and whether 3 or 4 characters

regexp: '^(?i)(\s*umask)\s+(?\!\d*[2,7]7)\d{3,4}'
replace: '\1 027'

Copy link
Member

@uk-bolly uk-bolly Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Just ran manually on bashrc

grep -i umask /etc/bashrc
    # By default, we want umask to get set. This sets it for non-login shell.
       umask 002
       umask 022
[ec2-user@test ~]$ cp -p /etc/bashrc /etc/bashrc_orig
cp: cannot create regular file '/etc/bashrc_orig': Permission denied
[ec2-user@test ~]$ sudo cp -p /etc/bashrc /etc/bashrc_orig
[ec2-user@test ~]$ grep -i umask /etc/bashrc
    # By default, we want umask to get set. This sets it for non-login shell.
       umask 027
       umask 027

Just checking we're in the same place i'm using this regexp

ansible.builtin.replace:
    path: /etc/bashrc
    regexp: ^(?i)(\s*umask)\s+(?!\d*[2,7]7)\d{3,4}
    replace: '\1 027'

p.s. i did forget to sudo :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the same regexp as well, still, it did not modify the lines...

Copy link
Member

Choose a reason for hiding this comment

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

Ok lets close this PR i will add the fix to the new Feb24 ive been working on, there seems to be so many commits for one line change for this control.
Let see if we reset and how we get on then, i have a feeling something is not quite right somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree! Thank you for your time!

Copy link
Member

Choose a reason for hiding this comment

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

Not a problem at all, will ensure credit added for the update
thank you for the time


- name: "4.6.5 | PATCH | Ensure default user umask is 027 or more restrictive | Editing USERGROUPS_ENAB"
ansible.builtin.lineinfile:
path: /etc/login.defs
regexp: '^USERGROUPS_ENAB'
Expand Down