Last Comment Bug 173729 - Deleting a builtin cert should behave better [certificate authority]
: Deleting a builtin cert should behave better [certificate authority]
Status: NEW
[kerh-coz]
:
Product: Core
Classification: Components
Component: Security: UI (show other bugs)
: Other Branch
: All All
: -- normal with 9 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 436619 542445 685525 (view as bug list)
Depends on:
Blocks: 345934 571329
  Show dependency treegraph
 
Reported: 2002-10-10 07:10 PDT by Kai Engert (:kaie)
Modified: 2015-06-20 20:47 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
screenshot - current user interface (88.41 KB, image/png)
2010-03-25 05:31 PDT, Kai Engert (:kaie)
no flags Details
screenshot - proposed new interface (96.03 KB, image/png)
2010-03-25 05:31 PDT, Kai Engert (:kaie)
no flags Details
Patch v1 (8.74 KB, patch)
2010-03-25 05:38 PDT, Kai Engert (:kaie)
rrelyea: review+
Details | Diff | Review

Description Kai Engert (:kaie) 2002-10-10 07:10:37 PDT
Create a new profile.
Open certificate manager.
Open the authorities tab.
Select a cert.
Click delete.
Confirm delete => the cert disappears from the displayed list.
Close cert manager window, or restart application.
Reopen cert manager window.
Open authorities tab.
The deleted cert is still there, but it has its trust removed, which is
essentially the same behaviour.

This is currently by design (see also bug 138818), but not the ideal behaviour.
(It is not possible to delete builtin certificates).


Actual behaviour:
- deleting a builtin CA cert appears to be possible
- this causes confusion when the user later discovers the cert again

Expected behaviour:
- when the user tries to delete a builtin cert, the UI confirmation message
should be different
- a proposed wording could be "This certificate can not be deleted, because it
came shipped with [product name]. However, you can mark this certificate as
untrusted, which has the same effect. Do you want to delete all trust from this
certificate?"
- when the user confirms to delete/untrust, do not delete the cert from the
shown list
Comment 1 John G. Myers 2004-02-27 00:13:03 PST
Mass reassign ssaux bugs to nobody
Comment 2 Mardeg 2010-01-26 23:25:50 PST
*** Bug 542445 has been marked as a duplicate of this bug. ***
Comment 3 alpha.mm 2010-02-01 00:26:17 PST
Well, it's been 7 years and this bug still exists.
Comment 4 W. Craig Trader 2010-03-24 14:57:39 PDT
Given the untrustability of some of the builtin CA's (particularly CNNIC), this is going to be a big problem for a lot of users.  If I personally don't trust a CA not to issue bogus certs, then I need to be able to tell my browser not to trust certs issued by the CA.  With the current behavior, I (as a user) expect that when I delete a CA and click to confirm the deletion, I should not see that CA back in the list the next time I open the certificate management window.
Comment 5 Kai Engert (:kaie) 2010-03-25 05:30:31 PDT
I agree it's time to improve the UI, but I want to propose an approach that is simple to do.

I propose to simply state more clearly what will happen by improving the button labels and warnings.

I will attach screenshots with my proposal.
Comment 6 Kai Engert (:kaie) 2010-03-25 05:31:19 PDT
Created attachment 434845 [details]
screenshot - current user interface
Comment 7 Kai Engert (:kaie) 2010-03-25 05:31:45 PDT
Created attachment 434846 [details]
screenshot - proposed new interface
Comment 8 Kai Engert (:kaie) 2010-03-25 05:37:45 PDT
Explanation of changes:

- rename the current "Edit..." button to "Edit trust..."
  (will also effect the people and server tabs)

- on the tab with certificate authorities (the only one showing built-ins)
  rename the button "Delete..." to "Delete or Distrust..."

- Enhance the text shown in the confirmation dialog, 
  as seen in the screenshot. I'll repeat the text here:


  It currently says:
    (title bar): Delete CA Certificates

    Are you sure you want to delete these CA certificates?

    If you delete a certificate authority (CA) certificate, 
    this application will no longer 
    trust any certificates issued by that CA.



  I propose to change the text to:

    (title bar): Delete or Distrust CA Certificates

    You have requested to delete these CA certificates.
    For built-in certificates all trust will be removed, 
    which has the same effect. 
    Are you sure you want to delete or distrust?

    If you delete or distrust a certificate authority 
    (CA) certificate, this application will no longer trust
    any certificates issued by that CA.
Comment 9 Kai Engert (:kaie) 2010-03-25 05:38:58 PDT
Created attachment 434849 [details] [diff] [review]
Patch v1
Comment 10 Kai Engert (:kaie) 2010-03-25 05:42:06 PDT
The reason why I have used the wording "delete or distrust":

The user interface allows to select multiple certificates, and the user could select a combination of both built-in certificates (which can be distrusted) and certificates stored in the software database (which can be deleted).

It would be good to avoid programming logic that tried to analyse whether it's all delete, all distrust, or a mix.

In order to keep the code simple, I'd prefer this flexible wording.
Comment 11 Honza Bambas (:mayhemer) 2010-03-25 06:20:24 PDT
According to the description (if nothing has changed in those 8 years), while the built-in certs are not deleted, but just the trust flags are changed, such certs should not be removed from the UI and should just be somehow marked as "inactive" or "untrusted".  Kai, are you going to have a patch for that?
Comment 12 Kai Engert (:kaie) 2010-03-25 07:47:28 PDT
(In reply to comment #11)
> According to the description (if nothing has changed in those 8 years), while
> the built-in certs are not deleted, but just the trust flags are changed, such
> certs should not be removed from the UI

I think it's problematic to remove the built-in certs from the UI, because they are still there and it could cause confusion if someone tries to add them back.

> and should just be somehow marked as
> "inactive" or "untrusted".  Kai, are you going to have a patch for that?

I don't plan to implement that.
Comment 13 W. Craig Trader 2010-03-25 08:28:08 PDT
Kai, I like your proposed wording changes (they're much clearer than the existing wording), but that isn't enough to fix the problem.

Right now, if you delete (or distrust) a built-in CA cert, the UI will remove the cert from the list, which indicates that the cert has been deleted.  Once you've closed and reopened the cert manager, the cert reappears, with no perceptable difference in appearance.  The end result is that it looks like a bug because the cert magically reappears after deletion.

If the cert is actually distrusted (even though it has not been removed because it is built-in), then the UI needs to show that the cert is no longer trusted.

It ought to be enough to show the distrust by rendering the cert information in the list via a strike-through.
Comment 14 Kai Engert (:kaie) 2010-03-25 08:53:14 PDT
I agree that removing the cert from the list is a bad idea, if it will reappear later, that must be fixed.

Regarding how to visualize "no trust", that's a tricky thing, because most certs in cert manager (in the other tabs) will always be in that state (no explicit trust). We should have some consistent display.

Also, there are 3 individual trust flags, and any combination is possible. If a cert has been set to be trusted for only email certs, but not for web sites, then both variants (with or without strikeout) will be semi-correct.

The detailed trust can always be examined using the "edit trust" button.
Comment 15 W. Craig Trader 2010-03-25 09:06:53 PDT
So that's what 'Edit...' means for built-in CA Certs.  Not what I expected (and I've been using PKI for 14 years now).

If there's only 3 aspects of trust for a CA Cert, why not show 3 icons (web, email, code), to represent which aspects are trusted?  Or if that's too complicated, add 3 more columns to the list, with a Yes/No for each cert.

Whatever is done, the user should be able to tell when looking at the Authorities tab that a given CA Cert is no longer trusted, if it can't be removed.  Otherwise it all ends up looking like nothing was really done, which is where we are today.
Comment 16 Kai Engert (:kaie) 2010-03-25 09:31:49 PDT
Sigh, I tried to get the "keep in list" done, but there is no clear API that will tell us at JavaScript implementation level whether a cert will be deleted or kept.

I tried to work around that by always reloading after deleting. That didn't work well, either, I got wrong results for non-builtin certs, because of the current implementation and because of JS garbage collection, we keep the certificate in memory while the window is still open (can't delete from database while there are still objects in memory referencing it).

So, even if we have deleted a cert, and try to reload, we'll still find the deleted cert in memory :-( (as long as the window is still open).

Yes, this could be fixed, but a real nice solution will be a lot of work.

Improving the UI with the patch I've attached is the best I can offer with the little attention I can give to this now.

The user will be aware that the trust will go away, the new UI explains that, and that certs may still be around.
Comment 17 W. Craig Trader 2010-03-27 12:00:54 PDT
Well, as long as this doesn't get closed, there's the chance that someone will have time to address it in the future.
Comment 18 Marc 2010-04-08 05:56:03 PDT
Why are you thinking so complicated?
How about 3 columns in the list, one for each checkbox?
In my opinion, when I want to delete a ca, it should be removed.
I hate it when developers always know better what the users want.
Better but it two buttons. "Delete", "Untrust".
Or you can only delete the ca in the edit window.
Comment 19 David E. Ross 2010-04-10 10:36:34 PDT
I suggest that implementation of this bug be done in conjunction with bug 545498 and bug 558222.
Comment 20 Kai Engert (:kaie) 2010-04-12 08:17:18 PDT
test
Comment 21 Robert Relyea 2010-04-29 16:15:34 PDT
Comment on attachment 434849 [details] [diff] [review]
Patch v1

r+ for the code.

This is only a string chang. I think we really should have mozilla UI review it.

bob
Comment 22 Kai Engert (:kaie) 2010-05-03 02:49:07 PDT
(In reply to comment #18)
> Why are you thinking so complicated?
> How about 3 columns in the list, one for each checkbox?
> In my opinion, when I want to delete a ca, it should be removed.

We can't remove CAs that are included in the binary software, that's the reason.


When the existing user interface was built, people simply ignored that builtin certs can't be deleted. That's bad. I fully agree. I don't have an easy solution right now. Yes, the user interface should get reviewed, rebuilt, enhanced. But that takes more time.

My intention is to do an incremental improvement which I'm able to do now, using the limited time that's available.

I believe the wording change is such an improvement. Yes, the UI will still not be perfect. But it will teach the users the information that "deleting" a cert is quite similar from "distrusting" from a technical point of view. Given that the underlying crypto library does not allow to delete some certs, but only distrust them, the wording change is the best I can think of to reduce confusion with limited programmer resources.
Comment 23 Kai Engert (:kaie) 2010-05-03 02:49:56 PDT
Please note that I won't claim this bug fixed after applying this patch.
Comment 24 Kai Engert (:kaie) 2010-05-03 04:44:08 PDT
Wording improvement landed
http://hg.mozilla.org/mozilla-central/rev/c7d54732994b

Keeping bug open, because the original request still applies.
Comment 25 Marc 2010-05-03 05:21:02 PDT
(In reply to comment #22)
> We can't remove CAs that are included in the binary software, that's the
> reason.

So why are they included in the binary and not in a database.

On Windows, it would be ok that you can delete built in CAs only as an
admin if the database would be installed with FF and not be in the
profile folder.
Comment 26 John David Galt 2011-09-03 12:51:29 PDT
I don't know if this related problem needs (or has!) a separate bug, but I thought it should be mentioned and hopefully fixed.

After news reports that Comodo (or someone who hacked them?) issued phony certs for Google and 200 others, I deleted Comodo from the list.  Result: Firefox now does not even give me the option to trust any Comodo-derived cert (example: eff.org).  Viewing the cert gives me a "Get me out of here!" button, but "I understand the Risks" does NOT appear.

The end user needs to ALWAYS be given the option to manually declare ANY cert trusted.
Comment 27 Matt McCutchen 2011-09-03 13:39:57 PDT
(In reply to John David Galt from comment #26)
> I don't know if this related problem needs (or has!) a separate bug, but I
> thought it should be mentioned and hopefully fixed.
>
> After news reports that Comodo (or someone who hacked them?) issued phony
> certs for Google and 200 others, I deleted Comodo from the list.  Result:
> Firefox now does not even give me the option to trust any Comodo-derived
> cert (example: eff.org).  Viewing the cert gives me a "Get me out of here!"
> button, but "I understand the Risks" does NOT appear.
>
> The end user needs to ALWAYS be given the option to manually declare ANY
> cert trusted.

Unrelated issue.  This is HTTP Strict Transport Security working precisely as intended, though it sucks if for whatever reason you don't have all the mainstream CAs enabled.  If you really want to proceed with this configuration, you can delete the sts/use entry from $PROFILE/permissions.sqlite while the browser is not running, add a cert override, and then use NoScript's less fascist HSTS implementation to get at least the "force HTTPS" semantic (the browser's will not engage at all on a site with a cert override).  If you have a considered idea to make this better, file a separate RFE.
Comment 28 Matthias Versen [:Matti] 2011-09-08 14:43:14 PDT
*** Bug 685525 has been marked as a duplicate of this bug. ***
Comment 29 :Cykesiopka 2015-06-20 20:47:12 PDT
*** Bug 436619 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.