Page MenuHomePhabricator

Quarantee to send key press/release pair exactly to each wayland client
AbandonedPublic

Authored by JHyun on Jan 14 2016, 11:43 PM.

Details

Summary

Assume that one module is set input_key_grabs after get key press.

(other conditions are same: e_client_action_get, e_menu_grab_window_get)
In that case, enlightenment send a key press to client,
but do not send a key release to client.(Because it is grabbed)
So to guarantee key event pair, I add delivered_key variable.
Test Plan

If a key press is sent to client, a key release must be sent to client.

Diff Detail

Repository
rE core/enlightenment
Branch
work
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1087
Build 1152: arc lint + arc unit
JHyun updated this revision to Diff 8164.Jan 14 2016, 11:43 PM
JHyun retitled this revision from to Quarantee to send key press/release pair exactly to each wayland client.
JHyun updated this object.
JHyun edited the test plan for this revision. (Show Details)
JHyun added reviewers: devilhorns, zmike, raster.
JHyun added subscribers: input.hacker, ohduna, cedric.
JHyun added a comment.Mar 23 2016, 7:18 PM

Hello.
I wonder how do you think of this commit.
I'll be very happy if you let me know your opinion of this commit.
This commit's purpose is to guarantee key events sequence(down/up) to client.
I think if a client get a key down event, that client must be get a key up event.
How about your opinion?

raster edited edge metadata.Mar 23 2016, 10:29 PM

i totally agree - a down should be followed by an up.. otherwise the key *IS* still down. pressed in. guys? comments?

zmike added a comment.Mar 24 2016, 9:39 AM

I think this is a valid issue but not the right way to go about it: in the case where the compositor has grabbed input, providing ANY genuine keystroke timestamp information is potentially a security risk.

A better way to handle this would be to directly trigger sending key-ups for all held keys to clients as soon as a compositor input grab occurs. This would create a more easily enforceable policy without leaking potentially compromising data.

zmike requested changes to this revision.Mar 24 2016, 9:39 AM
zmike edited edge metadata.
This revision now requires changes to proceed.Mar 24 2016, 9:39 AM
JHyun added a comment.Mar 25 2016, 3:41 AM

Your opinion is generate release key events when such conditions are set like input_key_grab.

That's not bad and satiate a key pair.
I have three questions.

1st, I don't completely understand your security risk.
My concerned of this patch is that we get client using e_client_focused_get() API.
But some case, a focused client have possible to different press time with release time.
In that case, key pair is not guaranteed.
But it is not related my patch... so I don't understand your security risk.
Will you give more information of this issue to me?

2nd, There are two more conditions exist except input_key_grabs.
e_client_action_get()(return action_client) and e_menu_grab_window_get()(return _e_menu_win) are that conditions.
action_client is set in _e_client_action_init() and _e_menu_win is set in e_menu_grab_window_get().
Following your opinion, generate key release events all of these points, right?

3rd is that how control real key release events?
If generate key release events, client will get paired events, it is okay.
But in enlightenment how control real key release events.
If we return False in e_comp_wl_key_up() another enlightenment modules can get release keys (except key press events).
If we return True in e_comp_wl_key_up() key events will be dropped.
Which case is better?

Thank you for information and opinions.

zmike added a comment.Mar 28 2016, 9:01 AM
In D3575#62667, @JHyun wrote:

Your opinion is generate release key events when such conditions are set like input_key_grab.

That's not bad and satiate a key pair.
I have three questions.

1st, I don't completely understand your security risk.
My concerned of this patch is that we get client using e_client_focused_get() API.
But some case, a focused client have possible to different press time with release time.
In that case, key pair is not guaranteed.
But it is not related my patch... so I don't understand your security risk.
Will you give more information of this issue to me?

At the time of a compositor keyboard grab, it's unknown what purpose the compositor is grabbing the keyboard for. As an example, suppose that the grab is to perform an authentication where the user must enter a password. If a modifier key is down when the grab occurs, a hostile application which tracks users across devices may be able to determine (with relative accuracy) the number of keys which are input into the authentication box based on having benchmarked the user's typing speed.

Given that on an input grab all clients should become unfocused, it makes sense to avoid this potential scenario altogether by ensuring that key-ups are sent for all held keys at the time of the grab.

2nd, There are two more conditions exist except input_key_grabs.
e_client_action_get()(return action_client) and e_menu_grab_window_get()(return _e_menu_win) are that conditions.
action_client is set in _e_client_action_init() and _e_menu_win is set in e_menu_grab_window_get().
Following your opinion, generate key release events all of these points, right?

These functions imply that a grab has occurred, so it's likely that the checks are not even needed if things are working properly.

3rd is that how control real key release events?
If generate key release events, client will get paired events, it is okay.
But in enlightenment how control real key release events.
If we return False in e_comp_wl_key_up() another enlightenment modules can get release keys (except key press events).
If we return True in e_comp_wl_key_up() key events will be dropped.
Which case is better?

The client should be unfocused on grab, forcing key-ups to be sent for all held keys. This is probably as simple as doing something like

if (e_client_focused_get()) evas_object_focus_set(e_client_focused_get()->frame, 0);

in the grab conditional in e_comp_grab_input()

Thank you for information and opinions.

yeah - zmike is right. sending the key ups for all held keys once a grab is "found" internally (eg alt-tab let's say. the alt does not begin the key grab, but is pressed first, then tab is pressed and this begins the grab, is when tab is pressed, send an up for alt then process the alt-tab as usual inside compositor). this is simple and leaks the minimum amount of info to clients.

JHyun abandoned this revision.Jun 27 2016, 9:32 PM

This patch is too old.