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

Fixes for systemd units #151

Merged
merged 6 commits into from Jan 30, 2020
Merged

Fixes for systemd units #151

merged 6 commits into from Jan 30, 2020

Conversation

Maryse47
Copy link
Contributor

@Maryse47 Maryse47 commented Jan 24, 2020

unbound.service.in: add RuntimeDirectory and ConfigurationDirectory

Adding 'RuntimeDirectory' is needed when pidfile path is set to
subdirectory under /run.

Adding ConfigurationDirectory may help in some non-standard setups.

Also add more descriptions about used rules to avoid user confusion
about they meaning and purpose.

Move unbound_nochroot.service to unbound_portable.service

The real purpose of this service is to make it work with
https://systemd.io/PORTABLE_SERVICES/ which are incompatible with
chroot workarounds from original unbound.service.

The service content is identical to unbound.service with exception
for chroot related rules which were modified as needed.

@Maryse47 Maryse47 changed the title unbound.service.in: add RuntimeDirectory and improve descriptions. Fixes for systemd units Jan 24, 2020
@wcawijngaards wcawijngaards self-assigned this Jan 27, 2020
@wcawijngaards
Copy link
Member

wcawijngaards commented Jan 27, 2020

Yes, as I wrote #150 (comment) in a comment for #150, this looks like a good fix. I'll wait a little more to hear your comments on the issue, perhaps it can be improved further.

@Maryse47
Copy link
Contributor Author

Maryse47 commented Jan 27, 2020

@wcawijngaards I left comment in #150 (comment) . We may find improvements for both generic and portable unit in the future but I think this one is good to go for now

@Maryse47
Copy link
Contributor Author

Maryse47 commented Jan 27, 2020

I rebased this to cover some new unbound_nochroot - unbound_portable transitions.

Maryse47 added 3 commits Jan 27, 2020
Adding 'RuntimeDirectory' is needed when pidfile path is set to
subdirectory under /run.

Adding ConfigurationDirectory may help in some non-standard setups.

Also add more descriptions about used rules to avoid user confusion
about they meaning and purpose.
The real purpose of this service is to make it work with
https://systemd.io/PORTABLE_SERVICES/ which are incompatible with
chroot workarounds from original unbound.service.

The service content is identical to unbound.service with exception
for chroot related rules which were modified as needed.
State directory will be created under /var/lib/unbound and will be
useful for writing various files managed at runtime like trust
anchors updates there instead of in ConfigureDirectory which could
be made read-only next. For this chroot needs to be disabled.
@Maryse47
Copy link
Contributor Author

Maryse47 commented Jan 27, 2020

Added StateDirectory=unbound as per #150 (comment)

Maryse47 added 3 commits Jan 27, 2020
CAP_CHOWN is needed for changing onwership of pidfile before
dropping privileges and truncate pidfile on exit.
CAP_IPC_LOCK controls whether a process can lock pages into physical
memory (for instance to prevent passwords or private keys from
being swapped to disk), e.g. mmap() with the MAP_LOCKED flag or
shmctl() with the SHM_LOCK command, neither of which seem to be
used by unbound.
Pidfiles aren't needed while running unbound through systemd.
The PID of the unbound daemon can still be obtained with:
'systemctl show --property MainPID --value unbound'.

While disabling pidfiles we can also drop CAP_CHOWN and writable
/run directory.
@Maryse47 Maryse47 requested a review from wcawijngaards Jan 30, 2020
@Maryse47
Copy link
Contributor Author

Maryse47 commented Jan 30, 2020

@wcawijngaards I think all discussions were addressed now.

Copy link
Member

@wcawijngaards wcawijngaards left a comment

Review of changes looks good. Thanks for the contributed patches!

@wcawijngaards wcawijngaards merged commit e4e00db into NLnetLabs:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants