[Kde-pim] [Differential] [Updated] D2324: Introduce Kleo::KeySelectionCombo class

Tue Aug 2 10:35:56 BST 2016

aheinecke added a comment.

  Hi,

  This Widget has much in common with the Key Selection for Signing / Encrypt to Self I need in the new Sigencfiles Dialog (which I should finally finish up and commit). I've added a very thin combobox for this in Kleo:

  https://quickgit.kde.org/?p=kleopatra.git&a=blob&h=a9fac6ea81af7801f6aa2d1c6619ead7173eb41d&hb=e9a2415c5d94614aa8c5ef0b1720358eda4b2a1f&f=src%2Fcrypto%2Fgui%2Fcertificatecombobox.cpp

  That Combobox uses the Keylistmodel from Kleopatra so that (configurable) presentation etc. is the same. But this is the big problem why I've done this in Kleopatra and not Libkleo as the Keylistmodel is neccessary, too. The Keylistmodel in Kleopatra is useful for widgets working on keys so we probably have to move more of that stuff into Libkleo if we want to have it as a good "Crypto Widget" Library.

  I have time for Kleo today so I'll see what I can do about this and ask you for a review of it later.

INLINE COMMENTS

> keyselectioncombo.cpp:161
> +            // no API for that. Would be nice to have a unified key generation UI
> +            // provided by libkleo...
> +            if (mBackend->name() == Kleo::CryptoBackend::OpenPGP) {

Yes Libkleo should imo provide a configurable new Key generation dialog. But I don't think you need a dialog at all from KMail because you can take the Name / Mailbox from the Identity configuration.

> keyselectioncombo.cpp:174
> +                                    "</GnupgKeyParms>").arg(email,
> +                                                            mNameEdit->text());
> +            } else {

With GnuPG 2.1.12 or so we could use a Quick Gen Key, but as we don't want to rely hard on such a new version we need these params :-(

> keyselectioncombo.cpp:176
> +            } else {
> +                args = QStringLiteral("<GnupgKeyParms format=\"internal\">\n"
> +                                    "key-type:      RSA\n"

I don't think this "Inline Keygen" can work for S/MIME in KMail this way as a new S/MIME Certificate only generates a CSR that has to be exported and sent to a Certificate Authority for signing. This is out of scope currently as it creates loads of problems usability wise.

I think for S/MIME the common case is that you either get your certificate handed to you by your CA or Administration and have to import it or that you start a dedicated tool (Kleo) to work this out.

> keyselectioncombo.cpp:255
> +
> +    bool lessThan(const QModelIndex &source_left, const QModelIndex &source_right) const Q_DECL_OVERRIDE
> +    {

I don't think this is needed as I think we should only show ultimately valid keys with the email matching the identity. Maybe if we make this more generic without a preset identity this could work.

> keyselectioncombo.cpp:411
> +
> +QIcon KeySelectionComboPrivate::iconForValidity(GpgME::UserID::Validity validity) const
> +{

This is something I always found annoying in the old selection that it showed you even keys that you could not select. We should imo only include keys with ultimate UID Trust as these are what GnuPG thinks are "your own keys" and you are currently selecting your own keys.

For the super corner case that someone wants to configure not his / her own key for an E-Mail address. (I'm maybe thinking about some "support at foo.bar" with a shared support signing key that does not have ultimate trust they can edit the configuration of KMail.

To quote Björn "We should design for the 95% and not the 5%".

> keyselectioncombo.cpp:464
> +            });
> +    job->start({ name, email }, true);
> +}

No name here imo. Otherwise you might get keys that do not provide the email, this would be wrong and should not be supported. This also might reduce results and thus simplify selection for the user imo.

> keyselectioncombo.cpp:464
> +            });
> +    job->start({ name, email }, true);
> +}

This can be a ridicoulously slow job:

e.g on my very high performance system (admittedly with a fairly large keyring):
time gpg2 -K
gpg2 -K  3.85s user 0.13s system 98% cpu 4.032 total

Because due to the Architecture GnuPG has to go over every public key and ask the gnupg-agent through IPC if he has a secret key for this public key. On slower Systems or if a TrustDB Update (which gnupg does automatically) is involved seen this job to take over 30 seconds. That's why Kleopatra now has a "Keycacheoverlay" to similarily to KMails Akonadioverlay disable widgets and show a "Loading Certificates.." Progress bar.
I've already complained to upstream about this and they might fix this in the future.

But in this widget this would result in a noticable delay between widget shown and entries visible. With GnuPG versions > 1.4 < 2.1.6 this job could even take an infinite amount of time as it resulted in CRL Checks for each S/MIME Certificate.

This could be solved by putting the keycache into libkleo so that it can be populated before in the background.

REPOSITORY
  rLIBKLEO PIM: Kleo Library

REVISION DETAIL
  https://phabricator.kde.org/D2324

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dvratil, aheinecke
Cc: kde-pim, spencerb, dvasin, winterz, smartins, vkrause, mlaurent, knauss, dvratil
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/