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

Status: RESOLVED FIXED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[Kai Engert (:KaiE:)]

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)

Comment 1

[Kai Engert (:KaiE:)]

Attachment: 1598478-v1.patch

Comment 2

[Kai Engert (:KaiE:)]

Attachment: 1598478-v1-no-ws.patch

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.

Comment 3

[Kai Engert (:KaiE:)]

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 4

[Patrick Brunschwig]

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.

Comment 5

[Kai Engert (:KaiE:)]

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

Comment 6

[Kai Engert (:KaiE:)]

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

Comment 7

[Kai Engert (:KaiE:)]

Attachment: 1598478-inc-v2.patch

incremental patch on top of v1

Comment 8

[Kai Engert (:KaiE:)]

Attachment: 1598478-v2.patch

Comment 9

[Kai Engert (:KaiE:)]

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.

Comment 10

[Kai Engert (:KaiE:)]

Attachment: 1598478-pass-revok.patch

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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