Created attachment 434271 [details] [diff] [review] change the caching semantics of side channel responses (by Adam Langley) This bug continues the work of bug 542538 on caching stapled OCSP responses. The first patch was originally attached in bug 542538 comment 27.
Created attachment 434274 [details] [diff] [review] fix bug that I noticed in my travels (by Adam Langley) This patch was originally attached by Adam Langley in bug 542538 comment 28, with the following description: I don't believe that cert_RememberOCSPProcessingFailure will currently work in all cases. It calls ocsp_CreateOrUpdateCacheEntry with single==NULL and that causes ocsp_CreateOrUpdateCacheEntry to set the missingResponseError on any cache entry that it finds. However, if there is already a cache entry, and that cache entry has a certStatusArena, then setting missingResponseError is not sufficient. It's the non-NULLness of certStatusArena that is checked in ocsp_GetCachedOCSPResponseStatusIfFresh
I'm really glad I just found this bug report and these patches!!! With the current state of NSS in CVS, OCSP stapling / side channel data will never be stored in the OCSP cache, if the OCSP response has a valid signature and says revoked cert. I agree with this patch to change this behaviour, and always cache such information, even if it came from a side channel. This change of behaviour is necessary for my work in bug 700701, where I want to enhance the tstclnt tool to report the information retrieved via the side channel.
These patches no longer apply cleanly, but luckily, it's only a matter of context. Luckily this means, it's still fine to review the patches that are attached in this bug. Unfortunately the context between your patch and the patch in bug 360420 is too close. I merged our patches, and there are just two lines of context between some of the changes, so it's impossible to create separate unified patches, not even with cvs -u2 I think we should get the code in this bug reviewed first and checked in, then I will attach an updated patch to bug 360420. Until then I'll carry the merged version of your patch in my tree and include it in future patches to bug 360420.
Comment on attachment 434271 [details] [diff] [review] change the caching semantics of side channel responses (by Adam Langley) I agree with these patches. I've merged them into the "injection" patches in bug 360420, since patches are overlapping.
Comment on attachment 434274 [details] [diff] [review] fix bug that I noticed in my travels (by Adam Langley) r=kaie on both patches
Marking as resolved, based on Kai's comment #4 and bug 360420
checked in, see bug 360420 comment 160