On Mon, Nov 23, 2015 at 11:04:10AM -0800, Keith Packard wrote: > > One of the many security holes in X is that any application can monitor > the state of the keyboard device by querying the list of pressed keys on > a regular basis. Here's a simple patch which makes that request report > only key state which the client itself has already seen through X > events. > > With this patch in place, grabbing the keyboard should be sufficient to > hide key presses from other clients. > > I think we need to try to fix some of these issues, even if the fixes > break existing applications. The next thing I'd like to try is to to > deliver input events to only one client (owner first, then > others). After that, apply the same rules to the input extension. > > In general, there are three areas that I'm wondering if we can fix: > > 1) input monitoring. This seems fairly "safe" as far as existing apps > go. > > 2) output monitoring. This seems much harder as so many useful hacks > and extensions take advantage of being able to get contents from > other windows. > > 3) breaking screen saver security. We've got an extension, let's make > it work. > > -keith > > From 627815391d2d6845f7e0a66d447c6b379be9d3cb Mon Sep 17 00:00:00 2001 > From: Keith Packard <kei...@keithp.com> > Date: Mon, 23 Nov 2015 10:01:10 -0800 > Subject: [PATCH xserver] Track keystate per client for QueryKeymap > > This adds a per-client key state vector and uses that for > ProcQueryKeymap instead of the device keymap. > > Signed-off-by: Keith Packard <kei...@keithp.com>
I _think_ this is correct, but you really want a lot of test cases for this to make sure you're not missing out on any shortcuts the event delivery code may take somewhere. I don't think there are any, but... Reviewed-by: Peter Hutterer <peter.hutte...@who-t.net> also, as I wrote in the other email this breaks syndaemon's disable-while-typing feature, especially together with RECORD disabled. Cheers, Peter > --- > dix/devices.c | 2 +- > dix/events.c | 13 +++++++++++++ > include/dixstruct.h | 1 + > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/dix/devices.c b/dix/devices.c > index 9b0c7d2..49b6994 100644 > --- a/dix/devices.c > +++ b/dix/devices.c > @@ -2405,7 +2405,7 @@ ProcQueryKeymap(ClientPtr client) > xQueryKeymapReply rep; > int rc, i; > DeviceIntPtr keybd = PickKeyboard(client); > - CARD8 *down = keybd->key->down; > + CARD8 *down = client->down; > > REQUEST_SIZE_MATCH(xReq); > rep = (xQueryKeymapReply) { > diff --git a/dix/events.c b/dix/events.c > index efaf91d..4114471 100644 > --- a/dix/events.c > +++ b/dix/events.c > @@ -2006,6 +2006,19 @@ TryClientEvents(ClientPtr client, DeviceIntPtr dev, > xEvent *pEvents, > } > } > > + /* Track keyboard state per client */ > + switch (type) { > + case KeyPress: > + SetBit(client->down, pEvents->u.u.detail); > + break; > + case KeyRelease: > + ClearBit(client->down, pEvents->u.u.detail); > + break; > + case KeymapNotify: > + memcpy(client->down+1, ((xKeymapEvent *) pEvents)->map, 31); > + break; > + } > + > if (BitIsOn(criticalEvents, type)) { > if (client->smart_priority < SMART_MAX_PRIORITY) > client->smart_priority++; > diff --git a/include/dixstruct.h b/include/dixstruct.h > index 8e70ae1..1e9f69e 100644 > --- a/include/dixstruct.h > +++ b/include/dixstruct.h > @@ -103,6 +103,7 @@ typedef struct _Client { > unsigned short newKeyboardNotifyMask; > unsigned short vMajor, vMinor; > KeyCode minKC, maxKC; > + CARD8 down[DOWN_LENGTH]; /* track key state for QueryKeymap */ > > int smart_start_tick; > int smart_stop_tick; > -- > 2.6.1 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel