Skip to content
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

Update unbound.service.in #149

Closed
wants to merge 1 commit into from
Closed

Update unbound.service.in #149

wants to merge 1 commit into from

Conversation

@Frzk
Copy link
Contributor

Frzk commented Jan 23, 2020

I'm trying to use (and deploy) unbound as a portable service. With the provided .service file, it seems very difficult, mostly because it puts everything under a chroot directory. The chroot thing seems a bit cumbersome though since we are using systemd to harden the environment unbound will run in.
So, here are a few changes I suggest to make things a bit more "standard" (IMHO). The changes are mostly inspired by what is provided by systemd in /usr/lib/systemd/portable/profile/default/service.conf.

I also added an ExecStop= directive because why not :)

The removed ReadWritePaths=, TemporaryFileSystem=, BindReadOnlyPaths= and BindPaths= directives have been replaced by ConfigurationDirectory= and RuntimeDirectory= which makes things clearer IMHO. Some of them were also already implied by ProtectSystem=strict.
I'll be happy to give some more precisions if needed.

These changes also require chroot: "" and pidfile: /run/unbound/unbound.pid but that might be something for the packagers to take care of when compiling (depending on the use of systemd in the distro ?) ?

I'm trying to use (and deploy) unbound as a portable service. With the provided .service file, it seems very difficult, mostly because it puts everything under a `chroot` directory. The chroot thing seems a bit cumbersome though since we are using systemd to harden the environment unbound will run in.
So, here are a few changes I suggest to make things a bit more "standard" (IMHO). The changes are mostly inspired by what is provided by systemd in /usr/lib/systemd/portable/profile/default/service.conf.

I also added an `ExecStop=` directive because why not :)

The removed `ReadWritePaths=`, `TemporaryFileSystem=`, `BindReadOnlyPaths=` and `BindPaths=` directives have been replaced by `ConfigurationDirectory=` and `RuntimeDirectory=` which makes things clearer IMHO. Some of them were also already implied by `ProtectSystem=strict`.
I'll be happy to give some more precisions if needed.

These changes also require `chroot: ""` and `pidfile: /run/unbound/unbound.pid` but that might be something for the packagers to take care of when compiling (depending on the use of systemd in the distro ?) ?
@wcawijngaards wcawijngaards self-assigned this Jan 23, 2020
@wcawijngaards

This comment has been minimized.

Copy link
Member

wcawijngaards commented Jan 23, 2020

Hi,
Thank you for the contribution. And I want to include it. But maybe these changes do not work together with other people's setups. Perhaps we should have a separate file for people that want chroot: "" setup and systemd files? And then also write some guidance (like in a README file with it, or comments in it if those are prominent to the user) on how to configure unbound to go with it?

@Frzk

This comment has been minimized.

Copy link
Contributor Author

Frzk commented Jan 23, 2020

Yes I agree. That's something I'm willing to contribute too but I'll need some guidance.
Where do we start ? :-)

@wcawijngaards

This comment has been minimized.

Copy link
Member

wcawijngaards commented Jan 23, 2020

Create a new file contrib/unbound_nochroot.service.in and copy your changes in there, leave the unbound.service.in unmodified. Then apply this change to configure.ac and that should make it work.
Put comments at the start of the new unbound_nochroot.service.in, describing the unbound config that is needed for this systemd integration.

This diff adds contrib/unbound_nochroot.service to the AC_CONFIG_FILES list.

diff --git a/configure.ac b/configure.ac
index e1cddc35..77e50cc5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2047,6 +2047,6 @@ dnl if this is a distro tarball, that was already done by makedist.sh
 AC_SUBST(version, [VERSION_MAJOR.VERSION_MINOR.VERSION_MICRO])
 AC_SUBST(date, [`date +'%b %e, %Y'`])
 
-AC_CONFIG_FILES([Makefile doc/example.conf doc/libunbound.3 doc/unbound.8 doc/unbound-anchor.8 doc/unbound-checkconf.8 doc/unbound.conf.5 doc/unbound-control.8 doc/unbound-host.1 smallapp/unbound-control-setup.sh dnstap/dnstap_config.h dnscrypt/dnscrypt_config.h contrib/libunbound.pc contrib/unbound.socket contrib/unbound.service])
+AC_CONFIG_FILES([Makefile doc/example.conf doc/libunbound.3 doc/unbound.8 doc/unbound-anchor.8 doc/unbound-checkconf.8 doc/unbound.conf.5 doc/unbound-control.8 doc/unbound-host.1 smallapp/unbound-control-setup.sh dnstap/dnstap_config.h dnscrypt/dnscrypt_config.h contrib/libunbound.pc contrib/unbound.socket contrib/unbound.service contrib/unbound_nochroot.service])
 AC_CONFIG_HEADER([config.h])
 AC_OUTPUT
@Frzk Frzk mentioned this pull request Jan 23, 2020
@Frzk

This comment has been minimized.

Copy link
Contributor Author

Frzk commented Jan 23, 2020

Follow-up in #150.

@Frzk Frzk closed this Jan 23, 2020
@Frzk Frzk deleted the Frzk:patch-1 branch Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.