Closed
Bug 1104109
Opened 9 years ago
Closed 9 years ago
December 2014 batch of EV root CA Changes
Categories
(Core :: Security: PSM, enhancement)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: kwilson, Assigned: jcj)
References
Details
Attachments
(2 files, 1 obsolete file)
4.82 KB,
patch
|
jcj
:
review+
|
Details | Diff | Splinter Review |
4.39 KB,
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
The purpose of this bug is to use a single patch to make the code changes for the December 2014 batch of EV-enablement changes (see the list of bugs this one blocks). Please enable EV treatment in source/security/certverifier/ExtendedValidation.cpp for the following root certs. == Bug #1062600 - Comodo - 3 roots == Test URL: https://comodorsacertificationauthority-ev.comodoca.com // CN=COMODO RSA Certification Authority,O=COMODO CA Limited,L=Salford,ST=Greater Manchester,C=GB "1.3.6.1.4.1.6449.1.2.1.5.1", "COMODORSACertificationAuthority", SEC_OID_UNKNOWN, { 0x52, 0xF0, 0xE1, 0xC4, 0xE5, 0x8E, 0xC6, 0x29, 0x29, 0x1B, 0x60, 0x31, 0x7F, 0x07, 0x46, 0x71, 0xB8, 0x5D, 0x7E, 0xA8, 0x0D, 0x5B, 0x07, 0x27, 0x34, 0x63, 0x53, 0x4B, 0x32, 0xB4, 0x02, 0x34 }, "MIGFMQswCQYDVQQGEwJHQjEbMBkGA1UECBMSR3JlYXRlciBNYW5jaGVzdGVyMRAw" "DgYDVQQHEwdTYWxmb3JkMRowGAYDVQQKExFDT01PRE8gQ0EgTGltaXRlZDErMCkG" "A1UEAxMiQ09NT0RPIFJTQSBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eQ==", "TKr5yttjb+Af907YWwOGnQ==", Test URL: https://usertrustrsacertificationauthority-ev.comodoca.com // CN=USERTrust RSA Certification Authority,O=The USERTRUST Network,L=Jersey City,ST=New Jersey,C=US "1.3.6.1.4.1.6449.1.2.1.5.1", "USERTrustRSACertificationAuthority", SEC_OID_UNKNOWN, { 0xE7, 0x93, 0xC9, 0xB0, 0x2F, 0xD8, 0xAA, 0x13, 0xE2, 0x1C, 0x31, 0x22, 0x8A, 0xCC, 0xB0, 0x81, 0x19, 0x64, 0x3B, 0x74, 0x9C, 0x89, 0x89, 0x64, 0xB1, 0x74, 0x6D, 0x46, 0xC3, 0xD4, 0xCB, 0xD2 }, "MIGIMQswCQYDVQQGEwJVUzETMBEGA1UECBMKTmV3IEplcnNleTEUMBIGA1UEBxML" "SmVyc2V5IENpdHkxHjAcBgNVBAoTFVRoZSBVU0VSVFJVU1QgTmV0d29yazEuMCwG" "A1UEAxMlVVNFUlRydXN0IFJTQSBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eQ==", "Af1tMPyjylGoG7xkDjUDLQ==", Test URL: https://usertrustecccertificationauthority-ev.comodoca.com // CN=USERTrust ECC Certification Authority,O=The USERTRUST Network,L=Jersey City,ST=New Jersey,C=US "1.3.6.1.4.1.6449.1.2.1.5.1", "USERTrust ECC Certification Authority", SEC_OID_UNKNOWN, { 0x4F, 0xF4, 0x60, 0xD5, 0x4B, 0x9C, 0x86, 0xDA, 0xBF, 0xBC, 0xFC, 0x57, 0x12, 0xE0, 0x40, 0x0D, 0x2B, 0xED, 0x3F, 0xBC, 0x4D, 0x4F, 0xBD, 0xAA, 0x86, 0xE0, 0x6A, 0xDC, 0xD2, 0xA9, 0xAD, 0x7A }, "MIGIMQswCQYDVQQGEwJVUzETMBEGA1UECBMKTmV3IEplcnNleTEUMBIGA1UEBxML" "SmVyc2V5IENpdHkxHjAcBgNVBAoTFVRoZSBVU0VSVFJVU1QgTmV0d29yazEuMCwG" "A1UEAxMlVVNFUlRydXN0IEVDQyBDZXJ0aWZpY2F0aW9uIEF1dGhvcml0eQ==", "XIuZxVqUxdJxVt7NiYDMJg==", == Bug #1069651 – GlobalSign – 2 roots == Test URL: https://2038r4.globalsign.com/ // CN=GlobalSign,O=GlobalSign,OU=GlobalSign ECC Root CA - R4 "1.3.6.1.4.1.4146.1.1", "GlobalSign ECC Root CA - R4", SEC_OID_UNKNOWN, { 0xBE, 0xC9, 0x49, 0x11, 0xC2, 0x95, 0x56, 0x76, 0xDB, 0x6C, 0x0A, 0x55, 0x09, 0x86, 0xD7, 0x6E, 0x3B, 0xA0, 0x05, 0x66, 0x7C, 0x44, 0x2C, 0x97, 0x62, 0xB4, 0xFB, 0xB7, 0x73, 0xDE, 0x22, 0x8C }, "MFAxJDAiBgNVBAsTG0dsb2JhbFNpZ24gRUNDIFJvb3QgQ0EgLSBSNDETMBEGA1UE" "ChMKR2xvYmFsU2lnbjETMBEGA1UEAxMKR2xvYmFsU2lnbg==", "KjikHJYKBN5CsiilC+g0mAI=", Test URL: https://2038r5.globalsign.com/ // CN=GlobalSign,O=GlobalSign,OU=GlobalSign ECC Root CA - R5 "1.3.6.1.4.1.4146.1.1", "GlobalSign ECC Root CA - R5", SEC_OID_UNKNOWN, { 0x17, 0x9F, 0xBC, 0x14, 0x8A, 0x3D, 0xD0, 0x0F, 0xD2, 0x4E, 0xA1, 0x34, 0x58, 0xCC, 0x43, 0xBF, 0xA7, 0xF5, 0x9C, 0x81, 0x82, 0xD7, 0x83, 0xA5, 0x13, 0xF6, 0xEB, 0xEC, 0x10, 0x0C, 0x89, 0x24 }, "MFAxJDAiBgNVBAsTG0dsb2JhbFNpZ24gRUNDIFJvb3QgQ0EgLSBSNTETMBEGA1UE" "ChMKR2xvYmFsU2lnbjETMBEGA1UEAxMKR2xvYmFsU2lnbg==", "YFlJ4CYuu1X5CneKcflK2Gw=", == Bug #1102519 - Entrust – 1 root == Test URL: https://2048test.entrust.net/ // CN=Entrust.net Certification Authority (2048),OU=(c) 1999 Entrust.net Limited,OU=www.entrust.net/CPS_2048 incorp. by ref. (limits liab.),O=Entrust.net "2.16.840.1.114028.10.1.2", "EntrustCA2048", SEC_OID_UNKNOWN, { 0x6D, 0xC4, 0x71, 0x72, 0xE0, 0x1C, 0xBC, 0xB0, 0xBF, 0x62, 0x58, 0x0D, 0x89, 0x5F, 0xE2, 0xB8, 0xAC, 0x9A, 0xD4, 0xF8, 0x73, 0x80, 0x1E, 0x0C, 0x10, 0xB9, 0xC8, 0x37, 0xD2, 0x1E, 0xB1, 0x77 }, "MIG0MRQwEgYDVQQKEwtFbnRydXN0Lm5ldDFAMD4GA1UECxQ3d3d3LmVudHJ1c3Qu" "bmV0L0NQU18yMDQ4IGluY29ycC4gYnkgcmVmLiAobGltaXRzIGxpYWIuKTElMCMG" "A1UECxMcKGMpIDE5OTkgRW50cnVzdC5uZXQgTGltaXRlZDEzMDEGA1UEAxMqRW50" "cnVzdC5uZXQgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkgKDIwNDgp", "OGPe+A==",
Assignee | ||
Comment 1•9 years ago
|
||
I'll get a patch put together.
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
This patch adds the new EV certificates to the list. Simple local testing at the provided test URLs and comparison to Aurora showed the expected EV behavior added for each of the certs. The build is processing on try right now; given the narrow scope, I don't expect any issues. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e32de55a1f82
Attachment #8527966 -
Flags: review?(dkeeler)
Assignee | ||
Comment 3•9 years ago
|
||
Try was happy. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e32de55a1f82
Comment on attachment 8527966 [details] [diff] [review] bug_1104109: Include the EV certificates as requested Review of attachment 8527966 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, although see comments. Thanks! ::: security/certverifier/ExtendedValidation.cpp @@ +1007,5 @@ > "VQQDExVRdW9WYWRpcyBSb290IENBIDIgRzM=", > "RFc0JFuBiZs18s64KztbpybwdSg=", > nullptr > + }, > + // == Bug #1062600 - Comodo - 3 roots == We probably don't need these lines - we can rely on mercurial/bugzilla history for this information (if you wanted, you could probably put it in the commit message on lines after the first one) @@ +1009,5 @@ > nullptr > + }, > + // == Bug #1062600 - Comodo - 3 roots == > + { > + // Test URL: https://comodorsacertificationauthority-ev.comodoca.com In the past, we haven't included the test urls here. I think they're of some use, although if we add them here, this list will be inconsistent. Would you mind either removing them from this patch or filing a follow-up bug to add test urls to each other entry? (Note that this information is in the root program Kathleen manages, so the information is available, just not as part of the source code. Also, when we've fully moved over to the new system, we can expose that information in a more dynamic way.)
Attachment #8527966 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 5•9 years ago
|
||
This patch removes the extra comments; thanks keeler! r=keeler
Attachment #8527966 -
Attachment is obsolete: true
Attachment #8528038 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 6•9 years ago
|
||
(In reply to J.C. Jones [:jcj] from comment #3) > Try was happy. > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e32de55a1f82 I only see Linux and Windows try builds. I have Mac OS X. So, if you need me to test this, can you create a try build for Mac for me?
Assignee | ||
Comment 7•9 years ago
|
||
Kathleen: I sure can; as I tested on MacOSX I didn't order a try build for that platform. The OSX try build is in progress here: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ade0347999e2
Comment 8•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f24a2739398d
Keywords: checkin-needed
Reporter | ||
Comment 9•9 years ago
|
||
(In reply to J.C. Jones [:jcj] from comment #7) > Kathleen: I sure can; as I tested on MacOSX I didn't order a try build for > that platform. > > The OSX try build is in progress here: > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ade0347999e2 I tested, and it all works as expected. Thanks!
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f24a2739398d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 11•9 years ago
|
||
I just noticed that the newly added EV roots aren't setting the oid_name field consistently. Please accept the attached patch. The inconsistencies were actually my doing, so I'm sorry about that. http://cert-checker.allizom.org/ seems to be using a build of GnuTLS that doesn't support ECC, so (in a hurry) I used a local build of https://github.com/mozkeeler/ev-checker to help Kathleen produce comment 0.
Attachment #8529004 -
Flags: review?(dkeeler)
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 12•9 years ago
|
||
(In reply to Rob Stradling from comment #11) > Created attachment 8529004 [details] [diff] [review] > Set nsMyTrustedEVInfo.oid_name values correctly > > I just noticed that the newly added EV roots aren't setting the oid_name > field consistently. Please accept the attached patch. > > The inconsistencies were actually my doing, so I'm sorry about that. > http://cert-checker.allizom.org/ seems to be using a build of GnuTLS that > doesn't support ECC, so (in a hurry) I used a local build of > https://github.com/mozkeeler/ev-checker to help Kathleen produce comment 0. I don't understand. Is the Cert Checker not producing the correct output? Or is there something we need to do with the output before copy-pasting it into ExtendedValidation.cpp ?
Comment on attachment 8529004 [details] [diff] [review] Set nsMyTrustedEVInfo.oid_name values correctly Review of attachment 8529004 [details] [diff] [review]: ----------------------------------------------------------------- I see. I think what's going on is that's a user-supplied description string that should have the property that if the OID matches another OID already present in the list, the descriptions should be the same too. We don't actually enforce this anywhere, and we probably should if it matters (it's not clear that it does, but if we register the same OID with different description strings, I imagine things could go wrong). In the meantime, I verified by hand that the new strings match the appropriate OID descriptions that are already present in the list. Thanks for following up on this, Rob.
Attachment #8529004 -
Flags: review?(dkeeler) → review+
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/61a1388dd1bc
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•