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

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: filter-expired-certificates-better.patch

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.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64719bb17179

Comment 2

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/64719bb17179