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

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

Conversation

DianaMariaDDM
Copy link
Contributor

Overall Review of Changes:
This PR contains the solution for issue 26

Issue Fixes:

Enhancements:
None

How has this been tested?:
Tested locally.

…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/3 by editing the destination path!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/4 by masking both the socket and the service!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/6 by editing the value of `clientalivecountmax` to 3!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/8 by editing the regexp value and adding another task with `replace` module!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/2 by using `import_tasks` module so as the rules will get added and executed!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/5 by adding the necessary lines to both sshd_config file and sshd_config.d/ files. The same method is used for all the rules from 4.2.x, to make them compliant with CISs checks.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
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!

- 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!

ansible.builtin.replace:
path: /etc/bashrc
regexp: '\s+umask\s*\d\d\d'
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

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

@mfortin mfortin mentioned this pull request Jan 25, 2024
uk-bolly and others added 20 commits January 26, 2024 09:28
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…that requires the root password to be set

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…ing "|".

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…e it was not supposed to be there!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
updates:
- [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0)
- [github.com/gitleaks/gitleaks: v8.17.0 → v8.18.2](gitleaks/gitleaks@v8.17.0...v8.18.2)
- [github.com/ansible-community/ansible-lint: v6.18.0 → v24.2.0](ansible/ansible-lint@v6.18.0...v24.2.0)
- [github.com/adrienverge/yamllint.git: v1.32.0 → v1.35.1](https://github.com/adrienverge/yamllint.git/compare/v1.32.0...v1.35.1)
…s it was implementing something needed by 6.1.10.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
DianaMariaDDM and others added 16 commits February 22, 2024 08:10
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…it-ci-update-config

[pre-commit.ci] pre-commit autoupdate
…ng_double_import_r_5_3

Removing double import of "cis_5.3.yml".
…nfig

Signed-off-by: DianaMariaDDM <143085010+DianaMariaDDM@users.noreply.github.com>
…ving_prelim_install_authconfig

Removing prelim for installing authconfig, as it is not used.
…r-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/8 by editing the regexp value and adding another task with `replace` module!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
…ub.com:siemens/AMAZON2023-CIS into siemens/feat/r_4.6.5_regexp_and_module_fixings
uk-bolly added a commit that referenced this pull request Feb 23, 2024
Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>
@uk-bolly uk-bolly closed this Feb 23, 2024
uk-bolly added a commit that referenced this pull request Mar 6, 2024
* updated to use ansible_facts variables

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* Squashed commit of the following:

commit 0cad737fe35db33ff0b867ac4439688cd2bcb009
Author: Mark Bolwell <mark.bollyuk@gmail.com>
Date:   Fri Feb 23 09:41:39 2024 +0000

    updated and tidied up

    Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

commit 1fa72013209866be66f7f2a353b9cf6264a3681d
Author: Mark Bolwell <mark.bollyuk@gmail.com>
Date:   Fri Feb 23 09:41:10 2024 +0000

    audit_only

    Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* Always tag for 1.2.1 gpg_key package update #14

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* tidy up prelim removal step

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* removed inject facts as vars

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* 4.6.5 related to #27 thanks to @DianaMariaDDM

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* 6.1.10 thanks to @DianaMariaDDM #37

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* tage change to alwasy thanks to @DianaMariaDDM #41

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

---------

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>
uk-bolly added a commit that referenced this pull request Apr 15, 2024
* Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/3 by editing the destination path!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/4 by masking both the socket and the service!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/6 by editing the value of `clientalivecountmax` to 3!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/2 by using `import_tasks` module so as the rules will get added and executed!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/5 by adding the necessary lines to both sshd_config file and sshd_config.d/ files. The same method is used for all the rules from 4.2.x, to make them compliant with CISs checks.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing trailing whitespaces

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing trailing whitespaces and fixing an end-of-file

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Refactoring docs

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Small fixings for https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/19

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing trailing whitespace

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing fail message so that is states the correct number of the rule that requires the root password to be set

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing inconsistencies for issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/22

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing minor syntax issues by adding missing "PATCH" keywords or missing "|".

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Fixing PRELIM task "PRELIM | 4.3.3 | Find all sudoers files" mentioned in issue https://code.siemens.com/infosec-pss-gov/security-crafter-baseline-automations/ansible-lockdown/amazon2023-cis/-/issues/22.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing 1.1.2.1 from multiline task 1.1.2.2 ,1.1.2.3, 1.1.2.4 because it was not supposed to be there!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing prelim for installing authconfig, as it is not used.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.4.0 → v4.5.0](pre-commit/pre-commit-hooks@v4.4.0...v4.5.0)
- [github.com/gitleaks/gitleaks: v8.17.0 → v8.18.2](gitleaks/gitleaks@v8.17.0...v8.18.2)
- [github.com/ansible-community/ansible-lint: v6.18.0 → v24.2.0](ansible/ansible-lint@v6.18.0...v24.2.0)
- [github.com/adrienverge/yamllint.git: v1.32.0 → v1.35.1](https://github.com/adrienverge/yamllint.git/compare/v1.32.0...v1.35.1)

* Removing the 6.1.12 duplicate task and adding it to the 6.1.10 task as it was implementing something needed by 6.1.10.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* De-commenting allow and deny variables for sshd.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Removing double import of cis_5.3.yml.

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* As authconfig is not needed anymore, the variable related to its installation is removed!

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>

* Feb 24 updates to devel (#58)

* updated to use ansible_facts variables

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* Squashed commit of the following:

commit 0cad737fe35db33ff0b867ac4439688cd2bcb009
Author: Mark Bolwell <mark.bollyuk@gmail.com>
Date:   Fri Feb 23 09:41:39 2024 +0000

    updated and tidied up

    Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

commit 1fa72013209866be66f7f2a353b9cf6264a3681d
Author: Mark Bolwell <mark.bollyuk@gmail.com>
Date:   Fri Feb 23 09:41:10 2024 +0000

    audit_only

    Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* Always tag for 1.2.1 gpg_key package update #14

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* tidy up prelim removal step

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* removed inject facts as vars

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* 4.6.5 related to #27 thanks to @DianaMariaDDM

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* 6.1.10 thanks to @DianaMariaDDM #37

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* tage change to alwasy thanks to @DianaMariaDDM #41

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated changelog

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

---------

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated ansible fact naming and checkout action (#64)

* updated ansible fact naming for ansible_facts.virtualization_type

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated checkout version

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* ansible fact update

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated ansible facts and timeout now inherited

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

---------

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* [pre-commit.ci] pre-commit autoupdate (#65)

updates:
- [github.com/ansible-community/ansible-lint: v24.2.0 → v24.2.1](ansible/ansible-lint@v24.2.0...v24.2.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* 4.2.16: Add variable for SSH MaxAuthTries (#66)

Signed-off-by: Tom Henderson <tom.henderson@pushpay.com>

* 4.2.16: Correct variable name and required max value (#67)

Signed-off-by: Tom Henderson <tom.henderson@pushpay.com>

* March 24 updates (#68)

* addressed #59 thanks to @DianaMariaDDM

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* issue #59 thanks to @DianaMariaDDM

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* issue #62 thanks to @DianaMariaDDM

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated variable name in conditional

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated container check

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* updated authselect PR

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* fix conditional typo

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* fix conditional typo

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* fix var typo

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

---------

Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>

* [pre-commit.ci] pre-commit autoupdate (#69)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.5.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.5.0...v4.6.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

---------

Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Signed-off-by: DianaMariaDDM <143085010+DianaMariaDDM@users.noreply.github.com>
Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>
Signed-off-by: Tom Henderson <tom.henderson@pushpay.com>
Co-authored-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: DianaMariaDDM <143085010+DianaMariaDDM@users.noreply.github.com>
Co-authored-by: Tom Henderson <tomhenderson@mac.com>
Co-authored-by: Tom Henderson <tom.henderson@pushpay.com>
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.

3 participants