diff --git a/.cargo/config.in b/.cargo/config.in index 29ef98f12e37..a2824596ed4f 100644 --- a/.cargo/config.in +++ b/.cargo/config.in @@ -20,7 +20,7 @@ tag = "v0.2.4" [source."https://github.com/mozilla/application-services"] git = "https://github.com/mozilla/application-services" replace-with = "vendored-sources" -rev = "43d69b250d8185ebc53e887b747d85a2a53c7298" +rev = "789a936d1a49917fe550360b8f3349fe4eba9169" [source."https://github.com/mozilla-spidermonkey/jsparagus"] git = "https://github.com/mozilla-spidermonkey/jsparagus" diff --git a/Cargo.lock b/Cargo.lock index 0f3c8dffb433..ca06666d7f4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1255,7 +1255,7 @@ checksum = "ff511d5dc435d703f4971bc399647c9bc38e20cb41452e3b9feb4765419ed3f3" [[package]] name = "error-support" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" dependencies = [ "failure", ] @@ -1985,6 +1985,7 @@ dependencies = [ "moz_task", "nserror", "nsstring", + "serde_json", "storage_variant", "sync15-traits", "thin-vec", @@ -2193,7 +2194,7 @@ dependencies = [ [[package]] name = "interrupt-support" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" [[package]] name = "intl-memoizer" @@ -3128,7 +3129,7 @@ dependencies = [ [[package]] name = "nss_build_common" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" [[package]] name = "nsstring" @@ -4271,7 +4272,7 @@ dependencies = [ [[package]] name = "sql-support" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" dependencies = [ "ffi-support", "interrupt-support", @@ -4468,7 +4469,7 @@ dependencies = [ [[package]] name = "sync-guid" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" dependencies = [ "base64 0.12.0", "rand", @@ -4479,7 +4480,7 @@ dependencies = [ [[package]] name = "sync15-traits" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" dependencies = [ "failure", "ffi-support", @@ -5194,7 +5195,7 @@ dependencies = [ [[package]] name = "webext-storage" version = "0.1.0" -source = "git+https://github.com/mozilla/application-services?rev=43d69b250d8185ebc53e887b747d85a2a53c7298#43d69b250d8185ebc53e887b747d85a2a53c7298" +source = "git+https://github.com/mozilla/application-services?rev=789a936d1a49917fe550360b8f3349fe4eba9169#789a936d1a49917fe550360b8f3349fe4eba9169" dependencies = [ "error-support", "failure", diff --git a/services/interfaces/mozIBridgedSyncEngine.idl b/services/interfaces/mozIBridgedSyncEngine.idl index 1f6e315e9151..111f6dbe3aa7 100644 --- a/services/interfaces/mozIBridgedSyncEngine.idl +++ b/services/interfaces/mozIBridgedSyncEngine.idl @@ -28,7 +28,7 @@ interface mozIBridgedSyncEngineApplyCallback : nsISupports { // records, and has outgoing records. Sync encrypts and uploads these // records, and notifies the engine that the upload succeeded by // calling `engine.setUploaded(uploadedOutgoingRecordIds, ...)`. - void handleSuccess(in Array outgoingRecordsAsJSON); + void handleSuccess(in Array outgoingEnvelopesAsJSON); // Notifies Sync that the bridged engine failed to apply the staged records. void handleError(in nsresult code, in AUTF8String message); @@ -111,7 +111,7 @@ interface mozIBridgedSyncEngine : nsISupports { // method that can be used to interrupt staging. Typically, engines will // stage incoming records in an SQLite temp table, and merge them with the // local database when `apply` is called. - void storeIncoming(in Array incomingRecordsAsJSON, + void storeIncoming(in Array incomingEnvelopesAsJSON, in mozIBridgedSyncEngineCallback callback); // Applies all the staged records, and calls the `callback` with diff --git a/services/sync/golden_gate/Cargo.toml b/services/sync/golden_gate/Cargo.toml index b6d74e5a6d73..dd099550c938 100644 --- a/services/sync/golden_gate/Cargo.toml +++ b/services/sync/golden_gate/Cargo.toml @@ -8,13 +8,14 @@ edition = "2018" [dependencies] atomic_refcell = "0.1" cstr = "0.1" -interrupt-support = { git = "https://github.com/mozilla/application-services", rev = "43d69b250d8185ebc53e887b747d85a2a53c7298" } +interrupt-support = { git = "https://github.com/mozilla/application-services", rev = "789a936d1a49917fe550360b8f3349fe4eba9169" } log = "0.4" moz_task = { path = "../../../xpcom/rust/moz_task" } nserror = { path = "../../../xpcom/rust/nserror" } nsstring = { path = "../../../xpcom/rust/nsstring" } +serde_json = "1" storage_variant = { path = "../../../storage/variant" } -sync15-traits = { git = "https://github.com/mozilla/application-services", rev = "43d69b250d8185ebc53e887b747d85a2a53c7298" } +sync15-traits = { git = "https://github.com/mozilla/application-services", rev = "789a936d1a49917fe550360b8f3349fe4eba9169" } xpcom = { path = "../../../xpcom/rust/xpcom" } [dependencies.thin-vec] diff --git a/services/sync/golden_gate/src/error.rs b/services/sync/golden_gate/src/error.rs index 121fc4be8b58..9d65df428b37 100644 --- a/services/sync/golden_gate/src/error.rs +++ b/services/sync/golden_gate/src/error.rs @@ -5,6 +5,7 @@ use std::{error, fmt, result, str::Utf8Error}; use nserror::{nsresult, NS_ERROR_INVALID_ARG, NS_ERROR_UNEXPECTED}; +use serde_json::Error as JsonError; /// A specialized `Result` type for Golden Gate. pub type Result = result::Result; @@ -18,7 +19,7 @@ pub enum Error { /// A ferry didn't run on the background task queue. DidNotRun(&'static str), - /// A Gecko string couldn't be converted to UTF-8. + /// A string contains invalid UTF-8 or JSON. MalformedString(Box), } @@ -53,6 +54,12 @@ impl From for Error { } } +impl From for Error { + fn from(error: JsonError) -> Error { + Error::MalformedString(error.into()) + } +} + impl From for nsresult { fn from(error: Error) -> nsresult { match error { diff --git a/services/sync/golden_gate/src/ferry.rs b/services/sync/golden_gate/src/ferry.rs index ad703f7574f8..229f7216cc93 100644 --- a/services/sync/golden_gate/src/ferry.rs +++ b/services/sync/golden_gate/src/ferry.rs @@ -4,6 +4,7 @@ use nsstring::nsCString; use storage_variant::VariantType; +use sync15_traits::IncomingEnvelope; use xpcom::{interfaces::nsIVariant, RefPtr}; /// An operation that runs on the background thread, and optionally passes a @@ -15,7 +16,7 @@ pub enum Ferry { SyncId, ResetSyncId, EnsureCurrentSyncId(String), - StoreIncoming(Vec), + StoreIncoming(Vec), SetUploaded(i64, Vec), SyncFinished, Reset, diff --git a/services/sync/golden_gate/src/lib.rs b/services/sync/golden_gate/src/lib.rs index e975111514d7..63442fae8769 100644 --- a/services/sync/golden_gate/src/lib.rs +++ b/services/sync/golden_gate/src/lib.rs @@ -113,5 +113,5 @@ pub use error::{Error, Result}; // Re-export items from `interrupt-support` and `sync15-traits`, so that // consumers of `golden_gate` don't have to depend on them. pub use interrupt_support::{Interrupted, Interruptee}; -pub use sync15_traits::{ApplyResults, BridgedEngine}; +pub use sync15_traits::{ApplyResults, BridgedEngine, IncomingEnvelope, OutgoingEnvelope}; pub use task::{ApplyTask, FerryTask}; diff --git a/services/sync/golden_gate/src/task.rs b/services/sync/golden_gate/src/task.rs index 4f3c2e24a2f8..a18016b6d399 100644 --- a/services/sync/golden_gate/src/task.rs +++ b/services/sync/golden_gate/src/task.rs @@ -111,20 +111,17 @@ where /// Creates a task to store incoming records. pub fn for_store_incoming( engine: &Arc, - incoming_cleartexts: &[nsCString], + incoming_envelopes_json: &[nsCString], callback: &mozIBridgedSyncEngineCallback, ) -> error::Result> { - let incoming_cleartexts = incoming_cleartexts.iter().try_fold( - Vec::with_capacity(incoming_cleartexts.len()), - |mut cleartexts, cleartext| -> error::Result<_> { - // We need to clone the string for the task to take ownership - // of it, anyway; might as well convert to a Rust string while - // we're here. - cleartexts.push(std::str::from_utf8(&*cleartext)?.into()); - Ok(cleartexts) + let incoming_envelopes = incoming_envelopes_json.iter().try_fold( + Vec::with_capacity(incoming_envelopes_json.len()), + |mut envelopes, envelope| -> error::Result<_> { + envelopes.push(serde_json::from_slice(&*envelope)?); + Ok(envelopes) }, )?; - Self::with_ferry(engine, Ferry::StoreIncoming(incoming_cleartexts), callback) + Self::with_ferry(engine, Ferry::StoreIncoming(incoming_envelopes), callback) } /// Creates a task to mark a subset of outgoing records as uploaded. This @@ -246,8 +243,8 @@ where Ferry::EnsureCurrentSyncId(new_sync_id) => { FerryResult::AssignedSyncId(engine.ensure_current_sync_id(&*new_sync_id)?) } - Ferry::StoreIncoming(incoming_cleartexts) => { - engine.store_incoming(incoming_cleartexts.as_slice())?; + Ferry::StoreIncoming(incoming_envelopes) => { + engine.store_incoming(incoming_envelopes.as_slice())?; FerryResult::default() } Ferry::SetUploaded(server_modified_millis, uploaded_ids) => { @@ -306,7 +303,7 @@ where pub struct ApplyTask { engine: Weak, callback: ThreadPtrHandle, - result: AtomicRefCell>, + result: AtomicRefCell, N::Error>>, } impl ApplyTask @@ -320,12 +317,23 @@ where } /// Runs the task on the background thread. - fn inner_run(&self) -> Result { + fn inner_run(&self) -> Result, N::Error> { let engine = match self.engine.upgrade() { Some(outer) => outer, None => return Err(Error::DidNotRun(Self::name()).into()), }; - Ok(engine.apply()?) + let ApplyResults { + envelopes: outgoing_envelopes, + .. + } = engine.apply()?; + let outgoing_envelopes_json = outgoing_envelopes.iter().try_fold( + Vec::with_capacity(outgoing_envelopes.len()), + |mut envelopes, envelope| { + envelopes.push(serde_json::to_string(envelope)?); + Ok(envelopes) + }, + )?; + Ok(outgoing_envelopes_json) } } @@ -373,11 +381,8 @@ where &mut *self.result.borrow_mut(), Err(Error::DidNotRun(Self::name()).into()), ) { - Ok(ApplyResults { - records, - num_reconciled: _, - }) => { - let result = records + Ok(envelopes) => { + let result = envelopes .into_iter() .map(nsCString::from) .collect::>(); diff --git a/services/sync/modules-testing/fakeservices.js b/services/sync/modules-testing/fakeservices.js index f8fe90f1df27..f13ef27c89de 100644 --- a/services/sync/modules-testing/fakeservices.js +++ b/services/sync/modules-testing/fakeservices.js @@ -12,7 +12,7 @@ var EXPORTED_SYMBOLS = [ ]; const { Weave } = ChromeUtils.import("resource://services-sync/main.js"); -const { CryptoWrapper } = ChromeUtils.import( +const { RawCryptoWrapper } = ChromeUtils.import( "resource://services-sync/record.js" ); const { Utils } = ChromeUtils.import("resource://services-sync/util.js"); @@ -90,7 +90,9 @@ function FakeCryptoService() { delete Weave.Crypto; // get rid of the getter first Weave.Crypto = this; - CryptoWrapper.prototype.ciphertextHMAC = function ciphertextHMAC(keyBundle) { + RawCryptoWrapper.prototype.ciphertextHMAC = function ciphertextHMAC( + keyBundle + ) { return fakeSHA256HMAC(this.ciphertext); }; } diff --git a/services/sync/modules/bridged_engine.js b/services/sync/modules/bridged_engine.js index 4421a688f503..ff2300d633c5 100644 --- a/services/sync/modules/bridged_engine.js +++ b/services/sync/modules/bridged_engine.js @@ -8,7 +8,7 @@ const { XPCOMUtils } = ChromeUtils.import( const { Changeset, SyncEngine } = ChromeUtils.import( "resource://services-sync/engines.js" ); -const { CryptoWrapper } = ChromeUtils.import( +const { RawCryptoWrapper } = ChromeUtils.import( "resource://services-sync/record.js" ); @@ -43,11 +43,13 @@ class BridgedStore { async applyIncomingBatch(records) { await this.engine.initialize(); for (let chunk of PlacesUtils.chunkArray(records, this._batchChunkSize)) { - // TODO: We can avoid parsing and re-serializing here... We also need to - // pass attributes like `modified` and `sortindex`, which are not part - // of the cleartext. - let incomingCleartexts = chunk.map(record => record.cleartextToString()); - await promisify(this.engine._bridge.storeIncoming, incomingCleartexts); + let incomingEnvelopesAsJSON = chunk.map(record => + JSON.stringify(record.toIncomingEnvelope()) + ); + await promisify( + this.engine._bridge.storeIncoming, + incomingEnvelopesAsJSON + ); } // Array of failed records. return []; @@ -104,9 +106,60 @@ class BridgedTracker { } } -class BridgedRecord extends CryptoWrapper { - constructor(collection, id, type) { - super(collection, id, type); +/** + * A wrapper class to convert between BSOs on the JS side, and envelopes on the + * Rust side. This class intentionally subclasses `RawCryptoWrapper`, because we + * don't want the stringification and parsing machinery in `CryptoWrapper`. + */ +class BridgedRecord extends RawCryptoWrapper { + /** + * Creates an outgoing record from an envelope returned by a bridged engine. + * This must be kept in sync with `sync15_traits::OutgoingEnvelope`. + * + * @param {String} collection The collection name. + * @param {Object} envelope The outgoing envelope, returned from + * `mozIBridgedSyncEngine::apply`. + * @return {BridgedRecord} A Sync record ready to encrypt and upload. + */ + static fromOutgoingEnvelope(collection, envelope) { + if (typeof envelope.id != "string") { + throw new TypeError("Outgoing envelope missing ID"); + } + if (typeof envelope.cleartext != "string") { + throw new TypeError("Outgoing envelope missing cleartext"); + } + let record = new BridgedRecord(collection, envelope.id); + record.cleartext = envelope.cleartext; + return record; + } + + transformBeforeEncrypt(cleartext) { + if (typeof cleartext != "string") { + throw new TypeError("Outgoing bridged engine records must be strings"); + } + return cleartext; + } + + transformAfterDecrypt(cleartext) { + if (typeof cleartext != "string") { + throw new TypeError("Incoming bridged engine records must be strings"); + } + return cleartext; + } + + /* + * Converts this incoming record into an envelope to pass to a bridged engine. + * This object must be kept in sync with `sync15_traits::IncomingEnvelope`. + * + * @return {Object} The incoming envelope, to pass to + * `mozIBridgedSyncEngine::storeIncoming`. + */ + toIncomingEnvelope() { + return { + id: this.data.id, + modified: this.data.modified, + cleartext: this.cleartext, + }; } } @@ -324,15 +377,16 @@ BridgedEngine.prototype = { await super._processIncoming(newitems); await this.initialize(); - let outgoingRecords = await promisify(this._bridge.apply); + let outgoingEnvelopesAsJSON = await promisify(this._bridge.apply); let changeset = {}; - for (let record of outgoingRecords) { - // TODO: It would be nice if we could pass the cleartext through as-is - // here, too, instead of parsing and re-wrapping for `BridgedRecord`. - let cleartext = JSON.parse(record); - changeset[cleartext.id] = { + for (let envelopeAsJSON of outgoingEnvelopesAsJSON) { + let record = BridgedRecord.fromOutgoingEnvelope( + this.name, + JSON.parse(envelopeAsJSON) + ); + changeset[record.id] = { synced: false, - cleartext, + record, }; } this._modified.replace(changeset); @@ -358,9 +412,7 @@ BridgedEngine.prototype = { if (!change) { throw new TypeError("Can't create record for unchanged item"); } - let record = new this._recordObj(this.name, id); - record.cleartext = change.cleartext; - return record; + return change.record; }, async _resetClient() { diff --git a/services/sync/modules/record.js b/services/sync/modules/record.js index 8bd29d7af341..e25029438ec3 100644 --- a/services/sync/modules/record.js +++ b/services/sync/modules/record.js @@ -5,6 +5,7 @@ var EXPORTED_SYMBOLS = [ "WBORecord", "RecordManager", + "RawCryptoWrapper", "CryptoWrapper", "CollectionKeyManager", "Collection", @@ -29,6 +30,16 @@ const { CommonUtils } = ChromeUtils.import( "resource://services-common/utils.js" ); +/** + * The base class for all Sync basic storage objects (BSOs). This is the format + * used to store all records on the Sync server. In an earlier version of the + * Sync protocol, BSOs used to be called WBOs, or Weave Basic Objects. This + * class retains the old name. + * + * @class + * @param {String} collection The collection name for this BSO. + * @param {String} id The ID of this BSO. + */ function WBORecord(collection, id) { this.data = {}; this.payload = {}; @@ -131,15 +142,70 @@ Utils.deferGetSet(WBORecord, "data", [ "payload", ]); -function CryptoWrapper(collection, id) { - this.cleartext = {}; +/** + * An encrypted BSO record. This subclass handles encrypting and decrypting the + * BSO payload, but doesn't parse or interpret the cleartext string. Subclasses + * must override `transformBeforeEncrypt` and `transformAfterDecrypt` to process + * the cleartext. + * + * This class is only exposed for bridged engines, which handle serialization + * and deserialization in Rust. Sync engines implemented in JS should subclass + * `CryptoWrapper` instead, which takes care of transforming the cleartext into + * an object, and ensuring its contents are valid. + * + * @class + * @template Cleartext + * @param {String} collection The collection name for this BSO. + * @param {String} id The ID of this BSO. + */ +function RawCryptoWrapper(collection, id) { + // Setting properties before calling the superclass constructor isn't allowed + // in new-style classes (`class MyRecord extends RawCryptoWrapper`), but + // allowed with plain functions. This is also why `defaultCleartext` is a + // method, and not simply set in the subclass constructor. + this.cleartext = this.defaultCleartext(); WBORecord.call(this, collection, id); this.ciphertext = null; - this.id = id; } -CryptoWrapper.prototype = { +RawCryptoWrapper.prototype = { __proto__: WBORecord.prototype, - _logName: "Sync.Record.CryptoWrapper", + _logName: "Sync.Record.RawCryptoWrapper", + + /** + * Returns the default empty cleartext for this record type. This is exposed + * as a method so that subclasses can override it, and access the default + * cleartext in their constructors. `CryptoWrapper`, for example, overrides + * this to return an empty object, so that initializing the `id` in its + * constructor calls its overridden `id` setter. + * + * @returns {Cleartext} An empty cleartext. + */ + defaultCleartext() { + return null; + }, + + /** + * Transforms the cleartext into a string that can be encrypted and wrapped + * in a BSO payload. This is called before uploading the record to the server. + * + * @param {Cleartext} outgoingCleartext The cleartext to upload. + * @returns {String} The serialized cleartext. + */ + transformBeforeEncrypt(outgoingCleartext) { + throw new TypeError("Override to stringify outgoing records"); + }, + + /** + * Transforms an incoming cleartext string into an instance of the + * `Cleartext` type. This is called when fetching the record from the + * server. + * + * @param {String} incomingCleartext The decrypted cleartext string. + * @returns {Cleartext} The parsed cleartext. + */ + transformAfterDecrypt(incomingCleartext) { + throw new TypeError("Override to parse incoming records"); + }, ciphertextHMAC: function ciphertextHMAC(keyBundle) { let hasher = keyBundle.sha256HMACHasher; @@ -166,7 +232,7 @@ CryptoWrapper.prototype = { this.IV = Weave.Crypto.generateRandomIV(); this.ciphertext = await Weave.Crypto.encrypt( - JSON.stringify(this.cleartext), + this.transformBeforeEncrypt(this.cleartext), keyBundle.encryptionKeyB64, this.IV ); @@ -191,29 +257,59 @@ CryptoWrapper.prototype = { Utils.throwHMACMismatch(this.hmac, computedHMAC); } - // Handle invalid data here. Elsewhere we assume that cleartext is an object. let cleartext = await Weave.Crypto.decrypt( this.ciphertext, keyBundle.encryptionKeyB64, this.IV ); + this.cleartext = this.transformAfterDecrypt(cleartext); + this.ciphertext = null; + + return this.cleartext; + }, +}; + +Utils.deferGetSet(RawCryptoWrapper, "payload", ["ciphertext", "IV", "hmac"]); + +/** + * An encrypted BSO record with a JSON payload. All engines implemented in JS + * should subclass this class to describe their own record types. + * + * @class + * @param {String} collection The collection name for this BSO. + * @param {String} id The ID of this BSO. + */ +function CryptoWrapper(collection, id) { + RawCryptoWrapper.call(this, collection, id); +} +CryptoWrapper.prototype = { + __proto__: RawCryptoWrapper.prototype, + _logName: "Sync.Record.CryptoWrapper", + + defaultCleartext() { + return {}; + }, + + transformBeforeEncrypt(cleartext) { + return JSON.stringify(cleartext); + }, + + transformAfterDecrypt(cleartext) { + // Handle invalid data here. Elsewhere we assume that cleartext is an object. let json_result = JSON.parse(cleartext); - if (json_result && json_result instanceof Object) { - this.cleartext = json_result; - this.ciphertext = null; - } else { + if (!(json_result && json_result instanceof Object)) { throw new Error( `Decryption failed: result is <${json_result}>, not an object.` ); } // Verify that the encrypted id matches the requested record's id. - if (this.cleartext.id != this.id) { - throw new Error(`Record id mismatch: ${this.cleartext.id} != ${this.id}`); + if (json_result.id != this.id) { + throw new Error(`Record id mismatch: ${json_result.id} != ${this.id}`); } - return this.cleartext; + return json_result; }, cleartextToString() { @@ -248,17 +344,16 @@ CryptoWrapper.prototype = { // The custom setter below masks the parent's getter, so explicitly call it :( get id() { - return WBORecord.prototype.__lookupGetter__("id").call(this); + return super.id; }, // Keep both plaintext and encrypted versions of the id to verify integrity set id(val) { - WBORecord.prototype.__lookupSetter__("id").call(this, val); + super.id = val; return (this.cleartext.id = val); }, }; -Utils.deferGetSet(CryptoWrapper, "payload", ["ciphertext", "IV", "hmac"]); Utils.deferGetSet(CryptoWrapper, "cleartext", "deleted"); /** diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index b8b3d1840d9b..ad92aca7e7b3 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -703,6 +703,7 @@ function doCheckWBOs(WBOs, expected) { } function FakeRecord(constructor, r) { + this.defaultCleartext = constructor.prototype.defaultCleartext; constructor.call(this, "bookmarks", r.id); for (let x in r) { this[x] = r[x]; diff --git a/services/sync/tests/unit/test_bridged_engine.js b/services/sync/tests/unit/test_bridged_engine.js index 2cb541788b58..bdb6b02dcd33 100644 --- a/services/sync/tests/unit/test_bridged_engine.js +++ b/services/sync/tests/unit/test_bridged_engine.js @@ -28,7 +28,7 @@ add_task(async function test_interface() { clear() { this.lastSyncMillis = 0; - this.incomingRecords = []; + this.incomingEnvelopes = []; this.uploadedIDs = []; this.wasSynced = false; this.wasReset = false; @@ -77,18 +77,18 @@ add_task(async function test_interface() { CommonUtils.nextTick(() => callback.handleSuccess(this.syncID)); } - storeIncoming(records, callback) { + storeIncoming(envelopes, callback) { ok( this.wasInitialized, "Should initialize before storing incoming records" ); - this.incomingRecords.push(...records.map(r => JSON.parse(r))); + this.incomingEnvelopes.push(...envelopes.map(r => JSON.parse(r))); CommonUtils.nextTick(() => callback.handleSuccess()); } apply(callback) { ok(this.wasInitialized, "Should initialize before applying records"); - let outgoingRecords = [ + let outgoingEnvelopes = [ { id: "hanson", data: { @@ -103,8 +103,13 @@ add_task(async function test_interface() { tomorrow: "winding 🛣", }, }, - ].map(r => JSON.stringify(r)); - CommonUtils.nextTick(() => callback.handleSuccess(outgoingRecords)); + ].map(cleartext => + JSON.stringify({ + id: cleartext.id, + cleartext: JSON.stringify(cleartext), + }) + ); + CommonUtils.nextTick(() => callback.handleSuccess(outgoingEnvelopes)); } setUploaded(millis, ids, callback) { @@ -195,13 +200,22 @@ add_task(async function test_interface() { greater(bridge.lastSyncMillis, 0, "Should update last sync time"); deepEqual( - bridge.incomingRecords.sort((a, b) => a.id.localeCompare(b.id)), + bridge.incomingEnvelopes + .sort((a, b) => a.id.localeCompare(b.id)) + .map(({ cleartext, ...envelope }) => ({ + cleartextAsObject: JSON.parse(cleartext), + ...envelope, + })), [ { id: "tlc", - data: { - forbidden: ["scrubs 🚫"], - numberAvailable: false, + modified: now + 5, + cleartextAsObject: { + id: "tlc", + data: { + forbidden: ["scrubs 🚫"], + numberAvailable: false, + }, }, }, ], diff --git a/third_party/rust/sync15-traits/.cargo-checksum.json b/third_party/rust/sync15-traits/.cargo-checksum.json index 8e734571ba53..850fa5141b18 100644 --- a/third_party/rust/sync15-traits/.cargo-checksum.json +++ b/third_party/rust/sync15-traits/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"656c4c4af39bcf924098be33996360250f9610ee3a4090b8152b68bdad03c46e","README.md":"396105211d8ce7f40b05d8062d7ab55d99674555f3ac81c061874ae26656ed7e","src/bridged_engine.rs":"3387cea6cdfe9cfbc837d52e79a5d8e5be75e9d468a030760e8e7b3c6784462d","src/changeset.rs":"442aa92b5130ec0f8f2b0054acb399c547380e0060015cbf4ca7a72027440d54","src/client.rs":"6be4f550ade823fafc350c5490e031f90a4af833a9bba9739b05568464255a74","src/lib.rs":"c1ca44e7bb6477b8018bd554479021dbf52754e64577185b3f7e208ae45bf754","src/payload.rs":"09db1a444e7893990a4f03cb16263b9c15abc9e48ec4f1343227be1b490865a5","src/request.rs":"9e656ec487e53c7485643687e605d73bb25e138056e920d6f4b7d63fc6a8c460","src/server_timestamp.rs":"43d1b98a90e55e49380a0b66c209c9eb393e2aeaa27d843a4726d93cdd4cea02","src/store.rs":"10e215dd24270b6bec10903ac1d5274ce997eb437134f43be7de44e36fb9d1e4","src/telemetry.rs":"027befb099a6fcded3457f7e566296548a0898ff613267190621856b9ef288f6"},"package":null} \ No newline at end of file +{"files":{"Cargo.toml":"656c4c4af39bcf924098be33996360250f9610ee3a4090b8152b68bdad03c46e","README.md":"396105211d8ce7f40b05d8062d7ab55d99674555f3ac81c061874ae26656ed7e","src/bridged_engine.rs":"abb9dc99f945983d885183da25dd05ff46ff1382e4e74c6421eb1a3ff51d116e","src/changeset.rs":"442aa92b5130ec0f8f2b0054acb399c547380e0060015cbf4ca7a72027440d54","src/client.rs":"6be4f550ade823fafc350c5490e031f90a4af833a9bba9739b05568464255a74","src/lib.rs":"3a3d278b4edb1d99e1fe5600ac8e77f44391f8bd7df059628a9aedb3e65eaa4c","src/payload.rs":"09db1a444e7893990a4f03cb16263b9c15abc9e48ec4f1343227be1b490865a5","src/request.rs":"9e656ec487e53c7485643687e605d73bb25e138056e920d6f4b7d63fc6a8c460","src/server_timestamp.rs":"43d1b98a90e55e49380a0b66c209c9eb393e2aeaa27d843a4726d93cdd4cea02","src/store.rs":"10e215dd24270b6bec10903ac1d5274ce997eb437134f43be7de44e36fb9d1e4","src/telemetry.rs":"027befb099a6fcded3457f7e566296548a0898ff613267190621856b9ef288f6"},"package":null} \ No newline at end of file diff --git a/third_party/rust/sync15-traits/src/bridged_engine.rs b/third_party/rust/sync15-traits/src/bridged_engine.rs index 51b3c882b9fd..e25e5a53f4d2 100644 --- a/third_party/rust/sync15-traits/src/bridged_engine.rs +++ b/third_party/rust/sync15-traits/src/bridged_engine.rs @@ -2,6 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use std::{error::Error, fmt}; + +use serde::{Deserialize, Serialize}; + +use super::{Guid, Payload, ServerTimestamp}; + /// A bridged Sync engine implements all the methods needed to support /// Desktop Sync. pub trait BridgedEngine { @@ -47,7 +53,7 @@ pub trait BridgedEngine { /// times per sync, once for each batch. Implementations can use the /// signal to check if the operation was aborted, and cancel any /// pending work. - fn store_incoming(&self, incoming_cleartexts: &[String]) -> Result<(), Self::Error>; + fn store_incoming(&self, incoming_cleartexts: &[IncomingEnvelope]) -> Result<(), Self::Error>; /// Applies all staged records, reconciling changes on both sides and /// resolving conflicts. Returns a list of records to upload. @@ -84,7 +90,7 @@ pub trait BridgedEngine { #[derive(Clone, Debug, Default)] pub struct ApplyResults { /// List of records - pub records: Vec, + pub envelopes: Vec, /// The number of incoming records whose contents were merged because they /// changed on both sides. None indicates we aren't reporting this /// information. @@ -92,20 +98,102 @@ pub struct ApplyResults { } impl ApplyResults { - pub fn new(records: Vec, num_reconciled: impl Into>) -> Self { + pub fn new(envelopes: Vec, num_reconciled: impl Into>) -> Self { Self { - records, + envelopes, num_reconciled: num_reconciled.into(), } } } // Shorthand for engines that don't care. -impl From> for ApplyResults { - fn from(records: Vec) -> Self { +impl From> for ApplyResults { + fn from(envelopes: Vec) -> Self { Self { - records, + envelopes, num_reconciled: None, } } } + +/// An envelope for an incoming item, passed to `BridgedEngine::store_incoming`. +/// Envelopes are a halfway point between BSOs, the format used for all items on +/// the Sync server, and records, which are specific to each engine. +/// +/// A BSO is a JSON object with metadata fields (`id`, `modifed`, `sortindex`), +/// and a BSO payload that is itself a JSON string. For encrypted records, the +/// BSO payload has a ciphertext, which must be decrypted to yield a cleartext. +/// The cleartext is a JSON string (that's three levels of JSON wrapping, if +/// you're keeping score: the BSO itself, BSO payload, and cleartext) with the +/// actual record payload. +/// +/// An envelope combines the metadata fields from the BSO, and the cleartext +/// from the encrypted BSO payload. +#[derive(Clone, Debug, Deserialize)] +pub struct IncomingEnvelope { + pub id: Guid, + pub modified: ServerTimestamp, + #[serde(default)] + pub sortindex: Option, + // Don't provide access to the cleartext directly. We want all callers to + // use `IncomingEnvelope::payload`, so that we can validate the cleartext. + cleartext: String, +} + +impl IncomingEnvelope { + /// Parses and returns the record payload from this envelope. Returns an + /// error if the envelope's cleartext isn't valid JSON, or the payload is + /// invalid. + pub fn payload(&self) -> Result> { + let payload: Payload = serde_json::from_str(&self.cleartext)?; + if payload.id != self.id { + return Err(MismatchedIdError { + envelope: self.id.clone(), + payload: payload.id, + } + .into()); + } + Ok(payload) + } +} + +/// An envelope for an outgoing item, returned from `BridgedEngine::apply`. This +/// is similar to `IncomingEnvelope`, but omits fields that are only set by the +/// server, like `modified`. +#[derive(Clone, Debug, Serialize)] +pub struct OutgoingEnvelope { + id: Guid, + cleartext: String, +} + +impl OutgoingEnvelope { + /// Creates an envelope for an outgoing item. Returns an error if the + /// payload can't be serialized to JSON. + pub fn new(payload: Payload) -> Result> { + let cleartext = serde_json::to_string(&payload)?; + Ok(OutgoingEnvelope { + id: payload.id, + cleartext, + }) + } +} + +/// An error returned when the ID of an incoming BSO doesn't match the ID in +/// its payload. +#[derive(Debug)] +pub struct MismatchedIdError { + pub envelope: Guid, + pub payload: Guid, +} + +impl Error for MismatchedIdError {} + +impl fmt::Display for MismatchedIdError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "ID `{}` in envelope doesn't match `{}` in payload", + self.envelope, self.payload + ) + } +} diff --git a/third_party/rust/sync15-traits/src/lib.rs b/third_party/rust/sync15-traits/src/lib.rs index d60747e6556e..93c9ecfd08a0 100644 --- a/third_party/rust/sync15-traits/src/lib.rs +++ b/third_party/rust/sync15-traits/src/lib.rs @@ -12,7 +12,7 @@ mod server_timestamp; mod store; pub mod telemetry; -pub use bridged_engine::{ApplyResults, BridgedEngine}; +pub use bridged_engine::{ApplyResults, BridgedEngine, IncomingEnvelope, OutgoingEnvelope}; pub use changeset::{IncomingChangeset, OutgoingChangeset, RecordChangeset}; pub use payload::Payload; pub use request::{CollectionRequest, RequestOrder}; diff --git a/toolkit/components/extensions/storage/webext_storage_bridge/Cargo.toml b/toolkit/components/extensions/storage/webext_storage_bridge/Cargo.toml index b4358545bfca..ebda51eecc83 100644 --- a/toolkit/components/extensions/storage/webext_storage_bridge/Cargo.toml +++ b/toolkit/components/extensions/storage/webext_storage_bridge/Cargo.toml @@ -16,5 +16,5 @@ xpcom = { path = "../../../../../xpcom/rust/xpcom" } serde = "1" serde_json = "1" storage_variant = { path = "../../../../../storage/variant" } -sql-support = { git = "https://github.com/mozilla/application-services", rev = "43d69b250d8185ebc53e887b747d85a2a53c7298" } -webext-storage = { git = "https://github.com/mozilla/application-services", rev = "43d69b250d8185ebc53e887b747d85a2a53c7298" } +sql-support = { git = "https://github.com/mozilla/application-services", rev = "789a936d1a49917fe550360b8f3349fe4eba9169" } +webext-storage = { git = "https://github.com/mozilla/application-services", rev = "789a936d1a49917fe550360b8f3349fe4eba9169" } diff --git a/toolkit/library/rust/shared/Cargo.toml b/toolkit/library/rust/shared/Cargo.toml index 88acbd0054ac..145352717ba6 100644 --- a/toolkit/library/rust/shared/Cargo.toml +++ b/toolkit/library/rust/shared/Cargo.toml @@ -55,7 +55,7 @@ unic-langid = { version = "0.8", features = ["likelysubtags"] } unic-langid-ffi = { path = "../../../../intl/locale/rust/unic-langid-ffi" } fluent-langneg = { version = "0.12.1", features = ["cldr"] } fluent-langneg-ffi = { path = "../../../../intl/locale/rust/fluent-langneg-ffi" } -webext-storage = { git = "https://github.com/mozilla/application-services", rev = "43d69b250d8185ebc53e887b747d85a2a53c7298", optional = true } +webext-storage = { git = "https://github.com/mozilla/application-services", rev = "789a936d1a49917fe550360b8f3349fe4eba9169", optional = true } golden_gate = { path = "../../../../services/sync/golden_gate", optional = true } # Note: `modern_sqlite` means rusqlite's bindings file be for a sqlite with