1603519 - Initial work for sending encrypted OpenPGP mail

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert [:KaiE:]]

I'd like to land initial code that hooks up the backend encryption code, and adds the necessary RNP bindings.

Because we don't have UI hooked up yet, my initial patch forces to always send encrypted, if the MOZ_OPENPGP build option is enabled. If keys for all recipients cannot be found automatically, sending will fail.

Comment 1

[Kai Engert [:KaiE:]]

Attachment: 1603519-loader.patch

Comment 2

[Kai Engert [:KaiE:]]

Attachment: 1603519-v1.patch

Comment 3

[Kai Engert [:KaiE:]]

This fixes a bug in keyObj.jsm, in both getSigningValidity() and getEncryptionValidity(), which I mentioned to Patrick (elsewhere).

If RNP imports a key, which has signatures on it, but the signing keys are unknown (not present in key store), then RNP will treat those keys as invalid, and will reject to encrypt with them. This is an acknowledged bug, and I assume we'll get a fix for that. Until then, only experiment using cleaned up keys, which only have a self-signature.
(Tracked upstream at: https://github.com/rnpgp/rnp/issues/1001 )

Patch enables RNP library error logging to stderr.

The patch loads the enigmail code from enigmailMsgComposeHelper.js and enigmailMsgComposeOverlay.js, however the dynamic loading of some UI overlays is disabled.

Also code that attempts to update UI status elements is disabled.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9115558 [details] [diff] [review]
1603519-v1.patch

Review of attachment 9115558 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/extensions/openpgp/content/modules/rnp.jsm
@@ +738,5 @@
> +    resultStatus.statusMsg = "";
> +    resultStatus.errorMsg = "";
> +
> +console.log("encryptAndOrSign, plaintext length: " + plaintext.length);
> +console.log(plaintext);

The patch has a lot of this kind of debug leftovers. I think it would be nice to clean them up a bit before landing. 

E.g. here just do
  console.debug(`Will encrypt and/or sign; plaintext=${plaintext}`);

@@ +842,5 @@
> +
> +      // TODO decide if our compatibility requirements allow us to
> +      // use AEAD
> +      if (RNPLib.rnp_op_encrypt_set_cipher(op, "AES256")) {
> +        throw "rnp_op_encrypt_set_cipher failed";

shouldn't throw string. Only throw new Error("foo") if needed.

But shouldn't this just log something, and return null?

Comment 5

[Patrick Brunschwig]

Comment on attachment 9115558 [details] [diff] [review]
1603519-v1.patch

I agree with Magnus' comments about console.log. Either use console.debug(), or if you want to stay in line with the rest of the code, use EnigmailLog.DEBUG().

Apart from this, the patch looks fine to me.

Comment 6

[Kai Engert [:KaiE:]]

(In reply to Patrick Brunschwig from comment #5)

if you want to stay in line with the rest of the code, use
EnigmailLog.DEBUG().

I'm using console output also as a way to learn about the flow of the enigmail code, and the involved objects.
Only console functions allow me to dump objects, and allow me to click/expand their structure, which is why I currently prefer to use that.

Using console.debug is a good suggestion.

Comment 7

[Kai Engert [:KaiE:]]

Attachment: 1603519-v2.patch

updated to use console.debug as requested

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0767ec016375
Initial work for sending encrypted OpenPGP mail, loader code. r=mkmelin DONTBUILD
https://hg.mozilla.org/comm-central/rev/64408ab633c4
Initial work for sending encrypted OpenPGP mail. r=patrick DONTBUILD