Hiding keyboard state

[Keith Packard]

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 <keithp at 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 <keithp at keithp.com>
---
 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20151123/965a36d3/attachment.sig>