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 https://github.com/ansible-lockdown/AMAZON2023-CIS/issues/36 #37

Conversation

DianaMariaDDM
Copy link
Contributor

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

Issue Fixes:

Enhancements:
None.

How has this been tested?:
Locally.

DianaMariaDDM and others added 24 commits February 1, 2024 15:59
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>
Signed-off-by: Diana-Maria Dumitru <diana.dumitru.ext@siemens.com>
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".
Copy link
Member

Choose a reason for hiding this comment

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

While i understand the issue, this change means that this could be searching any shared mounted drives which on shared systems may introduce major issues for some systems and configurations.
Passing it through the df with xargs allows us to separate those filesystems not part of the same device ID thereby excluding things like /sys and /proc.
Its current state allows it to run on most systems with local disk, it doesn't always allow for systems with multiple disks, again the impact on shared environments could have a very bad adverse effect.
Trying to minimise any possible impact we could change this to add a couple of extra excludes using grep at the end of the command.
e.g.
df --local -P | awk {'if (NR!=1) print $6'} | xargs -I '{}' find '{}' -xdev -type f -perm -0002 | grep -Ev "/snap|/containerd|/kubelet"
This allows greater control and having tested across a few different OSs seems to be a consistent output.
We could also add a variable if we wanted to allow users to defined their own filesystems to be excluded.

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 would try to modify the command by adding the exceptions in the find part of the command like this:

df --local -P | awk {'if (NR!=1) print $6'} | xargs -I '{}' find \( ! -path "/run/user/*" -a ! -path "/proc/*" -a ! -path "*/containerd/*" -a ! -path "*/kubelet/pods/*" -a ! -path "/sys/kernel/security/apparmor/*" -a ! -path "/snap/*" -a ! -path "/sys/fs/cgroup/memory*" -a ! -path "/sys/fs/selinux/*" \) -xdev -type f -perm -0002

Copy link
Member

Choose a reason for hiding this comment

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

the -xdev means that the filesystems /sys /run and /proc wouldn't be looked at anyway, so those in the ! -path matching those filesystems wouldn't be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, about this, I overlooked it. I will push the modification asap!

DianaMariaDDM and others added 5 commits February 23, 2024 08:04
…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.
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 8 commits February 23, 2024 14:03
…AZON2023-CIS into siemens/feat/r_6_1_10_updated
…allation is removed!

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

Removing `amzn2023cis_use_authconfig` variable.
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>
…AZON2023-CIS into siemens/feat/r_6_1_10_updated
uk-bolly added a commit that referenced this pull request Feb 23, 2024
Signed-off-by: Mark Bolwell <mark.bollyuk@gmail.com>
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
Copy link
Member

hi @DianaMariaDDM

Thank you again for you issues and PRs and all the work, i know we have seen this previously with one file being changes but having multiple commits being added. I believe this is the same issue that one of your colleagues is having.
If there is only one change that you wish to make we should only see that one change not the syncing of the commits we do see. Not sure how that happens, but it can cause issues and take alot longer for us to confirm all the changes in your PR.
I believe quite a few of the issues have been addressed now and merged.

If you could please check and close if this is done.

Many thanks and apologies for any confusion.

uk-bolly

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.

None yet

2 participants