-
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
accounts_user_dot_group_ownership: Improve OVAL to avoid nobody group #9956
accounts_user_dot_group_ownership: Improve OVAL to avoid nobody group #9956
Conversation
Code Climate has analyzed commit b3397d7 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 48.8% (0.0% change). View more on Code Climate. |
@@ -10,12 +10,18 @@ | |||
<unix:password_object id="object_accounts_user_dot_group_ownership_objects" version="1"> | |||
<unix:username datatype="string" operation="not equal">nobody</unix:username> | |||
<filter action="include">state_accounts_user_dot_group_ownership_interactive_gids</filter> | |||
<filter action="exclude">state_accounts_user_dot_group_ownership_nobody</filter> |
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.
The <unix:username datatype="string" operation="not equal">nobody</unix:username>
line (11) should do the trick. If the nobody string is not appropriate here, maybe we could check the gid
field in this object.
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.
not exactly, on that line you are just avoiding to match the user which name is nobody
.
My problem is users that have group nobody, e.g. in Ubuntu 22.04 default install:
$ sudo cat /etc/passwd | grep 65534
sync:x:4:65534:sync:/bin:/bin/sync
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin
_apt:x:105:65534::/nonexistent:/usr/sbin/nologin
dnsmasq:x:112:65534:dnsmasq,,,:/var/lib/misc:/usr/sbin/nologin
kernoops:x:113:65534:Kernel Oops Tracking Daemon,,,:/:/usr/sbin/nologin
gnome-initial-setup:x:125:65534::/run/gnome-initial-setup/:/bin/false
sshd:x:128:65534::/run/sshd:/usr/sbin/nologin
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.
Also this object only has the username and a filter, hence why I added such filter to avoid matching nobody group.
Unless I'm looking at the wrong spec file version:
https://oval.mitre.org/language/version5.11/ovaldefinition/documentation/unix-definitions-schema.html
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 see. My question is if wouldn't be enough to just update the <unix:username datatype="string" operation="not equal">nobody</unix:username>
line?
For example:
<unix:group_id datatype="int" operation="not equal">{{{ nobody_gid }}}</unix:group_id>
It would avoid the additional OVAL state and filter.
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.
It won't work, group_id
is part of the password_state
, not part of the password_object
.
What we could do is change the username line to match anything, and still have the filter
with the group_id
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.
Ohhh, that is true. It is part of the password_state
. Nevermind. Sorry for the confusion.
/retest |
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. All tests have passed locally. I will wait the CI tests to conclude. Thanks
|
Description: