Closed Bug 1064402 Opened 5 years ago Closed 4 years ago

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

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: hrosik, Assigned: Cykesiopka)

References

Details

(Keywords: addon-compat)

Attachments

(2 files, 2 obsolete files)

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.
Component: General → Security
Component: Security → Security: PSM
Product: Firefox → Core
Yes, the import button is pretty much useless now.
Assignee: nobody → cykesiopka.bmo
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Version: 31 Branch → unspecified
Attachment #8715642 - Flags: review?(dkeeler)
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.
Attachment #8715642 - Flags: review?(dkeeler) → review+
Attachment #8716153 - Flags: review?(dkeeler)
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().
Attachment #8716153 - Flags: review?(dkeeler) → review+
Thanks for the review!

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

Done.
+ Mention Bug 1202636 in commit message as another reason why the import functionality should go away.
Attachment #8715642 - Attachment is obsolete: true
Attachment #8716595 - Flags: review+
+ Remove nsNSSCertTrust::SetValidServerPeer() as well
Attachment #8716153 - Attachment is obsolete: true
Attachment #8716596 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/2d3ec6c8bfe4
https://hg.mozilla.org/mozilla-central/rev/1bde49e1fb13
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
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.
Keywords: addon-compat
Blocks: 1202636
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?
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 ).
(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.
Depends on: 1305385

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

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

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.

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.

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. :)

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