164707 - Provide a way to view/delete orphan certs

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Kai Engert (:KaiE:)]

Provide a way to view/delete orphan certs

Mozilla should provide a way to have a look at the list of certs that
Mozilla/NSS is storing in the user's profile, but which are not classified as
any of the 4 categories user/other people/web sites/CA.

I'm not sure whether we want to enable a 5th tab "orphans" by default, but at
least it can be helpful for analysis, as bug 163900 shows.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

This little patch, completely cloned from other already existant code, provides
the feature.

However, the new tab is not enabled by default. You have to manually set a pref
in prefs.js:
  user_pref("security.certmanager.showorphans", true);

If you do not have that pref, the tab will not be visible.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Patch v2

Comment 3

[Kai Engert (:KaiE:)]

I think we should take this patch.

Nelson, Sean, I have another suggestion for the additional "tab" of certificates
to add to cert manager.

What about naming the column "Miscellaneous"? And would it be fine to simply
call it "Misc"?

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Either Miscellaneous or Misc. would be fine with me.  Sean?

Comment 5

[Kai]

For what it's worth, stability enhanced Linux, 
Mac OSX and Windows builds based on Mozilla 1.3.1, 
that also contain the patch
from this bug, are available at http://wamcom.org

Comment 6

[Kai Engert (:KaiE:)]

Attachment: Patch v3

The previous patch bitrotted, updated to current trunk.

Comment 7

[Kai Engert (:KaiE:)]

Attachment: Screenshot

Comment 8

[Kai Engert (:KaiE:)]

Comment on attachment 240967 [details]
Screenshot

Hi Mike.
We would like to add a fifth tab to certificate manager.
While the existing four tabs display certs of well known types, the user might own other certificates.

Are you ok with the idea to add this extra tab?

In the past the wording "extra" has been suggested. Do you like that? (Another option would be "unknown")

Do you like the descriptive phrase (underlined in red in the screenshot).

Thanks in advance for your comments.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 240967 [details]
Screenshot

This will do as a stop-gap solution, but I'll file a follow-up bug to track what I think the real changes to the certificate manager should be in the long-term.

Comment 10

[Kai Engert (:KaiE:)]

Comment on attachment 240965 [details] [diff] [review]
Patch v3

Bob, can you please review the patch?

Comment 11

[Robert Relyea]

Comment on attachment 240965 [details] [diff] [review]
Patch v3

r+=relyea

Comment 12

[Kai Engert (:KaiE:)]

fixed on trunk

Comment 13

[Marek Stępień [:marcoos, inactive]]

deleteOrphanCertFlag=deleteOrphanFlag
deleteOrphanCertImpact=

I think you should add a localization note to these two properties, as it may not be clear to the localizers what to do with these... 

Comment 14

[Kai Engert (:KaiE:)]

(In reply to comment #13)
> deleteOrphanCertFlag=deleteOrphanFlag
> deleteOrphanCertImpact=
> 
> I think you should add a localization note to these two properties, as it may
> not be clear to the localizers what to do with these... 


Thanks for making me aware.
Actually, the deleteOrphanFlag is no longer necessary since bug 348259. It should have been removed from that older patch.

I'm reopening and will attach a supplemental patch.

Comment 15

[Kai Engert (:KaiE:)]

Attachment: Supplemental Patch v4

Comment 16

[Kai Engert (:KaiE:)]

Comment on attachment 248325 [details] [diff] [review]
Supplemental Patch v4

Bob, can you please review?

This patch removes the unnecessary strings from the bundle.

It fixes an issue with the delete function.

Before this patch, we defaulted to show all uncategorized certs in the server tab, which was incorrect. This patch will restored the UNKNOWN_CERT type when no other categorization is possible.

Comment 17

[Kai Engert (:KaiE:)]

Attachment: sample certdb

This is a sample cert db that can be used to QA this bug.
Extract this zip file to a new test profile directory.
With the patch applied, you'll get a cert in the Extra tab and you should be able to delete it.

Comment 18

[Robert Relyea]

Comment on attachment 248325 [details] [diff] [review]
Supplemental Patch v4

r=rrelyea

Comment 19

[Kai Engert (:KaiE:)]

Supplemental patch checked in, marking fixed.