Use ACLs to allow access to the cf-execd's runagent.socket #4513

vpodzime · 2021-02-18T09:27:07Z

No description provided.

vpodzime · 2021-02-18T18:23:53Z

@cf-bottom jenkins please

cf-bottom · 2021-02-18T18:28:55Z

Alright, I triggered a build:

Jenkins: https://ci.cfengine.com/job/pr-pipeline/6131/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-6131/

vpodzime · 2021-02-19T10:24:09Z

RHEL 8 builds (incl. the SELinux policy):

vpodzime · 2021-02-19T10:25:42Z

FR tests retry:
sequential tests retry:

cf-execd/cf-execd.c

larsewi

👍

olehermanse

Why is the first commit there?

Added changelog entry for unexpected consequence of change in and(), …

Seems like this should be removed if you rebase to upstream/master.

olehermanse

LGTM - just some smaller comments.

cf-execd/cf-execd.c

vpodzime · 2021-02-24T08:17:21Z

Seems like this should be removed if you rebase to upstream/master.

That's exactly what I did and how it got there. 😕

Ticket: ENT-6735 Changelog: None

Useful in places where we want to grant users access to some files (or directories) when then are not owners of the files. Ticket: ENT-6735 Changelog: None

A new attribute that tells cf-execd to grant access to the runagent.socket to the specified users. Ticket: ENT-6735 Changelog: Commit

…oy() Better explicitly compare to NULL. Ticket: ENT-6735 Changelog: None

So that the code can be reused when list of users allowed to access the socket changes. Ticket: ENT-6735 Changelog: None

cf-execd creates the runagent.socket when it starts and sets ACLs on it based on the policy. However, when policy changes, cf-execd is not restarted, it just loads the new policy and keeps running. Thus it needs to detect a change in the list of allowed users and make sure the ACLs are updated in case of a change. Ticket: ENT-6735 Changelog: None

It stops listening on the socket so the socket should not be left behind. Ticket: ENT-6735 Changelog: None

…missions The directory and the socket file inside of it should not be group- and world- readable, writable and executable.

So that it can set ACLs. Ticket: ENT-6735 Changelog: None

… sockets So that requests to trigger agent runs on hosts can be passed from the web UI to cf-execd's socket and handled by cf-execd. Ticket: ENT-6735 Changelog: None

lgtm-com · 2021-02-24T09:52:41Z

This pull request fixes 2 alerts when merging 26d70e9 into 040af13 - view on LGTM.com

fixed alerts:

2 for Pointer argument is dereferenced without checking for NULL

olehermanse · 2021-02-25T01:27:43Z

cf-execd/cf-execd.c

+            /* Check if the old list and the new one differ. */
+            if (!StringSetIsEqual(old_runagent_allow_users,
+                                  (*execd_config)->runagent_allow_users))
+            {


If I'm being nitpicky: This comment would not help me much. Right now, it says the same as the code. For it to be helpful, I think it needs to mention that the policy changed (not the runagent sockets/dirs/files). For example:

Suggested change

/* Check if the old list and the new one differ. */

if (!StringSetIsEqual(old_runagent_allow_users,

(*execd_config)->runagent_allow_users))

{

/* Check if the list of allowed users, from policy, has changed. */

if (!StringSetIsEqual(old_runagent_allow_users,

(*execd_config)->runagent_allow_users))

{

I think it's enough to look at where that list is taken from, just a few lines above.

vpodzime added the WIP Work in Progress label Feb 18, 2021

vpodzime force-pushed the master-cf_execd_socket_perms branch 6 times, most recently from 802cbec to 3ec6293 Compare February 18, 2021 11:33