Hi izzy,
On Thu, May 22, 2025 at 12:08:52PM -0500, izzy Meyer wrote:
> I said this in a previous mail [2], but I think it got overlooked:
>
> > Is it possible to backport this to -stable as the bsd-auth patch and
> > REBOOT_CMD fix breaking bugs in the port? Without these two changes,
> > the locking mechanism freezes up the xsession and reboot/shutdown/halt
> > do the same. I have a machine that I run -stable on, and it'd be nice
> > to have these fixes on that machine. I understand port updates usually
> > don't get backported to -stable, but would this be a valid enough
> > exception?
>
> Ideally, x11/emwm should be updated in tandem with x11/emwm-utils as
> they work as a team, so could that be backported as well?
I sent my reply to you privately on Sun, 18 May 2025 06:33:36 +0000:
| Backports to -stable are reserved for security or serious bugs. The fix
| freezings does fall in that category, but that's the work of your patch
| and not the release. In fact, the release is marked as "small fixes".
|
| Because of that, I'll need that you send me auth patches backported to
| 1.2 and then I can merge it.
> In a previous mail, I was asked to suppress a warning about xmsm lacking
> suid root. [1]
>
> I patched too much in my #ifndef/#endif to suppress the warning, so the
> logic ended up assuming that the non SUID root binary can be ran as
> root, and exec's /usr/libexec/login_passwd as root when
> attempting a lock. This is incorrect. I narrowed down the
> #ifndef/#ifdef logic to just include the warning and the failure to
> lock is now fixed.
>
> This also bumps REVISION to 1.
>
> Tested on macppc/powerpc, arm64, and amd64.
>
> Good to merge?
I have a few comments about this one.
> diff --git a/Makefile b/Makefile
> index 6a5e5822b36..127fceca498 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3,7 +3,7 @@ COMMENT = session manager and a toolchest-like
> application launcher
> V = 1.3
> DISTNAME = emwm-utils-src-${V}
> PKGNAME = emwm-utils-${V}
> -REVISION = 0
> +REVISION = 1
>
> CATEGORIES = x11
> HOMEPAGE = https://fastestcode.org/emwm.html
> diff --git a/patches/patch-src_smmain_c b/patches/patch-src_smmain_c
> index cf6ad7122bf..32ca62c615b 100644
> --- a/patches/patch-src_smmain_c
> +++ b/patches/patch-src_smmain_c
> @@ -61,21 +61,18 @@ Index: src/smmain.c
>
> memset(pwb,0,strlen(pwb));
> XmTextFieldSetString(wpasswd,"");
> -@@ -988,7 +1009,8 @@ static Boolean set_privileges(Boolean elevate)
> +@@ -988,10 +1009,13 @@ 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){
> ++ /* BSD-auth handles authentication, no SUID needed. */
> ++#ifndef __OpenBSD__
> 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)
> ++#endif /* __OpenBSD__ */
> ++
> + initialized = True;
> can_elevate = False;
> return False;
> - }
> -+#endif /* __OpenBSD__ */
> - initialized = True;
> - can_elevate = True;
> - }
First of all, does this solve anything? It looks correct and makes
sense, but doesn't really change the execution flow as it isn't
installed suid. orig_[gu]id get assigned the running user / group ID and
then the calls for sete[gu]id succeed.
> diff --git a/patches/patch-src_smmain_c.orig b/patches/patch-src_smmain_c.orig
> new file mode 100644
> index 00000000000..cf6ad7122bf
> --- /dev/null
> +++ b/patches/patch-src_smmain_c.orig
Are you sure this is the one you wanted to include in here? It creates
an additional file, patches/patch-src_smmain_c.orig, which won't be used
for patching the port. Is there a part for patches/patch-src_smmain_c
missing in this mail or is this inclusion an overlook?
> @@ -0,0 +1,81 @@
> +# For bsd-auth support
> +
> +Index: src/smmain.c
> +--- src/smmain.c.orig
> ++++ src/smmain.c
> +@@ -60,6 +60,9 @@
> + #if defined(__linux__) || defined(__svr4__)
> + #include <crypt.h>
> + #include <shadow.h>
> ++#elif defined(__OpenBSD__)
> ++#include <bsd_auth.h>
> ++#include <login_cap.h>
> + #endif
> + #include "smglobal.h"
> + #include "smconf.h"
> +@@ -419,6 +422,11 @@ static void lock_screen(void)
> + app_res.enable_locking = False;
> + return;
> + }
> ++
> ++#ifdef __OpenBSD__
> ++ /* BSD-auth handles authentication, no password hash check needed */
> ++ can_auth = True;
> ++#else
> +
> + if(set_privileges(True)) {
> +
> +@@ -441,7 +449,8 @@ static void lock_screen(void)
> +
> + set_privileges(False);
> + }
> +-
> ++#endif /* __OpenBSD__ */
> ++
> + if(!can_auth){
> + if(!app_res.silent) XBell(XtDisplay(wshell), 100);
> + log_msg("Cannot authenticate. Screen locking disabled!\n");
> +@@ -861,6 +870,17 @@ static void passwd_enter_cb(Widget w,
> + char *upw = NULL;
> +
> + login = get_login();
> ++
> ++#ifdef __OpenBSD__
> ++ int auth_result = auth_userokay(login, NULL, "auth-xmsm", pwb);
> ++ if (auth_result) {
> ++ unlock_screen();
> ++ set_unlock_message(NULL);
> ++ } else {
> ++ if(!app_res.silent) XBell(XtDisplay(w), 100);
> ++ set_unlock_message(MSG_NOACCESS);
> ++ }
> ++#else
> +
> + set_privileges(True);
> +
> +@@ -896,6 +916,7 @@ static void passwd_enter_cb(Widget w,
> + if(!app_res.silent) XBell(XtDisplay(w),100);
> + set_unlock_message(MSG_NOACCESS);
> + }
> ++#endif /* __OpenBSD__ */
> +
> + 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;
> + }
