TriMoon
commented
10 months ago :warning:
The service files as distributed in the deb package still has WRONG name !!!
Someone needs to fix the packaging script !
Open TriMoon opened 10 months ago
TriMoon
commented
10 months ago :warning:
The service files as distributed in the deb package still has WRONG name !!!
Someone needs to fix the packaging script !
lainedfles
commented
10 months ago @TriMoon Thanks for your attention to detail. I've not yet had a chance to test this personally but these are good ideas.
TriMoon
commented
10 months ago You can try the PR, linked above, locally and see if you like it :wink: (I'm running with that version)
lainedfles
commented
10 months ago The version under your merge request did work without modification but here is my critique:
ExecStart=%N without absolute path is bad practice from a security perspective although technically it should be safe under most modern distributions which have "merged" /bin via symbolic links and properly compiled systemd. It does also affect the process list as viewed by ps. My vote will always be to use absolute path if presented with a choice :thumbsup:
The present state of Makefile relies upon the OS/user umask for directory permissions during creation or packaging of /etc/opensnitch/rules, for this reason, hard coding ConfigurationDirectoryMode=0700 may not be a good idea as it can produce a mismatch warning. Omission of this statement may be a better option unless creation is also dependent on systemd which likely isn't the reality for most distributions.
WantedBy=basic.target may become problematic for initrd boot because basic.target is reached before any filesystems are mounted. However, I think that the directives under [Unit] should catch this case under most circumstances so it may be a non-issue but worth mention.
There is a benign warning if $opts is unset. I'd recommend setting an empty variable as a better practice. Like: Environment='opts='
The introduction of the _customconfig variable may be unnecessary unless use of EnvironmentFile= is planned. The structure of systemd makes it trivial to override ConfigurationDirectory= which, since v240, will expose the $CONFIGURATION_DIRECTORY environment variable automatically that we can re-use.
I'm presently using a modified version incorporating these items. This version produces no systemd warnings under my up-to-date Archlinux system(s):
--- /etc/systemd/system/opensnitchd.service_TriMoon 2023-09-03 19:54:32.833663095 -0600
+++ /etc/systemd/system/opensnitchd.service 2023-09-03 20:18:11.745180152 -0600
@@ -16,13 +16,13 @@
[Service]
Type=exec
ConfigurationDirectory=%N/rules
-ConfigurationDirectoryMode=0700
-Environment='custom_cfg=%E/%N/rules'
+# Example to enable daemon debug logging:
# Environment='opts=-debug'
+Environment='opts='
-ExecCondition=%N -check-requirements
-ExecStart=%N -rules-path $custom_cfg $opts
+ExecCondition=/usr/bin/%N -check-requirements
+ExecStart=/usr/bin/%N -rules-path $CONFIGURATION_DIRECTORY $opts
# Signal-info was taken from the init.d script, but it just exits and then systemd restarts the service...
ExecReload=kill -HUP $MAINPIDTypically the trends set by this project aim for general compatibility and my basic investigation and minimal testing suggest that it will be generally compatible. Nice work, thanks!
One additional note, many of these concepts may be applicable to the UI component. Someday I'd like to try implementing UI control via systemd unless someone beats me to it!
TriMoon
commented
10 months ago @lainedfles please see #1019 and use that in future to suggest changes :wink: I cross-posted and replied there :+1:
Summary:
The systemd service unit can be enhanced to start BEFORE any network is configured, and thus allow interception and protection at an earlier stage...
Service file:
Below is my crafted service file:
Notable changes/additions:
Unit filename: :warning: The service filename was changed to
opensnitchd.service!!! This was needed for automatically using the unit name in the service definition using the%Nspecifier... See: Specifiers@mansystemd.unit#Specifiersdin it's name !Unit ordering: Made sure the daemon starts before any network related devices or services are created/started by using
network-pre.target, See:Startup target / run-level: Changed the default install target to
basic.targetinstead ofmulti-user.target. This will allow it to run in any "run-level" in SysV terms. See: Units managed by the system service manager@mansystemd.special#basic.targetAutomatically disable using a kernel-command-line option: Just in case it is needed, i added the ability to disable the daemon using a kernel-command-line option using the
ConditionKernelCommandLinedirective. When theno-appfwoption is present in the kernel-command-line, the daemon will not startup. This option functions same as the well-knownquiteoption. (only applies when present as a separate word)Automatically create rules directory: Let systemd automatically create the "rules" directory with proper mode, when non-existent yet, upon starting the daemon service by using the
ConfigurationDirectoryandConfigurationDirectoryModedirectives.Automatically check for kernel support before starting: Automatically prevent startup when required kernel support is not present by using the
-check-requirementsflag in aExecConditiondirective. (This assumes the command returns a non-zero exit status when not satisfied.)Reload functionality: Added support for reloading the daemon using the
ExecReloaddirective. Signal-info was taken from the init.d script, but it just exits and then systemd restarts the service... So this functionality either needs to be implemented in the daemon's code or a different signal needs to be sent to it. But at least the functionality is now present in the unit file.Prevent from being killed by the OOM-Killer: Prevented the daemon to be killed by the Linux kernel's Out-Of-Memory (OOM) killer, using the
OOMScoreAdjust=-1000directive. See: OOMScoreAdjust@mansystemd.exec#OOMScoreAdjust(This will ensure that the protection keeps functioning even when other processes cause an OOM)Admin overrides using drop-ins: Added support for easily adjusting the directory used for rules and extra options by the local admin. The local admin can create "drop-in" config(s) under
/etc/systemd/system/opensnitchd.service.d/even when the service file is installed in other places by the package maintainer, See: 16.14. Extending the default unit configuration@redhat.com Example drop-in(s) contents that can-be-used:To change the rules directory to be used:
$custom_cfgis supplied as an argument to the-rules-pathoption of the daemon.systemd.unit#Specifiers%Eexpands to/etc%Nexpands to the unit nameopensnitchd/etc/opensnitchd/rules-specialTo enable debug output:
$optsis supplied as extra argument(s) to the daemon.Combination of both:
One last thing to improve systemd usage: Please add a way to prevent timestamps being output at all, when using
/dev/stdoutas log destination. Or at least add asyslogoption to use as logger, which also doesn't need timestamps because it adds them self. Look how ugly it is at moment: