From fdc3171cd176d5360c5f18c8268abad92e999948 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Fri, 1 Oct 2010 13:01:52 +0200 Subject: [PATCH 1/3] Bug 586867 - Use resource:///modules/services-sync/ instead of resource://gre/modules/services-sync, as it is an application (not GRE) module. [r=philikon] --- services/sync/Weave.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/sync/Weave.js b/services/sync/Weave.js index 6f21edb69004..a4645d7bf451 100644 --- a/services/sync/Weave.js +++ b/services/sync/Weave.js @@ -86,7 +86,7 @@ WeaveService.prototype = { if (resProt.hasSubstitution("services-sync")) return; - let uri = ioService.newURI("resource://gre/modules/services-sync/", + let uri = ioService.newURI("resource:///modules/services-sync/", null, null); resProt.setSubstitution("services-sync", uri); } From dc0e75b6ad98e8c654ada4c8f04b68b0b2c88496 Mon Sep 17 00:00:00 2001 From: Philipp von Weitershausen Date: Mon, 4 Oct 2010 22:39:08 +0200 Subject: [PATCH 2/3] Bug 600995 - Use a record's "encryption" property only as a fallback, default to the engine's value. [r=mconnor] --- services/sync/modules/base_records/crypto.js | 4 ++-- services/sync/modules/engines.js | 10 ++++++++-- services/sync/modules/type_records/bookmark.js | 2 +- services/sync/tests/unit/test_clients_escape.js | 2 +- services/sync/tests/unit/test_records_crypto.js | 13 +++++++------ services/sync/tests/unit/test_syncengine_sync.js | 16 ++++++++++++++-- 6 files changed, 33 insertions(+), 14 deletions(-) diff --git a/services/sync/modules/base_records/crypto.js b/services/sync/modules/base_records/crypto.js index f081cc778aa4..3217b728d27b 100644 --- a/services/sync/modules/base_records/crypto.js +++ b/services/sync/modules/base_records/crypto.js @@ -76,11 +76,11 @@ CryptoWrapper.prototype = { this.cleartext = null; }, - decrypt: function CryptoWrapper_decrypt(passphrase) { + decrypt: function CryptoWrapper_decrypt(passphrase, keyUri) { let pubkey = PubKeys.getDefaultKey(); let privkey = PrivKeys.get(pubkey.privateKeyUri); - let meta = CryptoMetas.get(this.encryption); + let meta = CryptoMetas.get(keyUri); let symkey = meta.getKey(privkey, passphrase); // Authenticate the encrypted blob with the expected HMAC diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 5e4bcef8ddbc..0ed525d46e40 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -468,7 +468,13 @@ SyncEngine.prototype = { handled.push(item.id); try { - item.decrypt(ID.get("WeaveCryptoID")); + // Short-circuit the key URI to the engine's one in case the WBO's + // might be wrong due to relative URI confusions (bug 600995). + try { + item.decrypt(ID.get("WeaveCryptoID"), this.cryptoMetaURL); + } catch (ex) { + item.decrypt(ID.get("WeaveCryptoID"), item.encryption); + } if (this._reconcile(item)) { count.applied++; this._tracker.ignoreAll = true; @@ -763,7 +769,7 @@ SyncEngine.prototype = { test.sort = "newest"; test.full = true; test.recordHandler = function(record) { - record.decrypt(ID.get("WeaveCryptoID")); + record.decrypt(ID.get("WeaveCryptoID"), this.cryptoMetaURL); canDecrypt = true; }; diff --git a/services/sync/modules/type_records/bookmark.js b/services/sync/modules/type_records/bookmark.js index 37d8290ddfce..6fd270b164b1 100644 --- a/services/sync/modules/type_records/bookmark.js +++ b/services/sync/modules/type_records/bookmark.js @@ -50,7 +50,7 @@ function PlacesItem(uri, type) { this.type = type || "item"; } PlacesItem.prototype = { - decrypt: function PlacesItem_decrypt(passphrase) { + decrypt: function PlacesItem_decrypt(passphrase, keyUri) { // Do the normal CryptoWrapper decrypt, but change types before returning let clear = CryptoWrapper.prototype.decrypt.apply(this, arguments); diff --git a/services/sync/tests/unit/test_clients_escape.js b/services/sync/tests/unit/test_clients_escape.js index 37fc23d31f6e..a8e6f69a3bab 100644 --- a/services/sync/tests/unit/test_clients_escape.js +++ b/services/sync/tests/unit/test_clients_escape.js @@ -51,7 +51,7 @@ function run_test() { do_check_eq(checkCount, serialized.length); _("Making sure the record still looks like it did before"); - record.decrypt(passphrase); + record.decrypt(passphrase, Clients.cryptoMetaURL); do_check_eq(record.id, "ascii"); do_check_eq(record.name, "wéävê"); diff --git a/services/sync/tests/unit/test_records_crypto.js b/services/sync/tests/unit/test_records_crypto.js index 1f15761c9c86..49fb0baf4697 100644 --- a/services/sync/tests/unit/test_records_crypto.js +++ b/services/sync/tests/unit/test_records_crypto.js @@ -74,9 +74,10 @@ function run_test() { log.info("Creating a record"); + let cryptoUri = "http://localhost:8080/crypto/steam"; cryptoWrap = new CryptoWrapper("http://localhost:8080/steam/resource"); - cryptoWrap.encryption = "http://localhost:8080/crypto/steam"; - do_check_eq(cryptoWrap.encryption, "http://localhost:8080/crypto/steam"); + cryptoWrap.encryption = cryptoUri; + do_check_eq(cryptoWrap.encryption, cryptoUri); do_check_eq(cryptoWrap.payload.encryption, "../crypto/steam"); log.info("Encrypting a record"); @@ -87,7 +88,7 @@ function run_test() { log.info("Decrypting the record"); - let payload = cryptoWrap.decrypt(passphrase); + let payload = cryptoWrap.decrypt(passphrase, cryptoUri); do_check_eq(payload.stuff, "my payload here"); do_check_neq(payload, cryptoWrap.payload); // wrap.data.payload is the encrypted one @@ -96,7 +97,7 @@ function run_test() { cryptoWrap.cleartext.stuff = "another payload"; cryptoWrap.encrypt(passphrase); let secondIV = cryptoWrap.IV; - payload = cryptoWrap.decrypt(passphrase); + payload = cryptoWrap.decrypt(passphrase, cryptoUri); do_check_eq(payload.stuff, "another payload"); log.info("Make sure multiple encrypts use different IVs"); @@ -107,7 +108,7 @@ function run_test() { cryptoWrap.data.id = "other"; let error = ""; try { - cryptoWrap.decrypt(passphrase); + cryptoWrap.decrypt(passphrase, cryptoUri); } catch(ex) { error = ex; @@ -119,7 +120,7 @@ function run_test() { cryptoWrap.hmac = "foo"; error = ""; try { - cryptoWrap.decrypt(passphrase); + cryptoWrap.decrypt(passphrase, cryptoUri); } catch(ex) { error = ex; diff --git a/services/sync/tests/unit/test_syncengine_sync.js b/services/sync/tests/unit/test_syncengine_sync.js index 085885df49ad..29327b3fb10e 100644 --- a/services/sync/tests/unit/test_syncengine_sync.js +++ b/services/sync/tests/unit/test_syncengine_sync.js @@ -48,7 +48,6 @@ SteamStore.prototype = { createRecord: function(id, uri) { var record = new SteamRecord(uri); - record.id = id; record.denomination = this.items[id] || "Data for new record: " + id; return record; }, @@ -125,7 +124,7 @@ function encryptPayload(cleartext) { cleartext = JSON.stringify(cleartext); } - return {encryption: "http://localhost:8080/1.0/foo/storage/crypto/steam", + return {encryption: "../crypto/steam", ciphertext: cleartext, // ciphertext == cleartext with fake crypto IV: "irrelevant", hmac: Utils.sha256HMAC(cleartext, null)}; @@ -502,6 +501,15 @@ function test_processIncoming_createFromServer() { 'scotsman', encryptPayload({id: 'scotsman', denomination: "Flying Scotsman"})); + // Two pathological cases involving relative URIs gone wrong. + collection.wbos['../pathological'] = new ServerWBO( + '../pathological', encryptPayload({id: '../pathological', + denomination: "Pathological Case"})); + let wrong_keyuri = encryptPayload({id: "wrong_keyuri", + denomination: "Wrong Key URI"}); + wrong_keyuri.encryption = "../../crypto/steam"; + collection.wbos["wrong_keyuri"] = new ServerWBO("wrong_keyuri", wrong_keyuri); + let server = sync_httpd_setup({ "/1.0/foo/storage/crypto/steam": crypto_steam.handler(), "/1.0/foo/storage/steam": collection.handler(), @@ -520,6 +528,8 @@ function test_processIncoming_createFromServer() { do_check_eq(engine.lastModified, null); do_check_eq(engine._store.items.flying, undefined); do_check_eq(engine._store.items.scotsman, undefined); + do_check_eq(engine._store.items['../pathological'], undefined); + do_check_eq(engine._store.items.wrong_keyuri, undefined); engine._processIncoming(); @@ -530,6 +540,8 @@ function test_processIncoming_createFromServer() { // Local records have been created from the server data. do_check_eq(engine._store.items.flying, "LNER Class A3 4472"); do_check_eq(engine._store.items.scotsman, "Flying Scotsman"); + do_check_eq(engine._store.items['../pathological'], "Pathological Case"); + do_check_eq(engine._store.items.wrong_keyuri, "Wrong Key URI"); } finally { server.stop(do_test_finished); From fef75cd21bbe027dbeceabbae9d32f1cde99da73 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 5 Oct 2010 08:32:37 +0200 Subject: [PATCH 3/3] Bug 583209 - Use ctypes.libraryName and don't use a full path to load libnss3 from weavecrypto. [r=dolske] --- services/crypto/WeaveCrypto.js | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/services/crypto/WeaveCrypto.js b/services/crypto/WeaveCrypto.js index 2939a527416c..81859c56e892 100644 --- a/services/crypto/WeaveCrypto.js +++ b/services/crypto/WeaveCrypto.js @@ -107,34 +107,12 @@ WeaveCrypto.prototype = { Cc["@mozilla.org/psm;1"].getService(Ci.nsISupports); // Open the NSS library. - let nssfile = Services.dirsvc.get("GreD", Ci.nsILocalFile); - let os = Services.appinfo.OS; - switch (os) { - case "WINNT": - case "WINMO": - case "WINCE": - nssfile.append("nss3.dll"); - break; - case "Darwin": - nssfile.append("libnss3.dylib"); - break; - case "Linux": - case "SunOS": - case "WebOS": // Palm Pre - nssfile.append("libnss3.so"); - break; - case "Android": - // Android uses a $GREDIR/lib/ subdir. - nssfile.append("lib"); - nssfile.append("libnss3.so"); - break; - default: - throw Components.Exception("unsupported platform: " + os, Cr.NS_ERROR_UNEXPECTED); - } - this.log("Using NSS library " + nssfile.path); + let path = ctypes.libraryName("nss3"); + + this.log("Using NSS library " + path); // XXX really want to be able to pass specific dlopen flags here. - let nsslib = ctypes.open(nssfile.path); + let nsslib = ctypes.open(path); this.log("Initializing NSS types and function declarations...");