On 2025/05/13 10:14, izzy Meyer wrote:
> On Tue, 13 May 2025 13:51:33 +0100
> Stuart Henderson <[email protected]> wrote:
>
> > On 2025/05/13 07:11, izzy Meyer wrote:
> > > On Sat, 10 May 2025 19:04:11 +0000
> > > Lucas Gabriel Vuotto <[email protected]> wrote:
> > >
> > > > On Sat, May 10, 2025 at 12:53:44PM -0500, izzy Meyer wrote:
> > > > > Ended up just patching the file for simplicity.
> > > >
> > > > I tend to avoid patching as much as possible, but to each one
> > > > their own. You're the maintainer anyways.
> > >
> > > This might make maintaining easier, but I couldn't get the -D flag
> > > in CFLAGS to apply correctly when setting REBOOT_CMD to
> > > "/sbin/shutdown -r now" in CFLAGS with -D. I must be missing
> > > something here:
> >
> > You can do it with
> >
> > MAKE_FLAGS = CFLAGS="${CFLAGS} -I./Xm -I${X11BASE}/include
> > -I${LOCALBASE}/include -D'REBOOT_CMD=\"/sbin/shutdown -r now\"'"
>
> Thanks!
>
> > but that's harder for maintenance because you don't get notified by
> > patch(1) if upstream changes that variable in their Makefile, which
> > you may need to adapt to.
> >
> > The version with the patch is imho saner.
>
> This is what I was thinking initially when doing it via a patch. I'll
> attach a new diff with this added back.
>
> > > > This package is not installed with SUID root; warnings about
> > > > lacking SUID root can be ignored.
> >
> > I don't particularly want to mess with my X setup to test it now, but
> > if that warning does still get displayed but is irrelevant after the
> > changes to the port, I would patch away the warning, rather than
> > adding to the pkg-readme telling people to ignore it.
>
> Good catch. Duh, why didn't I think of this! xD
>
> > : - To enable shutdown and reboot, the user should be in _shutdown
> > : group.
> > : - To enable suspend, the user needs to be able to run zzz(8).
> > :
> > : Use usermod(8) to add the user to _shutdown group.
> >
> > Might as well give an example for usermod:
> >
> > # usermod -G _shutdown <username>
> >
> > : Enable apmd(8) and check zzz(8) for details about the required
> > permissions.
> >
> > Neither apmd(8) nor zzz(8) are really up-front about permissions.
> >
> > zzz: "The protection modes on this socket govern which users may
> > access the APM functions"
> >
> > apmd: "The socket is protected to mode 0660, UID 0, GID 0; this
> > protects access to suspend requests to authorized users only."
> >
> > The experienced admin would realise what GID 0 is, but for user-level
> > docs it would be clearer to mention in the pkg-readme that the user
> > must be in group "wheel" for this to work -- however 'wheel' is a
> > significant escalation if the only reason is to permit sleep.
>
> Fair point. This is a large escalation for a seemingly simple
> operation.
>
> Does this read better?
It does. Slight tweak though as we can remove some of the text and
group the shutdown/reboot-related text+command in one place,
and zzz-related text+commmand in another (and fix line-ending in
pkg/README which was missing EOL)
I think this is ok
Index: Makefile
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/Makefile,v
diff -u -p -r1.2 Makefile
--- Makefile 8 May 2025 14:19:24 -0000 1.2
+++ Makefile 13 May 2025 15:21:00 -0000
@@ -3,6 +3,7 @@ COMMENT = session manager and a toolches
V = 1.3
DISTNAME = emwm-utils-src-${V}
PKGNAME = emwm-utils-${V}
+REVISION = 0
CATEGORIES = x11
HOMEPAGE = https://fastestcode.org/emwm.html
@@ -20,7 +21,8 @@ WANTLIB += X11 Xinerama Xm Xrandr Xss Xt
LIB_DEPENDS = x11/motif
MAKE_FLAGS = RCDIR=${PREFIX}/lib/X11 \
- CFLAGS="${CFLAGS} -I./Xm -I${X11BASE}/include
-I${LOCALBASE}/include"
+ CFLAGS="${CFLAGS} -DUNPRIVILEGED_SHUTDOWN \
+ -I./Xm -I${X11BASE}/include -I${LOCALBASE}/include"
FAKE_FLAGS = PREFIX=${WRKINST}${PREFIX} \
APPLRESDIR=${WRKINST}${PREFIX}/lib/X11/app-defaults \
Index: patches/patch-src_smconf_h
===================================================================
RCS file: patches/patch-src_smconf_h
diff -N patches/patch-src_smconf_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_smconf_h 13 May 2025 15:21:00 -0000
@@ -0,0 +1,12 @@
+Index: src/smconf.h
+--- src/smconf.h.orig
++++ src/smconf.h
+@@ -33,7 +33,7 @@
+ #endif /* SHUTDOWN_CMD */
+
+ #ifndef REBOOT_CMD
+-#define REBOOT_CMD "/sbin/reboot"
++#define REBOOT_CMD "/sbin/shutdown -r now"
+ #endif
+
+ #ifndef SUSPEND_CMD
Index: patches/patch-src_smmain_c
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/patches/patch-src_smmain_c,v
diff -u -p -r1.1 patch-src_smmain_c
--- patches/patch-src_smmain_c 8 May 2025 14:20:34 -0000 1.1
+++ patches/patch-src_smmain_c 13 May 2025 15:21:00 -0000
@@ -61,3 +61,21 @@ Index: src/smmain.c
memset(pwb,0,strlen(pwb));
XmTextFieldSetString(wpasswd,"");
+@@ -988,7 +1009,8 @@ static Boolean set_privileges(Boolean elevate)
+ if(!initialized){
+ orig_uid = geteuid();
+ orig_gid = getegid();
+-
++ /* BSD-auth handles authentication, no SUID needed. */
++#ifndef __OpenBSD__
+ if(orig_uid != 0){
+ log_msg("%s must be setuid root to enable "
+ "screen locking capabilities.\n",bin_name);
+@@ -996,6 +1018,7 @@ static Boolean set_privileges(Boolean elevate)
+ can_elevate = False;
+ return False;
+ }
++#endif /* __OpenBSD__ */
+ initialized = True;
+ can_elevate = True;
+ }
Index: pkg/PLIST
===================================================================
RCS file: /cvs/ports/x11/emwm-utils/pkg/PLIST,v
diff -u -p -r1.2 PLIST
--- pkg/PLIST 8 May 2025 14:19:24 -0000 1.2
+++ pkg/PLIST 13 May 2025 15:21:00 -0000
@@ -10,3 +10,4 @@ lib/X11/app-defaults/XmToolbox
lib/X11/toolboxrc
@man man/man1/xmsm.1
@man man/man1/xmtoolbox.1
+share/doc/pkg-readmes/${PKGSTEM}
Index: pkg/README
===================================================================
RCS file: pkg/README
diff -N pkg/README
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ pkg/README 13 May 2025 15:21:00 -0000
@@ -0,0 +1,15 @@
++-------------------------------------------------------------------------------
+| Running ${PKGSTEM} on OpenBSD
++-------------------------------------------------------------------------------
+
+To enable shutdown and reboot, the user should be in _shutdown group:
+
+ # usermod -G _shutdown <username>
+
+To enable suspend, the user needs to be able to run zzz(8).
+In order to do that, the user must be in group "wheel", and apmd(8)
+must be running:
+
+ # rcctl enable apmd
+ # rcctl start apmd
+ # usermod -G wheel <username>
