From 9d38fb8b7602dd0b70a3042f6e5ed94b869b3b78 Mon Sep 17 00:00:00 2001 From: ste Date: Fri, 13 Jul 2018 15:19:37 +0100 Subject: [PATCH 1/5] GPII-3102: Removed windows component from metrics; it's a mix-in --- gpii/node_modules/windowsMetrics/src/windowsMetrics.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js index 1a7aca1b2..c632ef719 100644 --- a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js +++ b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js @@ -111,14 +111,6 @@ fluid.defaults("gpii.windowsMetrics", { } }); -fluid.defaults("gpii.metrics.windows", { - components: { - windowsMetrics: { - type: "gpii.windowsMetrics" - } - } -}); - fluid.defaults("gpii.installID.windows", { invokers: { getMachineID: "gpii.windows.getMachineID" From 3e4d4f95470b1ebb1e316d583a8c2667e6d0b965 Mon Sep 17 00:00:00 2001 From: ste Date: Tue, 17 Jul 2018 15:28:03 +0100 Subject: [PATCH 2/5] GPII-3102: Added more special keys to logs, improved tests to ensure data isn't leaked. --- .../WindowsUtilities/WindowsUtilities.js | 56 +++++- .../windowsMetrics/src/windowsMetrics.js | 38 ++-- .../test/WindowsMetricsTests.js | 164 +++++++++++++++--- 3 files changed, 209 insertions(+), 49 deletions(-) diff --git a/gpii/node_modules/WindowsUtilities/WindowsUtilities.js b/gpii/node_modules/WindowsUtilities/WindowsUtilities.js index f6cb9ef5f..ad0fb2269 100644 --- a/gpii/node_modules/WindowsUtilities/WindowsUtilities.js +++ b/gpii/node_modules/WindowsUtilities/WindowsUtilities.js @@ -310,11 +310,57 @@ windows.API_constants = { DBT_DEVTYP_VOLUME: 0x2, // https://msdn.microsoft.com/library/dd375731 - VK_BACK: 0x08, - VK_ESCAPE: 0x1B, - VK_HOME: 0x24, - VK_LEFT: 0x25, - VK_DELETE: 0x2E, + virtualKeyCodes: { + VK_BACK: 0x08, + VK_TAB: 0x09, + VK_RETURN: 0x0D, + VK_ESCAPE: 0x1B, + VK_SPACE: 0x20, + VK_PAGEUP: 0x21, // VK_PRIOR + VK_PAGEDOWN: 0x22, // VK_NEXT + VK_END: 0x23, + VK_HOME: 0x24, + VK_LEFT: 0x25, + VK_UP: 0x26, + VK_RIGHT: 0x27, + VK_DOWN: 0x28, + VK_SELECT: 0x29, + VK_PRINT: 0x2A, + VK_EXECUTE: 0x2B, + VK_SNAPSHOT: 0x2C, + VK_INSERT: 0x2D, + VK_DELETE: 0x2E, + VK_HELP: 0x2F, + VK_LWIN: 0x5B, + VK_RWIN: 0x5C, + VK_APPS: 0x5D, + VK_F1: 0x70, + VK_F2: 0x71, + VK_F3: 0x72, + VK_F4: 0x73, + VK_F5: 0x74, + VK_F6: 0x75, + VK_F7: 0x76, + VK_F8: 0x77, + VK_F9: 0x78, + VK_F10: 0x79, + VK_F11: 0x7A, + VK_F12: 0x7B, + VK_F13: 0x7C, + VK_F14: 0x7D, + VK_F15: 0x7E, + VK_F16: 0x7F, + VK_F17: 0x80, + VK_F18: 0x81, + VK_F19: 0x82, + VK_F20: 0x83, + VK_F21: 0x84, + VK_F22: 0x85, + VK_F23: 0x86, + VK_F24: 0x87, + VK_NUMLOCK: 0x90, + VK_SCROLL: 0x91 + }, LLKHF_EXTENDED: 0x01, LLKHF_INJECTED: 0x10, diff --git a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js index c632ef719..848834f21 100644 --- a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js +++ b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js @@ -347,19 +347,22 @@ windows.stopInputMetrics = function (that) { }; /** - * Keys that have a special meaning. + * A value=>name map of only non-printable keys that will be logged. */ windows.specialKeys = fluid.freezeRecursive((function () { - var special = { - "BS": windows.API_constants.VK_BACK, - "DEL": windows.API_constants.VK_DELETE, - "ESC": windows.API_constants.VK_ESCAPE, - "LEFT": windows.API_constants.VK_LEFT - }; - - // Swap to ease look-ups. - fluid.each(special, function (value, key) { - special[value] = key; + var keys = [ + "BACK", "TAB", "RETURN", "ESCAPE", "PAGEUP", "PAGEDOWN", "END", "HOME", "LEFT", "UP", "RIGHT", "DOWN", + "SELECT", "PRINT", "EXECUTE", "SNAPSHOT", "INSERT", "DELETE", "HELP", "LWIN", "RWIN", "SCROLL", "NUMLOCK", + "F1", "F2", "F3", "F4", "F5", "F6", "F7", "F8", "F9", "F10", "F11", "F12", "F13", "F14", "F15", "F16", + "F17", "F18", "F19", "F20", "F21", "F22", "F23", "F24" + ]; + var special = {}; + + fluid.each(keys, function (keyName) { + var value = windows.API_constants.virtualKeyCodes["VK_" + keyName]; + if (value) { + special[value] = keyName; + } }); return special; @@ -425,8 +428,9 @@ windows.recordKeyTiming = function (that, timestamp, specialKey) { if (specialKey) { // Double-check that only certain keys are being recorded (it would be a serious blunder). - if (windows.specialKeys[specialKey] && typeof (specialKey) === "string" && specialKey.length > 1) { - record.key = windows.specialKeys[windows.specialKeys[specialKey]]; + var keycode = parseInt(fluid.keyForValue(windows.specialKeys, specialKey)); + if (!!keycode && typeof(specialKey) === "string" && specialKey.length > 1) { + record.key = windows.specialKeys[keycode]; } } @@ -522,12 +526,8 @@ windows.inputHook = function (that, code, wparam, lparam) { if ((eventData.flags & ignoreFlags) === 0) { // If the key doesn't generate a character, then don't count it. - wanted = specialKey || windows.user32.MapVirtualKeyW(eventData.vkCode, windows.API_constants.MAPVK_VK_TO_CHAR); - if (eventData.vkCode === windows.API_constants.VK_LEFT) { - // Special case for the 'left' cursor key - if the last key was logged then log this key as it was - // probably used for correcting a mistake. - wanted = that.state.input.lastKeyLogged; - } + wanted = specialKey || + windows.user32.MapVirtualKeyW(eventData.vkCode, windows.API_constants.MAPVK_VK_TO_CHAR); if (wanted) { // Process in the next tick, to allow this function to return soon. windows.recordKeyTiming(that, eventData.time, specialKey); diff --git a/gpii/node_modules/windowsMetrics/test/WindowsMetricsTests.js b/gpii/node_modules/windowsMetrics/test/WindowsMetricsTests.js index 03b3218d4..5a2a767cf 100644 --- a/gpii/node_modules/windowsMetrics/test/WindowsMetricsTests.js +++ b/gpii/node_modules/windowsMetrics/test/WindowsMetricsTests.js @@ -80,12 +80,12 @@ gpii.tests.metrics.recordKeyTimingKeyTests = fluid.freezeRecursive({ { // special key state: null, input: { - key: "BS" + key: "BACK" }, expect: { module: "metrics", event: "key-time", - data: {keyTime: 0, key: "BS"} + data: {keyTime: 0, key: "BACK"} } }, // Test that non-special keys don't get leaked. @@ -147,7 +147,7 @@ gpii.tests.metrics.recordKeyTimingKeyTests = fluid.freezeRecursive({ { // not a special key (virtual key code of a special key) state: null, input: { - key: gpii.windows.API_constants.VK_ESCAPE + key: gpii.windows.API_constants.virtualKeyCodes.VK_ESCAPE }, expect: { module: "metrics", @@ -308,7 +308,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ nCode: 0, wParam: gpii.windows.API_constants.WM_KEYUP, lParam: { - vkCode: gpii.windows.API_constants.VK_HOME, + vkCode: gpii.windows.API_constants.virtualKeyCodes.VK_HOME, scanCode: 0, flags: 0, time: 200, @@ -322,7 +322,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ nCode: 0, wParam: gpii.windows.API_constants.WM_KEYUP, lParam: { - vkCode: gpii.windows.API_constants.VK_BACK, + vkCode: gpii.windows.API_constants.virtualKeyCodes.VK_BACK, scanCode: 0, flags: 0, time: 200, @@ -332,7 +332,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ expect: { module: "metrics", event: "key-time", - data: { keyTime: 0, key: "BS" } + data: { keyTime: 0, key: "BACK" } } }, { // Special key: delete. @@ -340,7 +340,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ nCode: 0, wParam: gpii.windows.API_constants.WM_KEYUP, lParam: { - vkCode: gpii.windows.API_constants.VK_DELETE, + vkCode: gpii.windows.API_constants.virtualKeyCodes.VK_DELETE, scanCode: 0, flags: 0, time: 200, @@ -350,7 +350,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ expect: { module: "metrics", event: "key-time", - data: { keyTime: 0, key: "DEL" } + data: { keyTime: 0, key: "DELETE" } } }, { // Special key: escape. @@ -358,7 +358,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ nCode: 0, wParam: gpii.windows.API_constants.WM_KEYUP, lParam: { - vkCode: gpii.windows.API_constants.VK_ESCAPE, + vkCode: gpii.windows.API_constants.virtualKeyCodes.VK_ESCAPE, scanCode: 0, flags: 0, time: 200, @@ -368,23 +368,9 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ expect: { module: "metrics", event: "key-time", - data: { keyTime: 0, key: "ESC" } + data: { keyTime: 0, key: "ESCAPE" } } }, - { // Special key: left arrow. Expect nothing, because this only logs if hit after a character key. - input: { - nCode: 0, - wParam: gpii.windows.API_constants.WM_KEYUP, - lParam: { - vkCode: gpii.windows.API_constants.VK_LEFT, - scanCode: 0, - flags: 0, - time: 200, - dwExtraInfo: 0 - } - }, - expect: [] - }, { // Normal key: 'X' input: { nCode: 0, @@ -408,7 +394,7 @@ gpii.tests.metrics.inputHookTests = fluid.freezeRecursive([ nCode: 0, wParam: gpii.windows.API_constants.WM_KEYUP, lParam: { - vkCode: gpii.windows.API_constants.VK_LEFT, + vkCode: gpii.windows.API_constants.virtualKeyCodes.VK_LEFT, scanCode: 0, flags: 0, time: 200, @@ -840,6 +826,134 @@ jqUnit.asyncTest("Testing input metrics: inputHook", function () { }); }); +// Ensure no character keys are whitelisted for being logged. +jqUnit.test("Testing input metrics: inputHook - not logging character keys", function () { + + var keyCodes = Object.keys(gpii.windows.specialKeys); + jqUnit.expect(keyCodes.length + 2); + + // Return true if keyCode corresponds to a character key. + var isCharacterKey = function (keyCode) { + var char = gpii.windows.user32.MapVirtualKeyW(keyCode, gpii.windows.API_constants.MAPVK_VK_TO_CHAR); + return char && char >= 0x20; + }; + + isCharacterKey(gpii.windows.API_constants.virtualKeyCodes.VK_BACK); + + // Sanity check. + var keyA = 0x41; + jqUnit.assertTrue("'A' key should be a character key", isCharacterKey(keyA)); + jqUnit.assertFalse("'HOME' key should not be a character key", + isCharacterKey(gpii.windows.API_constants.virtualKeyCodes.VK_HOME)); + + fluid.each(keyCodes, function (key) { + jqUnit.assertFalse("Keys in windows.specialKeys must all be non-printable: " + gpii.windows.specialKeys[key], + isCharacterKey(key)); + }); +}); + +// Ensure only the value of whitelisted keys are being logged. +jqUnit.asyncTest("Testing input metrics: inputHook - only logging desired keys", function () { + var logFile = gpii.tests.metrics.setLogFile(); + + var windowsMetrics = gpii.tests.metrics.windowsMetricsWrapper({ + members: { + logFilePath: logFile + } + }); + + // Disable typing sessions + windowsMetrics.config.input.minSession = windowsMetrics.config.input.sessionTimeout = -1 >>> 0; + windowsMetrics.config.input.maxRecords = 1; + windowsMetrics.startInputMetrics(); + + var lParam = { + flags: 0, + time: 0, + dwExtraInfo: 0 + }; + + var keyCount = 0; + // Send every key. + for (var keyCode = 0; keyCode <= 0xff; keyCode++) { + lParam.vkCode = keyCode; + gpii.windows.inputHook(windowsMetrics, 0, gpii.windows.API_constants.WM_KEYUP, lParam); + keyCount++; + } + + // Log a bad key, to test the test. + windowsMetrics.logMetric("key-time", { + keyTime: 0, + key: "A", + // Used to mark this log entry as a test value. + selfTest: "this is a test" + }); + keyCount++; + + var expectedCount = 257; + jqUnit.assertEquals("self-test: there should be 257 key presses, including the bad one.", expectedCount, keyCount); + + // The events are put in the log in the next tick (to allow the hook handler to return quickly). + setImmediate(function () { + windowsMetrics.stopInputMetrics(); + + var gotSelfTest = false; + var loggedKeys = {}; + var loggedSpecialKeys = []; + + var logLines = fs.readFileSync(logFile).toString().trim().split(/[\n\r]+/); + fluid.each(logLines, function (line) { + var logEntry = JSON.parse(line); + if (logEntry.event === "key-time") { + + jqUnit.assertNotNull("Key event must have a data field", logEntry.data); + + var key = logEntry.data.key; + if (key) { + jqUnit.assertEquals("value of data.key must be a string", "string", typeof key); + jqUnit.assertTrue("value of data.key must not be a number", isNaN(parseInt(key))); + + // Failire means there's something wrong with the logging or this test. + jqUnit.assertFalse("Keys should only be logged once.", !!loggedKeys[key]); + loggedKeys[key] = true; + + // Make sure the key is supposed to be logged. + var keyCode = fluid.keyForValue(gpii.windows.specialKeys, key); + var found = keyCode !== undefined; + if (found) { + // Double check for the alpha-numeric keys + /* from WinUser.h: + * VK_0 - VK_9 are the same as ASCII '0' - '9' (0x30 - 0x39) + * 0x3A - 0x40 : unassigned + * VK_A - VK_Z are the same as ASCII 'A' - 'Z' (0x41 - 0x5A) + */ + var isAlphaNum = (keyCode >= 0x30 && keyCode <= 0x5a); + // This means a number/letter key is in the whitelist. + jqUnit.assertFalse("Alphanumeric keys should not be logged: " + key, isAlphaNum); + loggedSpecialKeys.push(key); + } else { + // This key should not be logged. + if (logEntry.data.selfTest === "this is a test" && !gotSelfTest) { + // The self-test key + gotSelfTest = true; + jqUnit.assert("Got self-test key."); + } else { + jqUnit.fail("An undesired key was logged"); + } + } + } + } + }); + + // Make sure the test ran fully. + var allKeys = fluid.values(gpii.windows.specialKeys); + jqUnit.assertDeepEq("All special keys should have been logged", allKeys, loggedSpecialKeys); + jqUnit.assertTrue("The self-test bad key should have been found", gotSelfTest); + + jqUnit.start(); + }); +}); + jqUnit.asyncTest("Testing input metrics: inactivity", function () { jqUnit.expect(4); var logFile = gpii.tests.metrics.setLogFile(); From 22bbd63401327199f729b72ed3dbf1aebce5a49f Mon Sep 17 00:00:00 2001 From: ste Date: Tue, 17 Jul 2018 15:52:02 +0100 Subject: [PATCH 3/5] GPII-3102: Comments about key values --- gpii/node_modules/windowsMetrics/src/windowsMetrics.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js index 848834f21..929ad2aa9 100644 --- a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js +++ b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js @@ -350,6 +350,8 @@ windows.stopInputMetrics = function (that) { * A value=>name map of only non-printable keys that will be logged. */ windows.specialKeys = fluid.freezeRecursive((function () { + // A white-list of key values that can be logged. It must not contain printable keys (like letters or numbers). + // Also, there needs to be a matching value in windows.API_constants.virtualKeyCodes with the VK_ prefix. var keys = [ "BACK", "TAB", "RETURN", "ESCAPE", "PAGEUP", "PAGEDOWN", "END", "HOME", "LEFT", "UP", "RIGHT", "DOWN", "SELECT", "PRINT", "EXECUTE", "SNAPSHOT", "INSERT", "DELETE", "HELP", "LWIN", "RWIN", "SCROLL", "NUMLOCK", @@ -430,6 +432,7 @@ windows.recordKeyTiming = function (that, timestamp, specialKey) { // Double-check that only certain keys are being recorded (it would be a serious blunder). var keycode = parseInt(fluid.keyForValue(windows.specialKeys, specialKey)); if (!!keycode && typeof(specialKey) === "string" && specialKey.length > 1) { + // Not logging the value of specialKey directly. record.key = windows.specialKeys[keycode]; } } From 7b3446d60af97afe9983c62d1bab63ab1196aef9 Mon Sep 17 00:00:00 2001 From: ste Date: Wed, 18 Jul 2018 11:35:31 +0100 Subject: [PATCH 4/5] GPII-3102: Logging start/stop once. --- gpii/node_modules/windowsMetrics/src/windowsMetrics.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js index 929ad2aa9..2840101f2 100644 --- a/gpii/node_modules/windowsMetrics/src/windowsMetrics.js +++ b/gpii/node_modules/windowsMetrics/src/windowsMetrics.js @@ -53,7 +53,7 @@ fluid.defaults("gpii.windowsMetrics", { invokers: { logMetric: { func: "{eventLog}.logEvent", - args: ["{eventLog}", "metrics", "{arguments}.0", "{arguments}.1"] + args: ["metrics", "{arguments}.0", "{arguments}.1"] }, startApplicationMetrics: { funcName: "gpii.windows.startApplicationMetrics", From cf66327ded5428a27c9e4117dcd5616e12501434 Mon Sep 17 00:00:00 2001 From: ste Date: Thu, 19 Jul 2018 16:19:44 +0100 Subject: [PATCH 5/5] GPII-3102: Referring to relevant universal branch. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a0229c637..7412adcf1 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,7 @@ "edge": "6.5.1", "edge-js": "8.8.1", "ffi": "2.0.0", - "gpii-universal": "0.3.0-dev.20180625T165346Z.4467c87", + "gpii-universal": "stegru/universal#GPII-3102", "@pokusew/pcsclite": "0.4.18", "ref": "1.3.4", "ref-struct": "1.1.0",