Last Comment Bug 733454 - Remove hard-coded blocklisting in PSM for Comodo and Diginotar
: Remove hard-coded blocklisting in PSM for Comodo and Diginotar
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: Trunk
: All All
-- minor (vote)
: mozilla29
Assigned To: Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
:
: Dana Keeler [:keeler] (she/her) (use needinfo)
Mentors:
Depends on:
Blocks: 730734
  Show dependency treegraph
 
Reported: 2012-03-06 10:40 PST by Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
Modified: 2014-07-11 17:42 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove hard-coded blocking of Comodogate certificates from PSM (4.00 KB, patch)
2012-03-06 10:45 PST, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
kaie: review-
Details | Diff | Splinter Review
Remove hard-coded blocking of Diginotar certificates (7.36 KB, patch)
2012-03-06 10:47 PST, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
kaie: review-
Details | Diff | Splinter Review
Remove DigiNotar and Comodo hacks (10.63 KB, patch)
2013-07-03 14:29 PDT, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
cviecco: review+
Details | Diff | Splinter Review
bug-733454.patch (10.43 KB, patch)
2014-01-21 20:43 PST, Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
briansmith: review+
Details | Diff | Splinter Review

Description User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 10:40:51 PST
This blocking is handled at the NSS level, so we don't need this PSM code. The DigiNotar-related code is part of the cause of bug 730734.
Comment 1 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 10:45:51 PST
Created attachment 603349 [details] [diff] [review]
Remove hard-coded blocking of Comodogate certificates from PSM
Comment 2 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 10:47:15 PST
Created attachment 603351 [details] [diff] [review]
Remove hard-coded blocking of Diginotar certificates
Comment 3 User image Kai Engert (:kaie:) 2012-03-06 10:54:02 PST
Brian, 

please explain for both Comodo and DigiNotar why your request is justified.
Comment 4 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 11:15:03 PST
Aren't these checks redundant with the checks that are done in NSS, now that these certs are blacklisted at the NSS level?
Comment 5 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 11:16:35 PST
And if the NSS-level checks are not sufficient, then we should fix that, since the NSS-level mechanism is the mechanism we are planning to use for future blocking, including blocking the TrustWave MITM cert. So, AFAICT, this is dead code.
Comment 6 User image Kai Engert (:kaie:) 2012-08-15 01:30:57 PDT
Brian, you should implemented automatest tests that proof that your proposed changes are fine and still block the bad certs (I'm worried this code is still necessary).
Comment 7 User image Kai Engert (:kaie:) 2012-08-15 01:31:12 PDT
Comment on attachment 603349 [details] [diff] [review]
Remove hard-coded blocking of Comodogate certificates from PSM

missing tests
Comment 8 User image Kai Engert (:kaie:) 2012-08-15 01:31:31 PDT
Comment on attachment 603351 [details] [diff] [review]
Remove hard-coded blocking of Diginotar certificates

missing tests
Comment 9 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-07-03 14:29:44 PDT
Created attachment 771026 [details] [diff] [review]
Remove DigiNotar and Comodo hacks

Still needs tests.
Comment 10 User image Camilo Viecco (:cviecco) 2013-07-03 14:47:00 PDT
Comment on attachment 771026 [details] [diff] [review]
Remove DigiNotar and Comodo hacks

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

Do we have any tests that this is still working with the nss blacklist?
Comment 11 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-01-21 20:43:30 PST
Created attachment 8363448 [details] [diff] [review]
bug-733454.patch

Rebased, carrying over r+
Comment 12 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-01-26 22:39:11 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c4220bf9e14
Comment 13 User image Carsten Book [:Tomcat] 2014-01-27 04:15:59 PST
https://hg.mozilla.org/mozilla-central/rev/4c4220bf9e14
Comment 14 User image Masatoshi Kimura [:emk] 2014-01-27 16:25:52 PST
Why is this bug fixed without adding any tests?
Comment 15 User image Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-07-11 17:42:48 PDT
(In reply to Masatoshi Kimura [:emk] from comment #14)
> Why is this bug fixed without adding any tests?

A combination of laziness and deadlines, and the fact that these certificates either expired already (2014-03-xx) and/or they were untrusted by other means. For example, we removed the Diginotar roots from NSS and we disabled MD5-based signature verification a while back.

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