content
content copied to clipboard
change: audit_rules_suid_privilege_function: better regexp
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.
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.
Start a new ephemeral environment with changes proposed in this pull request:
rhel8 (from CTF) Environment (using Fedora as testing environment)
/ok-to-test
New tests added. more_space.pass.sh, other_key.pass.sh fail w/o fix Add fedora support.
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.
Hi @maage , do you have plans to update this PR? Seems it is almost ready.
@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?
Rebased and added requested modifications as separate commits.
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.
@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.