diff --git a/P4TB-43/README.md b/P4TB-43/README.md index e7846c2..1acbbc7 100644 --- a/P4TB-43/README.md +++ b/P4TB-43/README.md @@ -7,7 +7,7 @@ This is one of the fundamental areas where we want to extend Thunderbird. We want to be able to transform a received message in order to decrypt it and perform other operations if needed. -## Short answer +## Main documentation This https://thunderbird.topicbox.com/groups/addons/Tefecd2c30eac581d question to the Thunderbird's addon dev community has been important @@ -16,10 +16,10 @@ search the question from its title "Transforming an incoming message through its MIME representation (multipart/encrypted)". Developing for the decryption workflow can be a bit tricky because the -logic is encapsulated in a factory that is called by Thuderbird when -we click on a `multipart/encrypted` message. For most of the tests i -used a prototype of the `onPgpMime` function listed in the question -above. +logic is encapsulated in a component that is created by a Thuderbird's +factory when we click on a `multipart/encrypted` message. For most of +the tests i used a prototype of the `onPgpMime` function listed in the +question above. I would call `onPgpMime` with functions that returned specific messages, so that i could unregister and try again from the command @@ -27,6 +27,12 @@ line without reinstalling the plugin. Towards the end when it was more stable i just called `pEpController.addFactories/removeFactories`, which is slower. +### Links + +- `README.md` you are reading this +- `decryption-control-flow-in-enigmail.md` control flow for PGP/MIME in Enigmail, which led to the solution we are actually adopting +- `from-enigmail-to-pep.md` why Enigmail's solution is not enough for us and how our current approach differs + ## Other contents The investigation for P4TB-43 was long and i took some notes while @@ -41,11 +47,9 @@ related to PGP/Inline is larger, it's the first i found and the first i tried to use. The code related to PGP/MIME contains the solution we are using now. -### Index +### Links -- `README.md` you are reading this - `thunderbird.txt` i started from analysing Thunderbird's code, then i moved to Enigmail - `style-and-markup.md` initial search to figure out whether Enigmail is duplicating Thunderbird's markup for attachments - `enigmailMessengerOverlay.md` notes about PGP/Inline code in Enigmail. This involves a set of solutions we eventually discarded -- `decryption-control-flow-in-enigmail.md` control flow for PGP/MIME in Enigmail, which led to the solution we are actually adopting - `open-enigmail-questions.md` some open directions for deepening how PGP/MIME currently works in Enigmail diff --git a/P4TB-43/from-enigmail-to-pep.md b/P4TB-43/from-enigmail-to-pep.md new file mode 100644 index 0000000..b6a7873 --- /dev/null +++ b/P4TB-43/from-enigmail-to-pep.md @@ -0,0 +1,42 @@ + +# Enigmail's solution was not a solution for pEp + +In our case getting the multipart body is not enough, we want to pass +the whole message to the engine as it's written in the topicbox +question above. Getting the whole message is not easy in the +component, as the URI we get there does not work with the most common +methods lise nsIMessenger.msgHdrFromURI. + +Another problem with this approach is that after decrypting the +message we want to use its contents to modify parts of the view, and +this is not possible from the component interface. + +The component can only return a different MIME message that will be +parsed by the mime proxy and showed by the interface. While the +component handler is executed, there is no way to refer to the +thunderbird window. + +## Duplicated decryption call + +If the component runs in isolation from the view then we need to +decrypt the message twice, once for passing the decrypted attachments +as a result of the component handler and a second time for updating +the parts of the reading window with the contents coming from the +decrypted message. + +## Current solution + +We want to connect the decryption handler component with the view +scope for the reasons explained above. Such a connection is based +upon some assumptions, for instance we assume that the view will +always be called after the component in the case of decryption (for +encryption, the opposite might be true). + +The decryption handler is asynchronous and experiments with +Thunderbird 68.4 show that we can postpone the resolution of the +handler indefinitely, so we can wait for a promise to be resolved by +the view. + +This is the reason why the component control flow is depending on the +view control flow. Every time the decryption component is created it +adds an handler that can be called by the reading view. diff --git a/chrome/content/p4tb.js b/chrome/content/p4tb.js index 73a52dd..38ae5b3 100644 --- a/chrome/content/p4tb.js +++ b/chrome/content/p4tb.js @@ -139,15 +139,43 @@ pEpController.messageToCompFields = (pEpMessage, compFields) => { pEpController.mimeFromMessage = emitter.fromMessage.bind(emitter); -pEpController.addFactories = init; -pEpController.removeFactories = shutdown; - -async function decryptionHandler (partialMime, boundary, fromAddress) { - let mime = pEpMimeDecrypt.addWrapper(partialMime, boundary); - let decoded = await pEpController.messageFromMIME(mime); - decoded.from = new pEp.Identity(fromAddress); - let [innerMessage, innerMIMEMessage] = await decryptFromMessage(decoded); - return innerMIMEMessage; +pEpController.factories = (() => { + let registered = {}; + let decrypt = pEpMimeDecrypt.init(); + return { + register: () => { + /* + + pEpMimeEncrypt is stateless, and the encryption view + communicates with it through message header properties + + */ + registered.encryption = new Factory(pEpMimeEncrypt.Handler); + /* + + the `decrypt` object created by pEpMimeDecrypt holds a + promise to the last decryption component that was + created, which is waiting for contents from a view + + */ + registered.decryption = new Factory(decrypt.component); + }, + unregister: () => { + Object + .values(registered) + .map(factory => factory.unregister()); + registered = {}; + }, + resolveDecryptComponent: decrypt.resolveComponent.bind(decrypt) + } +})() + +function init () { + pEpController.factories.register(); +} + +function shutdown () { + pEpController.factories.unregister(); } async function decryptFromMessage (message) { @@ -177,21 +205,6 @@ async function decryptFromMessage (message) { return [innerMessage, innerMIMEMessage]; } -let factories = []; - -function init () { - factories = [ - new Factory(pEpMimeEncrypt.Handler), - new Factory(pEpMimeDecrypt.makeComponent(decryptionHandler)) - ]; - return factories; -} - -function shutdown () { - factories.map(factory=>factory.unregister()); - factories = []; -} - Cm.QueryInterface(Ci.nsIComponentRegistrar); class Factory { diff --git a/chrome/content/pEpMimeDecrypt.js b/chrome/content/pEpMimeDecrypt.js index 61f7c59..c80292c 100644 --- a/chrome/content/pEpMimeDecrypt.js +++ b/chrome/content/pEpMimeDecrypt.js @@ -1,8 +1,16 @@ -function makeComponent (funct) { +function makeComponent (waitView) { let component = class { - onStartRequest (request, ctxt) { + constructor () { + // a new component is initialised for every new request, + // when users click on a received message in the messenger + // view. Every component handles only one request, that + // means a sequence of one call to `onStartRequest`, one + // or more calls to `onDataAvailable` and a final call to + // `onStopRequest` this.data = ""; + } + onStartRequest (request/*, ctxt*/) { this.decoder = request .QueryInterface(Components.interfaces.nsIPgpMimeProxy); this.boundary = getBoundary(request.contentType); @@ -22,20 +30,44 @@ function makeComponent (funct) { return reader.read(count); } } - onStopRequest (request, status) { - let messenger = Components.classes["@mozilla.org/messenger;1"] - .createInstance(Components.interfaces.nsIMessenger), - uri = request.messageURI.spec, // alternatively decoder.messageURI - header = messenger.msgHdrFromURI(uri), - fromAddress = "test@test.com"; - // having an `async onStopRequest` would crash Thunderbird - // so we use `synchronise` on `funct` which is async - let promise = funct(this.data, this.boundary, fromAddress); + onStopRequest (/*request, status*/) { + let mime = addWrapper(this.data, this.boundary); + /* + + Decryption view-component tunnel, component + end. `waitView` gives us a promise to wait until + `resolveComponent` receives a function that returns a + MIME message. The message can be passed to the MIME + proxy's outputDecryptedData in order to trigger updates + in the user interface like showing attachments or + updating the body + + */ + let promise = waitView(mime); + /* + + Having an `async onStopRequest` would crash + Thunderbird. Maybe if we allow `onStopRequest` to + terminate, some objects are deleted and we cannot call + `decoder` any longer. So we use `synchronise` to pause + the execution of `onStopRequest` until the promise is + finalised + + */ let decoded = synchronise(promise); - // TB >= 57, for other versions see the body of - // MimeDecryptHandler.returnData - // https://gitlab.com/enigmail/enigmail/blob/master/package/mimeDecrypt.jsm#L702-706 - this.decoder.outputDecryptedData(decoded, decoded.length); + if (decoded) { + /* + + This way to interface with the proxy decoder works + for thunderbird >= 57, for other versions see the + body of MimeDecryptHandler.returnData + https://gitlab.com/enigmail/enigmail/blob/master/package/mimeDecrypt.jsm#L702-706 + + */ + this.decoder.outputDecryptedData(decoded, decoded.length); + } else { + console.log("this decryption component received no data from the view"); + } function synchronise (promise) { let inspector = Cc["@mozilla.org/jsinspector;1"].createInstance(Ci.nsIJSInspector), synchronous, @@ -43,6 +75,7 @@ function makeComponent (funct) { update = asynchronous => synchronous = asynchronous; promise .then(update) + .catch(console.log.bind(console)) .finally(unlock); // wait here for the promise to resolve inspector.enterNestedEventLoop(0); @@ -69,9 +102,117 @@ function addWrapper (decryptedData, boundary) { decryptedData; } +function promiseWithHandlers () { + let resolve, + reject, + promise = new Promise((resolve_, reject_) => { + resolve = resolve_; + reject = reject_; + }); + return [promise, resolve, reject]; +} + +function init () { + /* + + We need state to synchronise every component invocation + with the eventual view rendering. + + As we assume that our view is rendered once after every + component call, we could just store the last handler to be + called. + + Having a queue helps us in detecting unexpected timing + conditions. With current logic, this queue can either be empty + or contain one promise. + + */ + let waiting = false, + resolve, + reject, + lastArguments, + component = makeComponent(waitView); + return { component, resolveComponent }; + function waitView (...someArguments) { + let promise; + if (waiting) { + console.log("warning: a decryption component was waiting for a view and it's being discarded"); + /* + + Several components have been waiting for + rendering, each one expecting some data for + passing it to their mime decoder proxy. + + We reject former component promises in order not + to leave hanging threads. These rejections are + handled in component.onStopRequest synchronise + with a simple console.log + + */ + reject("another component was initialised after this one"); + waiting = false; + } + waiting = true; + lastArguments = someArguments; + [promise, resolve, reject] = promiseWithHandlers(); + return promise; + } + function resolveComponent (handler) { + if (!handler) { + /* + + This branch is useful for troubleshooting. By calling + `pEpController.factories.resolveDecryptComponent();` + without arguments in the view, you can expect the user + interface to show an encrypted message's parts as body + and attachments. + + The other code here in `init` handles the arguments in a + generic way so it can adapt to cases where waitView's + arguments will change. Here in this fallback handler we + use the current actual arguments for simplicity. + + */ + handler = (encryptedMime) => { + return Promise.resolve(encryptedMime); + } + } + if (waiting) { + resolve(handler(...lastArguments)); + waiting = false; + } else { + /* + + There are no components waiting for the view, we + cannot render this part of the message. Maybe the + view that called `resolveComponent` is not the right + one. + + When users click on a received message in a window with + location.href == + 'chrome://messenger/content/messenger.xhtml', we can + see that a decryption component is called before the + view initialisation. In the other cases we get no + guarantees about the timing. + + We pass through this branch also when visiting cached + messages through the view. In that case the decryption + component gets called only the first time users visit + the messages + + */ + console.log("no decryption component waiting to be resolved"); + } + } +} + var pEpMimeDecrypt = { - makeComponent, - addWrapper + // `init` is the main interface, currently used in p4tb.js + init, + // `makeComponent` is less complex than `init`. It can help + // testing and troubleshooting and it's used in the runtime + // experiments + makeComponent }; const EXPORTED_SYMBOLS = ["pEpMimeDecrypt"]; diff --git a/chrome/content/pepmsghdrview.js b/chrome/content/pepmsghdrview.js index 424e29b..b9be894 100644 --- a/chrome/content/pepmsghdrview.js +++ b/chrome/content/pepmsghdrview.js @@ -129,13 +129,13 @@ var pEpHdrView = { init: function () { console.debug("pEpHdrView: init()", this, window); this.onLoadListener(); - pEpController.addFactories(); + pEpController.factories.register(); }, destroy: function () { console.debug("pEpHdrView: destroy()"); ColumnOverlay.destroy(); - pEpController.removeFactories(); + pEpController.factories.unregister(); delete window.pEpHdrView; delete window.MessageView; delete window.Helper; @@ -293,11 +293,35 @@ var pEpHdrView = { // Store decrypted copy if possible storeDecryptedCopy(msgHdr, decryptedMIMEMessage); - }).catch(err => { console.error("pepmsghdrview.js: onLoadMsgPanelFrameListener(): getMimeDecodedMessage", err) }); + /* + + Decryption view-component tunnel, view end. Decryption + components wait for a view. Here from the view we pass a + function that returns a MIME message, so that the last + component can pass the message to Thunderbird's MIME proxy + service. + + */ + pEpController.factories.resolveDecryptComponent(); + + async function decryptionHandler (mime) { + // When ticket #204 will be closed there will be no need + // to call the engine twice here. We can just return one + // of the decrypted messages in the scope and ignore the + // arguments. Also, this `decryptionHandler` function and + // the call to `resolveDecryptComponent` can be moved + // within the `messageFromMIME(...).then(...)` branch + // above + let decoded = await pEpController.messageFromMIME(mime); + decoded.longmsg = atob(decoded.attachments[1].value); + let result = await pEpController.decryptMailWithMessage(decoded); + return ""; + return result.longmsg; + } } }; diff --git a/tests/runtime/pepmsghdrview.js b/tests/runtime/pepmsghdrview.js index 14f9170..2952238 100644 --- a/tests/runtime/pepmsghdrview.js +++ b/tests/runtime/pepmsghdrview.js @@ -28,16 +28,25 @@ let tests = [ } }, () => { + var [promise, resolve, reject] = pEpMimeDecrypt.promiseWithHandlers(); + function handler (mime) { + return promise.then(()=>mime); + } var component = pEpMimeDecrypt.makeComponent(handler); - new pEpController.Factory(component); // this also registers the factory - console.log("a new decryption component has been registered, click on a multipart/encrypted message to trigger the handler invocation"); + var factory = new pEpController.Factory(component); + /* + + A new decryption component has been registered, click on a + multipart/encrypted message to trigger the component + creation. The component will pause waiting for the + handler. Call `resolve()` to finalise the component's + lifecycle and see an encrypted message in the user + interface. In the extension code we will decrypt the + message in the handler + + */ + factory.unregister(); return true; - function handler (data, boundary, fromAddress) { - console.log("decryption component handler called with arguments:"); - console.log(arguments); - let mime = pEpMimeDecrypt.addWrapper(data, boundary); - return Promise.resolve(mime); - } } ];