Bug 278919
| Summary: | irc/ngircd: update to 27.0 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Siva Mahadevan <me> | ||||||||
| Component: | Individual Port(s) | Assignee: | Vladimir Druzenko <vvd> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Some People | CC: | me, vvd | ||||||||
| Priority: | --- | ||||||||||
| Version: | Latest | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| URL: | https://github.com/ngircd/ngircd/releases/tag/rel-27 | ||||||||||
| Attachments: |
|
||||||||||
Maintainer informed via mail Any update on this? MAINTAINER=fgsch@lodoss.net - no user in bugzilla with this email. Anyway maintainer timeout > 2 weeks. 1. I don't see how user and group used in rc.d script. 2. UID and GID 345 already occupied - want about 346? 3. Option DOCS already in DEFAULT by ports framework. 4. PORTVERSION => DISTVERSION. 5. Sort options to make happy portclippy. I can take over maintainership of this port if needed.
> UID and GID 345 already occupied - want about 346?
Yes I had submitted this patch a while ago and it must've gotten stale. I'll send a new set of updates soon along with portclippy/portlint.
> I can take over maintainership of this port if needed. All commits from 2017 are "maintainer timeout" or "portmgr blanket": https://cgit.freebsd.org/ports/log/irc/ngircd https://www.freshports.org/irc/ngircd/ I'll change maintainership. Add this change in patch too. Created attachment 252771 [details] [PATCH] irc/ngircd: update to 27.0 Updated with feedback. > I don't see how user and group used in rc.d script It's used by the RC framework as seen here https://github.com/freebsd/freebsd-src/blob/main/libexec/rc/rc.subr#L965 . This causes ${command} to be run as the 'ngircd' user. (In reply to Siva Mahadevan from comment #8) ${name}_user and ${name}_group both used for chroot: https://github.com/freebsd/freebsd-src/blob/main/libexec/rc/rc.subr#L1476 I don't see port is "chroot ready". Did you tested it and it run as your custom user? And for kill process: https://github.com/freebsd/freebsd-src/blob/main/libexec/rc/rc.subr#L1784 (In reply to Vladimir Druzenko from comment #9) Yes I am running this in production and can confirm that 'service ngircd restart' is able to correctly run (and kill) as the 'ngircd' user. Created attachment 252789 [details]
v1
ngircd change user self after start - we don't need to pass user to start script.
(In reply to Vladimir Druzenko from comment #12) I actually like the approach better that I had in my patch; to use 'ngircd_user=ngircd' and 'ngircd_group=ngircd'. This ensures that the command itself is started from the beginning as 'ngircd'. I don't think that ngircd should even have support for switching users through its '-u' switch, and will be petitioning to remove that support (since tools like 'su' and 'doas' exist to do the same thing in a more correct way). Also, according to hier(7), /nonexistent is the correct home directory to use here since ngircd does NOT need a home directory, rather than needing an empty one with /var/empty. (In reply to Siva Mahadevan from comment #13) > I actually like the approach better that I had in my patch; to use 'ngircd_user=ngircd' and 'ngircd_group=ngircd'. This ensures that the command itself is started from the beginning as 'ngircd'. I don't think that ngircd should even have support for switching users through its '-u' switch, and will be petitioning to remove that support (since tools like 'su' and 'doas' exist to do the same thing in a more correct way). If so then it need correct permissions for dir and files /usr/local/etc/ngircd - by default it can't read config file (I tested run): > Aug 15 22:35:15 myhost ngircd[83022]: Can't read specified configuration file "/usr/local/etc/ngircd/ngircd.conf": Permission denied > Aug 15 22:35:15 myhost ngircd[83022]: ngIRCd exiting due to fatal errors! Also he tried to change UID and GID self: > Aug 16 00:22:23 myhost ngircd[86307]: Can't change group ID to nobody(65534): Operation not permitted! > Aug 16 00:22:23 myhost ngircd[86307]: Can't drop supplementary group IDs: Operation not permitted! > Aug 16 00:22:23 myhost ngircd[86307]: Can't change user ID to nobody(65534): Operation not permitted! > Also, according to hier(7), /nonexistent is the correct home directory to use here since ngircd does NOT need a home directory, rather than needing an empty one with /var/empty. ngircd trying cd to this dir: > Aug 16 00:22:23 myhost ngircd[86308]: Can't change working directory to "/nonexistent": No such file or directory! (In reply to Vladimir Druzenko from comment #14) > If so then it need correct permissions for dir and files /usr/local/etc/ngircd - by default it can't read config file (I tested run): > Aug 15 22:35:15 myhost ngircd[83022]: Can't read specified configuration file "/usr/local/etc/ngircd/ngircd.conf": Permission denied > Aug 15 22:35:15 myhost ngircd[83022]: ngIRCd exiting due to fatal errors! I have a patch open upstream for this fix already: https://github.com/ngircd/ngircd/pull/321 > Also he tried to change UID and GID self: > Aug 16 00:22:23 myhost ngircd[86307]: Can't change group ID to nobody(65534): Operation not permitted! > Aug 16 00:22:23 myhost ngircd[86307]: Can't drop supplementary group IDs: Operation not permitted! > Aug 16 00:22:23 myhost ngircd[86307]: Can't change user ID to nobody(65534): Operation not permitted! I will try to submit a patch for this upstream as well. I don't think a simple user application daemon should do any of this, since the tools are already available to do so if needed externally. In the meantime though, I think your patch is fine since it avoids this issue. > ngircd trying cd to this dir: >> Aug 16 00:22:23 myhost ngircd[86308]: Can't change working directory to "/nonexistent": No such file or directory! I think this is just a warning and is unnecessary. I'll look into this upstream as well. (In reply to Siva Mahadevan from comment #15) > I don't think a simple user application daemon should do any of this Maybe it's not customary now, but that's how it was always done in the past - daemons discard permissions self just after bind on ports and open all necessary files. (In reply to Vladimir Druzenko from comment #16) Makes sense. I like the patch you provided then since I do prioritize updating the port over any other minor concerns like this. In the meantime, I'll work on proposing changes upstream. (In reply to Siva Mahadevan from comment #17) So what do we do now: do you make any edits based on my suggestions? For example, GROUP is not used at all in the rc.d script. So why transfer it there? (In reply to Vladimir Druzenko from comment #18) I have tested your patch named 'v1' as-is in my production setup, things are working great. I'm good with submitting your patch, no additional edits necessary. (In reply to Siva Mahadevan from comment #19) Ok. If you decide to change something in the future - make a patch, create a PR and you can add me to the CC. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=5c11246a4716e4f841b348917f3be544ba22817f commit 5c11246a4716e4f841b348917f3be544ba22817f Author: Siva Mahadevan <me@svmhdvn.name> AuthorDate: 2024-08-21 16:57:50 +0000 Commit: Vladimir Druzenko <vvd@FreeBSD.org> CommitDate: 2024-08-21 17:07:49 +0000 irc/ngircd: Update 26.1 → 27, change maintainership Changelog: https://github.com/ngircd/ngircd/releases/tag/rel-27 Change maintainership: * all commits from 2017 are "maintainer timeout" or "portmgr blanket": https://cgit.freebsd.org/ports/log/irc/ngircd https://www.freshports.org/irc/ngircd/ * fgsch@lodoss.net - no user in bugzilla with this email Port changes: * Add a dedicated system user/group pair for better daemon permissions * Move PLIST files into pkg-plist for better conditional installation of files * Fix installation of documentation files to %%DOCSDIR%% * Put configuration file (and sample) into %%ETCDIR%% * Run a --configtest before starting daemon for sanity check * Replace PORTVERSION with DISTVERSION * Remove GNU_CONFIGURE_MANPREFIX * Sort options to make happy portclippy PR: 278919 GIDs | 2 +- UIDs | 2 +- irc/ngircd/Makefile | 42 +++++++++++++++++++++--------------------- irc/ngircd/distinfo | 6 +++--- irc/ngircd/files/ngircd.in | 16 +++++++++++++--- irc/ngircd/pkg-plist (new) | 27 +++++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 29 deletions(-) Thanks. |
Created attachment 250588 [details] [PATCH] irc/ngircd: update to 27.0 Patch attached. Other minor improvements: * Add a dedicated system user/group pair for better daemon permissions * Move PLIST files into pkg-plist for better conditional installation of files * Fix installation of documentation files to %%DOCSDIR%% * Put configuration file (and sample) into %%ETCDIR%% * Run a --configtest before starting daemon for sanity check