Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--pass-*-keys options (#136) #231

Merged
merged 8 commits into from
Aug 9, 2021
Merged

--pass-*-keys options (#136) #231

merged 8 commits into from
Aug 9, 2021

Conversation

JezerM
Copy link

@JezerM JezerM commented Jul 10, 2021

Closes #136

Description

  • This could solve the --pass-media-keys, --pass-screen-keys and --pass-power-keys options
  • Uses xtest library and xcb_test_fake_input to send keys.
  • Ungrabs the keyboard, sends the key event, and grabs the keyboard again

Release notes

Notes: Solves --pass-media-keys, --pass-screen-keys and --pass-power-keys options

i3lock.c Show resolved Hide resolved
@Rio6
Copy link
Collaborator

Rio6 commented Jul 15, 2021

What happens if someone spams multiple other keys at the same time? Does ungrabbing the keyboard let those keys go through as well?

@JezerM
Copy link
Author

JezerM commented Jul 15, 2021

I guess you would need to be really fast to do that, even when pressing them at the same time. I tested it right now and nothing happened, except that spamming slowed down my PC.
After the media or screen key is sent to root, other keys seems to be catched normally, they don't go through i3lock to the root window.

Copy link
Owner

@Raymo111 Raymo111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not send the keys while the keyboard is grabbed? (i.e. don't ungrab and regrab so we eliminate the chance of any security issue?

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

Can we not send the keys while the keyboard is grabbed? (i.e. don't ungrab and regrab so we eliminate the chance of any security issue?

No, I think it's not possible. I tried grabbing every key with xcb_grab_key so later it would just release the media and brightness keys with xcb_ungrab_key, but the first comes with some problems: (see docs)

  • If a key is grabbed by another program, like a keylogger, it won't grab those keys.
  • Trying to grab all keys with XCB_GRAB_ANY value fails if at least one key is not grabbed.

xcb_grab_keyboard is the solution to grab the keyboard without any error, and so it won't send any key to the root window (or window manager). Therefore, I see grabbing and ungrabbing the keyboard as the only way to solve this (without passing commands as commented in the issue)

And, yeah, it could be a possible security issue, but as I said before, you would need to be faster than the process of ungrabbing, sending, and grabbing, to send any unexpected key to the root window. (even faster than pressing two keys at the same time)

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

@JezerM grab_keyboard is fine, what I meant is can we not send the keys using xtest while the keyboard is grabbed? I'm just worried about something like plugging in a runberducky-type device, using media keys to ungrab the keyboard for a split second, and then doing bad things like opening a terminal and killing the lockscreen for example.

If we can't send the keys while the keyboard is grabbed, we should leave the existing pass-*-keys as-is and make new "unsafe" versions of the args.

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

Sending a key to the root while i3lock is grabbing the root keyboard just makes a infinite loop. So, sadly, it's not possible.

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

I've tested some things, with a Python script that tries to grab the keyboard and stops when it is grabbed:

from time import sleep
from ewmh import EWMH
from Xlib.ext import xtest
from Xlib.X import GrabModeSync

ewmh = EWMH()
display = ewmh.display
root = display.screen().root

while (root.grab_keyboard(display, False, False, GrabModeSync) == 1):
    print("NOPE")

print("YES!")

sleep(2)
display.ungrab_keyboard(0)
display.flush()

The same with a c script (if python's "slowness" is a problem):

#include <stdbool.h>
#include <stdio.h>
#include <xcb/xcb.h>
#include <unistd.h>
#include <malloc.h>

xcb_connection_t *conn;
xcb_screen_t *screen;
const xcb_setup_t *setup;
xcb_grab_keyboard_cookie_t kcookie;
xcb_grab_keyboard_reply_t *kreply;

int main() {
  int screenr;
  int done = 0;
  conn = xcb_connect(NULL, &screenr);
  setup = xcb_get_setup(conn);
  screen = xcb_setup_roots_iterator(setup).data;

  while (done == 0) {
    kcookie = xcb_grab_keyboard(conn, true, screen->root, XCB_CURRENT_TIME, XCB_GRAB_MODE_ASYNC, XCB_GRAB_MODE_ASYNC);
    if ((kreply = xcb_grab_keyboard_reply(conn, kcookie, NULL)) &&
        kreply->status == XCB_GRAB_STATUS_SUCCESS) {
      done = 1;
      free(kreply);
      break;
    }
    free(kreply);
    printf("NOPE\n");
  }

  printf("YES!\n");
  sleep(2);

  xcb_ungrab_keyboard(conn, XCB_CURRENT_TIME);
  xcb_disconnect(conn);
  return 0;
}

And then, run i3lock --pass-screen-keys with the python script:

./i3lock --pass-screen-keys --no-verify & \
sleep 2 && python3 ewmh-test.py

Or, compile the c script with gcc ewmh-test.c -lxcb -lxcb-xtest -o ewmh-test, and run:

./i3lock --pass-screen-keys --no-verify & \
sleep 2 && ./ewmh-test

Whenever I press the screen keys, i3lock sends them as expected, and it's not until i3lock releases the keyboard that the python/c script grabs the keyboard. Therefore, scripts that tries to grab the keyboard in between the ungrab-send-grab process just doesn't work.

I also tried with a key emitter loop, but no key was received by the root window, except for those permitted and/or pressed (screen and media keys).

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

Interesting. Now my only worry is, what happens if the regrab fails?

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

I think that couldn't happen. As far as I've seen, i3lock tries lots of times to grab the mouse and keyboard until it succeeds; it only fails when another program it's grabbing the keyboard. So, if no program is grabbing the keyboard besides i3lock and no program can grab the keyboard because i3lock is grabbing it, the regrab should always work, unless the X server dies or something like that.

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

There was a problem before when an action provoked by the key sets the focus to another window, which could make the regrab to "fail", but setting the focus to i3lock after sending the event solved it.

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

Hmm okay. I'd like to keep the current approach to passing keys for WMs though (DEs can use the new function). How about we add a --de flag, make that a global variable, and check for it in send_key_to_root? If it's true we can use the new approach, if it's false we can use the old one?

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

That's okay, though this is not a DE only problem, as it also affects some WM (AwesomeWM in my case), so I don't know if calling it --de would be correct.

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

Oh I see. Maybe like --special-passthrough then? Or I'm all ears if you have a better suggestion?

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

Hmm, I have no problem with that, --special-passthrough looks good.

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

Ok. Make sure to add that flag to the manpage as well as your tab completion PR. Also we should test this on all the major target environments just in case. I can handle KDE, XFCE, and i3, if you can take awesomeWM and GNOME? just to make sure pass keys now work on those (sanity check)?

@JezerM
Copy link
Author

JezerM commented Aug 9, 2021

I tested it before in XFCE, KDE, AwesomeWM and Ubuntu, and I guess everything works well.
I'm gonna add the flag and changes to manpages.

@Raymo111
Copy link
Owner

Raymo111 commented Aug 9, 2021

Oh ok, sick.

Copy link
Owner

@Raymo111 Raymo111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Raymo111 Raymo111 merged commit 4ff79c0 into Raymo111:master Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--pass-media-keys, --pass-screen-keys and --pass-power-keys options do not work in DEs
3 participants