diff --git a/services/sync/modules/base_records/crypto.js b/services/sync/modules/base_records/crypto.js index 18b5efcc256d..34a8a32041bc 100644 --- a/services/sync/modules/base_records/crypto.js +++ b/services/sync/modules/base_records/crypto.js @@ -65,6 +65,7 @@ CryptoWrapper.prototype = { this.IV = Svc.Crypto.generateRandomIV(); this.ciphertext = Svc.Crypto.encrypt(JSON.stringify(this.cleartext), symkey, this.IV); + this.hmac = Utils.sha256HMAC(this.ciphertext, symkey.hmacKey); this.cleartext = null; }, @@ -75,6 +76,10 @@ CryptoWrapper.prototype = { let meta = CryptoMetas.get(this.encryption); let symkey = meta.getKey(privkey, passphrase); + // Authenticate the encrypted blob with the expected HMAC + if (Utils.sha256HMAC(this.ciphertext, symkey.hmacKey) != this.hmac) + throw "Server attack?! SHA256 HMAC mismatch: " + this.hmac; + this.cleartext = JSON.parse(Svc.Crypto.decrypt(this.ciphertext, symkey, this.IV)); this.ciphertext = null; @@ -103,7 +108,8 @@ CryptoWrapper.prototype = { }, }; -Utils.deferGetSet(CryptoWrapper, "payload", ["ciphertext", "encryption", "IV"]); +Utils.deferGetSet(CryptoWrapper, "payload", ["ciphertext", "encryption", "IV", + "hmac"]); Utils.deferGetSet(CryptoWrapper, "cleartext", "deleted"); function CryptoMeta(uri) { @@ -115,10 +121,6 @@ CryptoMeta.prototype = { _logName: "Record.CryptoMeta", getKey: function CryptoMeta_getKey(privkey, passphrase) { - // We cache the unwrapped key, as it's expensive to generate - if (this._unwrappedKey) - return this._unwrappedKey; - // get the uri to our public key let pubkeyUri = privkey.publicKeyUri.spec; @@ -131,8 +133,14 @@ CryptoMeta.prototype = { if (!wrapped_key) throw "keyring doesn't contain a key for " + pubkeyUri; - return this._unwrappedKey = Svc.Crypto.unwrapSymmetricKey(wrapped_key, - privkey.keyData, passphrase.password, privkey.salt, privkey.iv); + let unwrappedKey = new String(Svc.Crypto.unwrapSymmetricKey(wrapped_key, + privkey.keyData, passphrase.password, privkey.salt, privkey.iv)); + + unwrappedKey.hmacKey = Svc.KeyFactory.keyFromString(Ci.nsIKeyObject.HMAC, + unwrappedKey); + + // Cache the result after the first get and just return it + return (this.getKey = function() unwrappedKey)(); }, addKey: function CryptoMeta_addKey(new_pubkey, privkey, passphrase) { diff --git a/services/sync/modules/util.js b/services/sync/modules/util.js index a6fe44019e33..4a8eabdd70c9 100644 --- a/services/sync/modules/util.js +++ b/services/sync/modules/util.js @@ -475,26 +475,34 @@ let Utils = { throw 'checkStatus failed'; }, - sha1: function Weave_sha1(string) { + digest: function digest(message, hasher) { let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]. createInstance(Ci.nsIScriptableUnicodeConverter); converter.charset = "UTF-8"; - let hasher = Cc["@mozilla.org/security/hash;1"] - .createInstance(Ci.nsICryptoHash); - hasher.init(hasher.SHA1); - - let data = converter.convertToByteArray(string, {}); + let data = converter.convertToByteArray(message, {}); hasher.update(data, data.length); - let rawHash = hasher.finish(false); - // return the two-digit hexadecimal code for a byte - function toHexString(charCode) { - return ("0" + charCode.toString(16)).slice(-2); - } + // Convert each hashed byte into 2-hex strings then combine them + return [("0" + byte.charCodeAt().toString(16)).slice(-2) for each (byte in + hasher.finish(false))].join(""); + }, - let hash = [toHexString(rawHash.charCodeAt(i)) for (i in rawHash)].join(""); - return hash; + sha1: function sha1(message) { + let hasher = Cc["@mozilla.org/security/hash;1"]. + createInstance(Ci.nsICryptoHash); + hasher.init(hasher.SHA1); + return Utils.digest(message, hasher); + }, + + /** + * Generate a sha256 HMAC for a string message and a given nsIKeyObject + */ + sha256HMAC: function sha256HMAC(message, key) { + let hasher = Cc["@mozilla.org/security/hmac;1"]. + createInstance(Ci.nsICryptoHMAC); + hasher.init(hasher.SHA256, key); + return Utils.digest(message, hasher); }, makeURI: function Weave_makeURI(URIString) { @@ -858,6 +866,7 @@ Svc.Obs = Observers; ["History", "@mozilla.org/browser/nav-history-service;1", "nsPIPlacesDatabase"], ["Idle", "@mozilla.org/widget/idleservice;1", "nsIIdleService"], ["IO", "@mozilla.org/network/io-service;1", "nsIIOService"], + ["KeyFactory", "@mozilla.org/security/keyobjectfactory;1", "nsIKeyObjectFactory"], ["Login", "@mozilla.org/login-manager;1", "nsILoginManager"], ["Memory", "@mozilla.org/xpcom/memory-service;1", "nsIMemory"], ["Observer", "@mozilla.org/observer-service;1", "nsIObserverService"], diff --git a/services/sync/tests/unit/test_records_crypto.js b/services/sync/tests/unit/test_records_crypto.js index f24c0914cebc..335f10fc1886 100644 --- a/services/sync/tests/unit/test_records_crypto.js +++ b/services/sync/tests/unit/test_records_crypto.js @@ -110,6 +110,18 @@ function run_test() { } do_check_eq(error, "Server attack?! Id mismatch: crypted-resource,other"); + log.info("Make sure wrong hmacs cause failures"); + cryptoWrap.encrypt(passphrase); + cryptoWrap.hmac = "foo"; + error = ""; + try { + cryptoWrap.decrypt(passphrase); + } + catch(ex) { + error = ex; + } + do_check_eq(error, "Server attack?! SHA256 HMAC mismatch: foo"); + log.info("Done!"); } finally { server.stop(); }