[bug#27083] screen-lockers: i3lock-color and i3lock-fancy

[Chris Marusich]

<address@hidden> writes:

> I have no idea about the error of i3lock-fancy (password is not
> accepted).  An commented phase in i3lock-color is still included where
> I tried to fix this, but I don't know much about PAM at this point.

When I checked using "guix system vm", the reason i3lock refused to
accept the password was because it could not read /etc/shadow.  One
solution to this problem is to do both of the following:

1) Do not wrap i3lock in a wrapper script.

2) Install the i3lock executable as setuid-root.

When installing i3lock as a setuid-root program, it is important not to
wrap it in a script.  It is common in UNIX-like systems to ignore the
setuid bit when the program is an interpreted script instead of an
executable (a simple Internet search for a phrase like "setuid bash
script" will provide lots of supporting information about this).  If you
do (2) without also removing the wrapping logic that you put into the
package definition, you will find that i3lock still rejects your
password - this is because it still can't read /etc/shadow.

The attached patch is a revised version of your first patch to add
i3lock-color.  I have basically just removed the wrapping phase.  To
install it setuid-root along with an appropriate PAM configuration file,
just add a screen-locker service like the following to your services in
your operating system declaration:

    (screen-locker-service i3lock-color "i3lock")

Note that this will create an appropriate PAM configuration file at
/etc/pam.d/i3lock, so you don't need to bother messing around with the
PAM configuration file that comes with i3lock-color.

I haven't looked at the fancy version yet.  I'll check it out when I get
a little more time.

> +  %D%/packages/patches/hypre-doc-tables.patch                        \
> +  %D%/packages/patches/hypre-ldflags.patch                   \

Did you intend to include these in the patch?  They aren't used, so I've
removed them in my version of the patch.

> +---
> + Makefile | 7 +------
> + 1 file changed, 1 insertion(+), 6 deletions(-)
> +
> +diff --git a/Makefile b/Makefile
> +index c0fe888..b4b6cd9 100644
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -6,11 +6,6 @@ SYSCONFDIR=/etc
> + PKG_CONFIG=pkg-config
> + MANDIR=/usr/share/man
> + 
> +-# Check if pkg-config is installed, we need it for building CFLAGS/LIBS
> +-ifeq ($(shell which $(PKG_CONFIG) 2>/dev/null 1>/dev/null || echo 1),1)
> +-$(error "$(PKG_CONFIG) was not found")
> +-endif
> +-
> + CFLAGS += -std=c99
> + CFLAGS += -pipe
> + CFLAGS += -Wall
> +@@ -18,7 +13,7 @@ CFLAGS += -O2
> + SIMD_CFLAGS += -funroll-loops
> + SIMD_CFLAGS += -msse2
> + CPPFLAGS += -D_GNU_SOURCE
> +-CPPFLAGS += -DXKBCOMPOSE=$(shell if test -e 
> /usr/include/xkbcommon/xkbcommon-compose.h ; then echo 1 ; else echo 0 ; fi )
> ++CPPFLAGS += -DXKBCOMPOSE=1
> + CFLAGS += $(shell $(PKG_CONFIG) --cflags cairo xcb-composite xcb-xinerama 
> xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
> + LIBS += $(shell $(PKG_CONFIG) --libs cairo xcb-composite xcb-xinerama 
> xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
> + LIBS += -lpam
> +-- 
> +2.13.0

FYI, it looks like maybe we could probably remove this patch file
entirely, and just use substitute* to modify the Makefile, if we
provided the "which" program as an input.  Specifically, if the which
program were present, then probably the check you removed would
succeed.  I haven't tried it myself, though.

> -;;; Copyright (c) 2016, 2017 ng0 <address@hidden>
> +;;; Copyright (c) 2016, 2017 ng0 <address@hidden>

FYI, the address@hidden email address is still mentioned in
multiple other files, too.

> +    (description
> +     "I3lock-color is a screen locker.  It is a re-patched version of
> +i3lock, which is a simple screen locker like slock.  Features include:
> +
> address@hidden
> address@hidden forking process, the locked screen is preserved when you 
> suspend from RAM
> address@hidden specify background color or PNG image to be displayed in the 
> lock screen
> address@hidden many additional color options
> address@hidden enumerate\n")

I'm not sure if the trailing newline is necessary.  Unless you beat me
to it, I'll check on this detail the next time I return to this thread.

-- 
Chris

0001-gnu-Add-i3lock-color.patch
Description: Text Data

signature.asc
Description: PGP signature