Revoked certificate verification succeeds if there is an error during CRL lookup

RESOLVED FIXED in 3.6

Status

NSS
Libraries
P1
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Julien Pierre, Assigned: Wan-Teh Chang)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

15 years ago
The function SEC_CheckCrl, which is used as part of certificate verification,
uses  SEC_FindCrlByName, whicih returns a pointer to a decoded CRL object, a
CERTSignedCRL*.

If a CRL is found in a token, but its DER fails to decode, then a NULL pointer
is returned to SEC_CheckCRL, which never knows that there was a bad DER CRL, and
therefore returns success.

The correct behavior is to do it all in SEC_CheckCrl . It should call
PK11_FindCRLByName, which will return the DER CRL.

It can then do the decoding by calling CERT_DecodeDERCrl, and fail the
verification if appropriate.

Doing the decoding in SEC_CheckCrl also provides the opportunity for a
performance optimization : it can do only a partial decoding of the CRL before
the signature verification, instead of a full decode. The entries don't need to
be decoded until after the signature verification has succeeded, and this can be
done with CERT_CompleteCRLDecodeEntries.

An actual example of how this can happen is if there is some disk corruption in
the cert database, which affects one of the ASN.1 tags in the CRL, that causes
the decoding to fail, and therefore the CRL validation to be bypassed.

If CRLs are stored on other tokens, the type of attack to corrupt it may be
different - and our current code would happily discard the corrupt CRL and
validate the cert.
(Reporter)

Comment 1

15 years ago
FYI, much of this would become irrelevant with the CRL cache, as SEC_CheckCrl
will end up going to the cache. This is an existing issue in our previous
implementations. I need to make sure that the cache handles this correctly and
it has an "invalid" state in which all the certs are rejected.
(Reporter)

Comment 2

15 years ago
There are other conditions that can cause this erroneous successful verification
to occur. The CRL lookup can also  fail because of an insufficient memory condition.
Depends on: 160635, 164501
Priority: -- → P1
Summary: Certificate verification succeeds if a token supplies an invalid DER CRL for an issuer → Revoked certificate verification succeeds if there is an error during CRL lookup
Target Milestone: --- → 3.6
(Reporter)

Comment 3

15 years ago
Resolved by CRL cache changes.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 4

15 years ago
Actually, I wasn't checking for invalid cache status. I need to automatically
consider all certs revoked in case I have a bad CRL in the cache . Patch to follow.

(Reporter)

Comment 5

15 years ago
Created attachment 98095 [details] [diff] [review]
patch to consider certs revoked when the cache has found a bad CRL for the cert issuer

as checked in to the tip
(Reporter)

Comment 6

15 years ago
FYI, a test case for this can be created. A bogus CRL can be inserted into the
database via crlutil -B (bypass checks, which skips the signature check).
It should be a valid DER CRL, but some data should be modified to make the
signature fail.

The CRL cache will then load the CRL from the token during the cert
verification. It will verify its signature, and that will fail. The cert
verification will therefore be failed for all certs from that issuer.

This prevents an attack against the CRL revocation mechanism. The only way to
defeat the revocation at that point is to completely delete the CRL from the token.

FYI, I ran into this in another case :

The softoken provided an invalid DER CRL to the cache. It was a SECItem with a 0
length and non-NULL pointer. This was caused by an out-of-memory condition. See
bug 164501. The attached patch also catches that case.
You need to log in before you can comment on or make changes to this bug.