Skip to content

Commit

Permalink
Issue #86 - Implemented sync protocol modifications to prevent data t…
Browse files Browse the repository at this point in the history 
…ampering by the sync provider
  • Loading branch information
@palant
palant committed Jun 5, 2019
1 parent 3bc3dbb commit 45c090e
Show file tree
Hide file tree
Showing 6 changed files with 425 additions and 95 deletions.
10 changes: 3 additions & 7 deletions lib/passwords.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,18 +289,14 @@ function getAllSites()
}
exports.getAllSites = getAllSites;

function exportPasswordData()
function exportPasswordData(extraKeys = [])
{
let exportedKeys = [masterPassword.saltKey, masterPassword.hmacSecretKey].concat(extraKeys);
return storage.getAllByPrefix("", null).then(data =>
{
for (let key of Object.keys(data))
{
if (!key.startsWith(STORAGE_PREFIX) && key != masterPassword.saltKey &&
key != masterPassword.hmacSecretKey)
{
if (!key.startsWith(STORAGE_PREFIX) && !exportedKeys.includes(key))
delete data[key];
}
}

return JSON.stringify({
application: "pfp",
Expand Down
155 changes: 124 additions & 31 deletions lib/sync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

"use strict";

let storage = require("./storage");
let passwords = require("./passwords");
let crypto = require("./crypto");
let masterPassword = require("./masterPassword");
let passwords = require("./passwords");
let storage = require("./storage");

let {lock, STORAGE_PREFIX: passwordsPrefix} = passwords;

const MAX_RETRIES = 5;
Expand Down Expand Up @@ -94,6 +96,7 @@ tracker.onModified = tracker.onModified.bind(tracker);

let engine = {
storageKey: "syncData",
secretKey: "sync-secret",
checkInterval: 10 * MILLIS_IN_MINUTE,
syncInterval: MILLIS_IN_HOUR,
data: null,
Expand Down Expand Up @@ -161,6 +164,28 @@ let engine = {
});
},

_calculateSignature(data, revision, rawSecret)
{
let values = [revision];
let keys = Object.keys(data);
keys.sort();
for (let key of keys)
values.push([key, data[key]]);
return crypto.importHmacSecret(rawSecret).then(secret =>
{
return crypto.getDigest(secret, JSON.stringify(values));
});
},

_validateSignature(data, revision, rawSecret, expectedSignature)
{
return this._calculateSignature(data, revision, rawSecret).then(signature =>
{
if (signature != expectedSignature)
throw "sync_tampered_data";
});
},

sync(nestingLevel = 0)
{
if (!nestingLevel && this.currentSync)
Expand All @@ -180,21 +205,14 @@ let engine = {
{
return Promise.all([
getProvider(provider).get(path, token, username),
passwords.exportPasswordData(),
tracker.getModifiedKeys()
passwords.exportPasswordData([this.secretKey])
]);
}).then(([remoteData, localData, modifiedKeys]) =>
}).then(([remoteData, localData]) =>
{
let local = JSON.parse(localData);
if (!remoteData)
{
return Promise.all([
[],
[],
getProvider(provider).put(path, localData, null, token, username)
]);
}
return Promise.resolve([null, null, local, null]);

let local = JSON.parse(localData);
let remote;
try
{
Expand Down Expand Up @@ -226,39 +244,113 @@ let engine = {
};
}

let modified = false;
let additions = [];
for (let key of Object.keys(remote.data))
if (local.data[this.secretKey] && local.data[this.secretKey] != remote.data[this.secretKey])
throw "sync_unrelated_client";

let validation = Promise.resolve();
if (remote.data[this.secretKey])
{
if (!modifiedKeys.has(key))
if (typeof remote.revision != "number" || remote.revision <= 0 || typeof remote.signature != "string")
throw "sync_unknown_data_format";

if (this.data.revision && remote.revision < this.data.revision)
throw "sync_tampered_data";

if (this.data.secret)
validation = validation.then(() => this.data.secret);
else
validation = validation.then(() => masterPassword.decrypt(remote.data[this.secretKey]));

validation = validation.then(secret =>
{
local.data[key] = remote.data[key];
additions.push([key, local.data[key]]);
}
else if (local.data[key] != remote.data[key])
modified = true;
return Promise.all([
secret,
this._validateSignature(remote.data, remote.revision, secret, remote.signature)
]);
}).then(([secret]) =>
{
this.data.secret = secret;
this.data.revision = remote.revision;
triggerModificationListeners();
return storage.set(this.storageKey, this.data, null);
});
}

return validation.then(() =>
{
return Promise.all([
remote,
remoteData.revision,
local,
tracker.getModifiedKeys()
]);
});
}).then(([remote, fileRevision, local, modifiedKeys]) =>
{
let modified = !remote;
let additions = [];
let removals = [];
for (let key of Object.keys(local.data))
if (remote)
{
if (!remote.data.hasOwnProperty(key))
for (let key of Object.keys(remote.data))
{
if (!modifiedKeys.has(key))
{
delete local.data[key];
removals.push(key);
local.data[key] = remote.data[key];
additions.push([key, local.data[key]]);
}
else
else if (local.data[key] != remote.data[key])
modified = true;
}

for (let key of Object.keys(local.data))
{
if (!remote.data.hasOwnProperty(key))
{
if (!modifiedKeys.has(key))
{
delete local.data[key];
removals.push(key);
}
else
modified = true;
}
}
}

let updateAction;
let updateAction = Promise.resolve();
if (modified)
updateAction = getProvider(provider).put(path, JSON.stringify(local), remoteData.revision, token, username);
else
updateAction = Promise.resolve();
{
local.revision = (this.data.revision || 0) + 1;
if (!this.data.secret)
{
updateAction = updateAction.then(() =>
{
this.data.secret = crypto.generateRandom(32);
return storage.set(this.secretKey, this.data.secret);
}).then(() =>
{
return storage.get(this.secretKey, null);
}).then(encrypted =>
{
local.data[this.secretKey] = encrypted;
});
}

updateAction = updateAction.then(() =>
{
return this._calculateSignature(local.data, local.revision, this.data.secret);
}).then(signature =>
{
local.signature = signature;
return getProvider(provider).put(path, JSON.stringify(local), fileRevision, token, username);
}).then(() =>
{
this.data.revision = local.revision;
triggerModificationListeners();
storage.set(this.storageKey, this.data, null);
});
}

return Promise.all([
additions,
Expand Down Expand Up @@ -358,6 +450,7 @@ let engine = {
triggerModificationListeners();
return Promise.all([
storage.delete(this.storageKey),
storage.delete(this.secretKey),
tracker.disable()
]);
}).finally(() => noLock || lock.release());
Expand Down
2 changes: 1 addition & 1 deletion locale/en-US.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ allpasswords_import_success = Passwords data has been imported.
allpasswords_show_confirm = This will display all your passwords on screen, please only proceed if nobody can watch over your shoulder. This action might take some time to complete.

# web
web_compat_message = Your browser lacks the required functionality for this application. At least Mozilla Firefox 40, Google Chrome 51, Opera 38, Apple Safari 11 or Microsoft Edge 12 is required.
web_compat_message = Your browser lacks the required functionality for this application. At least Mozilla Firefox 43, Google Chrome 51, Opera 38, Apple Safari 11 or Microsoft Edge 12 is required.

# options
autolock_title = Enable auto-lock
Expand Down
23 changes: 17 additions & 6 deletions test-lib/fake-crypto.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,11 @@ function getEncryptionPrefix(algoName, key, iv)
]);
}

const AES_KEY_LENGTH = 32;
const AES_IV_LENGTH = 12;

exports.subtle = {
importKey: function(format, keyData, algo, extractable, usages)
importKey(format, keyData, algo, extractable, usages)
{
return Promise.resolve().then(() =>
{
Expand All @@ -90,7 +93,7 @@ exports.subtle = {
});
},

encrypt: function(algo, key, cleartext)
encrypt(algo, key, cleartext)
{
return Promise.resolve().then(() =>
{
Expand All @@ -103,7 +106,7 @@ exports.subtle = {
return Buffer.concat([getEncryptionPrefix(algo.name, key, algo.iv), Buffer.from(cleartext)]);
else
{
if (algo.name != "AES-GCM" || key._data.length != 32)
if (algo.name != "AES-GCM" || key._data.length != AES_KEY_LENGTH)
throw new Error("Only AES256-GCM is supported");

let cipher = crypto.createCipheriv("aes-256-gcm", key._data, algo.iv);
Expand All @@ -114,7 +117,7 @@ exports.subtle = {
});
},

decrypt: function(algo, key, ciphertext)
decrypt(algo, key, ciphertext)
{
return Promise.resolve().then(() =>
{
Expand All @@ -137,7 +140,7 @@ exports.subtle = {
}
else
{
if (algo.name != "AES-GCM" || key._data.length != 32)
if (algo.name != "AES-GCM" || key._data.length != AES_KEY_LENGTH)
throw new Error("Only AES256-GCM is supported");

let decipher = crypto.createDecipheriv("aes-256-gcm", key._data, algo.iv);
Expand All @@ -148,7 +151,15 @@ exports.subtle = {
});
},

sign: function(algo, key, cleartext)
_fakeDecrypt(text)
{
if (!text.startsWith("AES-GCM!"))
throw new Error("Unexpected encryption algorithm");

return text.substr("AES-GCM!".length + AES_KEY_LENGTH + "!".length + AES_IV_LENGTH + "!".length);
},

sign(algo, key, cleartext)
{
return Promise.resolve().then(() =>
{
Expand Down
Loading

0 comments on commit 45c090e

Please sign in to comment.