390536 - Cert validation functions must validate leaf cert themselves

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Alexei Volkov]

EE cert is not check for valid date, valid key usage, eku.

libpkix only checks leaf for revocation.

Comment 1

[Julien Pierre]

Alexei,

Are you certain about that ?
There is code in pkix_pl_ekuchecker.c to check the EKU, code in pkix_pl_cert.c to check key usage and validity .
Is this not specifically not happening on the EE but on other certs ?

Comment 2

[Alexei Volkov]

it does not check leaf cert only. All others get checked as a part of pkix_BuildForwardDepthFirstSearch. Caller of pkix_BuildForwardDepthFirstSearch does not check the cert and sets state->status to BUILD_INITIAL that makes libpkix to search for an issuer of leaf cert without it validation. Chain building should probably start from state->status = BUILD_CERTVALIDATING to validate the leaf.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

comment 0 seems like two separate bug reports:
a) EE certs not checked for validity, key usage, etc.

b) CA certs not checked for revocation.

Those are independent problems.  Please file another bug for the latter.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I corresponded with some other folks about this issue.  They stated that
RFC 3280's cert patch validation algorithm assumes that the caller has 
already tested that the leaf certs meets its expectations.  They stated 
that the algorithm as stated in RFC 3280 does not validate the leaf cert
except to show that it does or does not chain to a trust anchor. They
stated that the Java code that calls the Java version of libPKIX always
checks the leaf cert itself first.  So, I think that's the right thing 
for our CERT_* verification functions to do, rather than trying to change
libPKIX itself to do that.

Note that this requirement affects all our "wrapper" functions, including
the old CERT_VreifyCert* functions, CERT_VerifyCACertForUsage, and also
our NEW function that Kai, Steve and Bob are working on in bug 294531.

Comment 5

[Alexei Volkov]

this bug is fixed by patch to bug 390888, when we start using libpkix wrapper as a  substitution to CERT_VerifyCertChain function.

Comment 6

[Steve Parkinson]

Can you explain this a little? I don't see where you're calling any functions
to check the leaf. I see that you're calling cert_ProcessingParamsSetKuAndEku()
to set the KU and EKU, but doesn't that just check that the issuer chain is
valid, not the leaf?

FYI, we can add policy OIDs to the list of things that PKIX is not checking in
the leaf.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Steve,  The 4 old public CERT_VerifyCert* functions 
  CERT_VerifyCert
  CERT_VerifyCertNow
  CERT_VerifyCertificate
  CERT_VerifyCertificateNow
all check that the leaf meets their criteria, then they call 
CERT_VerifyCertChain to check that it chains to a trust anchor trusted for 
the purpose.  

When we set out to replace the old CERT_Verify code with libPKIX, we 
initially attempted to replace all 4 of those functions with code that
merely calls libPKIX (see the early patches for bug 390888).  Then we 
discovered that libPKIX wasn't doing any checks on the leaf.  To fix that,
we completely changed our approach.  We left the four functions intact, 
and instead we changed CERT_VerifyCertChain to call libPKIX.  So, by the 
time CERT_VerifyCertChain has been called, the leaf checking is all done 
(apart from the signature, which is checked using the issuer cert).  That 
takes care of all the old CERT_VerifyCert* functions.

So, now this bug applies mostly to the new CERT_PKIXVerifyCert function(s).
If those functions make the same mistaken assumption that we initially 
made when we tried to replace the 4 functions named above, namely that
the PKIX code will check the leaf cert for suitability for the caller's
purpose, then it will have the same flaw as our initial implementation 
of the replacements for those 4 functions, before we changed it.

This bug now serves to remind us that the new CERT_PKIXVerifyCert function(s)
must do all the relevant checks on the leaf cert itself, and not rely on 
libPKIX to do any of them.  Maybe that function does all the leaf checking
already, or maybe not.  If not, this bug should be reopened (IMO) until 
CERT_PKIXVerifyCert does all the necessary parts.

Comment 8

[Alexei Volkov]

I believe leaf policy OIDs are checked as a part of cert chain verification process, which happens in pkix_CheckChain.

Comment 9

[Steve Parkinson]

Thanks for the comments nelson. Now I understand.

Alexei - I've been stepping through pkix_CheckChain, and although the
pkix_PolicyChecker_Check() function does get called for the leaf, it
doesn't actually compare the OID.  I will go through it again to figure out
why.

Comment 10

[Alexei Volkov]

Steve, please reopen the bug if you find that leaf cert is not checked for policy OIDs.

Comment 11

[Kai Engert (:KaiE:)]

This bug sounds related to bug 408903