Closed Bug 1047792 Opened 10 years ago Closed 10 years ago

Filter out expired issuer certificates within mozilla::pkix, instead of relying on NSS's CERT_CreateSubjectCertList to do it


(Core :: Security: PSM, defect)

Not set





(Reporter: briansmith, Assigned: briansmith)




(1 file)

David Keeler (:keeler) wrote in bug 1043041 comment 3:
> ::: security/pkix/lib/pkixbuild.cpp
> @@ +91,5 @@
> >  {
> >    if (newResult == Result::ERROR_UNTRUSTED_CERT) {
> >      newResult = Result::ERROR_UNTRUSTED_ISSUER;
> > +  } else if (newResult == Result::ERROR_EXPIRED_CERTIFICATE) {
> > +    newResult = Result::ERROR_EXPIRED_ISSUER_CERTIFICATE;
> I agree this is a good change to make, but it seems to violate the "one bug
> per bug" idea. Does it work to have the changes related to this be a
> separate patch? (it can be part 2 or part 3 of this bug)

In bug 1043041, we'll change the signature of TrustDomain::FindIssuer so that it no longer has a PRTime to pass to CERT_CreateSubjectCertList. Since we can't pass a time to that function, it won't be able to filter out expired issuer certificates. But, that's OK, because mozilla::pkix already checks the expiration itself anyway, so the check that CERT_CreateSubjectCertList is redundant.

A non-obvious consequence of this is that, in the case where there is only one potential issuer and it is expired, we may calculate a different result than we did previously. Previously, we'd return ERROR_UNKNOWN_ISSUER, but now we'll return ERROR_EXPIRED_ISSUER_CERTIFICATE for that case. Since ERROR_EXPIRED_ISSUER_CERTIFICATE is a more specific error, this side-effect is positive.

David had suggested that these changes be split out into a separate patch in bug 1043041. However, these changes can actually be made independently of the changes in bug 1043041, so I filed this bug to do that. David already reviewed these changes in bug 1043041.
Attachment #8466628 - Flags: review+
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.