Last Comment Bug 867465 - Remove "Revocation Lists" feature
: Remove "Revocation Lists" feature
Status: RESOLVED FIXED
: user-doc-needed
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla24
Assigned To: Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
:
: David Keeler [:keeler] (use needinfo?)
Mentors:
Depends on:
Blocks: 83509 87325 91395 91534 91676 98193 102006 104137 119046 149835 150589 161820 169315 170317 193564 200630 233126 282945 322070 371501 379298 406559 418921 441742 524448 584066 645683 645825 645832 646445 646446 646447 646534 647726 647952 682830 783185 816495 836400 886099 892255
  Show dependency treegraph
 
Reported: 2013-04-30 17:24 PDT by Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
Modified: 2016-02-09 01:47 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
24+


Attachments
Before/after screenshot (108.19 KB, image/png)
2013-06-03 21:30 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
no flags Details
remove "Revocation Lists" UI (103.35 KB, patch)
2013-06-03 21:38 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
cviecco: review+
Details | Diff | Splinter Review
Remove "Revocation Lists" UI [v2] (110.22 KB, patch)
2013-06-16 22:28 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
brian: review+
MattN+bmo: review+
Details | Diff | Splinter Review

Description Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-04-30 17:24:21 PDT
Steps involved:

1. Remove nsICRLManager and nsICRLInfo, and their implementations.
2. Remove all the CRL updating code from nsNSSComponent
3. Remove the UI in Options -> Advanced -> Certificates -> Revocation Lists
4. Remove localized strings.

See the justification in the discussion:
https://mail.mozilla.org/pipermail/firefox-dev/2013-April/000329.html
Comment 1 Kaspar Brand 2013-04-30 23:43:08 PDT
(In reply to Brian Smith (:bsmith) from comment #0)
> 2. Remove all the CRL updating code from nsNSSComponent

For the sake of completeness: the PSM content listeners for application/x-pkcs7-crl, application/x-x509-crl and application/pkix-crl should be removed as well, I guess - see https://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSModule.cpp#339. Otherwise, clicking a CRLDP URL will still download a CRL, but it can't be deleted from the DB any longer.
Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-03 21:30:44 PDT
Created attachment 757790 [details]
Before/after screenshot
Comment 3 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-03 21:38:43 PDT
Created attachment 757794 [details] [diff] [review]
remove "Revocation Lists" UI

This patch removes the entire UI. This patch does not try to address the issue of CRLs that are already imported into the database, because we don't know whether or not those CRLs were imported via command-line tools by the system administrator. Since there is no change to NSS, NSS will continue to use those CRLs.
Comment 4 Camilo Viecco (:cviecco) 2013-06-13 16:57:48 PDT
Comment on attachment 757794 [details] [diff] [review]
remove "Revocation Lists" UI

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

Needs ui review too. 
Otherwise r+ for security/manager
Comment 5 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-14 17:24:22 PDT
Comment on attachment 757794 [details] [diff] [review]
remove "Revocation Lists" UI

This is the thing that was discussed here:
https://mail.mozilla.org/pipermail/firefox-dev/2013-May/000333.html
https://mail.mozilla.org/pipermail/firefox-dev/2013-May/000335.html
Comment 6 u60234 2013-06-14 23:40:14 PDT
There are some more strings in pippki.properties that will be unused and can be removed:
NoUpdateFailure
undefinedValStr
undefinedURL
yesButton
noButton

The following strings in pipnss.properties can also be removed:
CrlImportFailure1x
CrlImportFailureExpired
CrlImportFailureBadSignature
CrlImportFailureInvalid
CrlImportFailureOld
CrlImportFailureNotYetValid
CrlImportFailureNetworkProblem
CrlImportFailureReasonUnknown
CrlImportFailure2
Comment 7 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-16 22:28:14 PDT
Created attachment 763366 [details] [diff] [review]
Remove "Revocation Lists" UI [v2]

Hasse, thanks for looking at this. I updated the patch to address your suggestions.

dolske, could you please briefly take a look at the UI-related (XUL/JS/CSS) changes. If you are too busy, a referral to somebody else would be appreciated. Also, see the before/after screen shot in the other attachment.
Comment 8 Matthew N. [:MattN] (PM me if requests are blocking you) 2013-06-21 22:38:53 PDT
Comment on attachment 763366 [details] [diff] [review]
Remove "Revocation Lists" UI [v2]

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

Brian was looking to expedite this review so poked me about it tonight. r=me

Remove the services.sync.prefs.sync.security.OCSP.disable_button.managecrl pref from firefox.js as that pref won't be useful anymore.

::: browser/components/preferences/in-content/advanced.xul
@@ +439,3 @@
>  #ifdef XP_MACOSX
>          <vbox>
>  #endif

Remove these ifdef's for OS X because the three buttons should fit on one line now.

@@ -446,5 @@
>                    preference="security.disable_button.openCertManager"/>
> -          <button id="viewCRLButton"
> -                  label="&viewCRLs.label;" accesskey="&viewCRLs.accesskey;"
> -                  oncommand="gAdvancedPane.showCRLs();"
> -                  preference="security.OCSP.disable_button.managecrl"/>

Remove the associated <preference> elements from this file.
Comment 9 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-22 16:03:02 PDT
Thanks for the review Matt (and Hasse). I made all the suggested changes:

https://hg.mozilla.org/integration/mozilla-inbound/rev/308e3cd73c5f
Comment 10 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-22 21:00:07 PDT
Backed out in because of a build failure in another patch I checked in at the same time:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cac85f8f512

Re-landed
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd6e10c7db02
Comment 11 Phil Ringnalda (:philor) 2013-06-23 15:14:27 PDT
https://hg.mozilla.org/mozilla-central/rev/dd6e10c7db02
Comment 12 Kathleen Wilson 2013-08-13 15:09:03 PDT
I just learned about this bug today.

To summarize:

- As of Firefox 24 there is no user-interface for importing a CRL or modifying the CRLs that you have set to auto-import.

- All of the CRLs that you have setup for auto-import will continue to be auto-imported as per your previous settings. See Comment #3 for details.

- If you want to see and/or modify your list of auto-importing CRLs, you will need to install a previous version of Firefox.

- Or you can use crlutil
https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_crlutil
Comment 13 Kathleen Wilson 2013-08-14 17:22:10 PDT
Will the CRL UI remain in Thunderbird and SeaMonkey? Or will it be removed from those products too?
Comment 14 Matthew N. [:MattN] (PM me if requests are blocking you) 2013-08-14 17:37:14 PDT
(In reply to Kathleen Wilson from comment #12)
> - If you want to see and/or modify your list of auto-importing CRLs, you
> will need to install a previous version of Firefox.

While this is true, I don't think we should recommend this.

(In reply to Kathleen Wilson from comment #13)
> Will the CRL UI remain in Thunderbird and SeaMonkey? Or will it be removed
> from those products too?

This patch removed the UI that was shared by all of those applications. Bug 892255 and bug 886099 removed the button to open the UI from Thunderbird and Seamonkey respectively.
Comment 15 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-08-18 20:32:31 PDT
(In reply to Kathleen Wilson from comment #12)
> - As of Firefox 24 there is no user-interface for importing a CRL or
> modifying the CRLs that you have set to auto-import.

Yes.

> - All of the CRLs that you have setup for auto-import will continue to be
> auto-imported as per your previous settings. See Comment #3 for details.

No, they will not auto-update anymore. But, the last version that was imported will be used by NSS.

> - If you want to see and/or modify your list of auto-importing CRLs, you
> will need to install a previous version of Firefox.

No. The feature is effectively gone.

> - Or you can use crlutil
> https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_crlutil

That will work for NSS but not for insanity::pkix.
Comment 16 Kathleen Wilson 2013-08-19 14:25:02 PDT
> 
> > - Or you can use crlutil
> > https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_crlutil
> 
> That will work for NSS but not for insanity::pkix.


Just trying to understand... Does that mean that when Firefox picks up insanity::pkix, no more CRL checking will be done, even if someone has imported CRLs into NSS?
Comment 17 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-08-19 15:00:21 PDT
(In reply to Kathleen Wilson from comment #16)
> > 
> > > - Or you can use crlutil
> > > https://developer.mozilla.org/en-US/docs/NSS/tools/NSS_Tools_crlutil
> > 
> > That will work for NSS but not for insanity::pkix.
> 
> 
> Just trying to understand... Does that mean that when Firefox picks up
> insanity::pkix, no more CRL checking will be done, even if someone has
> imported CRLs into NSS?

Right.

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