From cf2d068ed7e4ed4cef6faf54e7ccdb1ab09116e5 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Thu, 23 Nov 2017 18:02:07 +0200 Subject: [PATCH 1/4] Fixed HTTP auth, decrypt error handling and database-locked response handling --- CHANGELOG | 7 + README.md | 1 + keepassxc-browser/background/httpauth.js | 10 +- keepassxc-browser/background/keepass.js | 233 ++++++++++++--------- keepassxc-browser/background/page.js | 2 +- keepassxc-browser/manifest.json | 2 +- keepassxc-browser/popups/popup_httpauth.js | 20 +- 7 files changed, 159 insertions(+), 116 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 320892e..979cd36 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,10 @@ +0.4.2 (??-??-2017) +========================= +- Fixed HTTP authentication with multiple credentials (credits to smorks) +- Fixed error handling when decrypt fails +- Fixed database-locked response handling +- TODO: Fixed nonce increment when encrypting messages + 0.4.1 (18-11-2017) ========================= - Added support for the credentials dropdown menu with only password field visible diff --git a/README.md b/README.md index 04a9c7b..6b04489 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ The following improvements and features have been made after the fork. At this p - New buttons, icons and settings page graphics - Redesigned password generator dialog - Password generator supports diceware passphrases and extended ASCII characters +- Autocomplete works also when only password fields are visible ## Protocol diff --git a/keepassxc-browser/background/httpauth.js b/keepassxc-browser/background/httpauth.js index 35dc99e..f74277d 100644 --- a/keepassxc-browser/background/httpauth.js +++ b/keepassxc-browser/background/httpauth.js @@ -5,7 +5,7 @@ httpAuth.pendingCallbacks = []; httpAuth.requestCompleted = function(details) { let index = httpAuth.requests.indexOf(details.requestId); - if (index > -1) { + if (index >= 0) { httpAuth.requests.splice(index, 1); } }; @@ -23,6 +23,7 @@ httpAuth.handleRequestCallback = function(details, callback) { httpAuth.processPendingCallbacks = function(details, resolve, reject) { if (httpAuth.requests.indexOf(details.requestId) >= 0 || !page.tabs[details.tabId]) { reject({}); + return; } httpAuth.requests.push(details.requestId); @@ -41,10 +42,7 @@ httpAuth.processPendingCallbacks = function(details, resolve, reject) { httpAuth.loginOrShowCredentials = function(logins, details, resolve, reject) { // at least one login found --> use first to login if (logins.length > 0) { - kpxcEvent.onHTTPAuthPopup(null, { "id": details.tabId }, { "logins": logins, "url": details.searchUrl }); - //generate popup-list for HTTP Auth usernames + descriptions - - if (page.settings.autoFillAndSend) { + if (logins.length == 1 && page.settings.autoFillAndSend) { resolve({ authCredentials: { username: logins[0].login, @@ -52,7 +50,7 @@ httpAuth.loginOrShowCredentials = function(logins, details, resolve, reject) { } }); } else { - reject({}); + kpxcEvent.onHTTPAuthPopup(null, { 'id': details.tabId }, { 'logins': logins, 'url': details.searchUrl, 'resolve': resolve }); } } // no logins found diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index 2a5665d..55b52ca 100644 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -167,11 +167,15 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - callback(keepass.verifyResponse(parsed, response.nonce) ? 'success' : 'error'); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback('error'); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + callback(keepass.verifyResponse(parsed, response.nonce) ? 'success' : 'error'); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -228,25 +232,29 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - - if (keepass.verifyResponse(parsed, response.nonce)) { - entries = parsed.entries; - keepass.updateLastUsed(keepass.databaseHash); - if (entries.length === 0) { - // questionmark-icon is not triggered, so we have to trigger for the normal symbol - browserAction.showDefault(null, tab); - } - callback(entries); - } - else { - console.log('RetrieveCredentials for ' + url + ' rejected'); - } - page.debug('keepass.retrieveCredentials() => entries.length = {1}', entries.length); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback([]); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + entries = parsed.entries; + keepass.updateLastUsed(keepass.databaseHash); + if (entries.length === 0) { + // questionmark-icon is not triggered, so we have to trigger for the normal symbol + browserAction.showDefault(null, tab); + } + callback(entries); + } + else { + console.log('RetrieveCredentials for ' + url + ' rejected'); + } + page.debug('keepass.retrieveCredentials() => entries.length = {1}', entries.length); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -292,25 +300,29 @@ keepass.generatePassword = function(callback, tab, forceCallback) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback([]); + return; + } - if (keepass.verifyResponse(parsed, response.nonce)) { - if (parsed.entries) { - passwords = parsed.entries; - keepass.updateLastUsed(keepass.databaseHash); - } - else { - console.log('No entries returned. Is KeePassXC up-to-date?'); - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + if (parsed.entries) { + passwords = parsed.entries; + keepass.updateLastUsed(keepass.databaseHash); } else { - console.log('GeneratePassword rejected'); + console.log('No entries returned. Is KeePassXC up-to-date?'); } - callback(passwords); } + else { + console.log('GeneratePassword rejected'); + } + callback(passwords); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -352,23 +364,26 @@ keepass.associate = function(callback, tab) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - const id = parsed.id; - - if (!keepass.verifyResponse(parsed, response.nonce)) { - keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); - } - else { - keepass.setCryptoKey(id, key); // Save the current public key as id key for the database - keepass.associated.value = true; - keepass.associated.hash = parsed.hash || 0; - } - - browserAction.show(callback, tab); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + return; } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + const id = parsed.id; + + if (!keepass.verifyResponse(parsed, response.nonce)) { + keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); + } + else { + keepass.setCryptoKey(id, key); // Save the current public key as id key for the database + keepass.associated.value = true; + keepass.associated.hash = parsed.hash || 0; + } + + browserAction.show(callback, tab); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -434,27 +449,31 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); - keepass.isEncryptionKeyUnrecognized = false; + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback(false); + return; + } - if (!keepass.verifyResponse(parsed, response.nonce)) { - const hash = response.hash || 0; - keepass.deleteKey(hash); - keepass.isEncryptionKeyUnrecognized = true; - keepass.handleError(tab, kpErrors.ENCRYPTION_KEY_UNRECOGNIZED); - keepass.associated.value = false; - keepass.associated.hash = null; - } - else if (!keepass.isAssociated()) { - keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); - } - else { - if (tab && page.tabs[tab.id]) { - delete page.tabs[tab.id].errorMessage; - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + keepass.isEncryptionKeyUnrecognized = false; + + if (!keepass.verifyResponse(parsed, response.nonce)) { + const hash = response.hash || 0; + keepass.deleteKey(hash); + keepass.isEncryptionKeyUnrecognized = true; + keepass.handleError(tab, kpErrors.ENCRYPTION_KEY_UNRECOGNIZED); + keepass.associated.value = false; + keepass.associated.hash = null; + } + else if (!keepass.isAssociated()) { + keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); + } + else { + if (tab && page.tabs[tab.id]) { + delete page.tabs[tab.id].errorMessage; } } } @@ -501,30 +520,34 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + callback('no-hash'); + return; + } + + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); - if (parsed.hash) { - const oldDatabaseHash = keepass.databaseHash; - keepass.setcurrentKeePassXCVersion(parsed.version); - keepass.databaseHash = parsed.hash || 'no-hash'; + if (parsed.hash) { + const oldDatabaseHash = keepass.databaseHash; + keepass.setcurrentKeePassXCVersion(parsed.version); + keepass.databaseHash = parsed.hash || 'no-hash'; - if (oldDatabaseHash && oldDatabaseHash != keepass.databaseHash) { - keepass.associated.value = false; - keepass.associated.hash = null; - } - - keepass.isDatabaseClosed = false; - keepass.isKeePassXCAvailable = true; - callback(parsed.hash); - } - else if (parsed.errorCode) { - keepass.databaseHash = 'no-hash'; - keepass.isDatabaseClosed = true; - keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); - callback(keepass.databaseHash); + if (oldDatabaseHash && oldDatabaseHash != keepass.databaseHash) { + keepass.associated.value = false; + keepass.associated.hash = null; } + + keepass.isDatabaseClosed = false; + keepass.isKeePassXCAvailable = true; + callback(parsed.hash); + } + else if (parsed.errorCode) { + keepass.databaseHash = 'no-hash'; + keepass.isDatabaseClosed = true; + keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); + callback(keepass.databaseHash); } } else { @@ -604,16 +627,22 @@ keepass.lockDatabase = function(tab) { keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); - if (res) { - const message = nacl.util.encodeUTF8(res); - const parsed = JSON.parse(message); - keepass.setcurrentKeePassXCVersion(parsed.version); + if (!res) { + keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); + resolve(false); + return; + } - if (keepass.verifyResponse(parsed, response.nonce)) { - keepass.isDatabaseClosed = true; - keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); - resolve(false); - } + const message = nacl.util.encodeUTF8(res); + const parsed = JSON.parse(message); + keepass.setcurrentKeePassXCVersion(parsed.version); + + if (keepass.verifyResponse(parsed, response.nonce)) { + keepass.isDatabaseClosed = true; + + // Display error message in the popup + keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); + resolve(true); } } else if (response.error && response.errorCode) { diff --git a/keepassxc-browser/background/page.js b/keepassxc-browser/background/page.js index 5b9c525..8a9d5c2 100644 --- a/keepassxc-browser/background/page.js +++ b/keepassxc-browser/background/page.js @@ -48,7 +48,7 @@ page.initOpenedTabs = function() { } // set initial tab-ID - browser.tabs.query({ "active": true, "currentWindow": true }).then((tabs) => { + browser.tabs.query({ 'active': true, 'currentWindow': true }).then((tabs) => { if (tabs.length === 0) { resolve(); return; // For example: only the background devtools or a popup are opened diff --git a/keepassxc-browser/manifest.json b/keepassxc-browser/manifest.json index 98a1b1a..ae67e9e 100644 --- a/keepassxc-browser/manifest.json +++ b/keepassxc-browser/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 2, "name": "keepassxc-browser", - "version": "0.4.1", + "version": "0.4.2", "description": "KeePassXC integration for modern web browsers", "author": "Sami Vänttinen", "icons": { diff --git a/keepassxc-browser/popups/popup_httpauth.js b/keepassxc-browser/popups/popup_httpauth.js index a76a279..7aca550 100644 --- a/keepassxc-browser/popups/popup_httpauth.js +++ b/keepassxc-browser/popups/popup_httpauth.js @@ -1,17 +1,25 @@ $(function() { browser.runtime.getBackgroundPage().then((global) => { - browser.tabs.query({"active": true, "currentWindow": true}).then((tab) => { + browser.tabs.query({'active': true, 'currentWindow': true}).then((tabs) => { + let tab = tabs[0]; const data = global.page.tabs[tab.id].loginList; let ul = document.getElementById('login-list'); for (let i = 0; i < data.logins.length; i++) { const li = document.createElement('li'); const a = document.createElement('a'); - a.textContent = data.logins[i].login + ' (' + data.logins[i].name + ')'; - li.setAttribute('class', 'list-group-item'); + a.textContent = data.logins[i].login + " (" + data.logins[i].name + ")"; li.appendChild(a); - $(a).data('url', data.url.replace(/:\/\//g, '://' + data.logins[i].login + ':' + data.logins[i].password + '@')); - $(a).click(() => { - browser.tabs.update(tab.id, {'url': $(this).data('url')}); + $(a).data('creds', data.logins[i]); + $(a).click(function () { + if (data.resolve) { + const creds = $(this).data('creds'); + data.resolve({ + authCredentials: { + username: creds.login, + password: creds.password + } + }); + } close(); }); ul.appendChild(li); From 84642ffc553a86234d72307cd18d801ab2386e7d Mon Sep 17 00:00:00 2001 From: varjolintu Date: Fri, 24 Nov 2017 14:57:36 +0200 Subject: [PATCH 2/4] Nonce incrementation part1 --- keepassxc-browser/background/keepass.js | 119 +++++++++++++++++------- 1 file changed, 84 insertions(+), 35 deletions(-) diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index 55b52ca..ddda312 100644 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -21,6 +21,7 @@ keepass.databaseHash = 'no-hash'; //no-hash = KeePassXC is too old and does not keepass.keyId = 'keepassxc-browser-cryptokey-name'; keepass.keyBody = 'keepassxc-browser-key'; keepass.messageTimeout = 500; // milliseconds +keepass.nonce = nacl.util.encodeBase64(nacl.randomBytes(keepass.keySize)); const kpActions = { SET_LOGIN: 'set-login', @@ -142,7 +143,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, const kpAction = kpActions.SET_LOGIN; const {dbid} = keepass.getCryptoKey(); - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); let messageData = { action: kpAction, @@ -160,10 +161,10 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, const request = { action: kpAction, message: keepass.encrypt(messageData, nonce), - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -175,7 +176,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, const message = nacl.util.encodeUTF8(res); const parsed = JSON.parse(message); - callback(keepass.verifyResponse(parsed, response.nonce) ? 'success' : 'error'); + callback(keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce)) ? 'success' : 'error'); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -209,7 +210,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb let entries = []; const kpAction = kpActions.GET_LOGINS; - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const {dbid} = keepass.getCryptoKey(); let messageData = { @@ -225,10 +226,10 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb const request = { action: kpAction, message: keepass.encrypt(messageData, nonce), - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -242,7 +243,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, response.nonce)) { + if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { entries = parsed.entries; keepass.updateLastUsed(keepass.databaseHash); if (entries.length === 0) { @@ -289,14 +290,14 @@ keepass.generatePassword = function(callback, tab, forceCallback) { let passwords = []; const kpAction = kpActions.GENERATE_PASSWORD; - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const request = { action: kpAction, - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -310,7 +311,7 @@ keepass.generatePassword = function(callback, tab, forceCallback) { const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, response.nonce)) { + if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { if (parsed.entries) { passwords = parsed.entries; keepass.updateLastUsed(keepass.databaseHash); @@ -347,7 +348,7 @@ keepass.associate = function(callback, tab) { const kpAction = kpActions.ASSOCIATE; const key = nacl.util.encodeBase64(keepass.keyPair.publicKey); - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const messageData = { action: kpAction, @@ -357,10 +358,10 @@ keepass.associate = function(callback, tab) { const request = { action: kpAction, message: keepass.encrypt(messageData, nonce), - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -374,7 +375,7 @@ keepass.associate = function(callback, tab) { keepass.setcurrentKeePassXCVersion(parsed.version); const id = parsed.id; - if (!keepass.verifyResponse(parsed, response.nonce)) { + if (!keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); } else { @@ -422,7 +423,7 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { } const kpAction = kpActions.TEST_ASSOCIATE; - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const {dbid, dbkey} = keepass.getCryptoKey(); if (dbkey === null || dbid === null) { @@ -442,10 +443,10 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { const request = { action: kpAction, message: keepass.encrypt(messageData, nonce), - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -460,7 +461,7 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { keepass.setcurrentKeePassXCVersion(parsed.version); keepass.isEncryptionKeyUnrecognized = false; - if (!keepass.verifyResponse(parsed, response.nonce)) { + if (!keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { const hash = response.hash || 0; keepass.deleteKey(hash); keepass.isEncryptionKeyUnrecognized = true; @@ -497,7 +498,7 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { } const kpAction = kpActions.GET_DATABASE_HASH; - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const messageData = { action: kpAction @@ -513,10 +514,10 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { const request = { action: kpAction, message: encrypted, - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -525,7 +526,7 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { callback('no-hash'); return; } - + const message = nacl.util.encodeUTF8(res); const parsed = JSON.parse(message); @@ -574,8 +575,7 @@ keepass.changePublicKeys = function(tab, enableTimeout = false) { const kpAction = kpActions.CHANGE_PUBLIC_KEYS; const key = nacl.util.encodeBase64(keepass.keyPair.publicKey); - let nonce = nacl.randomBytes(keepass.keySize); - nonce = nacl.util.encodeBase64(nonce); + const nonce = keepass.getNonce(); keepass.clientID = nacl.util.encodeBase64(nacl.randomBytes(keepass.keySize)); const request = { @@ -584,11 +584,11 @@ keepass.changePublicKeys = function(tab, enableTimeout = false) { nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request, enableTimeout).then((response) => { keepass.setcurrentKeePassXCVersion(response.version); - if (!keepass.verifyKeyResponse(response, key, nonce)) { + if (!keepass.verifyKeyResponse(response, key, keepass.incrementedNonce(nonce))) { if (tab && page.tabs[tab.id]) { keepass.handleError(tab, kpErrors.KEY_CHANGE_FAILED); reject(false); @@ -611,7 +611,7 @@ keepass.lockDatabase = function(tab) { } const kpAction = kpActions.LOCK_DATABASE; - const nonce = nacl.randomBytes(keepass.keySize); + const nonce = keepass.getNonce(); const messageData = { action: kpAction @@ -620,10 +620,10 @@ keepass.lockDatabase = function(tab) { const request = { action: kpAction, message: keepass.encrypt(messageData, nonce), - nonce: nacl.util.encodeBase64(nonce), + nonce: nonce, clientID: keepass.clientID }; - + console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -631,13 +631,13 @@ keepass.lockDatabase = function(tab) { keepass.handleError(tab, kpErrors.CANNOT_DECRYPT_MESSAGE); resolve(false); return; - } + } const message = nacl.util.encodeUTF8(res); const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, response.nonce)) { + if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { keepass.isDatabaseClosed = true; // Display error message in the popup @@ -824,6 +824,51 @@ function onDisconnected() { console.log('Failed to connect: ' + (browser.runtime.lastError === null ? 'Unknown error' : browser.runtime.lastError.message)); } +keepass.getNonce = function() { + return nacl.util.encodeBase64(nacl.randomBytes(keepass.keySize)); + + // New implementation + const oldNonce = nacl.util.decodeBase64(keepass.nonce); + + let newNonce = []; + for (let i = 0; i < 24; i++){ + newNonce[i] = oldNonce[i]; + } + + for (let i = 0; i < 24; i++) { + newNonce[i]++; + if (newNonce[i]) { + break; + } + } + + console.log("New: " + newNonce); + console.log("Old: " + oldNonce); + keepass.nonce = nacl.util.encodeBase64(newNonce); + return nacl.util.encodeBase64(oldNonce); +}; + +keepass.incrementedNonce = function(nonce) { + const oldNonce = nacl.util.decodeBase64(nonce); + + // TODO: fix the incrementation, it's not complete yet + let newNonce = []; + for (let i = 0; i < 24; i++){ + newNonce[i] = oldNonce[i]; + } + + for (let i = 0; i < 24; i++) { + newNonce[i]++; + if (newNonce[i]) { + break; + } + } + + console.log("New: " + newNonce); + console.log("Old: " + oldNonce); + return nacl.util.encodeBase64(newNonce); +}; + keepass.nativeConnect = function() { console.log('Connecting to native messaging host ' + keepass.nativeHostName); keepass.nativePort = browser.runtime.connectNative(keepass.nativeHostName); @@ -867,6 +912,9 @@ keepass.verifyResponse = function(response, nonce, id) { } keepass.associated.value = (response.nonce === nonce); + if (keepass.associated.value === false) { + console.log("Compare failed"); + } if (id) { keepass.associated.value = (keepass.associated.value && id === response.id); @@ -909,9 +957,10 @@ keepass.setCryptoKey = function(id, key) { keepass.encrypt = function(input, nonce) { const messageData = nacl.util.decodeUTF8(JSON.stringify(input)); + const messageNonce = nacl.util.decodeBase64(nonce); if (keepass.serverPublicKey) { - const message = nacl.box(messageData, nonce, keepass.serverPublicKey, keepass.keyPair.secretKey); + const message = nacl.box(messageData, messageNonce, keepass.serverPublicKey, keepass.keyPair.secretKey); if (message) { return nacl.util.encodeBase64(message); } @@ -919,7 +968,7 @@ keepass.encrypt = function(input, nonce) { return ''; }; -keepass.decrypt = function(input, nonce, toStr) { +keepass.decrypt = function(input, nonce) { const m = nacl.util.decodeBase64(input); const n = nacl.util.decodeBase64(nonce); const res = nacl.box.open(m, n, keepass.serverPublicKey, keepass.keyPair.secretKey); From c491356dc9094b049dcd0ac0ce82770145027f35 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Sun, 26 Nov 2017 12:31:05 +0200 Subject: [PATCH 3/4] Nonce incrementation --- CHANGELOG | 2 +- keepassxc-browser/background/keepass.js | 92 ++++++++++--------------- keepassxc-protocol.md | 2 +- 3 files changed, 39 insertions(+), 57 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 979cd36..afe4ec6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,7 +3,7 @@ - Fixed HTTP authentication with multiple credentials (credits to smorks) - Fixed error handling when decrypt fails - Fixed database-locked response handling -- TODO: Fixed nonce increment when encrypting messages +- Fixed nonce increment when encrypting messages 0.4.1 (18-11-2017) ========================= diff --git a/keepassxc-browser/background/keepass.js b/keepassxc-browser/background/keepass.js index ddda312..ae435ac 100644 --- a/keepassxc-browser/background/keepass.js +++ b/keepassxc-browser/background/keepass.js @@ -134,8 +134,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, page.tabs[tab.id].errorMessage = null; keepass.testAssociation((response) => { - if (!response) - { + if (!response) { browserAction.showDefault(null, tab); callback([]); return; @@ -144,6 +143,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, const kpAction = kpActions.SET_LOGIN; const {dbid} = keepass.getCryptoKey(); const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); let messageData = { action: kpAction, @@ -164,7 +164,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -176,7 +176,7 @@ keepass.updateCredentials = function(callback, tab, entryId, username, password, const message = nacl.util.encodeUTF8(res); const parsed = JSON.parse(message); - callback(keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce)) ? 'success' : 'error'); + callback(keepass.verifyResponse(parsed, incrementedNonce) ? 'success' : 'error'); } else if (response.error && response.errorCode) { keepass.handleError(tab, response.errorCode, response.error); @@ -192,8 +192,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb page.debug('keepass.retrieveCredentials(callback, {1}, {2}, {3}, {4})', tab.id, url, submiturl, forceCallback); keepass.testAssociation((response) => { - if (!response) - { + if (!response) { browserAction.showDefault(null, tab); if (forceCallback) { callback([]); @@ -211,6 +210,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb let entries = []; const kpAction = kpActions.GET_LOGINS; const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const {dbid} = keepass.getCryptoKey(); let messageData = { @@ -229,7 +229,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -243,7 +243,7 @@ keepass.retrieveCredentials = function(callback, tab, url, submiturl, forceCallb const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { + if (keepass.verifyResponse(parsed, incrementedNonce)) { entries = parsed.entries; keepass.updateLastUsed(keepass.databaseHash); if (entries.length === 0) { @@ -274,8 +274,7 @@ keepass.generatePassword = function(callback, tab, forceCallback) { } keepass.testAssociation((taresponse) => { - if (!taresponse) - { + if (!taresponse) { browserAction.showDefault(null, tab); if (forceCallback) { callback([]); @@ -291,13 +290,14 @@ keepass.generatePassword = function(callback, tab, forceCallback) { let passwords = []; const kpAction = kpActions.GENERATE_PASSWORD; const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const request = { action: kpAction, nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -311,7 +311,7 @@ keepass.generatePassword = function(callback, tab, forceCallback) { const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { + if (keepass.verifyResponse(parsed, incrementedNonce)) { if (parsed.entries) { passwords = parsed.entries; keepass.updateLastUsed(keepass.databaseHash); @@ -349,6 +349,7 @@ keepass.associate = function(callback, tab) { const kpAction = kpActions.ASSOCIATE; const key = nacl.util.encodeBase64(keepass.keyPair.publicKey); const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const messageData = { action: kpAction, @@ -361,7 +362,7 @@ keepass.associate = function(callback, tab) { nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -375,7 +376,7 @@ keepass.associate = function(callback, tab) { keepass.setcurrentKeePassXCVersion(parsed.version); const id = parsed.id; - if (!keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { + if (!keepass.verifyResponse(parsed, incrementedNonce)) { keepass.handleError(tab, kpErrors.ASSOCIATION_FAILED); } else { @@ -424,6 +425,7 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { const kpAction = kpActions.TEST_ASSOCIATE; const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const {dbid, dbkey} = keepass.getCryptoKey(); if (dbkey === null || dbid === null) { @@ -446,7 +448,7 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -461,7 +463,7 @@ keepass.testAssociation = function(callback, tab, enableTimeout = false) { keepass.setcurrentKeePassXCVersion(parsed.version); keepass.isEncryptionKeyUnrecognized = false; - if (!keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { + if (!keepass.verifyResponse(parsed, incrementedNonce)) { const hash = response.hash || 0; keepass.deleteKey(hash); keepass.isEncryptionKeyUnrecognized = true; @@ -499,6 +501,7 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { const kpAction = kpActions.GET_DATABASE_HASH; const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const messageData = { action: kpAction @@ -517,7 +520,7 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request, enableTimeout).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -529,7 +532,6 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { const message = nacl.util.encodeUTF8(res); const parsed = JSON.parse(message); - if (parsed.hash) { const oldDatabaseHash = keepass.databaseHash; keepass.setcurrentKeePassXCVersion(parsed.version); @@ -543,12 +545,14 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { keepass.isDatabaseClosed = false; keepass.isKeePassXCAvailable = true; callback(parsed.hash); + return; } else if (parsed.errorCode) { keepass.databaseHash = 'no-hash'; keepass.isDatabaseClosed = true; keepass.handleError(tab, kpErrors.DATABASE_NOT_OPENED); callback(keepass.databaseHash); + return; } } else { @@ -562,6 +566,7 @@ keepass.getDatabaseHash = function(callback, tab, enableTimeout = false) { keepass.handleError(tab, response.errorCode, response.error); } callback(keepass.databaseHash); + return; } }); }; @@ -576,6 +581,7 @@ keepass.changePublicKeys = function(tab, enableTimeout = false) { const kpAction = kpActions.CHANGE_PUBLIC_KEYS; const key = nacl.util.encodeBase64(keepass.keyPair.publicKey); const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); keepass.clientID = nacl.util.encodeBase64(nacl.randomBytes(keepass.keySize)); const request = { @@ -584,11 +590,11 @@ keepass.changePublicKeys = function(tab, enableTimeout = false) { nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request, enableTimeout).then((response) => { keepass.setcurrentKeePassXCVersion(response.version); - if (!keepass.verifyKeyResponse(response, key, keepass.incrementedNonce(nonce))) { + if (!keepass.verifyKeyResponse(response, key, incrementedNonce)) { if (tab && page.tabs[tab.id]) { keepass.handleError(tab, kpErrors.KEY_CHANGE_FAILED); reject(false); @@ -612,6 +618,7 @@ keepass.lockDatabase = function(tab) { const kpAction = kpActions.LOCK_DATABASE; const nonce = keepass.getNonce(); + const incrementedNonce = keepass.incrementedNonce(nonce); const messageData = { action: kpAction @@ -623,7 +630,7 @@ keepass.lockDatabase = function(tab) { nonce: nonce, clientID: keepass.clientID }; - console.log(kpAction + " " + nacl.util.decodeBase64(nonce)); + keepass.sendNativeMessage(request).then((response) => { if (response.message && response.nonce) { const res = keepass.decrypt(response.message, response.nonce); @@ -637,7 +644,7 @@ keepass.lockDatabase = function(tab) { const parsed = JSON.parse(message); keepass.setcurrentKeePassXCVersion(parsed.version); - if (keepass.verifyResponse(parsed, keepass.incrementedNonce(nonce))) { + if (keepass.verifyResponse(parsed, incrementedNonce)) { keepass.isDatabaseClosed = true; // Display error message in the popup @@ -826,46 +833,21 @@ function onDisconnected() { keepass.getNonce = function() { return nacl.util.encodeBase64(nacl.randomBytes(keepass.keySize)); - - // New implementation - const oldNonce = nacl.util.decodeBase64(keepass.nonce); - - let newNonce = []; - for (let i = 0; i < 24; i++){ - newNonce[i] = oldNonce[i]; - } - - for (let i = 0; i < 24; i++) { - newNonce[i]++; - if (newNonce[i]) { - break; - } - } - - console.log("New: " + newNonce); - console.log("Old: " + oldNonce); - keepass.nonce = nacl.util.encodeBase64(newNonce); - return nacl.util.encodeBase64(oldNonce); }; keepass.incrementedNonce = function(nonce) { const oldNonce = nacl.util.decodeBase64(nonce); + let newNonce = oldNonce.slice(0); - // TODO: fix the incrementation, it's not complete yet - let newNonce = []; - for (let i = 0; i < 24; i++){ - newNonce[i] = oldNonce[i]; + // from libsodium/utils.c + let i = 0; + let c = 1; + for (; i < newNonce.length; ++i) { + c += newNonce[i]; + newNonce[i] = c; + c >>= 8; } - for (let i = 0; i < 24; i++) { - newNonce[i]++; - if (newNonce[i]) { - break; - } - } - - console.log("New: " + newNonce); - console.log("Old: " + oldNonce); return nacl.util.encodeBase64(newNonce); }; diff --git a/keepassxc-protocol.md b/keepassxc-protocol.md index 703d627..8ac2107 100644 --- a/keepassxc-protocol.md +++ b/keepassxc-protocol.md @@ -7,7 +7,7 @@ Now the requests are encrypted by [TweetNaCl.js](https://github.com/dchest/tweet 2. When KeePassXC receives the public key it generates its own key pair and transfers the public key to keepassxc-browser 3. All messages between the browser extension and KeePassXC are now encrypted. 4. When keepassxc-browser sends a message it is encrypted with KeePassXC's public key, a random generated nonce and keepassxc-browser's secret key. -5. When KeePassXC sends a message it is encrypted with keepassxc-browser's public key etc. +5. When KeePassXC sends a message it is encrypted with keepassxc-browser's public key and an incremented nonce. 6. Databases are stored based on the current public key used with `associate`. A new key pair for data transfer is generated each time keepassxc-browser is launched. Encrypted messages are built with these JSON parameters: From ffd6f088223119d5c3afde2ed84e1b8d839d1fd6 Mon Sep 17 00:00:00 2001 From: varjolintu Date: Mon, 27 Nov 2017 18:15:58 +0200 Subject: [PATCH 4/4] Changelog --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index afe4ec6..76ce284 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,4 +1,4 @@ -0.4.2 (??-??-2017) +0.4.2 (27-11-2017) ========================= - Fixed HTTP authentication with multiple credentials (credits to smorks) - Fixed error handling when decrypt fails