Opened 3 years ago

Closed 2 years ago

#15646 closed defect (fixed)

KeyboardEvent may allow fingerprinting of keyboard layout

Reported by: cypherpunks Owned by: arthuredelstein
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff38-esr, tbb-fingerprinting, tbb-5.0a-highrisk, GeorgKoppen201507R, TorBrowserTeam201512
Cc: gk, brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by gk)

E.g. https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code#Code_values_on_Windows_%28When_scancode_is_available%29 says:

The KeyboardEvent.code property represents a physical key, that is value not changed neither by the modifier state, nor by keyboard layout.

Child Tickets

Change History (50)

comment:1 Changed 3 years ago by gk

Cc: gk added
Description: modified (diff)
Keywords: ff38-esr tbb-fingerprinting added
Priority: minormajor
Summary: KeyboardEvent.code may allow fingerprinting of keyboard layoutKeyboardEvent may allow fingerprinting of keyboard layout

This landed in Firefox 32. We need to take care of it before we switch to ESR 38.

comment:2 Changed 3 years ago by arthuredelstein

Owner: changed from tbb-team to arthuredelstein
Status: newassigned

comment:3 Changed 3 years ago by mikeperry

Keywords: tbb-5.0a-highrisk added

Tag the set of things that are risky to debut in the 5.0-stable release without testing in a prior alpha.

comment:4 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201506 added

Ensure all tbb-5.0a items are on the June radar.

comment:5 Changed 3 years ago by arthuredelstein

It turns out that not only does the new property, KeyboardEvent.code, allow fingerprinting of keyboard layout, so does the old (deprecated but still retained) KeyboardEvent.keyCode. For example, here are keyCode values I get for the ? character, for layouts targeted to different languages on OS X:

English (US) 191
German 63
French 188
Spanish 190
Italian 222
Swedish 171

So I'm looking into how to create a map from commonly typed unicode characters to a consensus code and a consensuskeyCode.

comment:6 Changed 3 years ago by arthuredelstein

Status: assignedneeds_review

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/a409f8ffa3a26a3d96c5bba8ff6caa4e0b8d61db

To summarize: I provide consensus (US-English-style) fake properties for KeyboardEvent, namely code, keyCode, location and shiftKey. So, for example, if the user types ?, the result will be code = 'Slash', keyCode = 191, shiftKey = true, location = 0, regardless of the keyboard layout. Numbers are always reported as arriving from the keys located above "QWERTY", even if they are typed on the NumPad.

Note that for now I have focused on ASCII (US English) characters and standard keyboard control keys. Characters from other languages will simply return a KeyboardEvent.keyCode of 0 and an empty Keyboard.code. It should be straightforward to spoof .keyCode and .code for higher unicode points, but I think it makes sense to postpone that for another ticket while this approach is reviewed and perhaps tested by users in an alpha.

comment:7 Changed 3 years ago by arthuredelstein

Keywords: TorBrowserTeam201506R added; TorBrowserTeam201506 removed

comment:8 Changed 3 years ago by arthuredelstein

Navigate to this page to observe KeyboardEvent property values:
https://arthuredelstein.github.io/tordemos/keyboard.html

comment:9 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

I can't get your patch to compile on Linux:

/home/ubuntu/build/tor-browser/dom/events/KeyCodeConsensus.h:7:30: fatal error: nsClassHashTable.h: No such file or directory
 #include "nsClassHashTable.h"
                              ^
compilation terminated.
make[5]: *** [Unified_cpp_dom_events1.o] Error 1
}

comment:10 in reply to:  9 Changed 3 years ago by arthuredelstein

Replying to gk:

I can't get your patch to compile on Linux:

/home/ubuntu/build/tor-browser/dom/events/KeyCodeConsensus.h:7:30: fatal error: nsClassHashTable.h: No such file or directory
 #include "nsClassHashTable.h"
                              ^
compilation terminated.
make[5]: *** [Unified_cpp_dom_events1.o] Error 1
}

Oops -- that file has a capitalization error (unfortunately ignored by OS X), but, in any case, the file should be "nsDataHashtable.h". Here's a corrected version that built for me on Ubuntu:

https://github.com/arthuredelstein/tor-browser/commit/9b4f66455fe8aa6c6cf5a13907e720a83d80dade

comment:11 Changed 3 years ago by mcs

Cc: brade mcs added

comment:12 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:13 in reply to:  6 ; Changed 3 years ago by gk

Status: needs_reviewneeds_revision

Replying to arthuredelstein:

Here's a patch for review:
https://github.com/arthuredelstein/tor-browser/commit/a409f8ffa3a26a3d96c5bba8ff6caa4e0b8d61db

To summarize: I provide consensus (US-English-style) fake properties for KeyboardEvent, namely code, keyCode, location and shiftKey. So, for example, if the user types ?, the result will be code = 'Slash', keyCode = 191, shiftKey = true, location = 0, regardless of the keyboard layout. Numbers are always reported as arriving from the keys located above "QWERTY", even if they are typed on the NumPad.

The ? test with en-US bundles

On a Windows 7 machine with a german keyboard and german keyboard layout I get

event = keydown
key = ?
charCode = 0
which = 63
code = Minus
keyCode = 63
location = 0
event = keypress
key = ?
charCode = 63
which = 63
code = Minus
keyCode = 0
location = 0

On a Ubuntu 14.04 with a german keyboard and an en-US keyboard layout I get

event = keydown
key = ?
charCode = 0
which = 191
code = Slash
keyCode = 191
location = 0
event = keypress
key = ?
charCode = 63
which = 63
code = Slash
keyCode = 0
location = 0

Thus, I think this needs revision. I am not exactly sure in which regard though: s/63/191/g and s/Minus/Slash/g? What about the keyCode = 0 cases?

Last edited 3 years ago by gk (previous) (diff)

comment:14 Changed 3 years ago by mikeperry

I took a look at this code and it seems good enough to include in 5.0a3. From a pedantic correctness standpoint, I am wary of createKeyCodes()'s setup code being called from the first KeyboardEvent, which could potentially race with a KeyboardEvent from another thread and cause reentrancy issues with the hashtables. This does seem unlikely in practice, though. Perhaps even impossible, if keyboard events can only arrive on the main thread.

comment:15 Changed 3 years ago by mikeperry

Ok, I pushed this patch to our 38 branch for 5.0a3 exposure.

comment:16 in reply to:  13 Changed 3 years ago by arthuredelstein

Replying to gk:

[snip]
Thus, I think this needs revision. I am not exactly sure in which regard though: s/63/191/g and s/Minus/Slash/g? What about the keyCode = 0 cases?

Phooey, I think this didn't work because I forgot to add the "privacy.resistFingerprinting" pref to 0001-tor-browser.js. Here's a fixup pref:

https://github.com/arthuredelstein/tor-browser/commit/0bdac85c8cbc46b4d6a8ae81e2544bed6315bcd0

comment:17 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:18 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201507R added; TorBrowserTeam201506R removed

Transfer review tickets to next month.

comment:19 Changed 3 years ago by mikeperry

I just pushed a Torbutton commit to sync the privacy.resistFingerprinting pref with the torbutton pref.

Leaving this open for GK to test. I'm also wondering if we should move createKeyCodes() into a global static object with its own constructor (which should run at program initialization).

comment:20 in reply to:  19 Changed 3 years ago by mikeperry

Replying to mikeperry:

I'm also wondering if we should move createKeyCodes() into a global static object with its own constructor (which should run at program initialization).

https://developer.mozilla.org/en-US/docs/Mozilla/C++_Portability_Guide#Don%27t_use_static_constructors

Maybe don't listen to me wrt static constructors. Apparently ancient compiler bugs with static constructors used to be a thing (though I'm pretty sure Firefox won't compile with a C++ compiler older than like 5 years anyway). I'm still a bit worried about the potential initialization race here, though. I wonder if anyone at Mozilla might make any suggestions?

comment:21 Changed 3 years ago by gk

Keywords: GeorgKoppen201507R added

comment:22 Changed 3 years ago by mikeperry

Keywords: tbb-5.0a4 added

Tag some 5.0a4 goals.

comment:23 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

This looks better now. But we should give default values for altKey and ctrlKey as well (maybe even metaKey, too?) as not doing so might reveal the underlying keyboard layout (or maybe better: it might exclude possible keyboard layouts) as well:

German keyboard layout

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = true
ctrlKey = true
metaKey = false
shiftKey = true

English keyboard layout

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = false
ctrlKey = false
metaKey = false
shiftKey = true

Two nits:

// KEY and SHIFT Assign

s/Assign/assign

#define KEY_INTERNAL(key, code, keyCode, shift)                    \

It seems you wanted to align the backslashes but forgot one whitespace here?

Re: comment:6 I think the approach is okay. Could you take care of filing the new ticket you mentioned there? And a new ticket about investigating the possible initialization race Mike mentioned (or maybe you are already sure that can't bite us?)?

comment:24 Changed 3 years ago by mikeperry

Keywords: tbb-5.0a4 removed

Arthur: We have your pref flip for this ready to go for 5.0a4. I think fixing this up is lower priority for 5.0a4 than #16429.

comment:25 in reply to:  23 ; Changed 3 years ago by arthuredelstein

Replying to gk:

This looks better now. But we should give default values for altKey and ctrlKey as well (maybe even metaKey, too?) as not doing so might reveal the underlying keyboard layout (or maybe better: it might exclude possible keyboard layouts) as well:

German keyboard layout

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = true
ctrlKey = true
metaKey = false
shiftKey = true

I think you're right that we need to spoof the ALT key state. (I've done so in the new patch.) But I'm not so sure about the META and CTRL keys -- aren't these only used for non-printing commands? Correct me if I'm wrong.

Two nits:

// KEY and SHIFT Assign

s/Assign/assign

#define KEY_INTERNAL(key, code, keyCode, shift)                    \

It seems you wanted to align the backslashes but forgot one whitespace here?

Thanks. I've fixed both nits.

Re: comment:6 I think the approach is okay. Could you take care of filing the new ticket you mentioned there?

Here it is: #16678.

And a new ticket about investigating the possible initialization race Mike mentioned (or maybe you are already sure that can't bite us?)?

I've added a mutex to createKeyCodes which will prevent any such race problem.

Here's the new patch for review:

https://github.com/arthuredelstein/tor-browser/commit/15646+2

Last edited 3 years ago by gk (previous) (diff)

comment:26 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:27 Changed 3 years ago by mikeperry

I pushed the new patch for 5.0a4. Do we think that wraps up this ticket? Is everything else going to be saved for another ticket or is more testing needed first?

comment:28 in reply to:  25 Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

This looks better now. But we should give default values for altKey and ctrlKey as well (maybe even metaKey, too?) as not doing so might reveal the underlying keyboard layout (or maybe better: it might exclude possible keyboard layouts) as well:

German keyboard layout

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = true
ctrlKey = true
metaKey = false
shiftKey = true

I think you're right that we need to spoof the ALT key state. (I've done so in the new patch.) But I'm not so sure about the META and CTRL keys -- aren't these only used for non-printing commands? Correct me if I'm wrong.

Good question. That could be for the META key. I don't know how it works. The funny thing wrt ctrlkey is that I did not press that one in my example above on the german keyboard. I actually just pressed the right ALT key as modifier key, yet still CTRL showed up as true. I had hopes we can avoid that by making the return value for ctrlkey uniform as well in this case by spoofing the CTRL key state. But maybe doing so for the ALT key might already be enough. I'd need to test that with your new patch first.

comment:29 Changed 3 years ago by gk

Status: needs_reviewneeds_revision

I am getting

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = false
ctrlKey = true
metaKey = false
shiftKey = true

with 5.0a4.

comment:30 Changed 3 years ago by gk

Keywords: TorBrowserTeam201508 added; TorBrowserTeam201507R removed

comment:31 in reply to:  29 ; Changed 3 years ago by arthuredelstein

Replying to gk:

I am getting

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = false
ctrlKey = true
metaKey = false
shiftKey = true

with 5.0a4.

Interesting. What keys are you actually pressing to get this result? And what OS is this?

comment:32 in reply to:  31 ; Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

I am getting

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = false
ctrlKey = true
metaKey = false
shiftKey = true

with 5.0a4.

Interesting. What keys are you actually pressing to get this result? And what OS is this?

Windows 7 and I press Right ALT + "<".

comment:33 in reply to:  32 ; Changed 3 years ago by arthuredelstein

Replying to gk:

Replying to arthuredelstein:

Replying to gk:

I am getting

event = keydown
key = |
charCode = 0
which = 220
code = Backslash
keyCode = 220
location = 0
altKey = false
ctrlKey = true
metaKey = false
shiftKey = true

with 5.0a4.

Interesting. What keys are you actually pressing to get this result? And what OS is this?

Windows 7 and I press Right ALT + "<".

I think I understand what's happening now: on your keyboard, the Right Alt key is being interpreted as an AltGr key, which is often emulated with Ctrl+Alt. It seems Windows or Firefox is doing the inverse substitution by re-interpreting AltGr as Ctrl+Alt.

So if this understanding is correct, it means that if you use the Left Alt key instead, you should get ctrlKey = false. Is that what you see?

If so, then I will look into how to suppress this Ctrl signal when the Right Alt key is pressed.

comment:34 Changed 3 years ago by mikeperry

When using google docs with 5.0a4, I am unable to use hotkeys to switch tabs. I suspect this patch may be at fault?

I don't recall having this issue with 5.0a3...

comment:35 Changed 3 years ago by mikeperry

Backspace also doesn't work in google docs for me in 5.0a4 :/

comment:36 in reply to:  33 Changed 3 years ago by gk

Replying to arthuredelstein:

Replying to gk:

Windows 7 and I press Right ALT + "<".

I think I understand what's happening now: on your keyboard, the Right Alt key is being interpreted as an AltGr key, which is often emulated with Ctrl+Alt. It seems Windows or Firefox is doing the inverse substitution by re-interpreting AltGr as Ctrl+Alt.

So if this understanding is correct, it means that if you use the Left Alt key instead, you should get ctrlKey = false. Is that what you see?

Yes. I think your analysis is correct.

comment:37 in reply to:  34 ; Changed 3 years ago by arthuredelstein

Replying to mikeperry:

When using google docs with 5.0a4, I am unable to use hotkeys to switch tabs. I suspect this patch may be at fault?

I don't recall having this issue with 5.0a3...

What key combination and OS did you use here?

comment:38 in reply to:  37 ; Changed 3 years ago by mikeperry

Replying to arthuredelstein:

Replying to mikeperry:

When using google docs with 5.0a4, I am unable to use hotkeys to switch tabs. I suspect this patch may be at fault?

I don't recall having this issue with 5.0a3...

What key combination and OS did you use here?

Gnome on Linux. Alt+<num> while in the text edit window of a google doc. This just causes <num> to appear instead of switching tabs.

comment:39 in reply to:  38 Changed 3 years ago by mikeperry

Replying to mikeperry:

Replying to arthuredelstein:

Replying to mikeperry:

When using google docs with 5.0a4, I am unable to use hotkeys to switch tabs. I suspect this patch may be at fault?

I don't recall having this issue with 5.0a3...

What key combination and OS did you use here?

Gnome on Linux. Alt+<num> while in the text edit window of a google doc. This just causes <num> to appear instead of switching tabs.

I just double-checked, and the alt+<num> breakage was absent in 5.0a3, but backspace was still broken in 5.0a3. Both keys work fine in 4.5.3. For some reason, backspace and alt+<num> are only broken from within Google Docs. All other web editors/pads I've tested do not have any issues.

comment:40 Changed 3 years ago by mikeperry

I am also able to reproduce the backspace issue in EtherCalc. Here's an example spreadsheet:
https://storm.torproject.org/shared/kRXqrbzZPzH0u8iVSa1NRcOZxwv_t0nYPb0EFIEzxj1

Basically, start editing any cell, and then try to use backspace to delete some characters. It fails for me. Only the delete key works.

Ethercalc does not have the alt+<num> issue for me, though.

comment:41 in reply to:  40 Changed 3 years ago by arthuredelstein

Replying to mikeperry:

I am also able to reproduce the backspace issue in EtherCalc. Here's an example spreadsheet:
https://storm.torproject.org/shared/kRXqrbzZPzH0u8iVSa1NRcOZxwv_t0nYPb0EFIEzxj1

Basically, start editing any cell, and then try to use backspace to delete some characters. It fails for me. Only the delete key works.

Ethercalc does not have the alt+<num> issue for me, though.

I should mention that I have a solution for the backspace and Alt+<numeral> problems. I'm still working out the answer for AltGr.

comment:42 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

Here's a fix for the Alt+<numeral> problem and backspace. AltGr is a little tricky so I'm going to leave that out for now.

https://github.com/arthuredelstein/tor-browser/commits/15646+4

comment:43 Changed 3 years ago by gk

Status: needs_reviewassigned

Okay, this is merged with commit f1f66dc5096188a60a96df7ee5e32d9a4b81dbe5.

comment:44 Changed 3 years ago by gk

I see:

Key event not available on some keyboard layouts: key="r" modifiers="accel,alt" browser.xul
Key event not available on some keyboard layouts: key="c" modifiers="accel,alt"

on start-up on a Linux box with a german keyborad layout. Is this expected? I am not sure whether this breaks (chrome) stuff yet.

comment:45 in reply to:  44 Changed 3 years ago by gk

Replying to gk:

I see:

Key event not available on some keyboard layouts: key="r" modifiers="accel,alt" browser.xul
Key event not available on some keyboard layouts: key="c" modifiers="accel,alt"

on start-up on a Linux box with a german keyborad layout. Is this expected? I am not sure whether this breaks (chrome) stuff yet.

Seems to be unrelated to the keyboardevent patch as I see it in a vanilla Firefox as well.

comment:46 Changed 3 years ago by mikeperry

Keywords: TorBrowserTeam201509 added; TorBrowserTeam201508 removed

Move remaining August tickets to September.

comment:47 Changed 3 years ago by gk

Keywords: TorBrowserTeam201510 added; TorBrowserTeam201509 removed

Moving Tor Browser tickets to October 2015.

comment:48 Changed 3 years ago by gk

Keywords: TorBrowserTeam201511 added; TorBrowserTeam201510 removed

comment:49 Changed 2 years ago by mikeperry

Keywords: TorBrowserTeam201512 added; TorBrowserTeam201511 removed

comment:50 Changed 2 years ago by gk

Resolution: fixed
Severity: Normal
Status: assignedclosed

We are done here and have #16678 for remaining things.

Note: See TracTickets for help on using tickets.