From f53bec38a5b1fb55ba4c8118f61ec1527caef6b4 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Thu, 6 Dec 2018 11:33:11 +1100 Subject: [PATCH] Added password inputs on registration screen. Fix case where db is deleted but password hash still remains which causes user to never register. Allow password to have symbols and other characters. Added more tests. Moved passHash from config into the sqlite db. We can do this because we assume if sql failed to initialise then the key provided was wrong and thus we can show the user the password page. --- app/password_util.js | 22 ++++-- app/sql.js | 49 ++++++++++++-- background.html | 12 +++- js/modules/data.js | 8 +++ js/views/password_view.js | 3 +- js/views/standalone_registration_view.js | 86 +++++++++++++++++++++--- main.js | 58 ++++++++++------ password.html | 2 +- preload.js | 12 ++++ stylesheets/_global.scss | 22 +++++- test/app/password_util_test.js | 28 ++++++++ 11 files changed, 256 insertions(+), 46 deletions(-) diff --git a/app/password_util.js b/app/password_util.js index e2fef063a..b6c5e2b39 100644 --- a/app/password_util.js +++ b/app/password_util.js @@ -1,9 +1,23 @@ const { sha512 } = require('js-sha512'); -const generateHash = (phrase) => sha512(phrase); -const matchesHash = (phrase, hash) => sha512(phrase) === hash; +const generateHash = (phrase) => phrase && sha512(phrase.trim()); +const matchesHash = (phrase, hash) => phrase && sha512(phrase.trim()) === hash.trim(); + +const validatePassword = (phrase) => { + if (typeof phrase !== 'string') { + return 'Password must be a string' + } + + if (phrase && phrase.trim().length < 6) { + return 'Password must be atleast 6 characters long'; + } + + // An empty password is still valid :P + return null; +} module.exports = { - generateHash, - matchesHash, + generateHash, + matchesHash, + validatePassword, }; \ No newline at end of file diff --git a/app/sql.js b/app/sql.js index 1160c50e6..ea2509e17 100644 --- a/app/sql.js +++ b/app/sql.js @@ -1,10 +1,11 @@ +const fs = require('fs'); const path = require('path'); const mkdirp = require('mkdirp'); const rimraf = require('rimraf'); const sql = require('@journeyapps/sqlcipher'); const pify = require('pify'); const uuidv4 = require('uuid/v4'); -const { map, isString, fromPairs, forEach, last } = require('lodash'); +const { map, isString, fromPairs, forEach, last, isEmpty } = require('lodash'); // To get long stack traces // https://github.com/mapbox/node-sqlite3/wiki/API#sqlite3verbose @@ -15,6 +16,11 @@ module.exports = { close, removeDB, removeIndexedDBFiles, + setSQLPassword, + + getPasswordHash, + savePasswordHash, + removePasswordHash, createOrUpdateGroup, getGroupById, @@ -181,15 +187,25 @@ async function getSQLCipherVersion(instance) { } } -const INVALID_KEY = /[^0-9A-Fa-f]/; +const HEX_KEY = /[^0-9A-Fa-f]/; async function setupSQLCipher(instance, { key }) { - const match = INVALID_KEY.exec(key); - if (match) { - throw new Error(`setupSQLCipher: key '${key}' is not valid`); - } + // If the key isn't hex then we need to derive a hex key from it + const deriveKey = HEX_KEY.test(key); // https://www.zetetic.net/sqlcipher/sqlcipher-api/#key - await instance.run(`PRAGMA key = "x'${key}'";`); + const value = deriveKey ? `'${key}'` : `"x'${key}'"` + await instance.run(`PRAGMA key = ${value};`); +} + +async function setSQLPassword(password) { + if (!db) { + throw new Error('setSQLPassword: db is not initialized'); + } + + // If the password isn't hex then we need to derive a key from it + const deriveKey = HEX_KEY.test(password); + const value = deriveKey ? `'${password}'` : `"x'${password}'"` + await db.run(`PRAGMA rekey = ${value};`); } async function updateToSchemaVersion1(currentVersion, instance) { @@ -584,6 +600,25 @@ async function removeIndexedDBFiles() { indexedDBPath = null; } +// Password hash +async function getPasswordHash() { + const item = await getItemById('passHash'); + return item && item.value; +} +async function savePasswordHash(hash) { + if (isEmpty(hash)) { + return removePasswordHash(); + } + + const data = { id: 'passHash', value: hash }; + return createOrUpdateItem(data); +} +async function removePasswordHash() { + return removeItemById('passHash'); +} + +// Groups + const GROUPS_TABLE = 'groups'; async function createOrUpdateGroup(data) { return createOrUpdate(GROUPS_TABLE, data); diff --git a/background.html b/background.html index 482796562..83b1806b8 100644 --- a/background.html +++ b/background.html @@ -603,8 +603,16 @@
Create your Loki Messenger Account
-
Enter a name that will be shown to all your contacts
- +
+
Enter a name that will be shown to all your contacts
+ +
+
+
Type an optional password for added security
+ + +
+

Restore using seed

diff --git a/js/modules/data.js b/js/modules/data.js index 1002a743c..e7eb0614f 100644 --- a/js/modules/data.js +++ b/js/modules/data.js @@ -47,6 +47,8 @@ module.exports = { removeDB, removeIndexedDBFiles, + getPasswordHash, + createOrUpdateGroup, getGroupById, getAllGroupIds, @@ -405,6 +407,12 @@ async function removeIndexedDBFiles() { await channels.removeIndexedDBFiles(); } +// Password hash + +async function getPasswordHash() { + return channels.getPasswordHash(); +} + // Groups async function createOrUpdateGroup(data) { diff --git a/js/views/password_view.js b/js/views/password_view.js index e32cfb5e1..71a941d6c 100644 --- a/js/views/password_view.js +++ b/js/views/password_view.js @@ -26,9 +26,10 @@ }, async onLogin() { const passPhrase = this.$('#passPhrase').val(); + const trimmed = passPhrase ? passPhrase.trim() : passPhrase; this.setError(''); try { - await window.onLogin(passPhrase); + await window.onLogin(trimmed); } catch (e) { this.setError(`Error: ${e}`); } diff --git a/js/views/standalone_registration_view.js b/js/views/standalone_registration_view.js index b2eee33c6..6f32d874a 100644 --- a/js/views/standalone_registration_view.js +++ b/js/views/standalone_registration_view.js @@ -1,4 +1,4 @@ -/* global Whisper, $, getAccountManager, textsecure, i18n, storage, ConversationController */ +/* global Whisper, $, getAccountManager, textsecure, i18n, passwordUtil, ConversationController */ /* eslint-disable more/no-then */ @@ -35,6 +35,13 @@ }); this.$('#mnemonic-language').append(options); this.$('#mnemonic-display-language').append(options); + + this.$passwordInput = this.$('#password'); + this.$passwordConfirmationInput = this.$('#password-confirmation'); + this.$passwordConfirmationInput.hide(); + this.$passwordInputError = this.$('.password-inputs .error'); + + this.onValidatePassword(); }, events: { 'validation input.number': 'onValidation', @@ -48,18 +55,32 @@ 'change #mnemonic-display-language': 'onGenerateMnemonic', 'click #copy-mnemonic': 'onCopyMnemonic', 'click .section-toggle': 'toggleSection', + 'keyup #password': 'onPasswordChange', + 'keyup #password-confirmation': 'onValidatePassword', }, - register(mnemonic) { - this.accountManager - .registerSingleDevice( + async register(mnemonic) { + // Make sure the password is valid + if (this.validatePassword()) { + this.showToast('Invalid password'); + return; + } + + const input = this.trim(this.$passwordInput.val()); + + try { + await window.setPassword(input); + await this.accountManager.registerSingleDevice( mnemonic, this.$('#mnemonic-language').val(), this.$('#display-name').val() - ) - .then(() => { - this.$el.trigger('openInbox'); - }) - .catch(this.log.bind(this)); + ); + this.$el.trigger('openInbox'); + } catch (e) { + if (typeof e === 'string') { + this.showToast(e); + } + this.log(e); + } }, registerWithoutMnemonic() { const mnemonic = this.$('#mnemonic-display').text(); @@ -158,5 +179,52 @@ this.$('.section-toggle').not($target).removeClass('section-toggle-visible') this.$('.section-content').not($next).slideUp('fast'); }, + onPasswordChange() { + const input = this.$passwordInput.val(); + if (!input || input.length === 0) { + this.$passwordConfirmationInput.val(''); + this.$passwordConfirmationInput.hide(); + } else { + this.$passwordConfirmationInput.show(); + } + this.onValidatePassword(); + }, + validatePassword() { + const input = this.trim(this.$passwordInput.val()); + const confirmationInput = this.trim(this.$passwordConfirmationInput.val()); + + const error = passwordUtil.validatePassword(input); + if (error) + return error; + + if (input !== confirmationInput) + return 'Password don\'t match'; + + return null; + }, + onValidatePassword() { + const passwordValidation = this.validatePassword(); + if (passwordValidation) { + this.$passwordInput.addClass('error-input'); + this.$passwordConfirmationInput.addClass('error-input'); + this.$passwordInputError.text(passwordValidation); + this.$passwordInputError.show(); + } else { + this.$passwordInput.removeClass('error-input'); + this.$passwordConfirmationInput.removeClass('error-input'); + this.$passwordInputError.text(''); + this.$passwordInputError.hide(); + } + }, + trim(value) { + return value ? value.trim() : value; + }, + showToast(message) { + const toast = new Whisper.MessageToastView({ + message, + }); + toast.$el.appendTo(this.$el); + toast.render(); + }, }); })(); diff --git a/main.js b/main.js index a95b94a24..50e4f2b26 100644 --- a/main.js +++ b/main.js @@ -171,7 +171,7 @@ function captureClicks(window) { } const DEFAULT_WIDTH = 800; -const DEFAULT_HEIGHT = 610; +const DEFAULT_HEIGHT = 710; const MIN_WIDTH = 640; const MIN_HEIGHT = 360; const BOUNDS_BUFFER = 100; @@ -709,13 +709,12 @@ app.on('ready', async () => { const key = getDefaultSQLKey(); - // If we have a password set then show the password window - // Otherwise show the main window - const passHash = userConfig.get('passHash'); - if (passHash) { - showPasswordWindow(); - } else { + // Try to show the main window with the default key + // If that fails then show the password window + try { await showMainWindow(key); + } catch (e) { + showPasswordWindow(); } }); @@ -950,26 +949,45 @@ ipc.on('update-tray-icon', (event, unreadCount) => { // Password screen related IPC calls ipc.on('password-window-login', async (event, passPhrase) => { - const sendError = (e) => event.sender.send('password-window-login-response', e); + const sendResponse = (e) => event.sender.send('password-window-login-response', e); - // Check if the phrase matches with the hash we have stored - const hash = userConfig.get('passHash'); - const hashMatches = passPhrase && passwordUtil.matchesHash(passPhrase, hash); - if (hash && !hashMatches) { - sendError('Invalid password'); - return; - } - - // If we don't have a hash then use the default sql key to unlock the db - const key = hash ? passPhrase : getDefaultSQLKey(); try { - await showMainWindow(key); + await showMainWindow(passPhrase); + sendResponse(); if (passwordWindow) { passwordWindow.close(); passwordWindow = null; } } catch (e) { - sendError('Failed to decrypt SQL database'); + sendResponse('Invalid password'); + } +}); + +ipc.on('set-password', async (event, passPhrase, oldPhrase) => { + const sendResponse = (e) => event.sender.send('set-password-response', e); + + try { + // Check if the hash we have stored matches the hash of the old passphrase. + const hash = await sql.getPasswordHash(); + const hashMatches = oldPhrase && passwordUtil.matchesHash(oldPhrase, hash); + if (hash && !hashMatches) { + sendResponse('Failed to set password: Old password provided is invalid'); + return; + } + + if (_.isEmpty(passPhrase)) { + const defaultKey = getDefaultSQLKey(); + await sql.setSQLPassword(defaultKey); + await sql.removePasswordHash(); + } else { + await sql.setSQLPassword(passPhrase); + const newHash = passwordUtil.generateHash(passPhrase); + await sql.savePasswordHash(newHash); + } + + sendResponse(); + } catch (e) { + sendResponse('Failed to set password'); } }); diff --git a/password.html b/password.html index 557c74b7e..ab818dae3 100644 --- a/password.html +++ b/password.html @@ -21,7 +21,7 @@

{{ title }}

- + {{ buttonText }}
diff --git a/preload.js b/preload.js index 9c5ea103e..e55fedc0a 100644 --- a/preload.js +++ b/preload.js @@ -49,6 +49,17 @@ const localeMessages = ipc.sendSync('locale-data'); window.setBadgeCount = count => ipc.send('set-badge-count', count); +// Set the password for the database +window.setPassword = (passPhrase, oldPhrase) => new Promise((resolve, reject) => { + ipc.once('set-password-response', (event, error) => { + if (error) { + return reject(error); + } + return resolve(); + }); + ipc.send('set-password', passPhrase, oldPhrase); +}); + // We never do these in our code, so we'll prevent it everywhere window.open = () => null; // eslint-disable-next-line no-eval, no-multi-assign @@ -273,6 +284,7 @@ window.libphonenumber.PhoneNumberFormat = require('google-libphonenumber').Phone window.loadImage = require('blueimp-load-image'); window.getGuid = require('uuid/v4'); window.profileImages = require('./app/profile_images'); +window.passwordUtil = require('./app/password_util'); window.React = require('react'); window.ReactDOM = require('react-dom'); diff --git a/stylesheets/_global.scss b/stylesheets/_global.scss index b61fbf51c..3321781a2 100644 --- a/stylesheets/_global.scss +++ b/stylesheets/_global.scss @@ -919,7 +919,7 @@ textarea { } } - .display-name-header { + .input-header { margin-bottom: 8px; font-size: 14px; } @@ -977,8 +977,26 @@ textarea { } } + .password-inputs { + input { + margin-bottom: 0.5em; + } + + .error { + margin-bottom: 1em; + } + + .error-input { + border: 3px solid $color-vermilion; + + &:focus { + outline: none; + } + } + } + @media (min-height: 750px) and (min-width: 700px) { - .display-name-header { + .input-header { font-size: 18px; } diff --git a/test/app/password_util_test.js b/test/app/password_util_test.js index c1bf27a44..8256fe6b5 100644 --- a/test/app/password_util_test.js +++ b/test/app/password_util_test.js @@ -27,4 +27,32 @@ describe('Password Util', () => { assert.isFalse(passwordUtil.matchesHash('phrase2', hash)); }); }); + + describe('password validation', () => { + it('should return nothing if password is valid', () => { + const valid = [ + '123456', + '1a5b3C6g', + 'ABC#DE#F$IJ', + 'AabcDegf', + ]; + valid.forEach(pass => { + assert.isNull(passwordUtil.validatePassword(pass)); + }); + }); + + it('should return an error string if password is invalid', () => { + const invalid = [ + 0, + 123456, + [], + {}, + '123', + '1234$', + ]; + invalid.forEach(pass => { + assert.isNotNull(passwordUtil.validatePassword(pass)); + }); + }); + }); });