Pier Angelo Vendrame pushed to branch tor-browser-128.4.0esr-14.5-1 at The Tor Project / Applications / Tor Browser Commits: 5ebc9fc7 by Henry Wilkes at 2024-10-30T16:13:30+00:00 fixup! Lox integration Bug 42597: Fix `#updatePubkeys`. - - - - - 704133b3 by Henry Wilkes at 2024-10-30T16:13:31+00:00 fixup! Lox integration Bug 42597: Fix generateInvite. We make sure to call handle_issue_invite to convert the response from "issueinvite" into credentials. We also stop stringifying the lox credentials, which are already a string. - - - - - 27ec12e5 by Henry Wilkes at 2024-10-30T16:13:32+00:00 fixup! Lox integration Bug 42597: Made sure we're always using `JSON.parse(request).request` where `request` is generated by lox_wasm. - - - - - 8ca54076 by Henry Wilkes at 2024-10-30T16:24:48+00:00 fixup! Lox integration Bug 42597: Move duplicate logic into `#makeRequest`. Moreover, we pass in the common fetch arguments into buildPostRequest and treat the return value as a string instead of a JSON object. - - - - - d7c52d55 by Henry Wilkes at 2024-10-30T16:26:16+00:00 fixup! Lox integration Bug 42597: Refactor error handling. A lot of the errors can be propagated to the caller. - - - - - f08fc6e4 by Henry Wilkes at 2024-10-30T16:26:17+00:00 fixup! Bug 40597: Implement TorSettings module Bug 42597: Make buildPostRequest accept fetch arguments. This allows the request to buildPostRequest in the Lox module use the same arguments that would be passed to `fetch`. Also, return the text stream, rather than JSON. - - - - - 8dbd8dd6 by Henry Wilkes at 2024-10-30T16:26:18+00:00 fixup! Lox integration Bug 42597: Add note for missing trusted invites. - - - - - 3 changed files: - toolkit/components/lox/Lox.sys.mjs - toolkit/modules/DomainFrontedRequests.sys.mjs - toolkit/modules/Moat.sys.mjs Changes: ===================================== toolkit/components/lox/Lox.sys.mjs ===================================== @@ -37,6 +37,7 @@ XPCOMUtils.defineLazyModuleGetters(lazy, { set_panic_hook: "resource://gre/modules/lox_wasm.jsm", invitation_is_trusted: "resource://gre/modules/lox_wasm.jsm", issue_invite: "resource://gre/modules/lox_wasm.jsm", + handle_issue_invite: "resource://gre/modules/lox_wasm.jsm", prepare_invite: "resource://gre/modules/lox_wasm.jsm", get_invites_remaining: "resource://gre/modules/lox_wasm.jsm", get_trust_level: "resource://gre/modules/lox_wasm.jsm", @@ -91,6 +92,7 @@ const LoxSettingsPrefs = Object.freeze({ export class LoxError extends Error { static BadInvite = "BadInvite"; static LoxServerUnreachable = "LoxServerUnreachable"; + static ErrorResponse = "ErrorResponse"; /** * @param {string} message - The error message. @@ -408,89 +410,47 @@ class LoxImpl { } /** - * Update Lox credential after Lox key rotation - * Do not call directly, use #getPubKeys() instead to start the update only - * once + * Update Lox credential after Lox key rotation. * - * @param {string} prevkeys The public keys we are replacing + * Do not call directly, use #getPubKeys() instead to start the update only + * once. */ - async #updatePubkeys(prevkeys) { - let pubKeys; - try { - pubKeys = await this.#makeRequest("pubkeys", []); - } catch (error) { - lazy.logger.debug("Failed to get pubkeys", error); - // Make the next call try again. - this.#pubKeyPromise = null; - if (!this.#pubKeys) { - throw error; - } - return; - } + async #updatePubkeys() { + let pubKeys = await this.#makeRequest("pubkeys", null); const prevKeys = this.#pubKeys; if (prevKeys !== null) { // check if the lox pubkeys have changed and update the lox - // credentials if so - let lox_cred_req; - try { - lox_cred_req = JSON.parse( - lazy.check_lox_pubkeys_update( - JSON.stringify(pubKeys), - prevkeys, - this.#getCredentials(this.#activeLoxId) - ) - ); - } catch (error) { - lazy.logger.debug("Check lox pubkey update failed", error); - // Make the next call try again. - this.#pubKeyPromise = null; - return; - } - if (lox_cred_req.updated) { + // credentials if so. + // + // The UpdateCredOption rust struct serializes to "req" rather than + // "request". + const { updated, req: request } = JSON.parse( + lazy.check_lox_pubkeys_update( + pubKeys, + prevKeys, + this.#getCredentials(this.#activeLoxId) + ) + ); + if (updated) { + // Try update credentials. + // NOTE: This should be re-callable if any step fails. + // TODO: Verify this. lazy.logger.debug( `Lox pubkey updated, update Lox credential "${this.#activeLoxId}"` ); - let response; - try { - // TODO: If this call doesn't succeed due to a networking error, the Lox - // credential may be in an unusable state (spent but not updated) - // until this request can be completed successfully (and until Lox - // is refactored to send repeat responses: - // https://gitlab.torproject.org/tpo/anti-censorship/lox/-/issues/74) - response = await this.#makeRequest("updatecred", lox_cred_req.req); - } catch (error) { - lazy.logger.debug("Lox cred update failed.", error); - // Make the next call try again. - this.#pubKeyPromise = null; - return; - } - if (response.hasOwnProperty("error")) { - lazy.logger.error(response.error); - this.#pubKeyPromise = null; - lazy.logger.debug( - `Error response to Lox pubkey update request: "${response.error}", reverting to old pubkeys` - ); - return; - } - let cred; - try { - cred = lazy.handle_update_cred( - lox_cred_req.req, - JSON.stringify(response), - pubKeys - ); - } catch (error) { - lazy.logger.debug("Unable to handle updated Lox cred", error); - // Make the next call try again. - this.#pubKeyPromise = null; - return; - } + // TODO: If this call doesn't succeed due to a networking error, the Lox + // credential may be in an unusable state (spent but not updated) + // until this request can be completed successfully (and until Lox + // is refactored to send repeat responses: + // https://gitlab.torproject.org/tpo/anti-censorship/lox/-/issues/74) + let response = await this.#makeRequest("updatecred", request); + let cred = lazy.handle_update_cred(request, response, pubKeys); this.#changeCredentials(this.#activeLoxId, cred); } } // If we arrive here we haven't had other errors before, we can actually // store the new public key. - this.#pubKeys = JSON.stringify(pubKeys); + this.#pubKeys = pubKeys; this.#store(); } @@ -498,16 +458,24 @@ class LoxImpl { // FIXME: We are always refetching #pubKeys, #encTable and #constants once // per session, but they may change more frequently. tor-browser#42502 if (this.#pubKeyPromise === null) { - this.#pubKeyPromise = this.#updatePubkeys(); + this.#pubKeyPromise = this.#updatePubkeys().catch(error => { + lazy.logger.debug("Failed to update pubKeys", error); + // Try again with the next call. + this.#pubKeyPromise = null; + if (!this.#pubKeys) { + // Re-throw if we have no pubKeys value for the caller. + throw error; + } + }); } await this.#pubKeyPromise; } async #getEncTable() { if (this.#encTablePromise === null) { - this.#encTablePromise = this.#makeRequest("reachability", []) + this.#encTablePromise = this.#makeRequest("reachability", null) .then(encTable => { - this.#encTable = JSON.stringify(encTable); + this.#encTable = encTable; this.#store(); }) .catch(error => { @@ -526,10 +494,10 @@ class LoxImpl { async #getConstants() { if (this.#constantsPromise === null) { // Try to update first, but if that doesn't work fall back to stored data - this.#constantsPromise = this.#makeRequest("constants", []) + this.#constantsPromise = this.#makeRequest("constants", null) .then(constants => { const prevValue = this.#constants; - this.#constants = JSON.stringify(constants); + this.#constants = constants; this.#store(); if (prevValue !== this.#constants) { Services.obs.notifyObservers(null, LoxTopics.UpdateNextUnlock); @@ -579,7 +547,6 @@ class LoxImpl { */ async #backgroundTasks() { this.#assertInitialized(); - let addedEvent = false; // Only run background tasks for the active lox ID. const loxId = this.#activeLoxId; if (!loxId) { @@ -591,37 +558,39 @@ class LoxImpl { // this should catch key rotations (ideally some days) prior to the next // credential update await this.#getPubKeys(); + let levelup = false; try { - const levelup = await this.#attemptUpgrade(loxId); - if (levelup) { - const level = this.#getLevel(loxId); - const newEvent = { - type: "levelup", - newlevel: level, - }; - this.#events.push(newEvent); - this.#store(); - addedEvent = true; - } - } catch (err) { - lazy.logger.error(err); + levelup = await this.#attemptUpgrade(loxId); + } catch (error) { + lazy.logger.error(error); } + if (levelup) { + const level = this.#getLevel(loxId); + const newEvent = { + type: "levelup", + newlevel: level, + }; + this.#events.push(newEvent); + this.#store(); + } + + let leveldown = false; try { - const leveldown = await this.#blockageMigration(loxId); - if (leveldown) { - let level = this.#getLevel(loxId); - const newEvent = { - type: "blockage", - newlevel: level, - }; - this.#events.push(newEvent); - this.#store(); - addedEvent = true; - } - } catch (err) { - lazy.logger.error(err); + leveldown = await this.#blockageMigration(loxId); + } catch (error) { + lazy.logger.error(error); + } + if (leveldown) { + let level = this.#getLevel(loxId); + const newEvent = { + type: "blockage", + newlevel: level, + }; + this.#events.push(newEvent); + this.#store(); } - if (addedEvent) { + + if (levelup || leveldown) { Services.obs.notifyObservers(null, LoxTopics.UpdateEvents); } } @@ -708,7 +677,7 @@ class LoxImpl { // to issue open invitations for Lox bridges. async requestOpenInvite() { this.#assertInitialized(); - let invite = await this.#makeRequest("invite", []); + let invite = JSON.parse(await this.#makeRequest("invite", null)); lazy.logger.debug(invite); return invite; } @@ -725,23 +694,20 @@ class LoxImpl { // It's fine to get pubkey here without a delay since the user will not have a Lox // credential yet await this.#getPubKeys(); + // NOTE: We currently only handle "open invites". + // "trusted invites" are not yet supported. tor-browser#42974. let request = await lazy.open_invite(JSON.parse(invite).invite); - let response = await this.#makeRequest( - "openreq", - JSON.parse(request).request - ); - lazy.logger.debug("openreq response: ", response); - if (response.hasOwnProperty("error")) { - throw new LoxError( - `Error response to "openreq": ${response.error}`, - LoxError.BadInvite - ); + let response; + try { + response = await this.#makeRequest("openreq", request); + } catch (error) { + if (error instanceof LoxError && error.code === LoxError.ErrorResponse) { + throw new LoxError("Error response to openreq", LoxError.BadInvite); + } else { + throw error; + } } - let cred = lazy.handle_new_lox_credential( - request, - JSON.stringify(response), - this.#pubKeys - ); + let cred = lazy.handle_new_lox_credential(request, response, this.#pubKeys); // Generate an id that is not already in the #credentials map. let loxId; do { @@ -795,31 +761,32 @@ class LoxImpl { throw new LoxError(`Cannot generate invites at level ${level}`); } let request = lazy.issue_invite( - JSON.stringify(this.#getCredentials(loxId)), + this.#getCredentials(loxId), this.#encTable, this.#pubKeys ); - let response = await this.#makeRequest( - "issueinvite", - JSON.parse(request).request - ); - if (response.hasOwnProperty("error")) { - lazy.logger.error(response.error); - throw new LoxError(`Error response to "issueinvite": ${response.error}`); - } else { - const invite = lazy.prepare_invite(response); - this.#invites.push(invite); - // cap length of stored invites - if (this.#invites.len > 50) { - this.#invites.shift(); - } - this.#store(); - this.#changeCredentials(loxId, response); - Services.obs.notifyObservers(null, LoxTopics.NewInvite); - // Return a copy. - // Right now invite is just a string, but that might change in the future. - return structuredClone(invite); + let response = await this.#makeRequest("issueinvite", request); + // TODO: Do we ever expect handle_issue_invite to fail (beyond + // implementation bugs)? + // TODO: What happens if #pubkeys for `issue_invite` differs from the value + // when calling `handle_issue_invite`? Should we cache the value at the + // start of this method? + let cred = lazy.handle_issue_invite(request, response, this.#pubKeys); + + // Store the new credentials as a priority. + this.#changeCredentials(loxId, cred); + + const invite = lazy.prepare_invite(cred); + this.#invites.push(invite); + // cap length of stored invites + if (this.#invites.len > 50) { + this.#invites.shift(); } + this.#store(); + Services.obs.notifyObservers(null, LoxTopics.NewInvite); + // Return a copy. + // Right now invite is just a string, but that might change in the future. + return structuredClone(invite); } /** @@ -845,15 +812,13 @@ class LoxImpl { return false; } let response = await this.#makeRequest("checkblockage", request); - if (response.hasOwnProperty("error")) { - lazy.logger.error(response.error); - throw new LoxError( - `Error response to "checkblockage": ${response.error}` - ); - } + // NOTE: If a later method fails, we should be ok to re-call "checkblockage" + // from the Lox authority. So there shouldn't be any adverse side effects to + // loosing migrationCred. + // TODO: Confirm this is safe to lose. const migrationCred = lazy.handle_check_blockage( this.#getCredentials(loxId), - JSON.stringify(response) + response ); request = lazy.blockage_migration( this.#getCredentials(loxId), @@ -861,26 +826,21 @@ class LoxImpl { this.#pubKeys ); response = await this.#makeRequest("blockagemigration", request); - if (response.hasOwnProperty("error")) { - lazy.logger.error(response.error); - throw new LoxError( - `Error response to "blockagemigration": ${response.error}` - ); - } const cred = lazy.handle_blockage_migration( this.#getCredentials(loxId), - JSON.stringify(response), + response, this.#pubKeys ); this.#changeCredentials(loxId, cred); return true; } - /** Attempts to upgrade the currently saved Lox credential. - * If an upgrade is available, save an event in the event list. + /** + * Attempts to upgrade the currently saved Lox credential. + * If an upgrade is available, save an event in the event list. * - * @param {string} loxId Lox ID - * @returns {boolean} Whether a levelup event occurred. + * @param {string} loxId Lox ID + * @returns {boolean} Whether the credential was successfully migrated. */ async #attemptUpgrade(loxId) { await this.#getEncTable(); @@ -895,16 +855,18 @@ class LoxImpl { this.#encTable, this.#pubKeys ); - const response = await this.#makeRequest("levelup", request); - if (response.hasOwnProperty("error")) { - lazy.logger.error(response.error); - throw new LoxError(`Error response to "levelup": ${response.error}`); + let response; + try { + response = await this.#makeRequest("levelup", request); + } catch (error) { + if (error instanceof LoxError && error.code === LoxError.ErrorResponse) { + // Not an error. + lazy.logger.debug("Not ready for level up", error); + return false; + } + throw error; } - const cred = lazy.handle_level_up( - request, - JSON.stringify(response), - this.#pubKeys - ); + const cred = lazy.handle_level_up(request, response, this.#pubKeys); this.#changeCredentials(loxId, cred); return true; } @@ -922,76 +884,40 @@ class LoxImpl { this.#getPubKeys(); return false; } - let request, response; + let request; try { request = lazy.trust_promotion( this.#getCredentials(loxId), this.#pubKeys ); } catch (err) { + // This function is called routinely during the background tasks without + // previous checks on whether an upgrade is possible, so it is expected to + // fail with a certain frequency. Therefore, do not relay the error to the + // caller and just log the message for debugging. lazy.logger.debug("Not ready to upgrade", err); return false; } - try { - response = await this.#makeRequest( - "trustpromo", - JSON.parse(request).request - ); - } catch (err) { - lazy.logger.error("Failed trust promotion", err); - return false; - } - if (response.hasOwnProperty("error")) { - lazy.logger.error("Error response from trustpromo", response.error); - return false; - } - lazy.logger.debug("Got promotion cred", response, request); - let promoCred; - try { - promoCred = lazy.handle_trust_promotion( - request, - JSON.stringify(response) - ); - lazy.logger.debug("Formatted promotion cred"); - } catch (err) { - lazy.logger.error( - "Unable to handle trustpromo response properly", - response.error - ); - return false; - } - try { - request = lazy.trust_migration( - this.#getCredentials(loxId), - promoCred, - this.#pubKeys - ); - lazy.logger.debug("Formatted migration request"); - } catch (err) { - lazy.logger.error("Failed to generate trust migration request", err); - return false; - } - try { - response = await this.#makeRequest( - "trustmig", - JSON.parse(request).request - ); - } catch (err) { - lazy.logger.error("Failed trust migration", err); - return false; - } - if (response.hasOwnProperty("error")) { - lazy.logger.error("Error response from trustmig", response.error); - return false; - } - lazy.logger.debug("Got new credential"); - let cred; - try { - cred = lazy.handle_trust_migration(request, response); - } catch (err) { - lazy.logger.error("Failed to handle response from trustmig", err); - return false; - } + + let response = await this.#makeRequest("trustpromo", request); + // FIXME: Store response to "trustpromo" in case handle_trust_promotion + // or "trustmig" fails. The Lox authority will not accept a re-request + // to "trustpromo" with the same credentials. + let promoCred = lazy.handle_trust_promotion(request, response); + lazy.logger.debug("Formatted promotion cred: ", promoCred); + + request = lazy.trust_migration( + this.#getCredentials(loxId), + promoCred, + this.#pubKeys + ); + response = await this.#makeRequest("trustmig", request); + lazy.logger.debug("Got new credential: ", response); + + // FIXME: Store response to "trustmig" in case handle_trust_migration + // fails. The Lox authority will not accept a re-request to "trustmig" with + // the same credentials. + let cred = lazy.handle_trust_migration(request, response); this.#changeCredentials(loxId, cred); return true; } @@ -1079,38 +1005,47 @@ class LoxImpl { }; } - async #makeRequest(procedure, args) { + /** + * Fetch from the Lox authority. + * + * @param {string} procedure - The request endpoint. + * @param {string} body - The arguments to send in the body, if any. + * + * @returns {string} - The response body. + */ + async #fetch(procedure, body) { // TODO: Customize to for Lox - const serviceUrl = "https://lox.torproject.org"; - const url = `${serviceUrl}/${procedure}`; + const url = `https://lox.torproject.org/${procedure}`; + const method = "POST"; + const contentType = "application/vnd.api+json"; if (lazy.TorConnect.state === lazy.TorConnectState.Bootstrapped) { let request; try { request = await fetch(url, { - method: "POST", - headers: { - "Content-Type": "application/vnd.api+json", - }, - body: JSON.stringify(args), + method, + headers: { "Content-Type": contentType }, + body, }); } catch (error) { - lazy.logger.debug("fetch fail", url, args, error); + lazy.logger.debug("fetch fail", url, body, error); throw new LoxError( `fetch "${procedure}" from Lox authority failed: ${error?.message}`, LoxError.LoxServerUnreachable ); } if (!request.ok) { - lazy.logger.debug("fetch response", url, args, request); + lazy.logger.debug("fetch response", url, body, request); // Do not treat as a LoxServerUnreachable type. throw new LoxError( `Lox authority responded to "${procedure}" with ${request.status}: ${request.statusText}` ); } - return request.json(); + return request.text(); } + // TODO: Only make domain fronted requests with user permission. + // tor-browser#42606. if (this.#domainFrontedRequests === null) { this.#domainFrontedRequests = new Promise((resolve, reject) => { // TODO: Customize to the values for Lox @@ -1129,9 +1064,9 @@ class LoxImpl { } const builder = await this.#domainFrontedRequests; try { - return await builder.buildPostRequest(url, args); + return await builder.buildRequest(url, { method, contentType, body }); } catch (error) { - lazy.logger.debug("Domain front request fail", url, args, error); + lazy.logger.debug("Domain front request fail", url, body, error); if (error instanceof lazy.DomainFrontRequestNetworkError) { throw new LoxError( `Domain front fetch "${procedure}" from Lox authority failed: ${error?.message}`, @@ -1149,6 +1084,37 @@ class LoxImpl { ); } } + + /** + * Make a request to the lox authority, check for an error response, and + * convert it to a string. + * + * @param {string} procedure - The request endpoint. + * @param {?string} request - The request data, as a JSON string containing a + * "request" field. Or `null` to send no data. + * + * @returns {string} - The stringified JSON response. + */ + async #makeRequest(procedure, request) { + // Verify that the response is valid json, by parsing. + const jsonResponse = JSON.parse( + await this.#fetch( + procedure, + request ? JSON.stringify(JSON.parse(request).request) : "" + ) + ); + lazy.logger.debug(`${procedure} response:`, jsonResponse); + if (Object.hasOwn(jsonResponse, "error")) { + // TODO: Figure out if any of the "error" responses should be treated as + // an error. I.e. which of the procedures have soft failures and hard + // failures. + throw LoxError( + `Error response to ${procedure}: ${jsonResponse.error}`, + LoxError.ErrorResponse + ); + } + return JSON.stringify(jsonResponse); + } } export const Lox = new LoxImpl(); ===================================== toolkit/modules/DomainFrontedRequests.sys.mjs ===================================== @@ -523,34 +523,31 @@ export class DomainFrontRequestBuilder { } /** - * Make a POST request with a JSON body and a JSON response. + * Make a request. * - * @param {string} url The URL to load - * @param {object} args The arguments to send to the procedure. It will be - * serialized to JSON by this function and then set as POST body - * @returns {Promise<object>} A promise with the parsed response + * @param {string} url The URL to request. + * @param {object} args The arguments to send to the procedure. + * @param {string} args.method The request method. + * @param {string} args.body The request body. + * @param {string} args.contentType The "Content-Type" header to set. + * @returns {string} The response body. */ - async buildPostRequest(url, args) { + async buildRequest(url, args) { const ch = this.buildHttpHandler(url); - const argsJson = JSON.stringify(args); const inStream = Cc["@mozilla.org/io/string-input-stream;1"].createInstance( Ci.nsIStringInputStream ); - inStream.setData(argsJson, argsJson.length); + inStream.setData(args.body, args.body.length); const upChannel = ch.QueryInterface(Ci.nsIUploadChannel); - const contentType = "application/vnd.api+json"; - upChannel.setUploadStream(inStream, contentType, argsJson.length); - ch.requestMethod = "POST"; + upChannel.setUploadStream(inStream, args.contentType, args.body.length); + ch.requestMethod = args.method; // Make request const listener = new ResponseListener(); await ch.asyncOpen(listener, ch); // wait for response - const responseJSON = await listener.response(); - - // parse that JSON - return JSON.parse(responseJSON); + return listener.response(); } } ===================================== toolkit/modules/Moat.sys.mjs ===================================== @@ -108,7 +108,13 @@ export class MoatRPC { const procedureURIString = `${Services.prefs.getStringPref( TorLauncherPrefs.moat_service )}/${procedure}`; - return this.#requestBuilder.buildPostRequest(procedureURIString, args); + return JSON.parse( + await this.#requestBuilder.buildRequest(procedureURIString, { + method: "POST", + contentType: "application/vnd.api+json", + body: JSON.stringify(args), + }) + ); } async testInternetConnection() { View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/5e9deb4... -- View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/5e9deb4... You're receiving this email because of your account on gitlab.torproject.org.
participants (1)
-
Pier Angelo Vendrame (＠pierov)