From 01ad4d277861a12b156fc69f1c826fbd9452d8ce Mon Sep 17 00:00:00 2001 From: yan Date: Tue, 15 Aug 2017 21:35:57 +0000 Subject: [PATCH] Remove keytar and gnome-keyring deps Give up on migrating old passwords for users who are updating from <0.15.300 to 0.21 and above. Fix https://github.com/brave/browser-laptop/issues/10226 Test Plan: npm run test -- --grep='passwords' --- .travis.yml | 1 - README.md | 4 +- .../reducers/passwordManagerReducer.js | 116 ++---------------- docs/state.md | 18 --- js/lib/cryptoUtil.js | 112 ----------------- package.json | 1 - res/linuxPackaging.json | 4 +- test/unit/app/sessionStoreShutdownTest.js | 1 - test/unit/lib/cryptoUtilTest.js | 112 ----------------- test/vms/vagrant/jessie/init | 2 +- test/vms/vagrant/ubuntu-14.04/script.sh | 2 +- 11 files changed, 14 insertions(+), 359 deletions(-) delete mode 100644 js/lib/cryptoUtil.js delete mode 100644 test/unit/lib/cryptoUtilTest.js diff --git a/.travis.yml b/.travis.yml index 8cf9d40c0ae..6b0f8f6dd41 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,4 +37,3 @@ addons: packages: - xvfb - g++-4.8 - - libgnome-keyring-dev diff --git a/README.md b/README.md index 3e084e93a4d..0a00fe80801 100644 --- a/README.md +++ b/README.md @@ -59,13 +59,13 @@ For other platforms (macOS, Linux) You'll need certain packages installed before #### On Debian / Ubuntu /Mint ```` -apt-get install libgnome-keyring-dev build-essential rpm ninja-build +apt-get install build-essential rpm ninja-build ```` #### On Fedora ```` -dnf install libgnome-keyring-devel rpm-build +dnf install rpm-build dnf group install "Development Tools" "C Development Tools and Libraries" ```` diff --git a/app/browser/reducers/passwordManagerReducer.js b/app/browser/reducers/passwordManagerReducer.js index 2540050f0a5..94d1814e507 100644 --- a/app/browser/reducers/passwordManagerReducer.js +++ b/app/browser/reducers/passwordManagerReducer.js @@ -4,83 +4,18 @@ 'use strict' -const keytar = require('keytar') const appConstants = require('../../../js/constants/appConstants') const appActions = require('../../../js/actions/appActions') -const CryptoUtil = require('../../../js/lib/cryptoUtil') const locale = require('../../locale') const messages = require('../../../js/constants/messages') const {getWebContents} = require('../webContentsCache') const {makeImmutable} = require('../../common/state/immutableUtil') -const Immutable = require('immutable') const {ipcMain} = require('electron') const autofill = require('../../autofill') -const unsafeTestMasterKey = 'c66af15fc6555ebecf7cee3a5b82c108fd3cb4b587ab0b299d28e39c79ecc708' - -// Don't show the keytar prompt more than once per 24 hours -let throttleKeytar = false - // Map of password notification bar messages to their callbacks const passwordCallbacks = {} -let masterKey - -/** - * Gets the master key for encrypting login credentials from the OS keyring. - */ -const getMasterKey = () => { - if (throttleKeytar) { - return null - } - - if (process.env.NODE_ENV === 'test') { - // workaround for https://travis-ci.org/brave/browser-laptop/builds/132700770 - return (new Buffer(unsafeTestMasterKey, 'hex')).toString('binary') - } - - const appName = 'Brave' - // Previously the master key was binary encoded, which caused compatibility - // issues with various keyrings. In 0.8.3, switch to hex encoding for storage. - const oldAccountName = 'login master key' - const accountName = 'login master key v2' - let oldMasterKey = keytar.getPassword(appName, oldAccountName) - let masterKey = keytar.getPassword(appName, accountName) - - let success = false - - if (masterKey === null) { - if (typeof oldMasterKey === 'string') { - // The user made a v1 (binary) key. Try converting it to hex if it - // appears to be 32 bytes. - let oldBuffer = new Buffer(oldMasterKey, 'binary') - if (oldBuffer.length === 32) { - success = keytar.addPassword(appName, accountName, oldBuffer.toString('hex')) - } - } - - // Either the user denied access or no master key has ever been created. - // We can't tell the difference so try making a new master key. - success = success || keytar.addPassword(appName, accountName, CryptoUtil.getRandomBytes(32).toString('hex')) - - if (success) { - // A key should have been created - masterKey = keytar.getPassword(appName, accountName) - } - } - - if (typeof masterKey === 'string') { - // Convert from hex to binary - return (new Buffer(masterKey, 'hex')).toString('binary') - } else { - throttleKeytar = true - setTimeout(() => { - throttleKeytar = false - }, 1000 * 60 * 60 * 24) - return null - } -} - const init = () => { ipcMain.on(messages.NOTIFICATION_RESPONSE, (e, message, buttonIndex, persist) => { if (passwordCallbacks[message]) { @@ -190,53 +125,20 @@ const clearPasswords = () => { autofill.clearLogins() } -const migrate = (state) => { - const passwords = state.get('passwords') - if (passwords.size) { - masterKey = masterKey || getMasterKey() - if (!masterKey) { - console.log('Could not access master password; aborting') - return - } - passwords.forEach((password) => { - let decrypted = CryptoUtil.decryptVerify(password.get('encryptedPassword'), - password.get('authTag'), - masterKey, - password.get('iv')) - if (decrypted) { - let form = {} - form['origin'] = password.get('origin') - form['signon_realm'] = password.get('origin') + '/' - form['action'] = password.get('action') - form['username'] = password.get('username') - form['password'] = decrypted - autofill.addLogin(form) - } - }) - state = state.set('legacyPasswords', state.get('passwords')) - state = state.set('passwords', new Immutable.List()) - } - const allSiteSettings = state.get('siteSettings') - const blackedList = allSiteSettings.filter((setting) => setting.get('savePasswords') === false) - if (blackedList.size) { - blackedList.forEach((entry, index) => { - let form = {} - form['origin'] = index - form['signon_realm'] = index + '/' - form['blacklisted_by_user'] = true - autofill.addLogin(form) - appActions.deletePasswordSite(index) - }) - } - return state -} - const passwordManagerReducer = (state, action, immutableAction) => { action = immutableAction || makeImmutable(action) switch (action.get('actionType')) { case appConstants.APP_SET_STATE: init() - state = migrate(state) + // Log a warning if they are updating from <0.15.300 to 0.21 or higher + if (state.getIn(['passwords', 0])) { + console.log('Warning: unable to migrate old passwords.') + } + // legacyPasswords was added in 0.15.300 to backup old passwords in case + // the migration failed + if (state.get('legacyPasswords')) { + state = state.delete('legacyPasswords') + } break case appConstants.APP_SAVE_PASSWORD: savePassword(action.get('username'), action.get('origin'), action.get('tabId')) diff --git a/docs/state.md b/docs/state.md index 87920066a14..7d18b266a72 100644 --- a/docs/state.md +++ b/docs/state.md @@ -192,24 +192,6 @@ AppStore noScript: { enabled: boolean // enable noscript }, - passwords: [{ - // not being used anymore - action: string, // URL of the form action - authTag: string, // AES-GCM authentication data, binary-encoded - encryptedPassword: string, // encrypted by master password, binary-encoded - iv: string, // AES-GCM initialization vector, binary-encoded - origin: string, // origin of the form - username: string - }], - legacyPasswords: [{ - // backup of passwords migration - action: string, // URL of the form action - authTag: string, // AES-GCM authentication data, binary-encoded - encryptedPassword: string, // encrypted by master password, binary-encoded - iv: string, // AES-GCM initialization vector, binary-encoded - origin: string, // origin of the form - username: string - }], pinnedSites: { [siteKey]: { location: string, diff --git a/js/lib/cryptoUtil.js b/js/lib/cryptoUtil.js deleted file mode 100644 index 04fdb97b467..00000000000 --- a/js/lib/cryptoUtil.js +++ /dev/null @@ -1,112 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -'use strict' - -const crypto = require('crypto') - -const AES_256_GCM = 'aes-256-gcm' -const DEFAULT_ENCODING = 'binary' // encoding of ciphertexts, iv's, keys, etc. -const PLAINTEXT_ENCODING = 'utf8' // encoding of encryption input and decryption output - -/** - * Encrypts and integrity-protects some text using AES GCM 256 - * @param {string} text - utf8 encoded text to encrypt - * @param {string} key - binary encoded secret key, 32 bytes - * @return {{content: string, tag: string, iv: string}} - */ -module.exports.encryptAuthenticate = function (text, key) { - var iv = module.exports.getRandomBytes(12) - var encrypted = module.exports.encryptAuthenticateInternal( - new Buffer(text, PLAINTEXT_ENCODING), - new Buffer(key, DEFAULT_ENCODING), - iv, - iv, - DEFAULT_ENCODING - ) - return { - content: encrypted.ciphertext, - tag: encrypted.tag, - iv: iv.toString(DEFAULT_ENCODING) - } -} - -/** - * Decrypts and verifies text using AES GCM 256 - * @param {string} encrypted - binary string to decrypt - * @param {string} authTag - binary-encoded authentication tag - * @param {string} key - binary encoded secret key, 32 bytes - * @param {string} iv - binary encoded IV, 12 bytes - * @return {string|null} - */ -module.exports.decryptVerify = function (encrypted, authTag, key, iv) { - try { - let ivBuffer = new Buffer(iv, DEFAULT_ENCODING) - return module.exports.decryptVerifyInternal( - new Buffer(encrypted, DEFAULT_ENCODING), - new Buffer(key, DEFAULT_ENCODING), - ivBuffer, - ivBuffer, - new Buffer(authTag, DEFAULT_ENCODING), - PLAINTEXT_ENCODING - ) - } catch (e) { - // node throws error if authentication fails - console.log('got decryption failure', e) - return null - } -} - -/** - * Generates a buffer of N random bytes. - * @param {number} size - number of bytes to generate - * @return {Buffer} - */ -module.exports.getRandomBytes = function (size) { - return crypto.randomBytes(size) -} - -/** - * Encrypts and integrity-protects some data using AES GCM 256 - * @param {Buffer} pdata - plaintext data to encrypt - * @param {Buffer} key - secret key - * @param {Buffer} iv - initialization vector - * @param {Buffer|undefined} adata - additional authenticated data - * @param {string} outputEncoding - 'hex', 'binary', or 'base64' - * @return {{ciphertext: string, tag: string}} - */ -module.exports.encryptAuthenticateInternal = function (pdata, key, iv, adata, outputEncoding) { - // Note that Node's GCM implementation only accepts 12 byte IV's - var cipher = crypto.createCipheriv(AES_256_GCM, key, iv) - if (adata) { - cipher.setAAD(adata) - } - var encrypted = cipher.update(pdata, undefined, outputEncoding) - encrypted += cipher.final(outputEncoding) - return { - ciphertext: encrypted, - tag: cipher.getAuthTag().toString(outputEncoding) - } -} - -/** - * Decrypts and verifies some data using AES GCM 256 - * @param {Buffer} ciphertext - ciphertext - * @param {Buffer} key - secret key - * @param {Buffer} iv - initialization vector - * @param {Buffer|undefined} adata - additional authenticated data - * @param {Buffer} tag - authentication tag - * @param {string} outputEncoding - 'binary', 'ascii', 'utf8' - * @return {string} - */ -module.exports.decryptVerifyInternal = function (ciphertext, key, iv, adata, tag, outputEncoding) { - let decipher = crypto.createDecipheriv(AES_256_GCM, key, iv) - if (adata) { - decipher.setAAD(adata) - } - decipher.setAuthTag(tag) - let decrypted = decipher.update(ciphertext, undefined, outputEncoding) - decrypted += decipher.final(outputEncoding) - return decrypted -} diff --git a/package.json b/package.json index 4626da185cd..535163fdbb2 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,6 @@ "immutable": "^3.7.5", "immutablediff": "^0.4.2", "immutablepatch": "brave/immutable-js-patch", - "keytar": "^3.0.0", "l20n": "^3.5.1", "ledger-balance": "^0.9.1", "ledger-client": "^0.9.18", diff --git a/res/linuxPackaging.json b/res/linuxPackaging.json index e804f2ce30e..d3bc7a08e17 100644 --- a/res/linuxPackaging.json +++ b/res/linuxPackaging.json @@ -38,8 +38,6 @@ "libxtst6", "libxss1", "python", - "xdg-utils", - "gir1.2-gnomekeyring-1.0", - "libgnome-keyring0" + "xdg-utils" ] } diff --git a/test/unit/app/sessionStoreShutdownTest.js b/test/unit/app/sessionStoreShutdownTest.js index 0f621ef2697..3a53cbb4228 100644 --- a/test/unit/app/sessionStoreShutdownTest.js +++ b/test/unit/app/sessionStoreShutdownTest.js @@ -61,7 +61,6 @@ describe('sessionStoreShutdown unit tests', function () { mockery.registerMock('electron', fakeElectron) mockery.registerMock('ad-block', fakeAdBlock) mockery.registerMock('leveldown', {}) - mockery.registerMock('keytar', {}) sessionStore = require('../../../app/sessionStore') sessionStoreShutdown = require('../../../app/sessionStoreShutdown') appActions = require('../../../js/actions/appActions') diff --git a/test/unit/lib/cryptoUtilTest.js b/test/unit/lib/cryptoUtilTest.js deleted file mode 100644 index dfcae000f39..00000000000 --- a/test/unit/lib/cryptoUtilTest.js +++ /dev/null @@ -1,112 +0,0 @@ -/* global describe, it */ -const CryptoUtil = require('../../../js/lib/cryptoUtil') -const assert = require('assert') - -require('../braveUnit') - -describe('crypto util test', function () { - it('gets random bytes', function () { - assert.equal(CryptoUtil.getRandomBytes(256).length, 256) - }) - it('throws if random bytes argument is not a number', function () { - assert.throws( - () => { CryptoUtil.getRandomBytes('') } - ) - }) - it('encrypts and decrypts successfully', function () { - let message = 'hello world' - let key = '3zTvzr3p67VC61jmV54rIYu1545x4TlY' - let encrypted = CryptoUtil.encryptAuthenticate(message, key) - assert.equal(message, CryptoUtil.decryptVerify(encrypted.content, encrypted.tag, key, encrypted.iv)) - }) - it('decryption fails with wrong authentication tag', function () { - let message = 'hello world' - let tamperedMessage = 'i hate browsers' - let key = '3zTvzr3p67VC61jmV54rIYu1545x4TlY' - let encrypted = CryptoUtil.encryptAuthenticate(message, key) - let wrongEncrypted = CryptoUtil.encryptAuthenticate(tamperedMessage, key) - assert.equal(null, CryptoUtil.decryptVerify(encrypted.content, wrongEncrypted.tag, key, encrypted.iv)) - }) - it('decryption fails with wrong IV', function () { - let message = 'hello world' - let key = '3zTvzr3p67VC61jmV54rIYu1545x4TlY' - let encrypted = CryptoUtil.encryptAuthenticate(message, key) - assert.equal(null, CryptoUtil.decryptVerify(encrypted.content, encrypted.tag, key, - CryptoUtil.getRandomBytes(12))) - }) - // Try some test vectors from - // http://csrc.nist.gov/groups/ST/toolkit/BCM/documents/proposedmodes/gcm/gcm-revised-spec.pdf - - // fails with Error: Unsupported state or unable to authenticate data - it.skip('passes NIST Test Case 13', function () { - let K = '0000000000000000000000000000000000000000000000000000000000000000' - let P = '' - let IV = '000000000000000000000000' - let encrypted = CryptoUtil.encryptAuthenticateInternal( - new Buffer(P, 'hex'), - new Buffer(K, 'hex'), - new Buffer(IV, 'hex'), - undefined, - 'hex' - ) - let decrypted = CryptoUtil.decryptVerifyInternal( - new Buffer('', 'hex'), new Buffer(K, 'hex'), new Buffer(IV, 'hex'), undefined, - new Buffer('530f8afbc74536b9a963b4f1c4cb738b', 'hex'), 'binary' - ) - assert.equal(encrypted.ciphertext, '') - assert.equal(encrypted.tag, '530f8afbc74536b9a963b4f1c4cb738b') - assert.equal(decrypted, P) - }) - it('passes NIST Test Case 14', function () { - let K = '0000000000000000000000000000000000000000000000000000000000000000' - let P = '00000000000000000000000000000000' - let IV = '000000000000000000000000' - let encrypted = CryptoUtil.encryptAuthenticateInternal( - new Buffer(P, 'hex'), - new Buffer(K, 'hex'), - new Buffer(IV, 'hex'), - undefined, - 'hex' - ) - assert.equal(encrypted.ciphertext, 'cea7403d4d606b6e074ec5d3baf39d18') - assert.equal(encrypted.tag, 'd0d1c8a799996bf0265b98b5d48ab919') - }) - it('passes NIST Test Case 15', function () { - let K = 'feffe9928665731c6d6a8f9467308308feffe9928665731c6d6a8f9467308308' - let P = 'd9313225f88406e5a55909c5aff5269a86a7a9531534f7da2e4c303d8a318a721c3c0c95956809532fcf0e2449a6b525b16aedf5aa0de657ba637b391aafd255' - let IV = 'cafebabefacedbaddecaf888' - let encrypted = CryptoUtil.encryptAuthenticateInternal( - new Buffer(P, 'hex'), - new Buffer(K, 'hex'), - new Buffer(IV, 'hex'), - undefined, - 'hex' - ) - assert.equal(encrypted.ciphertext, '522dc1f099567d07f47f37a32a84427d' + - '643a8cdcbfe5c0c97598a2bd2555d1aa' + - '8cb08e48590dbb3da7b08b1056828838' + - 'c5f61e6393ba7a0abcc9f662898015ad') - assert.equal(encrypted.tag, 'b094dac5d93471bdec1a502270e3cc6c') - }) - it('passes NIST Test Case 16', function () { - let K = 'feffe9928665731c6d6a8f9467308308feffe9928665731c6d6a8f9467308308' - let P = 'd9313225f88406e5a55909c5aff5269a86a7a9531534f7da2e4c303d8a318a721c3c0c95956809532fcf0e2449a6b525b16aedf5aa0de657ba637b39' - let IV = 'cafebabefacedbaddecaf888' - let A = 'feedfacedeadbeeffeedfacedeadbeefabaddad2' - let encrypted = CryptoUtil.encryptAuthenticateInternal( - new Buffer(P, 'hex'), - new Buffer(K, 'hex'), - new Buffer(IV, 'hex'), - new Buffer(A, 'hex'), - 'hex' - ) - let decrypted = CryptoUtil.decryptVerifyInternal( - new Buffer('522dc1f099567d07f47f37a32a84427d643a8cdcbfe5c0c97598a2bd2555d1aa8cb08e48590dbb3da7b08b1056828838c5f61e6393ba7a0abcc9f662', 'hex'), - new Buffer(K, 'hex'), new Buffer(IV, 'hex'), new Buffer(A, 'hex'), - new Buffer('76fc6ece0f4e1768cddf8853bb2d551b', 'hex'), 'binary' - ) - assert.equal(encrypted.ciphertext, '522dc1f099567d07f47f37a32a84427d643a8cdcbfe5c0c97598a2bd2555d1aa8cb08e48590dbb3da7b08b1056828838c5f61e6393ba7a0abcc9f662') - assert.equal(encrypted.tag, '76fc6ece0f4e1768cddf8853bb2d551b') - assert.equal(P, new Buffer(decrypted, 'binary').toString('hex')) - }) -}) diff --git a/test/vms/vagrant/jessie/init b/test/vms/vagrant/jessie/init index 06fcba42eb1..5ae45548588 100755 --- a/test/vms/vagrant/jessie/init +++ b/test/vms/vagrant/jessie/init @@ -40,7 +40,7 @@ chown -R vagrant /build apt-get update -y apt-get install build-essential clang libdbus-1-dev libgtk2.0-dev \ - libnotify-dev libgnome-keyring-dev libgconf2-dev \ + libnotify-dev libgconf2-dev \ libasound2-dev libcap-dev libcups2-dev libxtst-dev \ libxss1 libnss3-dev gcc-multilib g++-multilib curl \ gperf bison git python-software-properties python g++ make libgl1-mesa-dev -y diff --git a/test/vms/vagrant/ubuntu-14.04/script.sh b/test/vms/vagrant/ubuntu-14.04/script.sh index 91855b670fc..fce2157f46e 100644 --- a/test/vms/vagrant/ubuntu-14.04/script.sh +++ b/test/vms/vagrant/ubuntu-14.04/script.sh @@ -1,6 +1,6 @@ set -e sudo apt-get update -sudo apt-get install -y dkms build-essential linux-headers-$(uname -r) clang libdbus-1-dev libgtk2.0-dev libnotify-dev libgnome-keyring-dev libgconf2-dev \ +sudo apt-get install -y dkms build-essential linux-headers-$(uname -r) clang libdbus-1-dev libgtk2.0-dev libnotify-dev libgconf2-dev \ libasound2-dev libcap-dev libcups2-dev libxtst-dev libxss1 libnss3-dev gcc-multilib g++-multilib libxss-dev libpci-dev libpulse-dev libexif-dev sudo apt-get install -y virtualbox-guest-dkms virtualbox-guest-utils virtualbox-guest-x11 sudo apt-get install -y --no-install-recommends ubuntu-desktop xterm