Open Bug 378098 Opened 17 years ago Updated 4 months ago

Do not expire OCSP responses that say "revoked"

Categories

(NSS :: Libraries, enhancement, P3)

3.11.7
enhancement

Tracking

(Not tracked)

People

(Reporter: KaiE, Unassigned)

References

Details

Attachments

(2 obsolete files)

This bug is a follow up to bug 205406.

Julien proposed: When an OCSP server reports an entry as revoked, we should not query the OCSP server again. We should cache the response forever.

I will attach a patch that implements it, on top of the code from bug 205406.
Julien, you said, as soon as a responder reports a cert as being revoked, this can never get reverted.

Even if an OCSP responder accidentially reports a cert as being revoked, it is not allowed to undo it, because of specs.

Two things come to my mind:


a) What happens if there is a bug in OCSP responder software, and a CA reports all its certs as being revoked?

This would be a dramatic problem for a CA. It would force most of its customers to install a new cert.
Should this ever happen in the real world, I'm tempted to say, a CA might try to avoid it, fix the bug in the responder (only), and hope that all OCSP clients will fetch new responses at some point.

A decision to cache revoked OCSP entries forever would mean: CA has no chance to fix it, but must issue new certs to all customers.


b) Change revocation timestamp.

Suppose the first customer report is "cert got compromised yesterday". And it gets revoked with a date of 2007-04-19.

The next day the customer learns the cert got compromised much earlier and asks the CA "please mark it revoked as of 2007-04-01".

This would not be possible with our cache-forever strategy.


Do you think our code should behave in a way that supports scenarios a and b?

(I suspect you'll say "no" but I wanted to ask.)
Attached patch Patch v1 (obsolete) — Splinter Review
Proposed patch
Attachment #262181 - Flags: review?(julien.pierre.boogz)
Attached patch Patch v2 (obsolete) — Splinter Review
Sorry, there was a typo in v1, attaching better patch v2.
Attachment #262181 - Attachment is obsolete: true
Attachment #262182 - Flags: review?(julien.pierre.boogz)
Attachment #262181 - Flags: review?(julien.pierre.boogz)
Kai,

You bring up good points.

As far as case a), I don't think we should support it. If the CA messes up this badly, they have to issue new certs. Unrevoking is not permitted by the standard.

But case b) is legit. In cases of key compromises in particular, the revocation information can change.

What I was trying to say this morning in the meeting was about the following case :
The cert is revoked and the OCSP response is in the cache.

Then, NSS goes back to the responder, and gets a response that either the CA doesn't know about the cert validity anymore (because it expired since, and it erased the revocation information, which is legit) or because the CA is messed up and tried to "unrevoke" the cert. For those two cases, we should still keep the old OCSP revoked response in the cache.

There is one exception, which is if the cert was marked revoked with a reason code of "on hold". In this one case, the CA is allowed to unrevoke the cert. So your patch needs to check the reason code in the response in the cache in order to decide what to do.

You said in the meeting that the solution was to just not check status anymore for certs that are already revoked. But in fact, as you found out, in case b, this is not correct - there may be an earlier revocation (although, all of this is really moot for offline protocols without a secure time source ... Separate discussion, though).

Another thing to take into account if the revocation information about a revoked cer tchanges is the reason code. If a cert was revoked with a reason "key compromise" for example, it should not be permitted to be put "on hold" later, since that would potentially allow it to be unrevoked later, which is illegal.

So, I think your patch v1/v2 is incomplete. Sorry about the confusion.
This how much earlier this issue would have been discovered if someone 
hadn't refused my SR request in bug 205406.
Attachment #262182 - Attachment is obsolete: true
Attachment #262182 - Flags: review?(julien.pierre.boogz)
In response to comment 5:
Nelson, I agree with you that additional review is good. I'm not sure about "earlier" because Julien is short of time. But, I'll be gladly accepting review input from Julien, if he finds time!

IIUC the issue reported by Julien is an improvement. The current OCSP cache behaviour is not making things worse than NSS behaviour without cache.

Because as of today, with or without the code from bug 205406, NSS will check with the responder for a current OCSP status.
Julien, thanks a lot for your comments. So here is my summary:

Old NSS code (without OCSP cache) will always check with the OCSP responder. It will directly use the responder's decision.
So, if the OCSP responder says "revoked", and later during the session the responder says "valid", NSS will directly follow it and will allow the cert again.

With the OCSP cache code from bug 205406, this behaviour does not change.
As before, NSS will use the latest information received by the responder.

Now to the purpose of this bug.
I think you are proposing an enhancement:

  "Have NSS never forget that a cert was revoked."

This enhancement couldn't get implemented before bug 205406, because there was no cache.

So I went ahead and attached a first proposed patch.
Now we learn, to make this enhancement be correct, we'd a smarter patch.

We'd require NSS to start evulating the revocationReason attribute.
As of today, NSS obtains the flag, but does not evaluate it.

revocationReason is a SECItem, which is currently not being parsed.


I propose we treat this enhancement request as "not urgent".

We'd need a patch that parses the revocationReason, to enable the OCSP cache code to differentiate the "on hold" revocation reason from other revocations.


Because as of today the OCSP cache item will expire after a while, a cached response of type "on hold" will eventually expire from our cache and the most recent official status will get fetched.
Assignee: kengert → nobody
Severity: normal → enhancement
Kai,

What I'm proposing to have NSS never forget that a cert was revoked, except if it was previously placed on hold. I'm not sure why the old response should expire from the cache as long as the program is still up and there is no new valid OCSP response to replace the old one. Are you concerned about memory constraints ?
Julien,

I think I understand your proposal.

Your proposal requires to make the cache behavior dependent on whether a response is "placed on hold".

All I say is: I don't have that information now.
We don't have code yet that gives me that information.

We need to implement new code to parse the response in more detail, in order to know whether it's on hold or not.

(If you think I still do not understand, please tell me, how can I tell whether something is "on hold" or not. I believe this is in field revocationReason, which do not yet parse.)

And yes, there are memory constraints. We have agreed that the cache size should be limited, it can be controlled by API functions. When we reach the limit, the least recently used entry gets removed from the cache.
Kaie, 

CMS does revoke certs with 'on hold' status.

This happens if the user leaves his token at home, we want to disable the use of the old token, while the user has his temparary token. When he returns the next day or week with his old token, the old token can be reinstated once the temparary token has been 'recycled' (destroyed, reformatted, etc). In this case we do not want to permanently cache the revocation status.

on hold basically means 'don't use this cert right now, but it may become valid later'. The only reason to parse this information is if you are trying to cache revocations for ever (thus we has no reason to parse it before).

bob
Bob, thanks for describing this use case.


(In reply to comment #10)
> The only reason to parse this information is if you are trying to cache
> revocations for ever (thus we has no reason to parse it before).

Right.
Because Julien proposes to enhance our code by caching revoked reponses forever, we'd need to start parsing the reason.
Severity: normal → S3
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.