401956 - improve how certificate error exceptions are displayed in the certificate manager

Status: NEW

(Core :: Security: PSM, defect, P5)

Description

[Kai Engert (:KaiE:)]

From bug 399275 comment 19:
> There's a column on the table in the Servers tab that says "Certificate Name",
> and that column says "(Not Stored)" on the exception entries I made.  That
> sounds a lot like it didn't store the actual certificate to me.  The only
> column with any data in it is the "Site" column.  The View and Edit buttons are
> also dimmed out when this exception entry is highlighted.


Which means, the exception is stored and active (identified by cert issuer, cert serial number, cert fingerprint), but cert manager was unable to find the cert in the database, therefore being unable to display the full details.


From bug 399275 comment 25:
> Maybe we should change 'not stored' to 'only hash available'? Our exceptions
> are tied to a specific cert, which we identify by the cert's hash. We store the
> cert only for UI purposes (so that you can see what cert you're exception
> applies to in the UI). It's possible that that cert may not be in the database
> (though it usually requires manual intervention for that). You can, for
> instance, share your exceptions file with othere. In that case they would still
> except a specific cert, but would not be able to display information about that
> cert.


Let's improve the labels in this bug.

Cert Manager groups certs by Organization.
When there is no cert stored, we don't know the Org name.

Right now all exceptions that lack a cert are grouped under:
  (Unknown)


Each certificate entry usually shows a cert's display name.
For each exception where the cert is missing, the leftmost column displays:
  (Not Stored)


CertOrgUnknown=(Unknown)
CertNotStored=(Not Stored)


What should we use instead?
Bob proposed "only hash available", but IIUC we should prefer positive statements.

FYI, the column header says "Certificate Name".

Maybe we should change (Unknown) to => (Name not stored)

Maybe we should change (Not Stored) to => (Details missing)


Comments? Other ideas?

Comment 1

[Johnathan Nightingale [:johnath]]

Are there other cases (I mean, besides the Add Exception dialog) where we have an entry in this list, but no cert?  If it's specific to this case, then our language can be more specific too - "(Manual Override)" or even "(Added by you)" is not really a *name*, but it is something that would likely confirm to users that everything is proceeding according to plan.  Obviously, though, that won't work if the string shows up in other non-hypothetical cases. :)

Or perhaps we could somehow tag the entries added from the exception list, so that those ones got a different string, allowing us to be more specific in that case, but still have general purpose text available the rest of the time?

Comment 2

[Kai Engert (:KaiE:)]

(In reply to comment #1)
> Are there other cases (I mean, besides the Add Exception dialog) where we have
> an entry in this list, but no cert?

Any chrome code can use nsICertOverrideService to create exceptions.
At creation time, the cert must be available, because the code needs to extract all the details.

The add exception dialog is simply a frontend to that service.

The certificate can get lost if cert database management is used to delete the cert. This can happen when users share their db between applications, and use a different tool to delete certs.

There is also bug 399057 which leads to the cert not being found.


> language can be more specific too - "(Manual Override)" or even "(Added by
> you)" is not really a *name*, but it is something that would likely confirm to
> users that everything is proceeding according to plan.  Obviously, though, that
> won't work if the string shows up in other non-hypothetical cases. :)


The strings are supposed to show up ONLY for displaying cert exceptions.

But add exception dialog is not the only mechanism to add exceptions, we don't know if future code will implicitly add exceptions (I hope not).


> Or perhaps we could somehow tag the entries added from the exception list, so
> that those ones got a different string, allowing us to be more specific in that
> case, but still have general purpose text available the rest of the time?

I would prefer not having to store additional tags.
If we offered such tags, the UI could would have to transport such tags to the backend service, and any other code could use the same mechanism, so we'd again be in the situation that you can't really distinguish them.


I propose we use general purpose strings.

Comment 3

[Johnathan Nightingale [:johnath]]

So if the original complaint was that text about "not stored" or "missing information" was confusing, and made it seem that the override didn't work, then I think that - even in light of Kai's points in comment 2 - we should consider language that describes what's happening, like "(Manual Override)" or "(Custom Exception)".

What do you think of those options, Kai?  I agree that the certOverrideService can be invoked through unknown paths in the future, including non-manual ones, but I also think that if what we're really saying is "these entries came from the override DB" then we should find a way to say that.  

Comment 4

[Kai Engert (:KaiE:)]

My concerns were mostly related to the proposal "(Added by You)".

"(Manual Override)" and "(Custom Exception)" both sound ok to me, I guess these proposals are for string ID CertNotStored.

What would you propose for string ID CertOrgUnknown?

Comment 5

[Johnathan Nightingale [:johnath]]

(In reply to comment #4)
> My concerns were mostly related to the proposal "(Added by You)".
> 
> "(Manual Override)" and "(Custom Exception)" both sound ok to me, I guess these
> proposals are for string ID CertNotStored.
> 
> What would you propose for string ID CertOrgUnknown?

would "(Custom)" work for CertOrgUnknown?  Certainly the things in that list are "Custom" insofar as they are not "Regular."  :)  And then "Manual Override" for CertNotStored sounds reasonable enough to me.  I certainly think that text would solve the original complaint, about it appearing that there was a bug.