1064402 - Remove Import button in Server Certificates tab (or add option to edit trust)

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Petr Cerny [:hrosik]]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 SeaMonkey/2.26
Build ID: 20140505221916

Steps to reproduce:

Go to server for which there is no trust chain.
Add exception
Export server certificate
Delete server certificate
Import previously exported server certificate


Actual results:

The "server" field in the certificate list changes from the hostname under which it was added as an exception to "*", but the certificate is not trusted and the trust can't be edited from UI.

While I understand that it is a Bad Idea (TM) to blindly trust for example the Subject or Subject Alternative Name fields on an imported certificate (or even accepting it for a wildcard "*" hostname with which it gets imported), it doesn't seem to make much sense to have an Import button when the import actually doesn't effectively do anything. It seems as if the "Import" button should be removed or a UI for editing server certificate trust added.

Comment 1

[:Cykesiopka]

Yes, the import button is pretty much useless now.

Comment 2

[:Cykesiopka]

Attachment: bug1064402_rm-import-btn-certmgr-servers-tab_v1.patch

Comment 3

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8715642 [details] [diff] [review]
bug1064402_rm-import-btn-certmgr-servers-tab_v1.patch

Review of attachment 8715642 [details] [diff] [review]:
-----------------------------------------------------------------

Good call. I think we can go further, though. This is the only code that calls nsNSSCertificateDB::ImportCertsFromFile with nsIX509Cert::SERVER_CERT as the type of certificate, which is the only way nsNSSCertificateDB::ImportServerCertificate (aka nsIX509CertDB.importServerCertificate) gets called, so we can remove all of that server cert-specific code.

Comment 4

[:Cykesiopka]

Attachment: bug1064402_part2_rm-nsIX509CertDB-import-server-cert-support_v1.patch

Comment 5

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8716153 [details] [diff] [review]
bug1064402_part2_rm-nsIX509CertDB-import-server-cert-support_v1.patch

Review of attachment 8716153 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome!

::: security/manager/ssl/nsNSSCertificateDB.cpp
@@ -711,5 @@
> -    nsrv = NS_ERROR_FAILURE;
> -    goto loser;
> -  }
> -
> -  trust.SetValidServerPeer();

Looks like we can also remove nsNSSCertTrust::SetValidServerPeer().

Comment 6

[:Cykesiopka]

Thanks for the review!

(In reply to David Keeler [:keeler] (use needinfo?) from comment #5)
> Looks like we can also remove nsNSSCertTrust::SetValidServerPeer().

Done.

Comment 7

[:Cykesiopka]

Attachment: bug1064402_part1_rm-import-btn-certmgr-servers-tab_v2.patch

+ Mention Bug 1202636 in commit message as another reason why the import functionality should go away.

Comment 8

[:Cykesiopka]

Attachment: bug1064402_part2_rm-nsIX509CertDB-import-server-cert-support_v2.patch

+ Remove nsNSSCertTrust::SetValidServerPeer() as well

Comment 9

[:Cykesiopka]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa205b7be666

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d3ec6c8bfe4
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bde49e1fb13

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2d3ec6c8bfe4
https://hg.mozilla.org/mozilla-central/rev/1bde49e1fb13

Comment 12

[:Cykesiopka]

Part 2 affects addon compat by removing nsIX509CertDB.importServerCertificate() and nsIX509Cert::SERVER_CERT support in nsIX509CertDB.importCertsFromFile().

The same effect can probably be achieved by using nsIX509CertDB.addCert() instead.

Comment 13

[Mike Kaply [:mkaply]]

So this breaks the CCK2.

We had specific requests from enterprises to import server certificates, so we use this API in the CCK2.

I don't believe addCert imports a server certificate specifically, does it? It just adds a generic certificate.

I honestly didn't know exactly what I was adding, I just know people said "please support server certificates" and I did using this API.

Can someone explain what a server certificate is and how it differs from a regular certificate?

Comment 14

[Dana Keeler (she/her) [:keeler]]

Generally, I think a "server certificate" is the end-entity certificate presented by a server in a TLS handshake. It may be that it used to be possible in Firefox to trust a server certificate as its own trust anchor (that is, when validating it, it wouldn't be necessary to find a trusted certificate that issued that certificate). However, this is not currently possible. What you can do instead is use nsICertOverrideService to add an override for a given host and certificate (and port, and expected error bits - see https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsICertOverrideService.idl ).

Comment 15

[Mike Kaply [:mkaply]]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #14)
> Generally, I think a "server certificate" is the end-entity certificate
> presented by a server in a TLS handshake. It may be that it used to be
> possible in Firefox to trust a server certificate as its own trust anchor
> (that is, when validating it, it wouldn't be necessary to find a trusted
> certificate that issued that certificate). However, this is not currently
> possible. What you can do instead is use nsICertOverrideService to add an
> override for a given host and certificate (and port, and expected error bits
> - see
> https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/
> nsICertOverrideService.idl ).

So given that I already support overrides for specific domains (using the cert override mechanism), it sounds like I should probably just remove support for server certs since they don't work anyway.

Thanks for the clarification.

Comment 16

[u218749]

I would like to import some server certificates. Since the button has been removed: What is the intended way to do that?

Comment 17

[Dana Keeler (she/her) [:keeler]]

You can use certutil, but I guess my question would be what effect are you hoping this will have?

Comment 18

[u218749]

Well, I got a new laptop and I wanted to export the certificates I had manually trusted to import them again and thus carry over the trust.

Comment 19

[Dana Keeler (she/her) [:keeler]]

Hmm - that probably won't work. Maybe copy over the files cert9.db and cert_override.txt from your old profile to the new one? Or maybe I'm misunderstanding. Are these root certificates that you're trying to trust? You should just be able to import them in the "Authorities" tab.

Comment 20

[u218749]

No, these are just individual servers with usually self-signed certs or from organizations with internal CAs which I don't have. Maybe copying some of the cert files would work but it's not a huge deal anyway. I was just wondering if there was some other easy way to import them but if there isn't then I'm not going to bother. :)