I'm in the market for a new authoritative DNS server and was snooping around. I noticed a few things that could be simplified in the nsd init script, some of which are covered in the service script guide: https://github.com/OpenRC/openrc/blob/master/service-script-guide.md The easy ones: 1. "need net" is probably wrong (covered in the guide) 2. The "test -e" for $NSD_CONFIG in checkconfig() can be handled with "required_files" (man openrc-run) 3. The "kill" command in reload() can be done better with SSD: start-stop-daemon --signal HUP --pidfile "${pidfile}" The hard one: nsd creates its PID file as nsd:nsd, which makes it unsafe to use in the init script. start-stop-daemon runs (and kills things) as root, and the nsd user can write to the PID file, meaning that he can write (for example) "1" to it, causing the system to be rebooted when nsd is stopped. (This is a problem for systemd, too). I don't see an easy way to fix this right now because nsd forks, and if we let OpenRC create its own PID file, it will contain the wrong PID. There's also no option to run nsd in the foreground. This is a confusing one to report upstream, but I'll have to try.
Created attachment 888522 [details] nsd init script I missed the "-d" flag that can be used to run the daemon in the foreground. It still creates an (insecure) PID file, but it can be worked around. Here's a version of the init script that lets OpenRC handle it.
Created attachment 888535 [details] nsd init script v2 v2: passing -P '' stops it from creating the extra PID file when it's running in the foreground
The upstream issue is now fixed, so the PID file will remain root:root after creation and there is no risk in later killing the PID it contains as root. The fork/pid approach can therefore stick around after the next release, although running in the foreground is probably simpler anyway. In any case the comment about the insecure PID file in my v2 is obsolete.
Do you also have a systemd setup and could maybe look at the service file in $FILESDIR and compare it to the upstream-provided service file in 'contrib/' and see if we can drop ours? I only use OpenRC, so vetting systemd stuff is out of my purview. Thanks!
(In reply to Joshua Kinard from comment #4) > Do you also have a systemd setup and could maybe look at the service file in > $FILESDIR and compare it to the upstream-provided service file in 'contrib/' > and see if we can drop ours? I only use OpenRC, so vetting systemd stuff is > out of my purview. Thanks! Uh oh, I also use OpenRC exclusively. I know a bit about how the service scripts work... I just can't test them. With that said, I think the upstream script should work as well as the one in $FILESDIR. The upstream copy has the small issue that it will still create a PID file even though the daemon is running in the foreground (type=simple, in systemd slang). As a result, it needs /run/nsd to exist to hold the PID file. But really, nothing is going to use the PID file, so we shouldn't bother: https://github.com/NLnetLabs/nsd/issues/317#issuecomment-2041438207 In Gentoo we could address that with --with-pidfile="" in src_configure(), for both OpenRC and systemd. In that case, the -P "" that I added to the OpenRC script is not needed.
Another thing I thought of in the meantime: My version of the OpenRC script avoids creating /run/nsd for the PID file, because a PID file is no longer created. However, some users may use nsd-control with a local socket. Where would they have put it? My guess is that they would have put it in /run/nsd. And in any case, we should probably support users who want to run nsd-control locally by providing a standard location with the correct permissions to hold the control socket. It's not safe to parse the config file and chown whatever path is in there to nsd:nsd. So instead, I would suggest that we create /run/nsd (owned by nsd) unconditionally, and document that if you want to use a local path for nsd-control, that you should use /run/nsd/something. We had this exact issue with clamav and its socket. It's not pretty, but using sed to add e.g. # control-interface: /run/nsd/nsd-control.sock into the sample config file has avoided most of the socket-related issues we used to have. So long as that example path works out-of-the-box, users don't try too hard to rock the boat.
The upstream systemd service script now avoids creating a PID file, so no tmpfiles.d entry (and no --with-pidfile="") is needed: https://github.com/NLnetLabs/nsd/commit/1366b6a9f3528ab6438013be8a62ddd3e64a210f
(In reply to Michael Orlitzky from comment #7) > The upstream systemd service script now avoids creating a PID file, so no > tmpfiles.d entry (and no --with-pidfile="") is needed: > https://github.com/NLnetLabs/nsd/commit/ > 1366b6a9f3528ab6438013be8a62ddd3e64a210f The only bits I think I need to copy over from our shipped copy of 'nsd.service' are the two flags to set the user and the group, and then that can be sunset w/ 4.8.x. For the nsd-control socket, I'll look into patching the sample config file over sed, so that it's a bit cleaner. Am thinking we really need to mod the build system to pass in a path for that, but that'll be a more intrusive change, so I can make that longer-term. That said, I have a cleaned-up 4.9.1 ebuild almost done. I've exposed more of the configure toggles as local USE flags, including making some of the forced settings in 4.8.x (ipv6, relro, largefile, etc) togglable. I need to add an exclusion for musl for year2038 support -- been seeing that toggle turn up lately in configure scripts, but only o0n glibc systems. It disappears under musl. The munin and dnstap bits I'll have to just keep my fingers crossed, as I don't use either of those, so hard to test.
(In reply to Joshua Kinard from comment #8) > > The only bits I think I need to copy over from our shipped copy of > 'nsd.service' are the two flags to set the user and the group, and then that > can be sunset w/ 4.8.x. Here's where it would be great if either of us could test the systemd service. The ./configure output doesn't mention it, but if you check configure.ac (in 4.9.1 at least), the default value for --with-user is "nsd". So I think in the absence of the -o and -g flags, it will use nsd:nsd automatically. In other words, the upstream .service file might work without modification.
(In reply to Michael Orlitzky from comment #9) > (In reply to Joshua Kinard from comment #8) > > > > The only bits I think I need to copy over from our shipped copy of > > 'nsd.service' are the two flags to set the user and the group, and then that > > can be sunset w/ 4.8.x. > > Here's where it would be great if either of us could test the systemd > service. The ./configure output doesn't mention it, but if you check > configure.ac (in 4.9.1 at least), the default value for --with-user is > "nsd". So I think in the absence of the -o and -g flags, it will use nsd:nsd > automatically. In other words, the upstream .service file might work without > modification. Yeah, I unfortunately have some...strong opinions...about systemd, so that's why I'm on OpenRC :) That said, I found that it does set the default user context to "nsd". After configure runs, it defines this in config.h: > /* the user name to drop privileges to */ > #define USER "nsd" Nothing about the default group context, though. I guess it just blindly assumes to use "nsd" also, which I suppose might be a minor issue, cause not everything out there uses the "group is same as user" setup. As for the socket...it looks like configure has a trick for that, too: > --localstatedir=DIR modifiable single-machine data [PREFIX/var] > --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] It's kludgy, because runstatedir is off of $localstatedir, which by default, is /var. But we have a /var/run symlink that points back to /run, so I think setting runstatedir to "LOCALSTATEDIR/run/nsd" might give nsd a safe place to store its nsd-control socket and thus, make it usable. Might cause issues with PID files created by NSD, though, but since we're intentionally disabling that, might not be an issue. Testing this is going to be interesting. I have two NSD instances already running on my home network, but they both live in FreeBSD jails on two different machines. Thinking of stringing up a third instance on my dev box and tying that in, then configuring an AXFR key to see if it receives local domain updates. If it can get that far and answer some queries about my local zone, then it should be good.
(In reply to Joshua Kinard from comment #10) > > As for the socket...it looks like configure has a trick for that, too: > > --localstatedir=DIR modifiable single-machine data [PREFIX/var] > > --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run] > It's kludgy, because runstatedir is off of $localstatedir, which by default, > is /var. But we have a /var/run symlink that points back to /run, so I > think setting runstatedir to "LOCALSTATEDIR/run/nsd" might give nsd a safe > place to store its nsd-control socket and thus, make it usable. Might cause > issues with PID files created by NSD, though, but since we're intentionally > disabling that, might not be an issue. Maybe we shouldn't worry about the socket for 4.9.1. The sample config file uses autoconf templating, so later on if we upstream the OpenRC script and have it also use autoconf templating, we can get everything just right: * --runstatedir=/run in the ebuild * checkpath -o @user@ -d @runstatedir@/${RC_SVCNAME} for the socket directory in the openrc script * # control-interface: @runstatedir@/nsd as an example in the config file * command=@sbindir@/nsd (et cetera) in the openrc script rather than hard-coding the paths If you can get the rest to a place where you're happy with it (and nobody reports any bugs), I'll work on a PR eventually.
Created attachment 894093 [details, diff] Draft patch for nsd-4.9.1 Give this patch a roll and let me know if it fixes things. Apply it on top of the existing net-dns/nsd directory and it'll basically match what I plan to commit (already found some minor nits to adjust, but they don't affect much). For the socket, I copied what net-misc/chrony was doing with tmpfiles.d to create /run/chrony for its pidfile and socket, and that seems to work well. Tested some basic lookups and also zone transfers of a dummy zone from my network's primary NSD server, and that looks to have worked out. Let me know if you encounter any issues that I still need to fix. Thanks!
Created attachment 894094 [details, diff] Draft patch for nsd-4.9.1 Updated the patch with some minor fixes for things pkgcheck complained about (ignore the one complaint about 'UnnecessaryManifest', because I work in a tree outside of the actual repo)
Thanks! I've been meaning to come back to this but I got stuck migrating 100+ websites to a new server in my "free time." NSD_CHECKCONF="${NSD_CHECKCONF:-/usr/sbin/nsd-checkconf}" Is there any benefit to having that be configurable? I can't think of a reason why the executable name would change. NSD_CONFIG="${NSD_CONFIG:-/etc/nsd/nsd.conf}" This could be /etc/nsd/${RC_SVCNAME}.conf? That way the default is unchanged, but if you create multiple instances with symlinks, the service started by e.g. "service start nsd-internal" would use /etc/nsd/nsd-internal.conf without having to configure anything. pidfile="/run/nsd/${RC_SVCNAME}.pid" That actually has to be /run/${RC_SVCNAME}.pid to avoid bringing back the security issue now that /run/nsd is writable by nsd. # By default, NSD will fork into the background and create a PID file in # an insecure manner. Since OpenRC will manage the PID file, tell NSD not # to create one, and also keep it in the foreground. Upstream fixed that bug, so I would suggest # Since OpenRC will manage the PID file, tell NSD not # to create one, and also keep it in the foreground. now.
(In reply to Michael Orlitzky from comment #14) > NSD_CHECKCONF="${NSD_CHECKCONF:-/usr/sbin/nsd-checkconf}" > > Is there any benefit to having that be configurable? I can't think of a > reason why the executable name would change. I cannot think of one. For sanity, I looked at what the rc.d script on my FreeBSD machine was doing, and they just flat hardcode the path, so I switched this variable out for a similar hardcode (lowered the casing to make it more apparent) and moved it down a few lines. > NSD_CONFIG="${NSD_CONFIG:-/etc/nsd/nsd.conf}" > > This could be /etc/nsd/${RC_SVCNAME}.conf? That way the default is > unchanged, but if you create multiple instances with symlinks, the service > started by e.g. "service start nsd-internal" would use > /etc/nsd/nsd-internal.conf without having to configure anything. I tweaked this a bit differently NSD_CONFBASE="${NSD_CONFBASE:-/etc/nsd}" NSD_CONFIG="${NSD_CONFIG:-${NSD_CONFBASE}/${RC_SVCNAME}.conf}" Basically used the ${RC_SVCNAME} per your example, but I kept the ability to change the default config file path as well. I should probably add an example /etc/conf.d/nsd file to things, too... > pidfile="/run/nsd/${RC_SVCNAME}.pid" > > That actually has to be /run/${RC_SVCNAME}.pid to avoid bringing back the > security issue now that /run/nsd is writable by nsd. Fixed by changing the pidfile location back to just /run. I still give a pidfile path to nsd's configure script as also being at the toplevel of /run, too, but that only comes into effect if nsd itself creates and pid, and it looks like we're fully leveraging OpenRC to do that now. I'll include an explanatory note in a conf.d bit about this, and set the default args to indicate foreground run + no pid, but leave it up to the user to change if they want.
Created attachment 894307 [details, diff] Draft patch for nsd-4.9.1 v3 Give this patch a once over. I made the initial adjustments before posting my last comment, then made some more tweaks. These are the three configurable vars in /etc/conf.d/nsd: NSD_ARGS="${NSD_ARGS:--d -P ''}" NSD_CONFBASE="${NSD_CONFBASE:-/etc/nsd}" NSD_CONFNAME="${NSD_CONFNAME:-${RC_SVCNAME}.conf}" The older initscript will be locked to 4.8 and will be removed once I stabilize 4.9 later on, after it soaks in ~arch for a bit. I think 4.10 is due out soon, too, as they announced an RC not too long ago.
Created attachment 894346 [details, diff] Draft patch for nsd-4.9.1 v4 Minor fixes in the ebuild.
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=8e0365b35f2190eb7f1cf28803e3e1ccced8039a commit 8e0365b35f2190eb7f1cf28803e3e1ccced8039a Author: Joshua Kinard <kumba@gentoo.org> AuthorDate: 2024-06-03 06:18:15 +0000 Commit: Joshua Kinard <kumba@gentoo.org> CommitDate: 2024-06-03 06:21:07 +0000 net-dns/nsd: Add an ebuild for nsd-4.9.1 w/ many improvements The ebuild for nsd-4.9.1 that includes a number of cleanups, including adding in more USE flags to support additional switches to nsd's configure script. Other fixes include: - Significant input from @mjo to revamp the OpenRC initscript to be smarter about things - Added a /etc/conf.d/nsd file with sane defaults - Added the use of tmpfiles.d for nsd-control's socket in /run - Switching to use of the upstream-provided systemd service unit In addition, the ebuilds for nsd-4.7.0 and nsd-4.8.0-r0 are dropped. Closes: https://bugs.gentoo.org/927552 Signed-off-by: Joshua Kinard <kumba@gentoo.org> net-dns/nsd/Manifest | 2 +- .../files/nsd-4.7.0-no-bind8-stats-no-ssl.patch | 23 ---- .../nsd/files/nsd-4.9.1-systemd-no-pidfile.patch | 16 +++ ...d_munin_.patch => nsd-munin-gentoo-paths.patch} | 0 net-dns/nsd/files/nsd.confd-r1 | 16 +++ net-dns/nsd/files/nsd.initd-r2 | 52 +++++++ net-dns/nsd/files/nsd.tmpfilesd-r1 | 1 + net-dns/nsd/metadata.xml | 27 ++-- net-dns/nsd/nsd-4.7.0.ebuild | 129 ------------------ net-dns/nsd/nsd-4.8.0-r1.ebuild | 4 +- net-dns/nsd/nsd-4.8.0.ebuild | 127 ------------------ net-dns/nsd/nsd-4.9.1.ebuild | 149 +++++++++++++++++++++ net-dns/nsd/nsd-9999.ebuild | 96 ++++++++----- 13 files changed, 311 insertions(+), 331 deletions(-)
I started a branch at, https://github.com/orlitzky/nsd/tree/openrc to move the OpenRC bits upstream: * I changed the name of NSD_ARGS to NSD_EXTRA_OPTS to agree with the name that upstream is already using for the systemd service. That might annoy a few people but etc-update will catch it and in the long run it's better than having two names for the same thing. * NSD_CONFBASE is now automatically set by the build system, so not needed any longer * NSD_CONFNAME is now ${RC_SVCNAME}.conf unconditionally. If someone wants to improve the upstream systemd file to bring it to feature-parity, I think the way they would do would be to use the instance name (%i) like we do with RC_SVCNAME. This might also annoy one or two people but it's easy enough to rename the file to agree with the service name.
I opened a PR to push the OpenRC bits upstream: https://github.com/NLnetLabs/nsd/pull/352 The only new change is that stop_pre() also checks the config file so a restart won't stop the daemon and then refuse to start it again.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=9bdd082a7cd14d3af5a7f0e5438e224d88e10fbb commit 9bdd082a7cd14d3af5a7f0e5438e224d88e10fbb Author: Joshua Kinard <kumba@gentoo.org> AuthorDate: 2024-08-04 10:13:04 +0000 Commit: Joshua Kinard <kumba@gentoo.org> CommitDate: 2024-08-04 10:15:21 +0000 net-dns/nsd: Cleanups, add 4.10.1 Clean out older ebuilds and files in FILESDIR and add a new ebuild for recently released 4.10.1, which includes fixes for parallelism in the simdzone Makefile and OpenRC init/conf files sent to upstream by @mjo. Closes: https://bugs.gentoo.org/936119 Bug: https://bugs.gentoo.org/927552 Co-authored-by: Michael Orlitzky <mjo@gentoo.org> Signed-off-by: Joshua Kinard <kumba@gentoo.org> net-dns/nsd/Manifest | 3 +- .../nsd/files/nsd-4.8.0-implausible-stats.patch | 22 ---- net-dns/nsd/files/nsd.initd-r1 | 59 ---------- net-dns/nsd/files/nsd.service | 14 --- net-dns/nsd/metadata.xml | 9 +- .../nsd/{nsd-4.10.0.ebuild => nsd-4.10.1.ebuild} | 12 +- net-dns/nsd/nsd-4.8.0-r1.ebuild | 128 --------------------- net-dns/nsd/nsd-4.9.1.ebuild | 4 +- net-dns/nsd/nsd-9999.ebuild | 12 +- 9 files changed, 20 insertions(+), 243 deletions(-)
