Open Bug 829677 Opened 7 years ago Updated 2 years ago

Remove cert entries for Actively Distrusted certs

Categories

(NSS :: CA Certificates Code, task)

task
Not set

Tracking

(Not tracked)

ASSIGNED
3.16.1

People

(Reporter: kwilson, Assigned: kaie)

References

Details

Attachments

(1 file, 3 obsolete files)

The NSS built-in cert list has two types of entries; cert entries and trust entries. A (dis)trust entry can be added without adding a corresponding cert entry. 

Please remove the cert entries for the Actively Distrusted certificates, so they don't show up in user interfaces for certificate management.
Assignee: nobody → brian
Target Milestone: --- → 3.15.2
TODO:

* Write tests that verify that these certs are still being blocked.
* Check if this change is enough to cause the certs to not show up in the PSM UI.
* Change the tool for generating these entries so that it omits the hash from the trust record and so that it omits the certificate entry by default.
Attached patch remove-distrusted-certs.patch (obsolete) — Splinter Review
Attachment #771029 - Attachment is obsolete: true
Attachment #812946 - Flags: review?(rrelyea)
Assignee: brian → nobody
Target Milestone: 3.15.2 → 3.15.5
Assignee: nobody → brian
Status: NEW → ASSIGNED
Target Milestone: 3.15.5 → 3.16
Kai, could you please review this for NSS 3.16? I am working on some tests for the distrust mechanism and these are getting in the way. Also, these certs are very confusing to end-users of Firefox who see them in the "Servers" tab of the certificate manager and freak out!
Attachment #812946 - Attachment is obsolete: true
Attachment #812946 - Flags: review?(rrelyea)
Attachment #8384868 - Flags: review?(kaie)
Comment on attachment 8384868 [details] [diff] [review]
remove-certs-from-distrust-records.patch

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

::: lib/ckfw/builtins/certdata.txt
@@ -13153,5 @@
> -\144\043\023\176\134\123\326\112\246\144\205\355\066\124\365\253
> -\005\132\213\212
> -END
> -CKA_CERT_MD5_HASH MULTILINE_OCTAL
> -\026\172\023\025\271\027\071\243\361\005\152\346\076\331\072\070

These hash entries are not needed for distrust to work. Note that our newer entries don't have them Also note that having these hash entries actually reduces the number of certs that are potentially blocked.

@@ +19853,5 @@
>  CKA_TRUST_EMAIL_PROTECTION CK_TRUST CKT_NSS_TRUSTED_DELEGATOR
>  CKA_TRUST_CODE_SIGNING CK_TRUST CKT_NSS_TRUSTED_DELEGATOR
>  CKA_TRUST_STEP_UP_APPROVED CK_BBOOL CK_FALSE
>  
> +# Trust for Certificate "Bogus Mozilla Addons"

Note that this diff gets formatted very poorly by the side-by-side diff mechanism in Splinter due to the repetition between the certificate entries that are removed and the trust entries that remain. I recommend reviewing the unified diff directly, instead of using a side-by-side diff review tool.
Comment on attachment 8384868 [details] [diff] [review]
remove-certs-from-distrust-records.patch

r- until you get approval from nss-dev and kwilson
Attachment #8384868 - Flags: review?(kaie) → review-
In my understanding, we still must block the certs.

If you want to remove the certificates, you would have to replace them with equivalent distrust records.

However, in the past, we didn't have the ability to block certs. We used a trick to override the bad certs using specially crafted, similar certs, with a newer date, which would be preferred by NSS.

If you wanted to replace those override certs using distrust records, the distrust records would have to be based on the properties of the original bad certs.

Did you make such changes?
Target Milestone: 3.16 → 3.16.1
Assignee: brian → nobody
Kai, can you own this?
(In reply to Ryan Sleevi from comment #7)
> Kai, can you own this?
Flags: needinfo?(kaie)
Assignee: nobody → kaie
Flags: needinfo?(kaie)
Kai, how does this look? I verified that each certificate removed has a distrust record.
Attachment #8384868 - Attachment is obsolete: true
Attachment #8810643 - Flags: review?(kaie)
The difficult is, there was a time, when NSS didn't support separate distrust records yet.

And to make things more difficult, we didn't have the ability to store a real "distrusted" flag, all we had was the "not trusted by default" flag.

I'm trying to remember all the details from that time, and will try to explain, why your suggested patch might be insufficient.


When we had to distrust a certificate, it was necessary to combine a certificate and a distrust record.

But we didn't want to ship the real bad certificates. Because if we had done that, users would have been able to go to certificate manager, and simple switch the embedded "not trusted by default" certificate to "trusted", and thereby explicitly and fully trusting a bad CA.

Because of that past unfortunate situation, we had decided to use a trick.

Instead of distrusting the real bad certificate, we manually created similarly looking certificates. For example, the certificate
  E=info@diginotar.nl,CN=DigiNotar Root CA,O=DigiNotar,C=NL
is such a manually created certificate, which I can identify easily, because I had used
  Serial Number:0f:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff:ff

We also called those special certificates "knockout" certificates.

(Not all our "knockout" certificates used such special serial number. Before I had the idea to mark these certificates with such a special serial number, we might have used knockout certificates that use random looking serial numbers.)

The knockout certificates look almost identical to the real certificates, but they differ in the NotBefore and NotAfter fields, and they differ in serial number from the original.

Also, because we created them by manually modifying the encoding of the original certificate, our knockout certificates don't have a valid signature.

Because the knockout certificate has the same issuer/subject names, NSS will find them as a potential issuer cert.

We changed the notBefore/notAfter timestamps to be one second later than the original certificates. That means, if NSS sees both the bad certificate and our knockout certificate at the same time, it will use the later one. It will ignore the bad certificate, and it will consider our knockout certificate as the better candidate.

Then NSS tries to find a trust record for our replacement knockout certificate, which said  "not trusted by default", which was sufficient for not trusting that chain.


In your patch, you have removed the DigiNotar certificate I have mentioned above, but the distrust record that you are keeping only matches our knockout certificate.

If the knockout certificate is gone, and we see the original DigiNotar certificate, the distrust record will be ignored, because the hashsum of the real certificate is different than the hashsum of the knockout certificate.


So, if you want to remove the old knockout certificates, and switch to using only distrust records, then you must carefully check what the new distrust record should look like. For that, you will need a copy of the real certificate that you want to block.


This is something that I could work on, but I never had the time to do so. Because I'm very busy, and because the issue that this bug attempts to solve is simply a cosmetic issue, I haven't been able to make it a high priority.
Comment on attachment 8810643 [details] [diff] [review]
remove certificate entries

I'm marking this as r-, because I believe with this patch applied, NSS would no longer block the DigiNotar CAs. I haven't had the time to check all changes, but I'm guessing there are additional changes which would remove the blocking of some bad CAs.
Attachment #8810643 - Flags: review?(kaie) → review-
You need to log in before you can comment on or make changes to this bug.