Drop present? and booted? systemd checks

[ehelms]

The check of whether NOTIFY_SOCKET is set should be sufficient for
enabling the plugin. Using this plugin and setting notify are
intentional use cases that should suffice. For systems with
SELinux enabled, the check of the systemctl binary and shelling
out to run systemctl can cause policy violations that provide
additional complications for users of the plugin.

[ehelms]

@sj26 If you could also take a look at this one for inclusion in the release... this simplifies things and makes SELinux less complex for projects. Appreciate you reviewing and considering!

[sj26]

I had this debate with myself when initially writing the plugin. But systemd's documentation specifically recommending using the sd_booted() function before activating systemd specific functionality, and that was calling systemd's dbus interface equivalent to this command line invocation, which is why I did it. I didn't want to introduce a dbus dependency. 😅 I'm torn between doing it "right" and making it "work".

Could you help me understand the failure case, maybe demonstrate it?

[ekohl]

From that page:

Internally, this function checks whether the directory /run/systemd/system/ exists. A simple check like this can also be implemented trivially in shell or any other language.

Perhaps this better? It doesn't have to exec anything which is safer.

[ehelms]

I tested this and it worked. I know the original comment seemed to warn against this:

    # We could check for the systemd run directory directly, but we can't be
    # absolutely sure of it's location and breaks encapsulation. We can be sure
    # that systemctl is present on a systemd system and understand whether
    # systemd is running. The systemd-notify binary actually recommends this.

However, if that page on sd_booted is correct this ought to be a safe option that doesn't make things more complex for SELinux users.

[ekohl]

I can't find that part in the current man pages. Perhaps the recommendations have changed?

[sj26]

Right, I was considering the edge case that systemd might be installed with a different prefix and so best deferred to the system's $PATH a la pkg-config. But a non-default prefix would be very irregular. I also thought that the implementation of sd_booted was more complex, but it is not, and is in fact just detecting that one and only directory path. So I'm not even sure a non-default directory is possible.

So this implementation looks good. 👍

[ehelms]

The failure case arises if you are running puma within a confined SELinux process; shelling out to systemctl requires an execption:

type=AVC msg=audit(1598616113.087:3870): avc:  denied  { getattr } for  pid=459 comm="ruby" path="/usr/bin/systemctl" dev="vda1" ino=925596 scontext=system_u:system_r:foreman_rails_t:s0 tcontext=system_u:object_r:systemd_systemctl_exec_t:s0 tclass=file

[ekohl]

Suggested change

	# Only install hooks if systemd is booted by
	# Only install hooks if the system is booted by

[sj26]

Thanks! 🙌

[sj26]

Released in v0.1.5