content icon indicating copy to clipboard operation
content copied to clipboard

change: audit_rules_suid_privilege_function: better regexp

Open maage opened this issue 3 years ago • 9 comments

Description:

audit_rules_suid_privilege_function regexp has variations where none wanted

Rationale:

Simplify rules how regexp is formed to make it easier to keep them in line with each other.

There is no need to require audit key to be \w (especially exclude -).

Some variations used only single \s, replace with \s+

Some variations used rx group with key, as it serves no purpose, just use (?:...)

Do not limit key with \w or exact string, just use \S+.

Instead of copy paste regexps multiple times with variations use variables to simplify. Now it is easier to see logic and that regexps are in line with each other.

Could include extended test scenarios to make previous config fail.

maage avatar Apr 25 '22 07:04 maage

Hi @maage. 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.

openshift-ci[bot] avatar Apr 25 '22 07:04 openshift-ci[bot]

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

github-actions[bot] avatar Apr 25 '22 07:04 github-actions[bot]

/ok-to-test

JAORMX avatar Apr 28 '22 08:04 JAORMX

New tests added. more_space.pass.sh, other_key.pass.sh fail w/o fix Add fedora support.

maage avatar Apr 29 '22 10:04 maage

Remediations fail with my new tests. Probably it can not remove wrong rules. Wrong audit rules are reasonably possible if admin creates them by hand and typos gid/uid etc.

I don't know how to these are normally fixed if they are fixed.

maage avatar Apr 29 '22 11:04 maage

Hi @maage , do you have plans to update this PR? Seems it is almost ready.

marcusburghardt avatar Jun 14 '22 15:06 marcusburghardt

@maage , could you rebase and resolve the conflict in this PR, please? In addition, there are some few considerations from my review and this PR should be ready to be merged. Could you update it, please?

marcusburghardt avatar Jul 26 '22 12:07 marcusburghardt

Rebased and added requested modifications as separate commits.

maage avatar Aug 17 '22 19:08 maage

Code Climate has analyzed commit 5095a7d3 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.

qlty-cloud-legacy[bot] avatar Aug 17 '22 19:08 qlty-cloud-legacy[bot]

@maage: The following tests 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-high 5095a7d330c6cd43bb43c8e933a8dd682d3b76fb link true /test e2e-aws-rhcos4-high
ci/prow/e2e-aws-rhcos4-moderate 5095a7d330c6cd43bb43c8e933a8dd682d3b76fb link true /test e2e-aws-rhcos4-moderate

Full PR test history. Your PR dashboard.

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.

openshift-ci[bot] avatar Aug 17 '22 22:08 openshift-ci[bot]