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

[PR #5830/c8a2ac3a backport][stable-6] sefcontext: add support for path substitutions #6098

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented Feb 26, 2023

This is a backport of PR #5830 as merged into main (c8a2ac3).

SUMMARY

Fixes #1193.

Add support for path substitution mappings in the sefcontext module.

I would like to get feedback on this architecture that is quite a bit different from another (dead) PR #5189:

  • Don't use new functions just for adding/deleting path substitutions. Modify the existing functions.
  • How deletion is done has minor change - current playbooks continue to work as before:
    • Don't require the setype argument with state=absent - it is not really required for deletion and it was not even used in the old code.
    • If neither setype or substitute arg passed: delete both path substitutions and regular context mappings for target if either was found when deletion requested.
    • If substitute was passed with state=absent then delete only that equivalence if found (don't delete regular context mappings); if not found then return unchanged, if found but target substitutes some other path then return unchanged.
    • If setype was passed with state=absent then delete only existing context mappings (don't delete path substitutions), don't care about the setype matching the current mapping to delete (this is the old behavior which seems a bit loose).

Fixes #4564.

Change the incorrect setype in documentation example to a working label.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

sefcontext

ADDITIONAL INFORMATION

Feedback please! It is the largest code change I have done yet so I really need feedback.

  1. Should adding/deleting path substitutions be moved to their own functions instead? Does it matter?
  2. Should deletion behavior be more strict?
    • Consider example sefcontext setype=tmp_t target=/opt/tmp state=absent: what if /opt/tmp is currently not mapped to tmp_t, say it's mapped to opt_t - in this case should the module just return changed=false? Or should it delete the mapping anyways? The old behavior is the latter and I've kept that unchanged. Personally I feel that it is wrong and would like to hear feedback on this.
  3. The arg name substitute? I couldn't come up with a great name for it. The man page for semanage-fcontext talks about "path substitution" and the switch -e for this is called "equals". Better args names welcome: current comments seem to support substitute but there is also request to add equal as an alias (I would hesitate, perhaps unnecessarily, to add an alias to a new feature).

Also adds "attributes" block to documentation & some minor documentation improvements.


* sefcontext: add path substitution support (#1193)

First commit for feedback, missing docs and tests.

* sefcontext: add documentation

* Add changelog fragment

* Documentation formatting

* Delete extra newline

* pep8 fixes

Fix indentation

* Add version_added to arg docs

* Add examples

* Don't delete non-matching path substitutions

* Add integration tests

* Delete only substitutions if such arg passed

Don't delete existing regular file context mappings if deletion of
a path substitution was requested with the presence of the
`equal` arg - delete only path substitutions in such case.

Path substitutions and regular mappings may overlap.

* Can only add args in minor releases

:(

* Cleanup before tests

* Fix deletion using substitution

Was comparing wrong var.

* Fix test checking wrong var

* Improve args documentation and examples

List the default values for selevel, seuser.
Add example for deleting path substitutions only.

* Add attributes documentation block

Not sure if should add become/delegate/async,
shouldn't those work just like that without any
specific code added for them?

* and fix indentation on attribute block

* Consistent indentation for attributes

Confusing, most plugins indent with 4 spaces.
But some use 2 like the rest of the code, so use 2.

* Add missing ref for attribute block

* Use correct c.g version in doc block

Co-authored-by: Felix Fontein <felix@fontein.de>

* Add full stop to changelog fragment

Co-authored-by: Felix Fontein <felix@fontein.de>

* Streamline documentation

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

* Support limiting deletion to setype

Deleting file context mappings may be limited by
passing setype or equal, if neither arg is passed
then delete either setype/equal mappings that match.

* Change arg name, diff mode output fix

Change arg name from equal to substitute.
Print target = subsitute in diff mode same way as
semanage does.

Also put back platform attribute, try to improve
clumsy language in the substitute arg docs.

* Delete even if arg setype not match existing

Test 5 indicates that deletion is supposed to not check that
the arg setype passed when deleting matches the setype
of the mapping to delete.
Delete any mapping that matches target, regardless of
setype arg value.

* Update arg name in tests

* Too eager replacing

Accidentally replaced seobject function names so fix them back

* 4564: Fix invalid setype in doc example

Change from httpd_git_rw_content_t which
does not exist to httpd_sys_rw_content_t

Fixes #4564

* Fix documentation attributes

Additional fragment

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update version_added in docs

Bumping minor to 6.4.0 since it didn't make 6.3.0.

* Add more description to the new arg docs

Try to improve discoverability of the new feature and make it easier to understand without deep SELinux understanding.

* Update platform to Linux in documentation

* Add equal as alias for the new argument

Improve discoverability of the new feature by adding an alias to the new module argument. The argument name "equal" will be easy to find for users who are not familiar with SELinux and who just try to match to the CLI tool `semanage`.

* And add alias argument properly

Previous commit missed actually adding the alias (added to docs only).

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
(cherry picked from commit c8a2ac3)
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added backport feature This issue/PR relates to a feature request integration tests/integration module module new_contributor Help guide this first time contributor plugins plugin (any type) system tests tests labels Feb 26, 2023
@felixfontein felixfontein merged commit 9bab144 into stable-6 Feb 26, 2023
@felixfontein felixfontein deleted the patchback/backports/stable-6/c8a2ac3a475ab490ad00f1db1a6197b108c66413/pr-5830 branch February 26, 2023 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request integration tests/integration module module new_contributor Help guide this first time contributor plugins plugin (any type) system tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants