On 2025/02/28 01:45, izzy Meyer wrote:
> On Fri, 28 Feb 2025 03:13:53 +0000
> Lucas Gabriel Vuotto <lu...@sexy.is> wrote:
>
> > Hello,
> >
> > On Thu, Feb 27, 2025 at 07:52:30PM -0600, izzy Meyer wrote:
> > > Hello ports@
> > >
> > > Attached is a diff to set SUID on xmsm and xmsession which fixes the
> > > locking/logout/poweroff/shutdown errors. This also bumps REVISION to
> > > 1.
shutdown should work already, as long as the user is in group _shutdown.
if you change the REBOOT_CMD #define to "/sbin/shutdown -r now" then the
same should apply for reboot.
> > > Index: x11/emwm-utils/Makefile
> > > ===================================================================
> > > RCS file: /cvs/ports/x11/emwm-utils/Makefile,v
> > > diff -u -r1.1.1.1 Makefile
> > > --- x11/emwm-utils/Makefile 23 Aug 2024 06:03:38
> > > -0000 1.1.1.1 +++ x11/emwm-utils/Makefile 28 Feb 2025
> > > 01:49:29 -0000 @@ -1,6 +1,7 @@
> > > COMMENT = session manager and a toolchest-like application
> > > launcher
> > > V = 1.2
> > > +REVISION = 1
> >
> > REVISION starts at 0.
>
> Whoops, bit of an oversight on my part. Thanks!
>
> > > DISTNAME = emwm-utils-src-${V}
> > > PKGNAME = emwm-utils-${V}
> > >
> > > @@ -28,5 +29,9 @@
> > > RCDIR=${WRKINST}${PREFIX}/lib/X11
> > >
> > > NO_TEST = Yes
> > > +
> > > +# xmsession and xmsm need suid to lock properly
> > > +post-install:
> > > + chmod u+s ${PREFIX}/bin/{xmsm,xmsession}
> >
> > Ports usually handle this differently, by hooking into the BSD
> > Authentication system (man authenticate). Take a look at x11/slock,
> > which might be one of the simplest implementations.
> >
>
> I looked into slock.c under the WRKSRC of x11/slock, and found no
> mention of the functions shown in authenticate(3). BUT- What I *do* see
> is an explicit mention os SUID/SGID:
>
> #ifdef __OpenBSD__
> if (!(pw = getpwuid_shadow(getuid())))
> die("slock: getpwnam_shadow: cannot retrieve
> shadow entry. " "Make sure to suid or sgid slock.\n");
> hash = pw->pw_passwd;
>
> Additionally, the Makefile in the WRKSRC of x11/slock specifically sets
> SUID:
>
> install: all
> @echo installing executable file to ${DESTDIR}${PREFIX}/bin
> @mkdir -p ${DESTDIR}${PREFIX}/bin
> @cp -f slock ${DESTDIR}${PREFIX}/bin
> @chmod 755 ${DESTDIR}${PREFIX}/bin/slock
> @chmod u+s ${DESTDIR}${PREFIX}/bin/slock # here
> [..snip]
>
> I must be missing something.
use 'make patch', not 'make extract'.
> Any specific hard-need for using authenticate(3) here? as simply
> setting SUID seems to do the trick fine.
if you don't understand the difference you definitely shouldn't be using
setuid root.
the buffer size calculations for sprintf do not give me a warm and fuzzy
feeling but might be ok. the expand_env_vars() implementation with its
strcat, multiplications for calloc, etc, should stay the hell away from
setuid root.
certainly should be no more than setgid _shadow and even that's a
stretch. converting to bsd-auth would be an improvement.
> Minor question, I actually don't see SUID being set with my diff when
> installed on my system, but inside of the fake install, it is SUID? Does
> this not get preserved when packaging or something?
correct, it needs to be annotated in the PLIST. it is simply too
dangerous to have these things easy to apply based on the port build,
it must at least be an explicit decision by the port maintainer and
whoever commits such a diff.
