The problem is explained in bug 592978 and bug 640892. Basically, libpkix's support for returning error logs (CERTVerifyLog) is incomplete or worse. Instead of fixing libpkix's verify log support, I am going to attempt to change PSM's cert override logic to not require the error log. Here is the current code: http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSIOLayer.cpp#3412 Note that CERT_VerifyCertificate and CERT_PKIXVerifyCert can never put SSL_ERROR_BAD_CERT_DOMAIN in the verify log, so that case is dead code. Only CERT_VerifyCertName can return SSL_ERROR_BAD_CERT_DOMAIN, and we already handle that case earlier in the function by setting the ERROR_MISMATCH flag. Also, we can check the validity time for the EE certificate with CERT_CheckCertValidTimes(cert, PR_Now(), PR_FALSE). This allows us to set the ERROR_TIME flag when necessary. That means that, if we could tell CERT_PKIXVerifyCert to ignore the validity time of the EE certificate, then we would be able to use the result of PORT_GetError() after CERT_PKIXVerifyCert to determine whether to set the ERROR_UNTRUSTED flag. So, if we add a new parameter cert_pi_ignoreEndEntityCertTimes parameter to CERT_PKIXVerifyCert, we could avoid needing to use the cert error log at all. Besides being much simpler to get implemented, this would have positive performance impact, especially if/when we remove the redundant call to CERT_PKIXVerifyCert in our bad cert handler.

assumes the psm-chainvalidate patch has been applied.

assumes the psm callback interface has been applied.

Assume the psm chainvalidate interface patch has been applied.

Comment on attachment 676291 [details] [diff] [review] fix-time-check pkix v2.3 Review of attachment 676291 [details] [diff] [review]: ----------------------------------------------------------------- I'm operating on the assumption that you're going to rebase this on top of bug 813418. Let me know if that's not the case. ::: security/manager/ssl/src/SSLServerCertVerification.cpp @@ +531,5 @@ > + //workaround to time revocation in addtion of other checks.. > + if (srv != SECSuccess) { > + nsCERTValInParamWrapper pkixInParams2; > + PRTime notBefore, notAfter; > + srv = CERT_GetCertTimes(cert, &notBefore, &notAfter); Check return value. You can treat this as a non-recoverable error. @@ +533,5 @@ > + nsCERTValInParamWrapper pkixInParams2; > + PRTime notBefore, notAfter; > + srv = CERT_GetCertTimes(cert, &notBefore, &notAfter); > + > + if (notAfter<PR_Now()) { Instead of calling PR_Now() again, you should use the value of PR_Now() from the original cert verification. @@ +541,5 @@ > + collected_errors |= nsICertOverrideService::ERROR_TIME; > + errorCodeExpired = SEC_ERROR_EXPIRED_CERTIFICATE; > + > + pkixParamsToUse = &pkixInParams2; > + pkixInParams2.Construct(*survivingParams, &callbackContainer,notAfter-1); Why notAfter - 1, instead of notAfter? A certificate is still valid at its notAfter time. What happens with revocation checking when we pass an old time? Presumably, the OCSP responder and/or the NSS OCSP cache is always going to return current information. Are the time checks on those OCSP responses checked against the time passed to CERT_PKIXVerifyCert, or are they compared to PR_Now()?

assumes centralized verification has landed

Since FF 31 libpkix verification is not used by default, starting 33 is is not even used. Because of this this bug wont be fixed.