Closed Bug 1104109 Opened 9 years ago Closed 9 years ago

December 2014 batch of EV root CA Changes

Categories

(Core :: Security: PSM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: kwilson, Assigned: jcj)

References

Details

Attachments

(2 files, 1 obsolete file)

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==",
I'll get a patch put together.
Assignee: nobody → jjones
Status: NEW → ASSIGNED
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)
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+
This patch removes the extra comments; thanks keeler! r=keeler
Attachment #8527966 - Attachment is obsolete: true
Attachment #8528038 - Flags: review+
Keywords: checkin-needed
(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?
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
(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!
https://hg.mozilla.org/mozilla-central/rev/f24a2739398d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
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)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(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+
https://hg.mozilla.org/mozilla-central/rev/61a1388dd1bc
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.