Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Update opensnitchd.service #1019

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

Open
TriMoon wants to merge 3 commits into evilsocket:master
base: master
Choose a base branch
Loading
from TriMoon:master-1
Open

Conversation

@TriMoon
Copy link

@TriMoon TriMoon commented Aug 15, 2023
edited
Loading

Implements: #1018


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...


Notable changes/additions:

  1. Unit filename:
    ⚠️ The service filename needs tobe changed to opensnitchd.service !!!
    This was needed for automatically using the unit name in the service definition using the %N specifier...
    See: Specifiers@man systemd.unit#Specifiers

    • The filename, as packaged in latest v1.6.2 is lacking the d in it's name !
    • Thus if you want to use this service file while at same time having the currently distributed unit file in your system you need to "mask" the old one to prevent it from being used and started:
      sudo systemctl mask --now opensnitch.service
  2. Unit ordering:
    Made sure the daemon starts before any network related devices or services are created/started by using network-pre.target, See:

  3. Startup target / run-level:
    Changed the default install target to basic.target instead of multi-user.target.
    This will allow it to run in any "run-level" in SysV terms.
    See: Units managed by the system service manager@man systemd.special#basic.target

  4. Automatically 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 ConditionKernelCommandLine directive.
    When the no-appfw option is present in the kernel-command-line, the daemon will not startup.
    This option functions same as the well-known quite option. (only applies when present as a separate word)

  5. 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 ConfigurationDirectory and ConfigurationDirectoryMode directives.

  6. Automatically check for kernel support before starting:
    Automatically prevent startup when required kernel support is not present by using the -check-requirements flag in a ExecCondition directive.
    (This assumes the command returns a non-zero exit status when not satisfied.)

  7. Reload functionality:
    Added support for reloading the daemon using the ExecReload directive.
    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.

  8. 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=-1000 directive.
    See: OOMScoreAdjust@man systemd.exec#OOMScoreAdjust
    (This will ensure that the protection keeps functioning even when other processes cause an OOM)

  9. 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:

      [Service]
      Environment='custom_cfg=%E/%N/rules-special'
      • The contents of $custom_cfg is supplied as an argument to the -rules-path option of the daemon.
      • See: Specifiers@man systemd.unit#Specifiers
        %E expands to /etc
        %N expands to the unit name opensnitchd
      • Thus the total expansion of the above will become: /etc/opensnitchd/rules-special
    • To enable debug output:

      [Service]
      Environment='opts=-debug'
      • The contents of $opts is supplied as extra argument(s) to the daemon.
    • Combination of both:

      [Service]
      Environment='custom_cfg=%E/%N/rules-special'
      Environment='opts=-debug'

Copy link
Author

TriMoon commented Aug 15, 2023
edited
Loading

⚠️
The service file as distributed in the deb package still has WRONG name !!!
Someone needs to fix the packaging script !

@evilsocket seems #118 was not proper after all? 🤔

deathtrip reacted with eyes emoji

Copy link
Author

TriMoon commented Aug 18, 2023
edited
Loading

I think i found the spot where the packaged version in the deb differs from what is actually meant to be used:

@gustavo-iniguez-goya could you please make that file a symbolic link to the one in the master tree at: https://github.com/evilsocket/opensnitch/blob/master/daemon/opensnitchd.service 🤔
(Ofcourse at same time fix the name of that file from opensnitch.service to opensnitchd.service, note the d at end of the name) 😉

It will eliminate further discrepancies between the packaged version and the sources of the daemon...

Copy link
Author

TriMoon commented Sep 4, 2023

@lainedfles in reply to #1018 (comment)

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 👍

When not using absolute paths, systemd is using it's own default (build-in) path to find the executables see: systemd-path search-binaries-default.
Thus if using non-absolute paths in this context would make you worry about security, you should not trust systemd to start with 😉

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.

That makefile could/should be changed to use this access-mode anyway to make that directory more secure... 🤷‍♀️
man systemd.exec mentions "Defaults to 0755" which is an insecure value for this directory, that's why i override the default access-mode used.
What is more important to you, improved security settings or warnings due to insecure settings?
In the later case a manual chmod of that directory will both eliminate the warning and secure your settings more...
I stand by my choice of making the default access-mode of this directory 0700 😉

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.

The opensnitchd service/daemon is not started inside the initrd so that's not an issue.
Also the dependencies used under [Unit] make sure it is started way after /etc is mounted anyhow.

There is a benign warning if $opts is unset. I'd recommend setting an empty variable as a better practice. Like: Environment='opts='

Nice catch, i expected for it to become empty in the expansion so i didn't expect that 👍

The introduction of the custom_config 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.

It can be removed when the functionality you mention is implemented, but until then lets keep it in the unit.

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 $MAINPID

Copy link
Author

@TriMoon TriMoon left a comment

Choose a reason for hiding this comment

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

Sugestions from @lainedfles

Applied suggested changes by @lainedfles
Copy link
Author

@TriMoon TriMoon left a comment
edited
Loading

Choose a reason for hiding this comment

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

Please see my comment on last commit...
(accessible using the chat icon on the commit line above)

Copy link
Author

TriMoon commented Sep 4, 2023
edited
Loading

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.

The opensnitchd service/daemon is not started inside the initrd so that's not an issue. Also the dependencies used under [Unit] make sure it is started way after /etc is mounted anyhow.

If things do go wrong or just to be extra specific we could add After=local-fs.target under [Unit], but i don't forsee any need at moment. 😉

Copy link
Contributor

luzpaz commented Oct 20, 2023

Any progress here ?

Copy link
Author

TriMoon commented Oct 22, 2023
edited
Loading

@luzpaz ,If i would have made any more changes they would be shown in this thread as commits so no, the PR is still waiting for merging/acceptance...

Feel free to do a review if you want though 👍

Wants=network-pre.target
Conflicts=shutdown.target
# Don't start when 'no-appfw` is in kernel command-line, to allow booting without it.
ConditionKernelCommandLine=!no-appfw
Copy link
Contributor

@petterreinholdtsen petterreinholdtsen Apr 20, 2025

Choose a reason for hiding this comment

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

Is no-appfw a well known kernel command line option, or introduced here?

Copy link
Author

Choose a reason for hiding this comment

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

I invented it just for this service 😉

Copy link
Contributor

Note, Debian uses fragments from this patch in 1030-systemd-service-earlier.patch. It would be nice to know if at least these parts will make it into master.

Copy link
Contributor

In NixOS we currently hand-roll our own minimal version of the systemd service here, without any of these patches: https://github.com/NixOS/nixpkgs/blob/ee930f9755f58096ac6e8ca94a1887e0534e2d81/nixos/modules/services/security/opensnitch.nix#L202-L245

I was intending to add the systemd hardening options to opensnitch, and i was considering upstreaming those - that would result in a merge conflict on this file, but is mostly an orthogonal change set in terms of logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@petterreinholdtsen petterreinholdtsen petterreinholdtsen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /