Closed Bug 1598478 Opened 5 years ago Closed 5 years ago

import, initial integration and more RNP bindings for: key manager, key details, key generation, am-prefs

Categories

(MailNews Core :: Security: OpenPGP, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 72.0

People

(Reporter: KaiE, Assigned: KaiE)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

I'll import the following additional files, as already approved by Patrick in bug 1594855:

strings/am-enigprefs.properties
ui/am-enigprefs.js
ui/enigmailEditIdentity.js
ui/enigmailEditIdentity.xul
ui/enigmailKeygen.js
ui/enigmailKeygen.xul
ui/enigmailMsgBox.js
ui/enigmailMsgBox.xul
ui/keyDetailsDlg.js
ui/keyDetailsDlg.xul

Also, I'd like to land changes, that already enable a subset of the functionality:

  • use keyring files in profile directory
  • key manager list is filled with keys from the RNP keyring
  • view details of a key
  • create a new key
  • load account manager prefs (not sure how much we will reuse, but as a starting point)
Attached patch 1598478-v1.patch (obsolete) (deleted) — Splinter Review
Assignee: nobody → kaie
Attached patch 1598478-v1-no-ws.patch (obsolete) (deleted) — Splinter Review

same patch, but ignoring whitespace changes.

Patrick, I recommend to look at this patch to see the changes I've made.
(Whitespace will get cleaned up later anyway.)

I've removed the overlay loading code from am-enigprefs.js.
Instead, I moved the xul code from enigmailEditIdentity directly into that am-enigprefs.xul.

I'm not yet ready to remove code that I might potentially need, such as the listener for background key generation. (Right now this code will do it in the foreground.) That's why I have commented out the listener. But my editor misinterpreted / inside a regular expression as a comment ending, that's why I temporarily have that "remove-space-between--and-/-to-unconfuse-syntax-highlighting-editor" text. I will all be cleaned up later.

Blocks: 1595318
Attachment #9110683 - Flags: review?(patrick)

The base code for this bug was imported here:
https://hg.mozilla.org/comm-central/rev/f0114d8e2104

The attached patch applies on top of that changeset.

Comment on attachment 9110683 [details] [diff] [review] 1598478-v1.patch in rnp.jsm: + let keyObj = {}; Do you also do `new EnigmailKeyObj(keyObj)` such that you have a proper key object? in enigmailKeygenStart() you do: ```javascript + RNP.genKey(idString, keyType, keySize, expiryTime, passphrase); + RNP.saveKeyRings(); ``` I think it would be better to always go via cryptoAPI.genKey, and not use RNP directly.

(In reply to Patrick Brunschwig from comment #4)

in rnp.jsm:

  • let keyObj = {};

Do you also do new EnigmailKeyObj(keyObj) such that you have a proper key
object?

I assumed I don't need to.

This new code is doing the same as the existing code in gnupg-keylist.jsm, which also creates a subset of the attributes, only:
https://searchfox.org/comm-central/rev/a7d599e6422ad3053384502bf1555e64a41c517e/mail/extensions/openpgp/content/modules/cryptoAPI/gnupg-keylist.jsm#208

The statement "let keyObj = {};" is in the implementation of CryptoAPI.getKeys().

The only place that calls getKeys() (for both GnuPG and RNP) is in keyRing.loadKeyList(), which will always call createAndSortKeyList() on the result data, which then calls newEnigmailKeyObj on the result data. Is that sufficient?

(In reply to Patrick Brunschwig from comment #4)

I think it would be better to always go via cryptoAPI.genKey, and not use
RNP directly.

enigmailKeygenStart() called EnigmailKeyRing.generateKey(), which executes the gnupg key generation commands directly, without going through cryptoAPI.

cryptoAPI doesn't have a genKey method yet. I can add one, and implement it for RNP, only. (Because we likely won't use GnuPG for key generation.)

Attached patch 1598478-inc-v2.patch (deleted) — Splinter Review

incremental patch on top of v1

Attached patch 1598478-v2.patch (deleted) — Splinter Review
Attachment #9110683 - Attachment is obsolete: true
Attachment #9110686 - Attachment is obsolete: true
Attachment #9110683 - Flags: review?(patrick)
Attachment #9111151 - Flags: review?(patrick)

Note that I'm not yet trying to clean up, and simply leave existing gnupg code mostly untouched. My current focus is on getting functionality working, as I'm interested to get clarity about the usability of RNP as soon as possible.

Attached patch 1598478-pass-revok.patch (deleted) — Splinter Review

Some additional related changes. Incremental patch to correctly handle the passphrase. Must set it for the subkey, too. Must unlock the primary key while generating the subkey. And get revocation status for key manager. (I'm not yet sure about treatment of other trust flags, that's for later.)

Attachment #9111165 - Flags: review?(patrick)
Attachment #9111165 - Flags: review?(patrick) → review+
Attachment #9111151 - Flags: review?(patrick) → review+

Pushed by kaie@kuix.de:
https://hg.mozilla.org/comm-central/rev/4213d50b5bfc
OpenPGP, initial integration and RNP bindings for: key manager, key details, key generation, am-prefs. r=patrick DONTBUILD

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

Attachment

General

Created:
Updated:
Size: