-
Notifications
You must be signed in to change notification settings - Fork 709
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
Ubuntu 24.04: Implement rule 5.4.2.8 Ensure accounts without a valid login shell are locked #12889
Conversation
Hi @alanmcanonical. 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 Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
Code Climate has analyzed commit 65140d2 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 61.9% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few comments
</criteria> | ||
</definition> | ||
|
||
<ind:textfilecontent54_test id="test_{{{ rule_id }}}_no_invalid_shell_accounts" check="all" check_existence="at_least_one_exists" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "at_least_one_exists" will result in fail when /etc/passwd
does not contain matching entries.
Usually, sync
is matched due to having /bin/sync
. But if that were to be changed, or if sync would be excluded in the future there will be no matches and the test would fail even though it shouldn't. Can you make a test for this case please? Maybe any_exist
would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the initial log for correct.pass.sh:
I: oscap: Variable 'oval:ssg-var_no_invalid_shell_accounts_unlocked_valid_shells:var:1' has values "/bin/sh", "/usr/bin/sh", "/bin/bash", "/usr/bin/bash", "/bin/rbash", "/usr/bin/rbash", "/usr/bin/dash", "/bin/bash". [oscap(75):oscap(768f591ddbc0):oval_variable.c:488:_dump_variable_values]
I: oscap: Test 'oval:ssg-test_no_invalid_shell_accounts_unlocked_no_invalid_shell_accounts:tst:1' requires that zero or more objects defined by 'oval:ssg-obj_no_invalid_shell_accounts_unlocked_shells:obj:1' exist on the system. [oscap(75):oscap(768f591ddbc0):oval_resultTest.c:903:_oval_result_test_evaluate_items]
I: oscap: 0 objects defined by 'oval:ssg-obj_no_invalid_shell_accounts_unlocked_shells:obj:1' exist on the system. [oscap(75):oscap(768f591ddbc0):oval_resultTest.c:918:_oval_result_test_evaluate_items]
I: oscap: No item matching object 'oval:ssg-obj_no_invalid_shell_accounts_unlocked_shells:obj:1' was found on the system. (flag=does not exist) [oscap(75):oscap(768f591ddbc0):oval_resultTest.c:954:_oval_result_test_evaluate_items]
I: oscap: Test 'oval:ssg-test_no_invalid_shell_accounts_unlocked_no_invalid_shell_accounts:tst:1' evaluated as true. [oscap(75):oscap(768f591ddbc0):oval_resultTest.c:1164:oval_result_test_eval]
I: oscap: Definition 'oval:ssg-no_invalid_shell_accounts_unlocked:def:1' evaluated as false. [oscap(75):oscap(768f591ddbc0):oval_resultDefinition.c:170:oval_result_definition_eval]
0 object is 'true' for any_exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed the negated test in the criteria
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work!
</criteria> | ||
</definition> | ||
|
||
<ind:textfilecontent54_test id="test_{{{ rule_id }}}_no_invalid_shell_accounts" check="all" check_existence="at_least_one_exists" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed the negated test in the criteria
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks!
Description:
Rationale: