Closed Bug 585122 Opened 14 years ago Closed 11 years ago

In PSM provide EV only with OCSP revocation checks (not CRL fallback)

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: kwilson, Assigned: cviecco)

References

Details

Attachments

(2 files, 4 obsolete files)

As per the EV Guidelines: CAs MUST support an OCSP capability for Subscriber Certificates that are issued after Dec 31, 2010. Please add code to PSM to not allow EV treatment when end-entity certs don't have an AIA OCSP URI.
(In reply to comment #0) > Please add code to PSM to not allow EV treatment when end-entity certs don't > have an AIA OCSP URI. ... but only for certs that were issued in year 2011 or later. If a cert has been issued prior to 2011, we still give EV treatment, even if the AIA OCSP URI is missing. Right?
But I would suggest that another revocation method is present and working. If the status of a certificate in the chain can't be confirmed, I suggest that the certificate shouldn't only show EV, it shouldn't trust it at all. As such, another enhancement would be to traverse all OCSP and CRL DPs before giving up. But that's probably a different bug.
(In reply to comment #2) > But I would suggest that another revocation method is present and working. If > the status of a certificate in the chain can't be confirmed, I suggest that the > certificate shouldn't only show EV, it shouldn't trust it at all. > > As such, another enhancement would be to traverse all OCSP and CRL DPs before > giving up. But that's probably a different bug. A different bug, quite. :)
> If a cert has been issued prior to 2011, we still give EV treatment, > even if the AIA OCSP URI is missing. > Right? Yes.
I believe this should also apply to intermediate CA's... EV treatment should only be allowed if both the end-entity cert and the intermediate certs have an AIA OCSP URI.
Kathleen, it is true that the EV Guidelines say "CAs MUST support an OCSP capability for Subscriber Certificates that are issued after Dec 31, 2010." Please re-read the first two words: "CAs MUST". I don't understand why you are interpreting "CAs MUST" as a requirement for PSM. When did PSM become a CA? Why shouldn't PSM continue to give EV treatment to EV certificates that only contain CRL Distribution Point URLs? Are CRLs no longer a good enough revocation mechanism? Is OCSP somehow "more secure" than CRLs? The EV Guidelines don't deprecate CRLs! Since when did "an OCSP capability" explicitly mean that the end-entity certificate must contain an AIA OCSP URL? If an Authorized OCSP Responder exists, but its URL is not quoted in end-entity certificates, does that mean that this Authorized OCSP Responder is not "an OCSP capability"?
Maybe a blue bar instead of green at EV certificates, when no aia ocsp is a good option.
(In reply to comment #7) > Maybe a blue bar instead of green at EV certificates, when no aia ocsp is a > good option. Please let's not invent yet another visible state for "half EV". Either it's EV or not.
Rob, why do you have a problem with adding an AIA OCSP URL to the certificates you will issue from 2011 ? If you dislike adding AIA OCSP URLs to your certificates from 2011, what equivalent OCSP capability do you offer, that can be discovered by looking at your certificates?
In reply to comment #9: > Rob, why do you have a problem with adding an AIA OCSP URL to the certificates > you will issue from 2011 ? Kai, we (Comodo) don't have a problem with this. We already include an AIA OCSP URL in every EV certificate we issue. However, some other CAs might have a problem with this. Hence my last post. I was trying to offer some constructive input regarding i) what the EV Guidelines actually mandate, and ii) what is or isn't reasonable for PSM to implement.
Some (mainly smaller) CAs might be reluctant to include AIA OCSP URLs at all, for fear that their OCSP Responders' (modest) server hardware might not cope with the (hard to predict) peak loads. However, those same CAs might be happy to deploy OCSP Responders that are intended to be used only by Servers that support OCSP "stapling", because the peak loads would be easier to predict. For certificate holders that use Apache: This could be achieved if the CA omits the AIA OCSP URL from the end-entity certificate, and instructs the certificate holder to use the "SSLStaplingForceURL" directive. Would you regard this as an "OCSP capability"? Or not?
(In reply to comment #5) > I believe this should also apply to intermediate CA's... EV treatment should > only be allowed if both the end-entity cert and the intermediate certs have an > AIA OCSP URI. We (DigiCert) already have an OCSP responder defined in our intermediate certificates (for both EV and non-EV) but I don't know if enforcing OCSP in the intermediate certificates is going to gain much aside from forcing certain CAs to reissue their intermediates. According to the way Mozilla treats failed OCSP requests, one could consider having an OCSP URI in an intermediate certificate a potential vulnerability, because of what Eddy points out: If Firefox tries to verify the intermediate certificate via OCSP and fails to receive an authoritative yes or no answer, then it will behave as if the answer was a 'yes' without checking the CRL. On the other hand, intermediate certificates which only have a CRL defined may be treated more securely if the CRL fails (I haven't tested this, so I don't know whether it behaves as it does for OCSP) I tend to agree with some of what Rob is saying: It's difficult to cope with a sudden spike of 2k+ or 10k+ OCSP requests per second, especially if serving responses via Apache/mod_worker. I'd love to see OCSP stapling encouraged in the way Rob describes, ESPECIALLY if it were possible to do that on a case by case basis. For example, to cope with the disproportionate load caused by our top 10 deployed certificates. OCSP stapling would really help here, but it would be difficult to achieve if Firefox automatically turned off the EV UI just because the end-entity certificate lacked an OCSP URI even though it received a valid stapled OCSP response... Paul Tiemann CTO, DigiCert
I think stapling needs to be first implement for Mozilla. This may take a while, in the meantime reasonable revocation checking must be enforced, specially for EV.
I think we need a deadline for the mandatory AIA:OCSP field. MAybe a later date, but a deadline. No deadline, no changes. :D
(In reply to comment #12) > According to the way Mozilla treats failed OCSP requests, one could consider > having an OCSP URI in an intermediate certificate a potential vulnerability, > because of what Eddy points out: > > If Firefox tries to verify the intermediate certificate via OCSP and fails to > receive an authoritative yes or no answer, then it will behave as if the answer > was a 'yes' without checking the CRL. On the other hand, intermediate > certificates which only have a CRL defined may be treated more securely if the > CRL fails (I haven't tested this, so I don't know whether it behaves as it does > for OCSP) That's not what I get from my recent tests. For a non-EV cert and an HTTP 403 error when asking for an OCSP status, you're right, Firefox 3.5.11 and 3.6.8 behave as if the answer is a 'yes', without checking the CRL. But for an EV cert, and an OCSP status "Try again later", the CRL is then downloaded, and the green bar displayed. > I tend to agree with some of what Rob is saying: It's difficult to cope with a > sudden spike of 2k+ or 10k+ OCSP requests per second, especially if serving > responses via Apache/mod_worker. I'd love to see OCSP stapling encouraged in > the way Rob describes, ESPECIALLY if it were possible to do that on a case by > case basis. For example, to cope with the disproportionate load caused by our > top 10 deployed certificates. OCSP stapling would really help here, but it > would be difficult to achieve if Firefox automatically turned off the EV UI > just because the end-entity certificate lacked an OCSP URI even though it > received a valid stapled OCSP response... Firefox could also use GET instead of POST to ask for an OCSP status. That way, an OCSP responder could use a classic static web cache (it's much easier to cache GET requests than POST ones). That's what Opera does.
(In reply to comment #15) > Firefox could also use GET instead of POST to ask for an OCSP status. Yes, that's the subject of bug 436414, actually. > That's what Opera does. And MS CAPI (on Vista and later).
(In reply to comment #16) > (In reply to comment #15) > > Firefox could also use GET instead of POST to ask for an OCSP status. > > Yes, that's the subject of bug 436414, actually. It's not very active. Has this proposition been abandoned?
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > Firefox could also use GET instead of POST to ask for an OCSP status. > > > > Yes, that's the subject of bug 436414, actually. > > It's not very active. Has this proposition been abandoned? There's isn't enough manpower to work on all bugs and enhancement requests, so inactivity often doesn't imply anything.
Johnathan, Kathleen: If I understand correctly, this change request also asks to disable the "embedded list of OCSP responders" starting on 2011-01-01, which got added in bug 485052. See http://mxr.mozilla.org/mozilla-central/ident?i=myDefaultOCSPResponders Can you please confirm?
I don't believe this bug necessarily requires the removal of the default OCSP responders, I think it could occur without that removal, but for what it's worth, I am in complete support of removing the default OCSP responder support on any branch where we now have CRLDP working (which, I think, is all of the supported ones, yes?) The callbacks might be nice to keep around, I'll leave that decision to you and the NSS guys, obviously, but the specific hard coded values don't need to persist, imo.
Put a later deadline on it. Force the AIA:OCSP at 2011-01-01 and drop the defaults at 2011-12-31 One year shoudl be enough for CAs, to rollover the certificates whic are not following these rules.
There is no rollover - this is a forward-looking rule about new certificates issued after a certain date which, at this point, is still in the future. (Though, regardless, EV CAs have had years of advance notice).
Here are some relevant quotes from The CA/Browser Forum’s current Guidelines for EV Certs, http://www.cabforum.org/Guidelines_v1_2.pdf -- Section 11: CAs MUST support an OCSP capability for Subscriber Certificates that are issued after Dec 31, 2010. Appendix B, Subordinate CA Certficates: (B) cRLDistributionPoint This extension MUST be present and MUST NOT be marked critical. It MUST contain the HTTP URL of the CA’s CRL service. (C) authorityInformationAccess This extension SHOULD be present and MUST NOT be marked critical. If present, it MUST contain the HTTP URL of the CA’s OCSP responder (accessMethod = 1.3.6.1.5.5.7.48.1). An HTTP URL MAY be included for the Root CA’s certificate (accessMethod = 1.3.6.1.5.5.7.48.2). Appendix B, Subscriber Certificates (B) cRLDistributionPoint This extension SHOULD be present and MUST NOT be marked critical. If present, it MUST contain the HTTP URL of the CA’s CRL service. This extension MUST be present if the certificate does not specify OCSP responder locations in an authorityInformationAccess extension. See Section 11 for details. (C) authorityInformationAccess This extension SHOULD be present and MUST NOT be marked critical. If present, it MUST contain the HTTP URL of the CA’s OCSP responder (accessMethod = 1.3.6.1.5.5.7.48.1). An HTTP URL MAY be included for the Subordinate CA’s certificate (accessMethod = 1.3.6.1.5.5.7.48.2) This extension MUST be present if the certificate does not contain a cRLDistributionPoint extension. See Section 11 for details. -- Therefore, please ignore Comment #5. My request in this bug is to update PSM to not allow EV treatment when end-entity certs that are issued after December 31, 2010, don't have an AIA OCSP URI. As stated in Comment #7, this essentially means that for end-entity certs that are issued after December 31, 2010, and don't have an AIA OCSP URI, only the blue bar may be displayed. Thanks, Kathleen
Hi Kathleen, Your submission of this bug used the EV guidelines as justification for the change in behaviour but as other contributors have recognised your requested change in behaviour goes beyond the requirements of the EV guidelines. Please would you help us understand why you are looking to introduce the new requirement for the inclusion of the AIA OCSP URI as a pre-requisite to giving consideration for EV treatment to a certificate? Regards Robin
According to the CA/Browser Forum's Guidelines for EV, after December 31, 2010, the end-entity cert is required to have an OCSP responder, and one way or another Firefox is going to have to find it to query the validity of the cert. We believe that putting the AIA OCSP URI into new end-entity certs is a reasonable requirement, especially since the OCSP data already must be available as per the guidelines.
Firefox does *not* have to find an OCSP responder to determine revocation status. The EV guidelines require that a URL to either CRL or OCSP is placed within the certificates. The EV guidelines require that OCSP responses are available after 31 Dec (at least for stapling). The EV guidelines do *not* require that OCSP responder URLs are published in certificates, and they do not therefore require that high volume OCSP responders are made available for relying party use. Mozilla is, of course, within its rights to insist on URLs pointing to high volume OCSP responders in certificates to which it grants EV chrome, but you should not (in my opinion) claim that this requirement comes from the current EV guidelines. If you feel the EV guidelines are deficient in this respect then you have representation on the CAB forum and a means to introduce your requirement there for the benefit of all relying parties.
I think Gerv tried to clarify that at the CAB Forum. Adding him to the CC list here.
According to the CA/Browser Forum's Guidelines for EV, after December 31, 2010, the end-entity cert is required to have an OCSP responder. We believe that one intention of the requirement is that browsers be able to find the OCSP responder to query the validity of the cert. The OCSP URI in the AIA is currently the best way for Firefox to find the OCSP responder. Firefox doesn’t currently support stapling; see bug #360420. Ideally, if a stapled OCSP response is included Firefox should honor it. If Firefox gets a stapled OCSP response from the server, then Firefox shouldn’t hit the OCSP responder specified in the certificate's AIA extension.
(In reply to comment #28) Kathleen, I'm going to try one more time to convince you/Mozilla to change your minds... > According to the CA/Browser Forum's Guidelines for EV, after December 31, > 2010, the end-entity cert is required to have an OCSP responder. I agree. There certainly cannot be an "OCSP capability" if no OCSP Responder server has ever been configured. > We believe that one intention of the requirement is that browsers be able to > find the OCSP responder to query the validity of the cert. Exactly which part(s) of the EV Guidelines cause you to believe this? Both before and after 31st Dec 2010, the EV Guidelines categorically *do not* require end-entity EV Certificates to contain the Authority Information Access extension (as long as the CRL Distribution Points extension is present). Appendix B(3)(C) of the EV Guidelines v1.2 says: "authorityInformationAccess This extension SHOULD be present and MUST NOT be marked critical. If present, it MUST contain the HTTP URL of the CA’s OCSP responder ... This extension MUST be present if the certificate does not contain a cRLDistributionPoint extension..." "SHOULD be present...If present" clearly means that the AIA extension can be optional! Section 11.1.1 of the EV Guidelines v1.2 says: "It is strongly RECOMMENDED that all CAs support OCSP when a majority of deployed Web servers support the TLS 1.0 extension in accordance to RFC 3546, to return “stapled” OCSP responses to EV-enabled applications. CAs MUST support an OCSP capability for Subscriber Certificates that are issued after Dec 31, 2010." Clearly the context of this paragraph is OCSP stapling, not AIA OCSP URLs. Therefore, I think it is entirely reasonable to declare Stapled OCSP Responses for EV Certificates that lack AIA OCSP URLs to be a perfectly valid "OCSP capability". > The OCSP URI in the AIA is currently the best way for Firefox to find the > OCSP responder. Firefox doesn’t currently support stapling; see bug #360420. True, and it seems to me that this state of affairs is blinkering/biasing your interpretation of the EV Guidelines. If Firefox does mandate that end-entity EV Certificates issued after 31st Dec 2010 MUST contain an AIA OCSP URL, then IMHO this would necessitate a change to the EV Guidelines in order for Mozilla's implementation of EV to be judged fully compliant. Also, please don't ignore the privacy issue that AIA OCSP URLs create. Stapled OCSP and CRLs don't suffer from this. Until such time as Firefox does support OCSP stapling, I would like to see it continue to treat both OCSP (via AIA OCSP URLs) and CRL as equally valid revocation mechanisms for allowing the EV green bar to appear. If you can't/won't agree with this, then I would like to see the EV Guidelines updated to unequivocally state that AIA OCSP URLs are a MUST post-2010. IMHO, the worst possible outcome of this discussion would be that we fail to reach (one way or another) the same interpretation of "OCSP capability".
Would the following flow satisfy these concerns? 1) Check for OCSP stapling response from server. 2) If cannot get a valid OCSP stapling response, then use OCSP URI in AIA to try to get OCSP response. 3) If these attempts to get a valid OCSP response fail, then EV treatment will not be given.
Kathleen, I'd be happy enough with that flow. I think that it is justifiable under the current EV Guidelines. It would of course mean that this bug becomes dependent on bug #360420. I'd be happier if CRLs could still be useable as an additional fallback revocation mechanism that can yield EV treatment. However, if nobody else wants this, I'm prepared to live without it.
(In reply to comment #31) > I'd be happier if CRLs could still be useable as an additional fallback > revocation mechanism that can yield EV treatment. However, if nobody else > wants this, I'm prepared to live without it. We all want this, the question is if somebody will code it.
Bug 360420 does not block this bug but work on bug 360420 is being prioritized with this in mind.
So, now we have OCSP stapling enabled. Also, the EV guidelines have said for more than 27 months that end-entity certificates MUST have an OCSP responder URI. Further, the EV guidelines have said for more than 27 months that intermediate CA certificates SHOULD have an OCSP responder URI. Further, 27 months is the maximum lifetime of an EV certificate, according to the EV guidelines. Finally, there is now a requirement that OCSP responders MUST NOT provide "good" responses for unknown certificates in effect; this is a feature that CRLs do not have. Therefore, I think we're more than justified in requiring that revocation information be provided by OCSP for EV certificates. Instead of checking explicitly for an OCSP responder URI in the AIA extension, let's simply remove the support for downloading CRLs from Firefox's EV checking. That will have the effect of enforcing that all certs in the chain have an OCSP AIA extension, except possibly for the end-entity certificate if the server stapled the end-entity OCSP response. I agree with the CA representatives that a missing OCSP AIA URL isn't harmful when a stapled OCSP response is provided. So, I am OK with allowing that exception, at least for now. A change to require OCSP for the intermediate certificates may negatively impact some sites that are using CAs that are still not including OCSP AIA URIs in their intermediate certificates in the sense that the EV indicator would no longer be shown. However, the sites would still work just fine so any negative impact will be very low. Let's try it out and see if there are any/many sites that actually lose the EV indicator because of this change.
Assignee: nobody → brian
Target Milestone: --- → mozilla25
QA Contact: cviecco
Assignee: brian → cviecco
QA Contact: cviecco
Depends on: 912155
As per discussions in the CA/Browser Forum Public list (Ballot 103), the OCSP URI needs to be in the AIA of the certificate, even if OCSP stapling is provided by the server. The browser should be able to fallback to the OCSP AIA URI if the server's OCSP stapling did not work. So I think it would be reasonable to not give EV treatment if the OCSP AIA URI is not present in the cert.
Target Milestone: mozilla25 → mozilla28
(In reply to Brian Smith (:briansmith, was :bsmith@mozilla.com) from comment #34) > let's simply > remove the support for downloading CRLs from Firefox's EV checking. That > will have the effect of enforcing that all certs in the chain have an OCSP > AIA extension, except possibly for the end-entity certificate if the server > stapled the end-entity OCSP response. I think ripping out CRL checking is a significant enough change that it should be discussed in m.d.t.crypto. Did I somehow miss that discussion? I saw the discussion about removing the user interface for CRL (though I didn't see closure on that discussion, even though the code change already happened). But I haven't seen discussion about ripping CRL checking out completely.
Attached patch require-oscp-aia-for-ev (obsolete) (deleted) — Splinter Review
What about this for checking BR requirements?
Attachment #8336775 - Flags: feedback?(dkeeler)
Comment on attachment 8336775 [details] [diff] [review] require-oscp-aia-for-ev Review of attachment 8336775 [details] [diff] [review]: ----------------------------------------------------------------- merge removed a call
Attachment #8336775 - Flags: feedback?(dkeeler)
Attached patch require-oscp-aia-for-ev (v2) (obsolete) (deleted) — Splinter Review
Attachment #8336775 - Attachment is obsolete: true
Attachment #8336781 - Flags: feedback?(dkeeler)
Comment on attachment 8336781 [details] [diff] [review] require-oscp-aia-for-ev (v2) Review of attachment 8336781 [details] [diff] [review]: ----------------------------------------------------------------- I think this is the right thing to do. Just a couple of comments. ::: security/manager/ssl/src/CertVerifier.cpp @@ +142,5 @@ > return rv; > } > > +static bool > +statisfiesEVBR(CERTCertificate * cert) { I have a feeling this should go in either an anonymous namespace or in CertVerifier:: - I would ask Brian. @@ +145,5 @@ > +static bool > +statisfiesEVBR(CERTCertificate * cert) { > + bool hasOCSPURL = false; > + > + char *loc = nullptr; You'll need to PORT_Free this pointer when done with it. @@ +150,5 @@ > + loc = CERT_GetOCSPAuthorityInfoAccessLocation(cert); > + if (!loc) { > + return false; > + } > + //Do we need to ensure the url has a n http or https scheme? rfc5019 says is must be http. @@ +152,5 @@ > + return false; > + } > + //Do we need to ensure the url has a n http or https scheme? > + hasOCSPURL = true; > + return hasOCSPURL; Tiny nit: I'm not sure it's important to have the variable hasOCSPURL - just return true or false as appropriate. @@ +201,5 @@ > // so we set the evPolicy to unkown in this case > if (CERT_LIST_EMPTY(trustAnchors)) { > evPolicy = SEC_OID_UNKNOWN; > } > + if (!statisfiesEVBR(cert)) { I think there are more to the baseline requirements than just an OCSP URI - maybe just call this hasOCSPURI or something for now.
Attachment #8336781 - Flags: feedback?(dkeeler) → feedback+
Summary: In PSM don't provide EV when cert doesn't have AIA OCSP URI → In PSM provide EV only with OCSP revocation checks (not CRL fallback)
Depends on: 942918
Attached patch ev-only-check-ocsp (obsolete) (deleted) — Splinter Review
Attachment #8336781 - Attachment is obsolete: true
Attached patch require-oscp-aia-for-ev-tests (obsolete) (deleted) — Splinter Review
Attachment #8341197 - Flags: review?(brian)
Attachment #8341198 - Flags: review?(brian)
Comment on attachment 8341197 [details] [diff] [review] ev-only-check-ocsp Review of attachment 8341197 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/manager/ssl/src/CertVerifier.cpp @@ +247,5 @@ > > if (evPolicy != SEC_OID_UNKNOWN) { > // EV setup! > // XXX 859872 The current flags are not quite correct. (use > // of ocsp flags for crl preferences). Please fix this comment. @@ +248,5 @@ > if (evPolicy != SEC_OID_UNKNOWN) { > // EV setup! > // XXX 859872 The current flags are not quite correct. (use > // of ocsp flags for crl preferences). > + uint64_t OCSPRevMethodFlags = variables names should not start with a capital letter. ocspRevMethodFlags would be better. Better still, since this variable is only used in one place, would be to just inline these values into the place where you reference OCSPRevMethodFlags below. @@ +263,3 @@ > rev.leafTests.cert_rev_flags_per_method[cert_revocation_method_crl] = > + rev.chainTests.cert_rev_flags_per_method[cert_revocation_method_crl] > + = CRLRevMethodFlags; I suggest you avoid the CRLRevMethodFlags variable and use CERT_REV_M_DO_NOT_TEST_USING_THIS_METHOD directly here. @@ +270,1 @@ > | (mOCSPGETEnabled ? 0 : CERT_REV_M_FORCE_POST_METHOD_FOR_OCSP); This doesn't make sense now. If we're going to have a separate OCSPRevMethodFlags variable, then (mOCSPGETEnabled ? 0 : CERT_REV_M_FORCE_POST_METHOD_FOR_OCSP) should be included in that variable's value.
Attachment #8341197 - Flags: review?(brian) → review+
Comment on attachment 8341198 [details] [diff] [review] require-oscp-aia-for-ev-tests Review of attachment 8341198 [details] [diff] [review]: ----------------------------------------------------------------- Out of curiosity, why are so many certificate files updated by this patch? Please double-check that all of the changes to binary files in this patch actually belong to this bug. Also, please verify that the test fails in the expected way without the fix for the bug, and that the test passes in the expected way with the fix for the bug. ::: security/manager/ssl/tests/unit/test_ev_certs.js @@ +17,5 @@ > let certList = [ > // Test for successful EV validation > 'int-ev-valid', > 'ev-valid', > + 'no-ocsp-url-cert', // a cert signed by the EV auth that has no ocsp url Nit: Capitalize OCSP in the comment. Add to the comment that the certificate does have a CRLDP. @@ +38,4 @@ > gHttpServer = new HttpServer(); > gHttpServer.registerPrefixHandler("/", > function handleServerCallback(aRequest, aResponse) { > + do_check_neq(aRequest.host, "crl.example.com"); //No CRL checks whitespace after "//" ::: security/manager/ssl/tests/unit/test_ev_certs/generate.py @@ +93,5 @@ > "int-" + prefix) > import_cert_and_pkcs12(int_cert, pk12file, "int-" + prefix, ",,") > import_untrusted_cert(ee_cert, prefix) > + > + #now we generate an end entity with-no-ocsp whitespace after "#" and before "with-no-ocsp" @@ +96,5 @@ > + > + #now we generate an end entity with-no-ocsp > + no_ocsp_url_ext_aia = ("authorityInfoAccess =" + > + "caIssuers;URI:http://www.example.com/ca.html\n"); > + [no_ocsp_key, no_ocsp_cert] = CertUtils.generate_cert_generic( db, whitespace after "("
Attachment #8341198 - Flags: review?(brian) → review+
Thanks for the reviews (this an the others) (In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #44) > Comment on attachment 8341198 [details] [diff] [review] > require-oscp-aia-for-ev-tests > > Review of attachment 8341198 [details] [diff] [review]: > ----------------------------------------------------------------- > > Out of curiosity, why are so many certificate files updated by this patch? Since generate.py is changed and the new certificate chains to an intermediate from whose key is not in the tree I decied to change all the cerificates (to match the new output from generate.py). > Please double-check that all of the changes to binary files in this patch > actually belong to this bug. There are only 2 cert files that could not be part of this patch, but for consistency I decided to change all to verify generate.py works as expected. > > Also, please verify that the test fails in the expected way without the fix > for the bug, and that the test passes in the expected way with the fix for > the bug. Done yes it does. Agree and implemented the restof your comments. > > ::: security/manager/ssl/tests/unit/test_ev_certs.js > @@ +17,5 @@ > > let certList = [ > > // Test for successful EV validation > > 'int-ev-valid', > > 'ev-valid', > > + 'no-ocsp-url-cert', // a cert signed by the EV auth that has no ocsp url > > Nit: Capitalize OCSP in the comment. Add to the comment that the certificate > does have a CRLDP. > > @@ +38,4 @@ > > gHttpServer = new HttpServer(); > > gHttpServer.registerPrefixHandler("/", > > function handleServerCallback(aRequest, aResponse) { > > + do_check_neq(aRequest.host, "crl.example.com"); //No CRL checks > > whitespace after "//" > > ::: security/manager/ssl/tests/unit/test_ev_certs/generate.py > @@ +93,5 @@ > > "int-" + prefix) > > import_cert_and_pkcs12(int_cert, pk12file, "int-" + prefix, ",,") > > import_untrusted_cert(ee_cert, prefix) > > + > > + #now we generate an end entity with-no-ocsp > > whitespace after "#" and before "with-no-ocsp" > > @@ +96,5 @@ > > + > > + #now we generate an end entity with-no-ocsp > > + no_ocsp_url_ext_aia = ("authorityInfoAccess =" + > > + "caIssuers;URI:http://www.example.com/ca.html\n"); > > + [no_ocsp_key, no_ocsp_cert] = CertUtils.generate_cert_generic( db, > > whitespace after "("
Attached patch ev-only-check-ocsp (deleted) — Splinter Review
keeping r+ from briansmith
Attachment #8341197 - Attachment is obsolete: true
Attachment #8342000 - Flags: review+
Attached patch require-oscp-aia-for-ev-tests (deleted) — Splinter Review
keeping r+ from bsmith
Attachment #8341198 - Attachment is obsolete: true
Attachment #8342002 - Flags: review+
cviecco landed this on inbound today: https://hg.mozilla.org/integration/mozilla-inbound/rev/7063dc99de44 https://hg.mozilla.org/integration/mozilla-inbound/rev/0c7a5bc1b06e Shortly after, we started seeing XPCShell issues like this: { TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/tests/xpcshell/tests/security/manager/ssl/tests/unit/test_ev_certs.js | test failed (with xpcshell return code: 0), see following log: TEST-UNEXPECTED-FAIL | /builds/slave/talos-slave/test/build/tests/xpcshell/tests/security/manager/ssl/tests/unit/head_psm.js | [addCertFromFile : 64] /builds/slave/talos-slave/test/build/tests/xpcshell/tests/security/manager/ssl/tests/unit/test_ev_certs/no-ocsp-url-cert.der does not exist TEST-UNEXPECTED-FAIL | xpcshell/head.js | NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init] } https://tbpl.mozilla.org/php/getParsedLog.php?id=31514218&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31514824&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31514645&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31515430&tree=Mozilla-Inbound Note that the not-found file, no-ocsp-url-cert.der, was added in the second patch here. We've had green XPCShell runs, too; it's not yet clear why the file is only reported as missing in some of these runs. But if the issue persists, we probably need to backout (or land a bustage fix if we can come up with one, or trigger a clobber if that happens to be required for whatever reason).
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: