Bug 578024 - Improve the TrustCertificateDialog
Summary: Improve the TrustCertificateDialog
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.23 M2   Edit
Assignee: P2 Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 314601 496730 541347 579281 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-01-03 07:58 EST by Ed Merks CLA
Modified: 2022-03-16 12:20 EDT (History)
6 users (show)

See Also:


Attachments
How the dialog could show PGP verified signed trust chain (31.92 KB, image/png)
2022-01-10 03:48 EST, Ed Merks CLA
no flags Details
How the dialog could show PGP verified signed trust chain with more keys present (35.87 KB, image/png)
2022-01-10 03:50 EST, Ed Merks CLA
no flags Details
trust dialog problems (57.54 KB, image/png)
2022-02-04 07:16 EST, Andrey Loskutov CLA
no flags Details
Usability improvements to the dialog (371.88 KB, image/png)
2022-02-05 07:55 EST, Ed Merks CLA
no flags Details
Screen captures and description of dialog changes (339.63 KB, application/pdf)
2022-02-07 03:58 EST, Ed Merks CLA
no flags Details
updated trust dialog issues (92.00 KB, image/png)
2022-02-09 02:57 EST, Andrey Loskutov CLA
no flags Details
This is what I mean by ugly wrapping. (7.74 KB, image/png)
2022-02-09 11:54 EST, Ed Merks CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Merks CLA 2022-01-03 07:58:27 EST
As described here

https://www.eclipse.org/lists/platform-dev/msg03238.html

the dialog for trusting PGP-signed artifacts is very poor, i.e., the user appears to have no basis for granting trust and is offered no guidance as to how to confirm that a trustworthy entity (where the entity is represented only by hexadecimal numbers) has signed the artifacts being installed.
Comment 1 Mickael Istria CLA 2022-01-03 08:45:46 EST
The most important part is the key fingerprint and even the key itself (that can be exported). For Java Certificates, I'm not aware of some explanation to users on how to decide whether to trust them or not. If something is existing, we can mimic it.
With https://evil32.com/ , the Key ID should never be shown to user, only fingerprint should be visible and used for verification.
Populating other columns is a "nice to have" but doesn't bring any value in term of security or assistance to trust decision, as metadata like "user name" can be too easily incorporated in unsafe key (ie I can easily create a PGP key pretending I'm webmaster@eclipse.org or Ed.Merks@gmail.com , however I'll never be able to find a reliable service or a workflow that allows to verify this key is safe). Somehow, we can even fear that populating those columns will give a false sense of trust against invalid keys that users won't try to verify if they just trust the metadata.
Comment 2 Ed Merks CLA 2022-01-03 09:26:56 EST
(In reply to Mickael Istria from comment #1)
> The most important part is the key fingerprint and even the key itself (that
> can be exported). 

The important parts aren't provided at a glance.  I suppose the Details... could show the key itself, but that's just more textual noise...

> For Java Certificates, I'm not aware of some explanation
> to users on how to decide whether to trust them or not. If something is
> existing, we can mimic it.

With certificates there is intrinsic meaningful information that can be extract and presented to the user:

Signed (5141) CN=Eclipse.org Foundation, Inc. OU=IT O=Eclipse.org Foundation, Inc. L=Ottawa ST=Ontario from=2021-03-16 to=2022-05-18
      Signed CN=DigiCert SHA2 Assured ID Code Signing CA OU=www.digicert.com O=DigiCert Inc from=2013-10-22 to=2028-10-22
      Signed CN=DigiCert Assured ID Root CA OU=www.digicert.com O=DigiCert Inc from=2006-11-10 to=2031-11-10 

So I can know this is signed by a certificate issued to the Eclipse.org and assured by www.digicert.com.  So if some root certificate is missing, I can find out about it...

I don't think we can mimic this because such information is not embedded in the PGP signatures nor in the keys but rather must be associated with these things externally.

I hate to say in a public record, but we have to assume that users are naive and they will do either a) or b).

a) Click the dialog away as fast as possible to make the installation proceed.
b) Be completely paranoid and want to know what/who the heck they are trusting.


> With https://evil32.com/ , the Key ID should never be shown to user, only
> fingerprint should be visible and used for verification.

It's a long not an int, but yes, I wonder what might be accomplished if one can create a PGPPublicKey whose getKeyID intentionally collides with one issued by Eclipse...

So you're saying we should redesign this not to present the key to user?

> Populating other columns is a "nice to have" but doesn't bring any value in
> term of security or assistance to trust decision, as metadata like "user
> name" can be too easily incorporated in unsafe key (ie I can easily create a
> PGP key pretending I'm webmaster@eclipse.org or Ed.Merks@gmail.com 

Yes there no point in presenting information that is not itself reliable/secure.

> , however
> I'll never be able to find a reliable service or a workflow that allows to
> verify this key is safe). Somehow, we can even fear that populating those
> columns will give a false sense of trust against invalid keys that users
> won't try to verify if they just trust the metadata.

I think for b) above we have to do something better.   Just braining possibilities...  What if we included in the artifact properties not just the PGPPublicKey but also an associated link to an https site that provides information about that key?  Then one could provide URLs in the dialog and the secure site could list the known fingerprints.  This would help the user understand who "issued" the key.  Or is this a stupid idea?
Comment 3 Mickael Istria CLA 2022-01-03 09:38:34 EST
(In reply to Ed Merks from comment #2)
> I think for b) above we have to do something better.   Just braining
> possibilities...  What if we included in the artifact properties not just
> the PGPPublicKey but also an associated link to an https site that provides
> information about that key?  Then one could provide URLs in the dialog and
> the secure site could list the known fingerprints.  This would help the user
> understand who "issued" the key.  Or is this a stupid idea?

It's not a stupid idea, but I think we can do better & simpler. For example, we could just add a link to a reliable keyserver to let users see whether the key is known there and matches requirements. Eg https://keyserver.ubuntu.com/pks/lookup?search=0xb6d3ab9bcc641282&fingerprint=on&op=index . The message could be "Verify this key is authenticated on <keyserver>".
Comment 4 Christoph Laeubrich CLA 2022-01-03 09:41:00 EST
(In reply to Ed Merks from comment #2)
> With certificates there is intrinsic meaningful information that can be
> extract and presented to the user:
> 
> Signed (5141) CN=Eclipse.org Foundation, Inc. OU=IT O=Eclipse.org
> Foundation, Inc. L=Ottawa ST=Ontario from=2021-03-16 to=2022-05-18
>       Signed CN=DigiCert SHA2 Assured ID Code Signing CA OU=www.digicert.com
> O=DigiCert Inc from=2013-10-22 to=2028-10-22
>       Signed CN=DigiCert Assured ID Root CA OU=www.digicert.com O=DigiCert
> Inc from=2006-11-10 to=2031-11-10 
> 
> So I can know this is signed by a certificate issued to the Eclipse.org and
> assured by www.digicert.com.  So if some root certificate is missing, I can
> find out about it...

The same could be accomplished with PGP "Web of trust", e.g. an eclipse.org keyserver "sign" other PGP keys that where considered trustful. 

> It's a long not an int, but yes, I wonder what might be accomplished if one
> can create a PGPPublicKey whose getKeyID intentionally collides with one
> issued by Eclipse...

I can just quote the RFC [1] here:

 A Key ID is an eight-octet scalar that identifies a key.
   Implementations SHOULD NOT assume that Key IDs are unique.

[1] https://datatracker.ietf.org/doc/html/rfc4880#section-3.3

> I think for b) above we have to do something better.   Just braining
> possibilities...  What if we included in the artifact properties not just
> the PGPPublicKey but also an associated link to an https site that provides
> information about that key?  Then one could provide URLs in the dialog and
> the secure site could list the known fingerprints.  This would help the user
> understand who "issued" the key.  Or is this a stupid idea?

See above, that's what key-servers are meant for and WebOfTrust so Eclipse.org could built an authoritative key-server that has trust for some common keys.
Comment 5 Mickael Istria CLA 2022-01-03 09:45:03 EST
@Christoph: At the moment, it seems too optimistic that Eclipse Foundation would build a keyserver and that it's the one keyserver all RCP applications should rely on to build their trust. We don't really need an automated decision about whether to trust the key or not, because after all Web Of Trust is also a hint that user has to take into account, it's not meant to automate a decision. The whole Web Of Trust concept still relies on the fact that users has to decide at some point whether to trust a keyserver, or some root PGP keys...
Keyservers simplify trust decision, but they do not fully resolve it. I think it's overkill to rely on them for automation at the moment.
Comment 6 Andrey Loskutov CLA 2022-01-03 10:22:14 EST
(In reply to Ed Merks from comment #2) 
> I hate to say in a public record, but we have to assume that users are naive
> and they will do either a) or b).
> 
> a) Click the dialog away as fast as possible to make the installation
> proceed.
> b) Be completely paranoid and want to know what/who the heck they are
> trusting.

*Both* is the reality I personally see in our lab full of experienced C/C++ developers. 

Once they see *existing* dialog about certificates they send me a mail asking if they should trust that dialog or not, even if there is a wiki explaining exact that dialog...  Others don't care at all and just click every button they can to get the dialog closed.

They will go amok looking at the dialog showing some hex numbers and asking if they should trust that number or not, and I will have fun trying to check what the heck the number is and what could they have installed.

Whatever we do, we should at least give some context to the PGP signatures we present.

"PGP signature $HEXHEXHEX presented by the bundle XYZ installed from ABC url" is *at least* required. Ideally, a link to some page explaining how that thing could be verified or directly to some list/directory/whatever with trusted keys.
Comment 7 Ed Merks CLA 2022-01-05 00:36:10 EST
(In reply to Christoph Laeubrich from comment #4)
> (In reply to Ed Merks from comment #2)
> > With certificates there is intrinsic meaningful information that can be
> > extract and presented to the user:
> > 
> > Signed (5141) CN=Eclipse.org Foundation, Inc. OU=IT O=Eclipse.org
> > Foundation, Inc. L=Ottawa ST=Ontario from=2021-03-16 to=2022-05-18
> >       Signed CN=DigiCert SHA2 Assured ID Code Signing CA OU=www.digicert.com
> > O=DigiCert Inc from=2013-10-22 to=2028-10-22
> >       Signed CN=DigiCert Assured ID Root CA OU=www.digicert.com O=DigiCert
> > Inc from=2006-11-10 to=2031-11-10 
> > 
> > So I can know this is signed by a certificate issued to the Eclipse.org and
> > assured by www.digicert.com.  So if some root certificate is missing, I can
> > find out about it...
> 
> The same could be accomplished with PGP "Web of trust", e.g. an eclipse.org
> keyserver "sign" other PGP keys that where considered trustful. 
> 

By the "same thing", you mean that the PGPPublicKey itself will have intrinsic information?  I don't really know what a "Web of trust"  totally means, but if it involves a key server then an internet connection is involved (or a least an offline cache of information provided by a key server) and that's not really the same thing as intrinsic information.


> > It's a long not an int, but yes, I wonder what might be accomplished if one
> > can create a PGPPublicKey whose getKeyID intentionally collides with one
> > issued by Eclipse...
> 
> I can just quote the RFC [1] here:
> 
>  A Key ID is an eight-octet scalar that identifies a key.
>    Implementations SHOULD NOT assume that Key IDs are unique.
> 
> [1] https://datatracker.ietf.org/doc/html/rfc4880#section-3.3
> 

Does that mean that org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPPublicKeyStore.keys which is implemented as

	private Map<Long, PGPPublicKey> keys = new HashMap<>()

is wrong and should instead be like this?

	private Map<Long, Set<PGPPublicKey>> keys = new HashMap<>()


If there are holes in the current implementation, we should really fix them sooner rather than later.  From what you say, I get the sense that org.bouncycastle.openpgp.PGPSignature.getKeyID() should allow to look up a set of PGPPublicKeys one of which should be able to verify the content of the artifact, e.g., in org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier.initialize(IProvisioningAgent, IProcessingStepDescriptor, IArtifactDescriptor) it should try all the keys with a given ID, shouldn't it?
 

> > I think for b) above we have to do something better.   Just braining
> > possibilities...  What if we included in the artifact properties not just
> > the PGPPublicKey but also an associated link to an https site that provides
> > information about that key?  Then one could provide URLs in the dialog and
> > the secure site could list the known fingerprints.  This would help the user
> > understand who "issued" the key.  Or is this a stupid idea?
> 
> See above, that's what key-servers are meant for and WebOfTrust so
> Eclipse.org could built an authoritative key-server that has trust for some
> common keys.

So you are suggesting that we can have a hard-coded link in the "name" column if the foundation provided such a thing?

Or do we need some way of associating a PGPPublicKey with a URL?  If so, then I assume org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPPublicKeyStore should support that.  Also https://bugs.eclipse.org/bugs/show_bug.cgi?id=577248 would be impacted by such a decision...
Comment 8 Mickael Istria CLA 2022-01-05 03:38:03 EST
(In reply to Ed Merks from comment #7)
> Does that mean that
> org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPPublicKeyStore.
> keys which is implemented as
> 
> 	private Map<Long, PGPPublicKey> keys = new HashMap<>()
> 
> is wrong

Probably. Can you please open a dedicated bug about this concern?

> and should instead be like this?
> 
> 	private Map<Long, Set<PGPPublicKey>> keys = new HashMap<>()

I don't think so. Instead of getKeyID(), the key (of the map) should be something that we can consider unique, such as the Fingerprint, or maybe something else.

> If there are holes in the current implementation, we should really fix them
> sooner rather than later.  From what you say, I get the sense that
> org.bouncycastle.openpgp.PGPSignature.getKeyID() should allow to look up a
> set of PGPPublicKeys one of which should be able to verify the content of
> the artifact, e.g., in
> org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier.
> initialize(IProvisioningAgent, IProcessingStepDescriptor,
> IArtifactDescriptor) it should try all the keys with a given ID, shouldn't
> it?

I think we should instead get rid of all usage of KeyIDs and find something else to ensure a 1-1 mapping with a key.

> So you are suggesting that we can have a hard-coded link in the "name"
> column if the foundation provided such a thing?

Note that such decision about what "master" keys or keyservers to trust is not a decision on p2 project, but a decision of the RCP application provider. Eg for Red Hat Code Ready Studio, we'd want to add some Red Hat public keys as trusted by default; but not necessarily the Foundation key as we do not provide full support for external artifacts even if coming from the Foundation.

> Or do we need some way of associating a PGPPublicKey with a URL?

A key can be declared as verified on multiple keyservers. There is no key->keyserver mapping in place.
Also, keyservers are only available online, so it doesn't solve the general issue of helping people to build trust in some cases.
At the moment, I think the simpler solution is to make the trust dialog "educate" about how to take a decision whether to trust a key or not, with some extra explanations.
Comment 9 Ed Merks CLA 2022-01-05 07:42:16 EST
(In reply to Mickael Istria from comment #8)
> (In reply to Ed Merks from comment #7)
> > Does that mean that
> > org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPPublicKeyStore.
> > keys which is implemented as
> > 
> > 	private Map<Long, PGPPublicKey> keys = new HashMap<>()
> > 
> > is wrong
> 
> Probably. Can you please open a dedicated bug about this concern?
> 
> > and should instead be like this?
> > 
> > 	private Map<Long, Set<PGPPublicKey>> keys = new HashMap<>()
> 
> I don't think so. Instead of getKeyID(), the key (of the map) should be
> something that we can consider unique, such as the Fingerprint, or maybe
> something else.
> 

I opened https://bugs.eclipse.org/bugs/show_bug.cgi?id=578056 but note that PGPSignature.getKeyID() is the *only* association between a key and the signature so unless we ALWAYS store the key(s) and the signature(s) on the artifact metadata, we will need to do a lookup with a signature's key ID and deal with collisions...


> > If there are holes in the current implementation, we should really fix them
> > sooner rather than later.  From what you say, I get the sense that
> > org.bouncycastle.openpgp.PGPSignature.getKeyID() should allow to look up a
> > set of PGPPublicKeys one of which should be able to verify the content of
> > the artifact, e.g., in
> > org.eclipse.equinox.internal.p2.artifact.processors.pgp.PGPSignatureVerifier.
> > initialize(IProvisioningAgent, IProcessingStepDescriptor,
> > IArtifactDescriptor) it should try all the keys with a given ID, shouldn't
> > it?
> 
> I think we should instead get rid of all usage of KeyIDs and find something
> else to ensure a 1-1 mapping with a key.
> 

Yes, but as mentioned above, there is a reason why one does a lookup by KeyID...

> > So you are suggesting that we can have a hard-coded link in the "name"
> > column if the foundation provided such a thing?
> 
> Note that such decision about what "master" keys or keyservers to trust is
> not a decision on p2 project, but a decision of the RCP application
> provider. Eg for Red Hat Code Ready Studio, we'd want to add some Red Hat
> public keys as trusted by default; but not necessarily the Foundation key as
> we do not provide full support for external artifacts even if coming from
> the Foundation.
> 

I'm not suggesting this as some form of automated trust but rather as some form of documentation or as a starting point from which a user can try to make sense of meaningless hex content. I feel like the current dialog looks like a work-in-progress that's missing some form of human-sensible content, i.e., the empty "name" column.

> > Or do we need some way of associating a PGPPublicKey with a URL?
> 
> A key can be declared as verified on multiple keyservers. There is no
> key->keyserver mapping in place.
> Also, keyservers are only available online, so it doesn't solve the general
> issue of helping people to build trust in some cases.
> At the moment, I think the simpler solution is to make the trust dialog
> "educate" about how to take a decision whether to trust a key or not, with
> some extra explanations.

Do you think a person can make an educated decision about a key/finger print without knowing the source of the key and with no mechanism for reliably determining the source?  Personally I would want to know who issued the key and I would want a reliable mechanism for being sure that they really did issue it.

As for being online, yes of course best not to assume one is always online, but this is typically happening during an install or update so generally one is online.

But again, I a not suggesting something needs to be automated, but rather that there is a link that I can look at in a web page that I can use to "verify manually" that the fingerprint I see in the dialog is really valid and that I can see who issued it.  We can't really just tell the user to go Google for key servers and see if they can find one or more and then try find the key on one of those, or?
Comment 10 Mickael Istria CLA 2022-01-05 08:13:36 EST
(In reply to Ed Merks from comment #9)
> I'm not suggesting this as some form of automated trust but rather as some
> form of documentation or as a starting point from which a user can try to
> make sense of meaningless hex content. I feel like the current dialog looks
> like a work-in-progress that's missing some form of human-sensible content,
> i.e., the empty "name" column.

That's right.

> Do you think a person can make an educated decision about a key/finger print
> without knowing the source of the key and with no mechanism for reliably
> determining the source?  Personally I would want to know who issued the key
> and I would want a reliable mechanism for being sure that they really did
> issue it.

The "educated decision" requires to verify the key in some external service (it can be a keyserver or even local pgp keyring). Verifying the key on a keyserver can happen thanks to the fingerprint. When verifying the key, user will show the issuer email, that happens to have been verified if present in some "safe" keyserver. So from that step, they do know who issued the key and know it was verified by the keyserver as being the right issuer.
So the current state gives enough for a decision.

However, although it's still nice to show this info, assuming that one can take a trust decision based on the "name" column would be a major error because anyone can forge a key with another name. I can create 50 keys for Ed.Merks@gmail.com or webmaster@eclipse.org in the next minutes, without even scripting... And all those wouldn't be trustworthy. If I use one of these key to sign an artifact, a user that gets only fingerprint will have to investigate the fingerprint, and will probably not trust the key because it's verified nowhere. If the dialog shows the name, then users installing my nasty artifact will see "Ed.Merks@gmail.com" and may think, "OK, it's Ed, I know him, I can install it" without even verifying the key.
So sure, we could present the name info to users, and we'll eventually will; but we need to make clear that this info itself is not trustworthy unless the key is verified.

> But again, I a not suggesting something needs to be automated, but rather
> that there is a link that I can look at in a web page that I can use to
> "verify manually" that the fingerprint I see in the dialog is really valid
> and that I can see who issued it.

You cannot trust who issued a PGP key just by looking at it. Building trust that a key belongs to the right issuer is the purpose of keyservers or "Web of Trust".
Having the fingerprint is usually enough to verify on a keyserver, doing more manual workflows typically requires to get the key locally and verify some other signatures of this key from trusted peers.

> We can't really just tell the user to go
> Google for key servers and see if they can find one or more and then try
> find the key on one of those, or?

That's how PGP works.
Comment 11 Mickael Istria CLA 2022-01-05 10:37:25 EST
Actually the issue about missing user id is not a client side issue but a signer side issue, it's https://bugs.eclipse.org/bugs/show_bug.cgi?id=578059 .
If you use a different repo (eg rt.equinox.p2/bundles/org.eclipse.equinox.p2.tests/testData/pgp/repoPGPOK ) which is signed with another key, you see the user id as expected.
Comment 12 Eclipse Genie CLA 2022-01-06 05:43:00 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/189341
Comment 14 Mickael Istria CLA 2022-01-09 08:10:29 EST
A patch was merged recently that's supposed to deal with most reported issues in the dialog and is now available in I-Build.
@Ed (and others): can you please have a look and evaluate whether it's good enough and mark as resolved, whether some more "nice to have" enhancements can be done and reported in separate bugs, or whether there are still important things we need to fix/improve for users to get a sense of how to react to this popup?
Comment 15 Ed Merks CLA 2022-01-09 08:19:12 EST
(In reply to Mickael Istria from comment #14)
> A patch was merged recently that's supposed to deal with most reported
> issues in the dialog and is now available in I-Build.
> @Ed (and others): can you please have a look and evaluate whether it's good
> enough and mark as resolved, whether some more "nice to have" enhancements
> can be done and reported in separate bugs, or whether there are still
> important things we need to fix/improve for users to get a sense of how to
> react to this popup?

I also made a number of fixes and improvements via https://bugs.eclipse.org/bugs/show_bug.cgi?id=474099 which is also now in master and no doubt in last night's IBuild.

I think the ?/help button should open a helpful page, even if for now it's just a simple placeholder page with only a minimal amount of content so that we have a place to provide actually helpful documentation.

I'm not so happy with the selected PGP node not showing anything in the tree viewer (other than an empty node that can be selected but has no text), but I don't have a good idea other than to repeat the fingerprint information...
Comment 16 Mickael Istria CLA 2022-01-09 08:26:13 EST
(In reply to Ed Merks from comment #15)
> I think the ?/help button should open a helpful page, even if for now it's
> just a simple placeholder page with only a minimal amount of content so that
> we have a place to provide actually helpful documentation.

While I agree it should, I don't think it's a new and thus a blocker problem here; the same concerns about unhelpful dialog are/were still present for unrooted certificates.
I plan to work on it anyway, as I'm investing in help stuff for p2/PGP as part of this whole story. Doing a page for the Trust dialog as part of this effort is affordable and profitable enough. Just it will be part of a separate commit in help first anyway.
 
> I'm not so happy with the selected PGP node not showing anything in the tree
> viewer (other than an empty node that can be selected but has no text), but
> I don't have a good idea other than to repeat the fingerprint information...

We can just hide it if a PGP key is selected.
Comment 17 Ed Merks CLA 2022-01-09 12:30:44 EST
We could show the links in the "web of trust" for PGP keys as described here:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=578059#c12

Of course if we validate the signing links with the web master's root PGP key, we wouldn't even need to bring up the dialog...
Comment 18 Ed Merks CLA 2022-01-10 03:48:13 EST
Created attachment 287800 [details]
How the dialog could show PGP verified signed trust chain

The information represented here is available via the PGP keys current present in the artifact metadata.
Comment 19 Ed Merks CLA 2022-01-10 03:50:20 EST
Created attachment 287801 [details]
How the dialog could show PGP verified signed trust chain with more keys present

The information represented here is available only if the full key for the platform and the webmaster are present in the local key store.
Comment 20 Mickael Istria CLA 2022-01-10 08:23:51 EST
Those are great proposals Ed. Do you plan to submit a Gerrit patch to implement them?
Comment 21 Ed Merks CLA 2022-01-10 08:54:39 EST
(In reply to Mickael Istria from comment #20)
> Those are great proposals Ed. Do you plan to submit a Gerrit patch to
> implement them?

Thanks.  Yes, I have the implementation sketched out, but it's a bit rough and I have become completely confused by package imports so finally resorted to bundle imports to make progress.

Part of the changes are of course in ValidationDialogServiceUI.createTreeNodes(Certificate[][], Collection<PGPPublicKey>) but that needs access to the keystore, which is tucked away in PGPSignatureVerifier.KNOWN_KEYS...


For now I hacked that class like this:

	static {
		PGPSignatureVerifier.KNOWN_KEYS.add(new File("D:/Users/merks/pgp-keys/platform-rootkey.asc"));
		PGPSignatureVerifier.KNOWN_KEYS.add(new File("D:/Users/merks/pgp-keys/webmaster.asc"));
	}


I.e., to load the extra keys for the second screen shot.

But I keep thinking about how we could ensure that such keys are found and loaded automatically because then we could validate the web of trust rooted to the webmaster automatically and could avoid the trust dialog for any keys transitively signed by the webmaster, much like what happens for unrooted certificates versus rooted ones...

So yes, I will try to into Gerrit in the coming days...
Comment 22 Eclipse Genie CLA 2022-01-11 09:52:03 EST
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.common/+/189479
Comment 24 Eclipse Genie CLA 2022-01-11 10:59:48 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/189481
Comment 26 Eclipse Genie CLA 2022-01-14 07:48:32 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/189634
Comment 28 Andrey Loskutov CLA 2022-02-04 07:16:30 EST
Created attachment 287985 [details]
trust dialog problems

I see some issues with the new "PGP trust" dialog. I've attached screenshot with my comments.

1) The warning that is *written into the column title*. Sorry, no way, this is not how a warning should be added to a wizard / dialog. Beside that it is highly unusual, it fully hides the next "validity" column.
I would propose to add only an icon to the column and a tooltip over it. Optionally, this warning could be added to the top of the wizard.

2) Even as experienced Eclipse user I have NO CLUE which bundle (or bundles) use this PGP key. Without this context I have no clue if the all 100 bundles I'm going to install are "not to be trusted" or just a few or a single one. IMHO this is crucial to know. There must be some info shown which bundles are using this PGP key.
Comment 29 Eclipse Genie CLA 2022-02-05 07:51:21 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/190485
Comment 30 Ed Merks CLA 2022-02-05 07:55:29 EST
Created attachment 287995 [details]
Usability improvements to the dialog

Use a SashForm so that relative sizes of the top table and the bottom tree can be controlled by the user; this has no visual impact, but is a nice-to-have.

Use a wrapping label for the description text such that a longer description doesn't make the dialog too wide.  The description is changed along the suggest line: move the warning text about the name not being reliable from the column label to the description.

Use a TableColumnLayout so that all the columns are visible without scrolling.

Fix the problem with the false indication of expiration, i.e., when the valid seconds is 0, that means there is no expiration.
Comment 31 Ed Merks CLA 2022-02-05 08:00:26 EST
Does anyone have an idea why the Gerrit changes don't trigger builds?
Comment 33 Ed Merks CLA 2022-02-07 03:58:12 EST
Created attachment 287996 [details]
Screen captures and description of dialog changes

I believe these changes address the concerns expressed here and in Bug 541347 too.  Feedback is welcome.
Comment 34 Eclipse Genie CLA 2022-02-07 06:45:48 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/190518
Comment 36 Eclipse Genie CLA 2022-02-09 01:19:37 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/190598
Comment 37 Andrey Loskutov CLA 2022-02-09 02:57:51 EST
Created attachment 288018 [details]
updated trust dialog issues

Hi Ed, many thanks working on this dialog, it is definitely a big difference now!

Still, there are few places that could be probably fixed for 4.23, see screenshot.

1) The "Warnings" shouldn't follow each other as plain text, each should start with a new line.

2) I see the unknown PGP warning dialog appearing also for bundles signed by platform-releng-dev@eclipse.org key. Why? What is missing to recognize our own keys? This seem wrong.

3) Is there a way now to mute the dialog (I do not mean remember the key, I mean first install ever)?
Comment 38 Ed Merks CLA 2022-02-09 03:26:52 EST
(In reply to Andrey Loskutov from comment #37)
> Created attachment 288018 [details]
> updated trust dialog issues
> 
> Hi Ed, many thanks working on this dialog, it is definitely a big difference
> now!
> 
> Still, there are few places that could be probably fixed for 4.23, see
> screenshot.
> 
> 1) The "Warnings" shouldn't follow each other as plain text, each should
> start with a new line.
> 

It will start to take quite a bit of vertical space with possibly 4 lines. I don't think there is any rule that each sentence necessarily should start on a new line.  Looking at these, there are many multi-sentence "paragraphs".   Generally I would expect paragraphs to be separated by two lines but for sentence that form a paragraph there are no line breaks.  Also when a single line break is present, the wrapping will be very ugly...

I agree that warning "icon" twice is overkill. I'm actually not a big fan of unicode glyphs because you never know how they will look or if the font actually supports it.  On my older Windows 7 machine, many of these were just empty boxes which is super ugly.


> 2) I see the unknown PGP warning dialog appearing also for bundles signed by
> platform-releng-dev@eclipse.org key. Why? What is missing to recognize our
> own keys? This seem wrong.
> 

I'm not sure how you are testing, or which version.  In older implementations, the keys stored in the artifact metadata on the remote repository were not transferred to the local repository.   So if the artifact you are installing is already in your local artifact repository, no key will be found and install/update fails completely.  For a while, we treated this as unsigned content, but that behavior created a security hole so it was reverted.  I think now if the key is not found, an error box will come up.  The only way to fix it is to add the key in the Install/Update > Trust page.


> 3) Is there a way now to mute the dialog (I do not mean remember the key, I
> mean first install ever)?


This Gerrit change introduced a "trust always" preference that can be used to mute the dialog.
Comment 39 Andrey Loskutov CLA 2022-02-09 03:55:16 EST
(In reply to Ed Merks from comment #38)
> (In reply to Andrey Loskutov from comment #37)
> > Created attachment 288018 [details]
> > updated trust dialog issues
> > 
> > Hi Ed, many thanks working on this dialog, it is definitely a big difference
> > now!
> > 
> > Still, there are few places that could be probably fixed for 4.23, see
> > screenshot.
> > 
> > 1) The "Warnings" shouldn't follow each other as plain text, each should
> > start with a new line.
> > 
> 
> It will start to take quite a bit of vertical space with possibly 4 lines. 

I think this is OK, see below.
But shouldn't be "Do you trust these signers?" (which is a bit redundant considering the next warning questions) in the dialog title.

> I
> don't think there is any rule that each sentence necessarily should start on
> a new line.  Looking at these, there are many multi-sentence "paragraphs".  
> Generally I would expect paragraphs to be separated by two lines but for
> sentence that form a paragraph there are no line breaks.  Also when a single
> line break is present, the wrapping will be very ugly...

I'm not saying every sentence should be starting with a new line, but the sentences (with warnings) that each requires an explicit user attention, which is the case here. Writing them one after each other doesn't look nice and doesn't catch attention as it supposed to do. And the third line (which is not a warning) should be separated by exact the reason to not to steal attention from the actual warning.

> I agree that warning "icon" twice is overkill. I'm actually not a big fan of
> unicode glyphs because you never know how they will look or if the font
> actually supports it.  On my older Windows 7 machine, many of these were
> just empty boxes which is super ugly.

Why not have use "Warning" dialog where the warning is the system default icon?
 
> > 2) I see the unknown PGP warning dialog appearing also for bundles signed by
> > platform-releng-dev@eclipse.org key. Why? What is missing to recognize our
> > own keys? This seem wrong.
> > 
> 
> I'm not sure how you are testing, or which version. 

Always the latest available nightly build, by manual SDK download & installation of additional bundles as described in https://wiki.eclipse.org/Platform/How_to_Contribute#.5B2.5D_Install_the_development_tools

> In older
> implementations, the keys stored in the artifact metadata on the remote
> repository were not transferred to the local repository.   So if the
> artifact you are installing is already in your local artifact repository, 

I don't have local repository, see above, I simply install list of bundles into a fresh SDK nightly build.
 
> > 3) Is there a way now to mute the dialog (I do not mean remember the key, I
> > mean first install ever)?
> 
> 
> This Gerrit change introduced a "trust always" preference that can be used
> to mute the dialog.

Can it be a system property please? Otherwise one needs to define a custom product with custom preferences and can't just start SDK with that dialog muted.
Comment 40 Ed Merks CLA 2022-02-09 03:58:36 EST
BTW, if the "mute" question is about the state of

https://bugs.eclipse.org/bugs/show_bug.cgi?id=578583

I did ask on the comment if everyone agrees when I added Mickael

https://bugs.eclipse.org/bugs/show_bug.cgi?id=578583#c5

But I've not heard from Mickael in a while so I've started to assume that silence is agreement, and yes, I will implement it as described there.
Comment 41 Ed Merks CLA 2022-02-09 04:09:01 EST
(In reply to Andrey Loskutov from comment #39)
> (In reply to Ed Merks from comment #38)
> > (In reply to Andrey Loskutov from comment #37)
> > > Created attachment 288018 [details]
> > > updated trust dialog issues
> > > 
> > > Hi Ed, many thanks working on this dialog, it is definitely a big difference
> > > now!
> > > 
> > > Still, there are few places that could be probably fixed for 4.23, see
> > > screenshot.
> > > 
> > > 1) The "Warnings" shouldn't follow each other as plain text, each should
> > > start with a new line.
> > > 
> > 
> > It will start to take quite a bit of vertical space with possibly 4 lines. 
> 
> I think this is OK, see below.
> But shouldn't be "Do you trust these signers?" (which is a bit redundant
> considering the next warning questions) in the dialog title.
> 
> > I
> > don't think there is any rule that each sentence necessarily should start on
> > a new line.  Looking at these, there are many multi-sentence "paragraphs".  
> > Generally I would expect paragraphs to be separated by two lines but for
> > sentence that form a paragraph there are no line breaks.  Also when a single
> > line break is present, the wrapping will be very ugly...
> 
> I'm not saying every sentence should be starting with a new line, but the
> sentences (with warnings) that each requires an explicit user attention,
> which is the case here. Writing them one after each other doesn't look nice
> and doesn't catch attention as it supposed to do. And the third line (which
> is not a warning) should be separated by exact the reason to not to steal
> attention from the actual warning.
> 
> > I agree that warning "icon" twice is overkill. I'm actually not a big fan of
> > unicode glyphs because you never know how they will look or if the font
> > actually supports it.  On my older Windows 7 machine, many of these were
> > just empty boxes which is super ugly.
> 
> Why not have use "Warning" dialog where the warning is the system default
> icon?
>  

I will have another look, but to me it's one paragraph...

> > > 2) I see the unknown PGP warning dialog appearing also for bundles signed by
> > > platform-releng-dev@eclipse.org key. Why? What is missing to recognize our
> > > own keys? This seem wrong.
> > > 
> > 
> > I'm not sure how you are testing, or which version. 
> 
> Always the latest available nightly build, by manual SDK download &
> installation of additional bundles as described in
> https://wiki.eclipse.org/Platform/How_to_Contribute#.5B2.
> 5D_Install_the_development_tools
> 

I misunderstood your statement about "what's missing" thinking that the keys were missing.  Nothing is missing.  Keys are not trusted until the user trusts them.  That's how it's supposed to work.  Perhaps the platform plans to use it's own extension point org.eclipse.equinox.p2.engine.pgp to register its own signing key as trusted in its own SDK, but that's not for me add or decide.


> > In older
> > implementations, the keys stored in the artifact metadata on the remote
> > repository were not transferred to the local repository.   So if the
> > artifact you are installing is already in your local artifact repository, 
> 
> I don't have local repository, see above, I simply install list of bundles
> into a fresh SDK nightly build.
> 
 
Actually you always do.  You'll see the eclipse installation folder itself is an artifact repository with an artifacts.xml.


> > > 3) Is there a way now to mute the dialog (I do not mean remember the key, I
> > > mean first install ever)?
> > 
> > 
> > This Gerrit change introduced a "trust always" preference that can be used
> > to mute the dialog.
> 
> Can it be a system property please? Otherwise one needs to define a custom
> product with custom preferences and can't just start SDK with that dialog
> muted.

Yes, that the other bug where I was hoping everyone would agree but I'll just assume that now...
Comment 43 Ed Merks CLA 2022-02-09 11:47:54 EST
*** Bug 541347 has been marked as a duplicate of this bug. ***
Comment 44 Ed Merks CLA 2022-02-09 11:54:32 EST
Created attachment 288023 [details]
This is what I mean by ugly wrapping.

I don't want a dialog that looks like this.  Too much vertical space is used and this version only has 3 lines, not 4 as in the most general case.  In translations you don't know how long the lines will be so it just seems a poor idea to have single line line breaks...
Comment 45 Ed Merks CLA 2022-02-10 02:14:03 EST
*** Bug 496730 has been marked as a duplicate of this bug. ***
Comment 46 Ed Merks CLA 2022-02-10 02:19:04 EST
*** Bug 314601 has been marked as a duplicate of this bug. ***
Comment 47 Eclipse Genie CLA 2022-02-13 10:44:15 EST
New Gerrit change created: https://git.eclipse.org/r/c/equinox/rt.equinox.p2/+/190750
Comment 49 Alexander Kurtakov CLA 2022-02-17 01:22:07 EST
Ed, do you consider this one done?
Comment 50 Ed Merks CLA 2022-02-17 01:31:18 EST
There is the issue of line wrapping of the message, but I really don't like the suggestion to add line breaks because its looks ugly and strange...
Comment 51 Ed Merks CLA 2022-02-27 02:20:01 EST
Given I don't think line splitting improves the readability and that it doesn't work well in the limited space, I consider this done as is.
Comment 52 Andrey Loskutov CLA 2022-03-16 12:20:11 EDT
*** Bug 579281 has been marked as a duplicate of this bug. ***