Closed Bug 1349762 Opened 7 years ago Closed 7 years ago

Disable EV treatment for two GlobalSign roots, and enable EV for an intermediate cert

Categories

(Core :: Security: PSM, enhancement, P1)

Product:
Core
Component:
Security: PSM
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: kathleen.a.wilson, Assigned: keeler)

References

Details

(Whiteboard: [psm-assigned])

Attachments

(2 files)

test-1349727.txt
7.78 KB, text/plain
Details
bug 1349762 - handle two GlobalSign EV root transfers
59 bytes, text/x-review-board-request
Cykesiopka
: review+
jcj
: review+
Details 
As explained in bug #1347882, Google Trust Services (GTS) recently purchased two roots from GlobalSign that are both enabled for EV treatment: "GlobalSign Root CA - R2" and "GlobalSign Root CA - R4".

However, GTS does not have an EV audit, so we are going to turn off EV treatment for both of those root certificates.

But "GlobalSign Root CA - R2" has intermediate cert "GlobalSign Extended Validation CA - SHA256 - G2" that continues to be controlled by GlobalSign, to be used to migrate their customers off dependence on that root. So we will turn on EV treatment for that intermediate cert, after it is added to the root store via bug #1349727.


Therefore, please enable the following certificate for EV treatment (after bug #1349727 is completed):

Friendly Name: GlobalSign Extended Validation CA - SHA256 - G2
SHA-1 Fingerprint: 65:BE:10:2B:E2:69:28:65:0E:0E:F5:4D:C8:F4:F1:5A:F5:F9:8E:8B
SHA-256 Fingerprint:
24:F9:1C:07:05:A0:A5:33:86:41:B3:65:FB:0D:9D:97:09:B5:62:97:CF:F1:85:7E:73:C0:2C:16:36:D4:86:AA
EV Policy OID: 1.3.6.1.4.1.4146.1.1
Test URL: https://2021.globalsign.com/

And please remove EV treatment for the following two root certificates:

1) GlobalSign ECC Root CA - R4
Certificate Serial Number: 2a38a41c960a04de42b228a50be8349802
SHA-1 Fingerprint: 69:69:56:2E:40:80:F4:24:A1:E7:19:9F:14:BA:F3:EE:58:AB:6A:BB
SHA-256 Fingerprint: BE:C9:49:11:C2:95:56:76:DB:6C:0A:55:09:86:D7:6E:3B:A0:05:66:7C:44:2C:97:62:B4:FB:B7:73:DE:22:8C

2) GlobalSign Root CA - R2
Certificate Serial Number: 0400000000010f8626e60d
SHA-1 Fingerprint: 75:E0:AB:B6:13:85:12:27:1C:04:F8:5F:DD:DE:38:E4:B7:24:2E:FE
SHA-256 Fingerprint: CA:42:DD:41:74:5F:D0:B8:1E:B9:02:36:2C:F9:D8:BF:71:9D:A1:BD:1B:1E:FC:94:6F:5B:4C:99:F4:2C:1B:9E
Attached file test-1349727.txt — 
David, I'm curious if the current PSM code will work as is, if the mentioned intermediate gets added as a trust anchor.

I'm attaching a file, which you could use for testing, by adding it at the end of certdata.txt

My question is:
After you remove the EV treatment for both roots as mentioned above, and after you add EV treatment for this intermediate, and after the attachment is added to certdata.txt, is that sufficient for the test suite to get EV treatment?

If yes, I'm OK with the request in bug 1349727.

If the above isn't sufficient, and PSM requires adjustment anyway to handle an intermediate whitelisted for EV, then my preference is to mark bug 1349727 wontfix, and rather have PSM treat that intermediate as EV, without requiring it to be marked as a trust anchor in addition.


It would be nice to get this clarified, prior to taking action in bug 1349727.
Thanks, Kai, for your suggestions and for raising concern. I am personally in agreement with your concern about Bug #1349727 -- It would be better if we could handle all of this exceptional case in PSM, without adding the intermediate cert to NSS. Therefore, I will greatly appreciate it if Keeler will look into this and respond in Bug #1349727 as to the possibility of updating PSM to allow for EV treatment to be specified for certs that are not directly in the NSS root store.
Flags: needinfo?(dkeeler)
I mentioned in bug 1349727 comment 14 that this is possible. However, if option 1 from bug 1349727 comment 13 is more viable, it would probably be best to go with that.
Flags: needinfo?(dkeeler)
Assignee: nobody → dkeeler
Priority: -- → P1
Whiteboard: [psm-assigned]
This was discussed in Bug #1349727, and decided "to special-case this intermediate and not build a new framework for something we're hopefully not going to do again." It was also decided that we will *not* add this intermediate cert to NSS.

So, please add the special-case code to enable EV treatment for the specific "GlobalSign Extended Validation CA - SHA256 - G2" intermediate cert listed above. And remove EV treatment for the two root certs listed above.

I am not aware of imminent security issues that this change will fix, but I will greatly appreciate it if this change can happen in Firefox 55.

Per https://bugzilla.mozilla.org/show_bug.cgi?id=1349727#c17 the last EV TLS cert signed by this intermediate expires in January 2019, so perhaps the special case code can be made to stop working then?

Thanks!
Comment on attachment 8854982 [details]
bug 1349762 - handle two GlobalSign EV root transfers

https://reviewboard.mozilla.org/r/126902/#review130442

Looks good.

::: security/certverifier/NSSCertDBTrustDomain.cpp:881
(Diff revision 1)
> +  0xda, 0x82, 0xd7, 0x02, 0x03, 0x01, 0x00, 0x01,
> +};
> +
> +template<size_t T, size_t R>
> +static bool
> +CertMatchesStaticData(const CERTCertificate* cert,

We should probably assert `cert` isn't null or something.
Attachment #8854982 - Flags: review?(cykesiopka.bmo) → review+
Comment on attachment 8854982 [details]
bug 1349762 - handle two GlobalSign EV root transfers

https://reviewboard.mozilla.org/r/126902/#review130506

I think it's good to go after the null-check.

::: security/certverifier/NSSCertDBTrustDomain.cpp:842
(Diff revision 1)
> +  0xbd, 0xdc, 0xd9, 0x5f, 0xdf, 0x72, 0xa9, 0x60, 0x13, 0x5e, 0x00, 0x01, 0xc9,
> +  0x4a, 0xfa, 0x3f, 0xa4, 0xea, 0x07, 0x03, 0x21, 0x02, 0x8e, 0x82, 0xca, 0x03,
> +  0xc2, 0x9b, 0x8f, 0x02, 0x03, 0x01, 0x00, 0x01,
> +};
> +
> +static const unsigned char sGlobalSignExtendedValidationCASHA256G2SubjectBytes[] = {

FWIW, I checked these against this ASN.1 decoder and they match the cert in question:

https://lapo.it/asn1js/#3082045D30820345A003020102020B040000000001444EF04A55300D06092A864886F70D01010B0500304C3120301E060355040B1317476C6F62616C5369676E20526F6F74204341202D20523231133011060355040A130A476C6F62616C5369676E311330110603550403130A476C6F62616C5369676E301E170D3134303232303130303030305A170D3231313231353038303030305A3062310B300906035504061302424531193017060355040A1310476C6F62616C5369676E206E762D7361313830360603550403132F476C6F62616C5369676E20457874656E6465642056616C69646174696F6E204341202D20534841323536202D20473230820122300D06092A864886F70D01010105000382010F003082010A0282010100A3EAA1D2C349E5F71C5DAFC39242AF8A3CDCEF4CE62F5F0C2B9F8A503066EF4EC84F214AF6E7F24E1B8C5357B09EC85BF7B84655B31AEDC26AFEF41BEC48460E8FE0FBE09119DF99186F2E51AFDAF69ACA646F99541074EA3CC8AA804D4337FBC8A47F059D3792BD9800355AAFBB5B74150EBCBCC6E9B786E7EEAE4D4B044C2BA0B46548B8C33ACD75BB37C94AC00111D9BF3F158660196B342046F586660F24F4CC629F9F9E1DFD10A4995EF041EBB094FF2CB336D6EB1DA7175FDFCE6A77C79AC43263A706ADF3121B9D3072590BEB72EB2AD2777B9177DB00FCD86FF52FD87AC50C3AA07B5E90F39D8459C801D9913756E53A5393AD60492725D9E1DA82D70203010001A382012830820124300E0603551D0F0101FF04040302010630120603551D130101FF040830060101FF020100301D0603551D0E04160414DA407743651CF8FEA7E3F464823E4D431322310230470603551D200440303E303C0604551D20003034303206082B06010505070201162668747470733A2F2F7777772E676C6F62616C7369676E2E636F6D2F7265706F7369746F72792F30360603551D1F042F302D302BA029A0278625687474703A2F2F63726C2E676C6F62616C7369676E2E6E65742F726F6F742D72322E63726C303D06082B060105050701010431302F302D06082B060105050730018621687474703A2F2F6F6373702E676C6F62616C7369676E2E636F6D2F726F6F747232301F0603551D230418301680149BE20757671C1EC06A06DE59B49A2DDFDC19862E300D06092A864886F70D01010B0500038201010040EF12908374968AF93ABA9B594A33D3EF4C132BB591CBC996ED6EF56C64F1C684B246595A588252F134A054416420ABD8573BD41474711836CC13C1C770C0F545660E71AE87AF9294EB714009F4CC77F71B93858A4AAE3385E674AEF510A63EC95983C3F95C96F928F7347BE938C6913C4F715875FEE1567576CD40C4154039A941FD64100F978507E87964D05B4D4C9B2797D3735E927E1F48E2CAB905974EEF2C1C6B4D8A5F785395CD0239C22FE6694FF671D199B57F6D20DE438FDB001BA33B37DED13F6DF3B690761DAC9D6F844F24940976E09DA84DF74D378FA42F5F4B41E4491697CC7B6CAF11CA9654098B2451AE5DEDA2F1BB53104D97FA1A7703

::: security/certverifier/NSSCertDBTrustDomain.cpp:881
(Diff revision 1)
> +  0xda, 0x82, 0xd7, 0x02, 0x03, 0x01, 0x00, 0x01,
> +};
> +
> +template<size_t T, size_t R>
> +static bool
> +CertMatchesStaticData(const CERTCertificate* cert,

Should check the cert pointer to be non-null
Attachment #8854982 - Flags: review?(jjones)
Comment on attachment 8854982 [details]
bug 1349762 - handle two GlobalSign EV root transfers

https://reviewboard.mozilla.org/r/126902/#review131662
Attachment #8854982 - Flags: review?(jjones) → review+
Pushed by dkeeler@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c1d15e5f6b0
handle two GlobalSign EV root transfers r=Cykesiopka,jcj
https://hg.mozilla.org/mozilla-central/rev/7c1d15e5f6b0
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: