399057 - CA certs used as server cert exceptions show no names

Status: RESOLVED WONTFIX

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

Description

[bill]

Attachment: security certificate exception wording issue

setup:
- using FireFox 3.0a9pre on Intel Mac OS X 10.4.10

steps to reproduce:
- go to https://www.dubin.org (which uses a self-signed CA to issue its cert)
- go through the steps to add this as a "security exception"
- view the security exceptions list

expected result:
- the Certificate name would identify the newly added CA/site certificate

actual result:
- Under "Certificate Name" (which should actually read "Issuer Name" I see "(Unknown)"
- And under "Certificate Name" for this exception I see "(Not Stored)"

Comment 1

[Kai Engert (:KaiE:)]

Bill, thanks for your bug report.
This must be some kind of edge case, and looks like a good test case.

We do import the cert obtained from www.dubin.org
I can see it displayed when using certutil to list all certs.

For some reason the dialog code is unable to find it for displaying.
I'll have a look.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: Patch v1

In your particular scenario, your cert got (incorrectly) sorted into the CA cert tab, because your server cert contains the "is a CA" flag!

It had not foreseen this scenario when I coded the new sorting logic.

I worked on a patch for this bug. I figured that we need to do test-for-stored-overrides in additional scnearios, and ended up doing a code simplification that is similar to what Bob R had proposed anyway.

In short, I have a patch that solves the problem for me.

Comment 3

[bill]

I figured this would be a good test case because it's also a highly likely case: I posit that most people who create self-signed and untrusted certificates also don't bother building a CA "hierarchy".

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Kai, why do you say it got incorrectly sorted?
It is (or claims to be) a CA cert.  
It was correctly sorted into the tab of CA certs.

Comment 5

[Kai Engert (:KaiE:)]

(In reply to comment #4)
> Kai, why do you say it got incorrectly sorted?
> It is (or claims to be) a CA cert.  
> It was correctly sorted into the tab of CA certs.

Well, but we didn't import it with the purpose of having it as a CA cert.

It was imported to "be available" for a stored server cert override.

Maybe what we really should do is: 
  Display the cert both in the CA tab and in the servers tab
?

But if we decide to do that, then we'll have to enhance the certificate manager code further. We'll have to support having a cert in both tabs. And when a cert gets deleted from one tab, ensure this doesn't cause problems for the other tabs. In other words, before deleting a cert, check whether the cert is used on other tabs, too.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Every cert has a fundamental binary property.
Like being pregnant: you are, or you are not.  There is no middle ground.
A cert is a CA cert, or it is not.  
We have two separate tabs, one for CA certs, and one for non-CA certs. 
People who want to create SSL server certs should not make them CA certs. 
On the other hand, if people choose to make and use CA certs for server 
certs, they should not complain if those certs are correctly displayed as 
CA certs.

Comment 7

[Bob Lord]

If it's a CA cert (intentional or not), I would expect to find it in the CA tab.

Of course, it would still need to be in the Server tab since that's where the exceptions live.

Comment 8

[Kai Engert (:KaiE:)]

I agree that people SHOULD NOT use CA certs for server certs, but the question is, what should we do, when people do anyway?

Should we reject attempts to add a server cert override for such bad certs who claim to be a CA cert? In my opinion: We should not reject them.


I would prefer this behavior:
- show the cert in the CA tab (because it's a CA cert)
- in addition, for each server override bound to that cert,
  display a line in the Servers Tab, 
  showing information from the associated cert (even if it's a CA cert)

Doing the above will require the improvements that I mentioned earlier:
Make the certificate manager smart enough to deal with certificates being referenced from multiple tabs.


I'm learning from your comments, my Patch v1 is a bad patch, because a CA cert stored in the database will not be shown in the CA tab.

Because of that I propose we wait until until we have the solution (cert referenced from multiple tabs).


Right now, the only problem is the incorrect display in the server tab, but everything will "work" as expected.

Comment 9

[Kai Engert (:KaiE:)]

Comment on attachment 284163 [details] [diff] [review]
Patch v1

removing review request and marking patch as obsolete

Comment 10

[Nelson Bolyard (seldom reads bugmail)]

Bob, in what sense do the exceptions live only in the server tab?
Do you mean that that is where the button lives?

This history of the cert manager has many examples where people import
a cert using a button in one tab, and then are surprised that the newly
imported cert shows up in a different tab, because it was not a cert of
the type to be shown in that tab.  

It appears that we are now saying that we wish to change the cert manager
so that a cert always shows up in the tab in which the import button was
pressed, even if that is the wrong tab for the type of cert that was 
imported.  

Comment 11

[Kai Engert (:KaiE:)]

Nelson, we are talking about "server overrides".
Overrides that are bound to a specific hostname, port and a particular cert.

Those overrides are designed to be shown in the Servers tab, only.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Kai, is there anything in the server tab that designates certs there as
override certs?  
Are all certs in that tab assumed to be overrides?
If not, does anything visually distinguish override certs from others?

Comment 13

[Kai Engert (:KaiE:)]

(In reply to comment #12)
> Kai, is there anything in the server tab that designates certs there as
> override certs? 

Yes. We have that "Site" column. If that column shows "*", then it's a real cert that is shown in that line. It's a server cert which got sorted into this column - and it's possible to edit its explicit trust.

All other entries which list a specific host:port in the "Site" column are "server override entries". Such an entry can have a cert associated. And it usually has. However, even if the cert is lost, like, it gets deleted externally from the NSS cert database, the "server override entry" will still be there. And you'll get the same display as shown in Bill's screenshot.


> Are all certs in that tab assumed to be overrides?

No. You can have both real-server-certs (distinguished by the "valid peer" flag, as used in the past when you used "permanently accept this cert"), and host:port-specific server overrides (with or without cert available in the NSS database).


> If not, does anything visually distinguish override certs from others?

See above.

Comment 14

[timeless]

fwiw, i hit this at:
https://svn.fmtc.be

(I was bugzilla hunting)

Comment 15

[(not currently active) Ted Mielczarek]

blocking+ since it seems likely that most self-signed certs are going to hit this.

Comment 16

[Kai Engert (:KaiE:)]

Adding a dependency to bug 401956, because they touch similar code, so we have a link between the bugs.

Comment 17

[Honza Bambas (:mayhemer)]

Attachment: Draft, displaying certs at all related tabs

I agree with possibility to display such (CA and server) certs in both tabs.

This patch adds new function that is based on fact that cert types are bit fields (1,2,4,8). It sets all bits in the mask according to what type the cert could be. It is then used when building the tree.

There is still the problem of deleting items from other tabs.
1. Easiest is to refresh the view (paid by performance)
2. Notify remaining tree views about deleting an item using some umbrella component (need new C code and iface)
3. Deleting by some unique identify of items (pointer to CERTCertificate NSS object? index in aCertList?). all items could be e.g. held in hash table by global index (aCertList index) and deleted in ALL tree views by this index (new method nsICertTree::DeleteEntryGlobalObject(in aGlobalIndex))

Opinions?

Comment 18

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 306394 [details] [diff] [review]
Draft, displaying certs at all related tabs

This patch looks OK to me, but I am not a peer of the PSM module, so I am forwarding this review request to Kai.

Comment 19

[Honza Bambas (:mayhemer)]

Attachment: Draft 2, displaying certs at all related tabs

I figured out that CA cert were displayed in People tab (as email certs). This patch fixes that problem.

Comment 20

[Kai Engert (:KaiE:)]

Comment on attachment 306526 [details] [diff] [review]
Draft 2, displaying certs at all related tabs

I really appreciate your work, but I think we must solve both problems at the same time.

Your patches solves the display-everywhere problem.

It does not solve the consistency-after-delete problem.

Comment 21

[Honza Bambas (:mayhemer)]

(In reply to comment #20)
> (From update of attachment 306526 [details] [diff] [review])
> I really appreciate your work, but I think we must solve both problems at the
> same time.
> 
> Your patches solves the display-everywhere problem.
> 
> It does not solve the consistency-after-delete problem.
> 

I know, only wanted to hear your opinion on this code. I didn't get any answer to comment 17. Do I have free hand on implementation?

Comment 22

[Honza Bambas (:mayhemer)]

Comment on attachment 306526 [details] [diff] [review]
Draft 2, displaying certs at all related tabs

It seems this needs a lot more work in nsCertTree component and certManager.js. UI is completely unprepared for displaying certificates in multiple tabs. Organization of the tree must be made in different way and referencing between several views must be reconsidered. This might take 5 days of intensive work, there is probably no one who understands this code very well.

Comment 23

[Honza Bambas (:mayhemer)]

Is this bug still actual?

Comment 24

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I can reproduce this bug, but I don't think it's worth the effort to fix it.