Closed
Bug 108021
Opened 23 years ago
Closed 23 years ago
validation against CRL fails if next update is in past
Categories
(NSS :: Libraries, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
3.4
People
(Reporter: rangansen, Assigned: rrelyea)
References
Details
Attachments
(2 files)
(deleted),
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
By default, validation against crl should not fail even if 'next-update' is in the past. We should have some kind of an option/flag , and only when it is set, we should get validation error for 'next-update' being in the past. cc-ing Bob Relyea on this bug, for we had a discussion on this issue...
Assignee | ||
Comment 1•23 years ago
|
||
This needs to go into NSS 3.4. Assigning to me and setting the Priority and targets.
Assignee: wtc → relyea
Priority: -- → P1
Target Milestone: --- → 3.4
Updated•23 years ago
|
Version: unspecified → 3.3.1
Assignee | ||
Comment 2•23 years ago
|
||
Fixed in 3.4
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 3•23 years ago
|
||
Bob, how do I make use this feature in PSM? Do I have to enable some kind of a flag, or so? Shall greatly appreciate any advice on this...
Assignee | ||
Comment 4•23 years ago
|
||
Just use 3.4. Vallidate a cert which is certified by a CA which a loaded CRL. It should no longer look at the 'next update' time on CRL. bob
Reporter | ||
Comment 5•23 years ago
|
||
With the present fix, we do not have that flag/option [mentioned in the opening description, ie, above comment# 1 of this bug] to indicate whether the 'next-update' field should be taken into consideration. With this fix, validation against a crl does not at all look into the 'next-update' field. I believe we really should have this flag - it would be extremely useful. For example, a user has enabled autoupdate for a CRL and the auto-update fails for some reason[eg, for networking issues]. With this capability and the user having enabled strict validation [ie, always consider the 'next-update' date] he is still assured that he will never be using an old crl for validation. We might want to re-open this...
Reporter | ||
Comment 6•23 years ago
|
||
CERT_VerifyCertNow() still returns SECFailure when next-update is in the past. Actually, SEC_CheckCRL returns SECWouldBlock when next-update is in the past, but inside CERT_VerifyCertChain() we do something like: rv = SEC_CheckCRL(handle, subjectCert, issuerCert, t, wincx); if (rv == SECFailure) { LOG_ERROR_OR_EXIT(log,subjectCert,count,0); } else if (rv == SECWouldBlock) { /* We found something fishy, so we intend to issue an * error to the user, but the user may wish to continue * processing, in which case we better make sure nothing * worse has happened... so keep cranking the loop */ rvFinal = SECFailure; LOG_ERROR(log,subjectCert,count,0); } if ( issuerCert->trust ) { /* * check the trust parms of the issuer */ if ( certUsage == certUsageVerifyCA ) { if ( subjectCert->nsCertType & NS_CERT_TYPE_EMAIL_CA ) { trustType = trustEmail; } else if ( subjectCert->nsCertType & NS_CERT_TYPE_SSL_CA ) { trustType = trustSSL; } else { trustType = trustObjectSigning; } } flags = SEC_GET_TRUST_FLAGS(issuerCert->trust, trustType); if ( (flags & CERTDB_VALID_CA) || (certUsage == certUsageStatusResponder)) { if ( ( flags & requiredFlags ) == requiredFlags || certUsage == certUsageStatusResponder ) { /* we found a trusted one, so return */ rv = rvFinal; goto done; And finally, CERT_VerifyCertNow returns SECFailure ...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 7•23 years ago
|
||
This is really disturbing. The fix was clearly not checked in, in any of the files that should have affected this. I wonder if I did the commit from the wrong tree at some point?
Comment 9•23 years ago
|
||
Comment on attachment 71704 [details] [diff] [review] Remove LastUpdate checks when verifying certs. r=wtc. The local variable 'validity' should also be removed because it is now an unused variable. Bob, could you answer Rangan's question in comment #5? He suggested that we provide the user with an option to check the next-update field.
Attachment #71704 -
Flags: review+
Comment 10•23 years ago
|
||
Assignee | ||
Comment 11•23 years ago
|
||
The flag would require changes to the NSS 3 API in a number of functions (VerifyCert, VerifyCertChain, VerifyCrl). It has always been 'out of plan'. bob
Comment 12•23 years ago
|
||
No, I think that we had reached a compromise that didn't require us to modify the API. The flag was "global". On -> old behavior all cert verification fails is nextupdate is in the past. off-> new behavior, nextupdate is not used. I'm not sure it's super important to have it, especially since we don't have a way to manipulate that global pref.
Assignee | ||
Comment 13•23 years ago
|
||
We *proposed* a global pref. I remember specifically not agreeing to the pref, because it required to many changes to NSS. We settled on NSS not checking and applications deciding if they want to deal with CRL update fields on their own. bob
Comment 14•23 years ago
|
||
The fix has been checked into the tip and the NSS_CLIENT_TAG of NSS. Marked the bug fixed.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•