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

Add support for nlmsg extended permission #138

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

tweksteen
Copy link
Contributor

No description provided.

@pebenito
Copy link
Member

you'll need to at least update th github action to use main, so the builds will work. Also you should look at the tests and add some nlmsg examples to prove that this works.

@tweksteen
Copy link
Contributor Author

Thanks for the review.

I changed the default SELinux branch in the github action as you suggested. I assume this implies that setools will require SELinux >= 3.8 in the future. Is that ok?

There is also a commit to rename IoctlSet to XpermSet. I don't know the compatibility guarantee for the setools API, but its worth mentioning. (This commit is not strictly necessary, but it helps clarifying the role of the class).

I added a test in test_terulequery.py. I don't know what's the best place otherwise. Please let me know and I can relocate it or add further tests. Also, I came across the issue that the tests in TERuleQueryXperm weren't being executed (as the test class does not starts with "Test"). This is fixed in the commit as well.

@pebenito
Copy link
Member

Thanks for the review.

I changed the default SELinux branch in the github action as you suggested. I assume this implies that setools will require SELinux >= 3.8 in the future. Is that ok?

That's fine. SETools needs to keep up with new SELinux features.

There is also a commit to rename IoctlSet to XpermSet. I don't know the compatibility guarantee for the setools API, but its worth mentioning. (This commit is not strictly necessary, but it helps clarifying the role of the class).

SETools only has best-effort compat guarantees. This rename should be ok, but we should probably have a IoctlSet class that inherits XpermSet and throws a deprecation warning in its __init__ before calling super().__init__.

I added a test in test_terulequery.py. I don't know what's the best place otherwise. Please let me know and I can relocate it or add further tests.

I'd add one test in tests/library/policyrep/test_rules.py.

Then in tests/library/policyrep/selinuxpolicyy.conf replace one of each of the xperm rule types (allowxperm, auditallowxperm, etc.) to make sure this counts towards the rule counts. Don't add rules, as I intentionally made the rule counts prime numbers.

Also, I came across the issue that the tests in TERuleQueryXperm weren't being executed (as the test class does not starts with "Test"). This is fixed in the commit as well.

Thanks!

@pebenito
Copy link
Member

Looks like your build issues are due to issues I've since fixed in #139. You should also rebase to get those fixes.

@tweksteen
Copy link
Contributor Author

Done. Let me know your thoughts. Thanks.

setools/policyrep/terule.pxi Outdated Show resolved Hide resolved
The same class can be used for both ioctl and nlmsg extended
permissions. Rename the current class and mark IoctlSet as deprecated.

Signed-off-by: Thiébaud Weksteen <[email protected]>
The "Test" prefix is added to TERuleQueryXperm to ensure it is executed.

Signed-off-by: Thiébaud Weksteen <[email protected]>
@pebenito pebenito merged commit 6352feb into SELinuxProject:main Nov 6, 2024
9 checks passed
@pebenito
Copy link
Member

pebenito commented Nov 6, 2024

thanks!

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.

2 participants