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

Add event buffering for cloaking user input patterns #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 3 additions & 12 deletions gui-common/txrx-vchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ int vchan_is_closed = 0;
*/
int double_buffered = 1;

static int wait_for_vchan_or_argfd_once(libvchan_t *vchan, int fd);

void vchan_register_at_eof(void (*new_vchan_at_eof)(void)) {
vchan_at_eof = new_vchan_at_eof;
}
Expand Down Expand Up @@ -99,7 +97,7 @@ int read_data(libvchan_t *vchan, char *buf, int size)
int ret;
while (written < size) {
while (!libvchan_data_ready(vchan))
wait_for_vchan_or_argfd_once(vchan, -1);
wait_for_vchan_or_argfd_once(vchan, -1, 1000);
ret = libvchan_read(vchan, buf + written, size - written);
if (ret <= 0)
handle_vchan_error(vchan, "read data");
Expand All @@ -109,15 +107,15 @@ int read_data(libvchan_t *vchan, char *buf, int size)
return size;
}

static int wait_for_vchan_or_argfd_once(libvchan_t *vchan, int fd)
int wait_for_vchan_or_argfd_once(libvchan_t *vchan, int fd, int timeout)
{
int ret;
write_data(vchan, NULL, 0); // trigger write of queued data, if any present
struct pollfd fds[] = {
{ .fd = libvchan_fd_for_select(vchan), .events = POLLIN, .revents = 0 },
{ .fd = fd, .events = POLLIN, .revents = 0 },
};
ret = poll(fds, fd == -1 ? 1 : 2, 1000);
ret = poll(fds, fd == -1 ? 1 : 2, timeout);
if (ret < 0) {
if (errno == EINTR)
return -1;
Expand All @@ -140,10 +138,3 @@ static int wait_for_vchan_or_argfd_once(libvchan_t *vchan, int fd)
}
return ret;
}

int wait_for_vchan_or_argfd(libvchan_t *vchan, int fd)
{
int ret;
while ((ret=wait_for_vchan_or_argfd_once(vchan, fd)) == 0);
return ret;
}
6 changes: 6 additions & 0 deletions gui-daemon/guid.conf
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,10 @@ global: {
# Timeout when waiting for qubes-gui-agent
#
# startup_timeout = 45;

# Maximum delay in milliseconds for event buffering (used for obfuscating
# biometric data that could be obtained by observing keyboard and mouse
# patterns). Set to 0 to disable event buffering entirely.
#
# events_max_delay = 0;
}
129 changes: 123 additions & 6 deletions gui-daemon/xside.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
#include <sys/wait.h>
#include <sys/resource.h>
#include <sys/uio.h>
#include <sys/queue.h>
#include <sys/random.h>
#include <signal.h>
#include <poll.h>
#include <errno.h>
Expand Down Expand Up @@ -2386,11 +2388,71 @@ static void process_xevent_xembed(Ghandles * g, const XClientMessageEvent * ev)

}

/* get current time */
static int64_t ebuf_current_time_ms()
{
int64_t timeval;
struct timespec spec;
clock_gettime(CLOCK_MONOTONIC, &spec);
timeval = (((int64_t)spec.tv_sec) * 1000LL) + (((int64_t)spec.tv_nsec) / 1000000LL);
return timeval;
}

/* get random delay value */
static uint32_t ebuf_random_delay(Ghandles * g, uint32_t lower_bound)
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the function is otherwise self-contained, I think it would be better to pass the upper bound instead of the full Ghandles.

{
uint32_t maxval;
uint32_t randval;
size_t randsize;
union ebuf_rand ebuf_rand_data;

if (lower_bound >= g->ebuf_max_delay) {
fprintf(stderr,
"Bug detected - lower_bound >= g->ebuf_max_delay, events may get briefly stuck");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is reachable if 2 events arrive within the same ms and the ebuf_random_delay generates e_buf_max_delay by random for the first event.

return g->ebuf_max_delay;
}

maxval = g->ebuf_max_delay - lower_bound + 1;

Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: trainling whitespace (for some reason GitHub's web UI doesn't show it, but it's in commit 541c10d?)

do {
randsize = getrandom(ebuf_rand_data.raw, sizeof(uint32_t), 0);
if (randsize != sizeof(uint32_t))
continue;
} while (ebuf_rand_data.val >= UINT32_MAX - (UINT32_MAX % maxval));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check is correct. But it unnecessarily throws away value if 2**32 is a multiple of maxval. That was a bit confusing during review.


randval = ebuf_rand_data.val % maxval;
return lower_bound + randval;
}

/* queue input event */
static void ebuf_queue_xevent(Ghandles * g, XEvent xev)
{
int64_t current_time;
uint32_t random_delay;
struct ebuf_entry *new_ebuf_entry;
uint32_t lower_bound;

current_time = ebuf_current_time_ms();
lower_bound = min(max(g->ebuf_prev_release_time - current_time, 0), g->ebuf_max_delay);
Copy link
Contributor

Choose a reason for hiding this comment

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

This deserves some explanation. Why do you make the random delay dependent on the previous random delay value?

random_delay = ebuf_random_delay(g, lower_bound);
new_ebuf_entry = malloc(sizeof(struct ebuf_entry));
if (new_ebuf_entry == NULL) {
perror("Could not allocate ebuf_entry:");
exit(1);
}
if (current_time > 0 && random_delay > (LONG_MAX - current_time)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (current_time > 0 && random_delay > (LONG_MAX - current_time)) {
if (current_time > 0 && random_delay > (INT64_MAX - current_time)) {

fprintf(stderr, "Event scheduler overflow detected, cannot continue");
exit(1);
}
new_ebuf_entry->time = current_time + random_delay;
new_ebuf_entry->xev = xev;
TAILQ_INSERT_TAIL(&(g->ebuf_head), new_ebuf_entry, entries);
g->ebuf_prev_release_time = new_ebuf_entry->time;
}

/* dispatch local Xserver event */
static void process_xevent(Ghandles * g)
static void process_xevent_core(Ghandles * g, XEvent event_buffer)
{
XEvent event_buffer;
XNextEvent(g->display, &event_buffer);
switch (event_buffer.type) {
case KeyPress:
case KeyRelease:
Expand Down Expand Up @@ -2449,6 +2511,40 @@ static void process_xevent(Ghandles * g)
}
}

/* dispatch queued events */
static void ebuf_release_xevents(Ghandles * g)
{
int64_t current_time;
struct ebuf_entry *current_ebuf_entry;

current_time = ebuf_current_time_ms();
while ((current_ebuf_entry = TAILQ_FIRST(&(g->ebuf_head)))
&& (current_time >= current_ebuf_entry->time)) {
XEvent event_buffer = current_ebuf_entry->xev;
process_xevent_core(g, event_buffer);
TAILQ_REMOVE(&(g->ebuf_head), current_ebuf_entry, entries);
free(current_ebuf_entry);
}
current_ebuf_entry = TAILQ_FIRST(&(g->ebuf_head));
if (current_ebuf_entry == NULL) {
g->ebuf_next_timeout = VCHAN_DEFAULT_POLL_DURATION;
} else {
g->ebuf_next_timeout = (int)(current_ebuf_entry->time - current_time);
}
}

/* handle or queue local Xserver event */
static void process_xevent(Ghandles * g)
{
XEvent event_buffer;
XNextEvent(g->display, &event_buffer);
if (g->ebuf_max_delay > 0) {
ebuf_queue_xevent(g, event_buffer);
} else {
process_xevent_core(g, event_buffer);
}
}


/* handle VM message: MSG_SHMIMAGE
* pass message data to do_shm_update - there input validation will be done */
Expand Down Expand Up @@ -4237,11 +4333,23 @@ static void parse_vm_config(Ghandles * g, config_setting_t * group)
g->disable_override_redirect = 0;
else {
fprintf(stderr,
"unsupported value ‘%s’ for override_redirect (must be disabled or allow\n",
"unsupported value '%s' for override_redirect (must be 'disabled' or 'allow')\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Including such an unrelated small change in a PR is ok, but belongs into a separate commit.

value);
exit(1);
}
}

if ((setting =
config_setting_get_member(group, "events_max_delay"))) {
int delay_val = config_setting_get_int(setting);
if (delay_val < 0 || delay_val > 5000) {
fprintf(stderr,
"unsupported value '%d' for events_max_delay (must be >= 0 and <= 5000)",
delay_val);
exit(1);
}
g->ebuf_max_delay = delay_val;
}
}

static void parse_config(Ghandles * g)
Expand Down Expand Up @@ -4364,11 +4472,13 @@ int main(int argc, char **argv)
/* parse cmdline, possibly overriding values from config */
parse_cmdline(&ghandles, argc, argv);
get_boot_lock(ghandles.domid);
/* init event queue */
TAILQ_INIT(&(ghandles.ebuf_head));

if (!ghandles.nofork) {
// daemonize...
if (pipe(pipe_notify) < 0) {
perror("canot create pipe:");
perror("cannot create pipe:");
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Including such an unrelated typo fix in a PR is ok, but belongs into a separate commit.

exit(1);
}

Expand Down Expand Up @@ -4559,8 +4669,15 @@ int main(int argc, char **argv)
handle_message(&ghandles);
busy = 1;
}
if (ghandles.ebuf_max_delay > 0) {
ebuf_release_xevents(&ghandles);
}
} while (busy);
wait_for_vchan_or_argfd(ghandles.vchan, xfd);
if (ghandles.ebuf_max_delay > 0) {
wait_for_vchan_or_argfd_once(ghandles.vchan, xfd, ghandles.ebuf_next_timeout);
} else {
wait_for_vchan_or_argfd_once(ghandles.vchan, xfd, VCHAN_DEFAULT_POLL_DURATION);
}
}
return 0;
}
19 changes: 19 additions & 0 deletions gui-daemon/xside.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@

#define MAX_SCREENSAVER_NAMES 10

#define VCHAN_DEFAULT_POLL_DURATION 1000

#ifdef __GNUC__
# define UNUSED(x) UNUSED_ ## x __attribute__((__unused__))
#else
Expand All @@ -82,6 +84,7 @@
#include <stdbool.h>
#include <assert.h>
#include <unistd.h>
#include <sys/queue.h>
#include <libvchan.h>
#include <X11/Xlib.h>
#include <xcb/xcb.h>
Expand Down Expand Up @@ -139,6 +142,17 @@ struct extra_prop {
int nelements; /* data size, in "format" units */
};

struct ebuf_entry {
XEvent xev;
int64_t time;
TAILQ_ENTRY(ebuf_entry) entries;
};

union ebuf_rand {
uint32_t val;
char raw[sizeof(uint32_t)];
};

/* global variables
* keep them in this struct for readability
*/
Expand Down Expand Up @@ -239,6 +253,11 @@ struct _global_handles {
int xen_fd; /* O_PATH file descriptor to /dev/xen/gntdev */
int xen_dir_fd; /* file descriptor to /dev/xen */
bool permit_subwindows : 1; /* Permit subwindows */
uint32_t ebuf_max_delay;
/* ebuf state */
TAILQ_HEAD(tailhead, ebuf_entry) ebuf_head;
int64_t ebuf_prev_release_time;
int ebuf_next_timeout;
};

typedef struct _global_handles Ghandles;
Expand Down
2 changes: 1 addition & 1 deletion include/txrx.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ int read_data(libvchan_t *vchan, char *buf, int size);
x.untrusted_len = sizeof(y); \
real_write_message(vchan, (char*)&x, sizeof(x), (char*)&y, sizeof(y)); \
} while(0)
int wait_for_vchan_or_argfd(libvchan_t *vchan, int fd);
int wait_for_vchan_or_argfd_once(libvchan_t *vchan, int fd, int timeout);
void vchan_register_at_eof(void (*new_vchan_at_eof)(void));

#endif /* _QUBES_TXRX_H */