Closed Bug 1603519 Opened 5 years ago Closed 5 years ago

Initial work for sending encrypted OpenPGP mail

Categories

(MailNews Core :: Security: OpenPGP, enhancement)

Product:
MailNews Core
Component:
Security: OpenPGP
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 74.0

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

1603519-loader.patch
(deleted), patch
mkmelin
: review+
Details | Diff | Splinter Review
1603519-v2.patch
(deleted), patch
KaiE
: review+
Details | Diff | Splinter Review

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.

Blocks: 1595318
Attached patch 1603519-loader.patch (deleted) — — Splinter Review
Assignee: nobody → kaie
Attachment #9115557 - Flags: review?(mkmelin+mozilla)
Attached patch 1603519-v1.patch (obsolete) (deleted) — — Splinter Review
Attachment #9115558 - Flags: review?(patrick)

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.

Attachment #9115557 - Flags: review?(mkmelin+mozilla) → review+
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?
Blocks: 1603774
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.
Attachment #9115558 - Flags: review?(patrick) → review+

(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.

Attached patch 1603519-v2.patch (deleted) — — Splinter Review

updated to use console.debug as requested

Attachment #9115558 - Attachment is obsolete: true
Attachment #9122305 - Flags: review+

Pushed by kaie@kuix.de:
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

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 74.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: