Skip to content

Conversation

@vpodzime
Copy link
Contributor

@vpodzime vpodzime commented Jan 2, 2024

Multiple issues discovered and fixed. Plus one piece of polishing.

And add a comment describing the meaning of the logical
expression.

Ticket: ENT-11136
Changelog: None
Ticket: ENT-11136
Changelog: Federated reporting policy now properly fixes SELinux
           context of the ~cftransport/.ssh directory and its
           contents.
We only run a single command, no need to use shell.

Also, reformat the policy a bit for better readability.

Ticket: ENT-11136
Changelog: None
@vpodzime vpodzime requested a review from craigcomstock January 2, 2024 14:20
@vpodzime
Copy link
Contributor Author

vpodzime commented Jan 2, 2024

Manually tested to verify that it fixes issues on a CentOS 7 hub enabled as a feeder. Really strange it hasn't been breaking FR tests.

@vpodzime
Copy link
Contributor Author

vpodzime commented Jan 2, 2024

Manually tested to verify that it fixes issues on a CentOS 7 hub enabled as a feeder.

It just requires two agent runs to properly do the full setup. #2817 adds a more complicated change making sure a single agent run is enough, but we may want to stick to this simpler change for backports.

@vpodzime vpodzime added the cherry-pick? Fixes which may need to be cherry-picked to LTS branches label Jan 2, 2024
expression => not(returnszero("$(default:paths.semanage) fcontext -l | grep '$(home)/.ssh(/.*)?'", "useshell")),
if => fileexists("$(home)");
enabled.selinux_enabled::
# For all the files below it must be true that if they exist they need
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate your explanation here but have to say this block has become rather unreadable. I wonder if we could do better? The logic reads to me as correct after some time looking at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I know how to make this a lot nicer in Python. Or any language with functions and loops, really. But no clue how to make it nicer in CFEngine policy. Maybe @nickanderson can help polish this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use a list of files and then create an array with those files as keys and the results of the SELinux labels as values and then search for a false value in getindices() on the array. But that's similarly ugly, AFAICT.

@vpodzime vpodzime merged commit 2e67c64 into cfengine:master Jan 3, 2024
This was referenced Jan 3, 2024
@craigcomstock craigcomstock removed the cherry-pick? Fixes which may need to be cherry-picked to LTS branches label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants