search

i3 / i3lock

improved screen locker
https://i3wm.org/i3lock
BSD 3-Clause "New" or "Revised" License
921 stars 404 forks source link

New release needed #231

Closed jallbrit closed 5 years ago

jallbrit commented 5 years ago

I'm submitting a…

[ ] Bug
[ ] Feature Request
[x] Other (Please describe in detail)

Current Behavior

The --lock-console option is not available on the most recent release. It has been a considerable amount of time since the last release and I was wondering when the next one is planned.

Expected Behavior

Latest release was expected to contain the --lock-console option.

Reproduction Instructions

i3lock --lock-console

Environment

Output of i3lock --version:

i3lock version: 2.12.c-4-gfb9d064
orestisfl commented 5 years ago

@stapelberg

stapelberg commented 5 years ago

I just merged https://github.com/i3/i3lock/pull/226, so let’s give people a few days to test it out and report any issues they might face.

I’ll aim for doing a new release this weekend.

stapelberg commented 5 years ago

Only noticed this while going through the changelog: @LorianColtof’s https://github.com/i3/i3lock/pull/183 changed the Makefile so that i3lock is now installed setuid root. Can we change this to only grant the required capability (using setcap)?

I’d prefer it if i3lock did not run as root, or at least not by default.

mortie commented 5 years ago

I looked through how the TTY locking works; we could probably use setcap to add the cap_sys_tty_config capability, but the issue is that we open /dev/console as read/write, and /dev/console is 0700 and owned by root:root. AFAIK, the extended attributes system isn't flexible enough to add read/write permission to one particular file.

It's possible to make a separate locking binary which is simple enough to be much less likely to have security bugs, and make that a suid binary. I made one here: https://github.com/mortie/i3lock/blob/feature/separate-locking-binary/i3lock-tty-switching.c

However, we're fundamentally giving unprivileged users permission to globally lock and unlock TTY switching. This feels very wrong. I wouldn't want some admin to install i3lock on a shared system and suddenly without knowing it allow any of their users to disable TTY switching. At the same time, TTY switching is obviously a potential security issue.

It seems like neither xscreensaver nor google's xsecurelock feature TTY switch locking. physlock is the only screen locker I've seen thus far which disables it, and that's also just a suid binary which uses VT_{UN,}LOCKSWITCH ioctls.

What do you think @stapelberg?

stapelberg commented 5 years ago

I looked through how the TTY locking works; we could probably use setcap to add the cap_sys_tty_config capability, but the issue is that we open /dev/console as read/write, and /dev/console is 0700 and owned by root:root. AFAIK, the extended attributes system isn't flexible enough to add read/write permission to one particular file.

It’s possible to do this using setfacl(1), but we can’t rely on that across distributions.

It seems like neither xscreensaver nor google's xsecurelock feature TTY switch locking. physlock is the only screen locker I've seen thus far which disables it, and that's also just a suid binary which uses VT_{UN,}LOCKSWITCH ioctls.

What do you think @stapelberg?

Yeah, I think we should revert our merging of that feature. People should use vlock(1) instead if they care about virtual console locking. Many users will never use virtual consoles. I will do the revert soon, unless someone convinces me not to.

stapelberg commented 5 years ago

i3lock 2.12 was released! \o/