Use the stapled OCSP response from TLS, when available. #67011
Conversation
Based on (non-exhaustive) testing, chain builds from a Let's Encrypt issued certificate have the following characteristics: * Live OCSP request required (uncached/unstapled): 577ms * OCSP response retrieved from cache (unstapled): 183ms * OCSP response utilized from TLS stapling (bypasses cache): 182ms In both cached and stapled the revocation portion was about 39ms. (The revocation mode was ExcludeRoot, the CRL pertaining to the intermediate was cached for all three measurements.)
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsBased on (non-exhaustive) testing, chain builds from a Let's Encrypt issued certificate have
In both cached and stapled the revocation portion was about 39ms. Contributes to #33377.
|
| static X509VerifyStatusCode CheckOcsp(OCSP_REQUEST* req, | ||
| OCSP_RESPONSE* resp, | ||
| X509* subject, | ||
| X509* issuer, | ||
| X509_STORE_CTX* storeCtx, | ||
| ASN1_GENERALIZEDTIME** thisUpdate, | ||
| ASN1_GENERALIZEDTIME** nextUpdate) |
There was a problem hiding this comment.
The cache policy was repeated across two callers, and almost got repeated to a third. Now it's inside this routine and just expressed via canCache... so no one needs these values any more.
| return true; | ||
| } | ||
|
|
||
| Debug.Assert(resp == 0, $"Unexpected response from X509ChainHasStapledOcsp: {resp}"); |
There was a problem hiding this comment.
Looks like the function CryptoNative_X509ChainHasStapledOcsp could return -2? I'd also be nervous about assuming hat the || operator in C++ result in exactly 0 or exactly 1. Per C++, any non-zero value should be interpreted as true.
There was a problem hiding this comment.
The -2 is a "something has gone horribly wrong" code. For this particular case it would require that the X509_STORE_CTX value has no associated chain (had never tried chain building, or someone had just reset it), which is pretty unexpected by the time you're calling this.
There was a problem hiding this comment.
C99 said:
Semantics
3 The == (equal to) and != (not equal to) operators are analogous to the relational
operators except for their lower precedence. 93) Each of the operators yields 1 if the
specified relation is true and 0 if it is false.
and
Semantics
3 The || operator shall yield 1 if either of its operands compare unequal to 0; otherwise, it
yields 0. The result has type int.
So, in C, it seems to be well defined to be 1 or 0. And, if the compiler disagrees, the assert'll start firing 😄
| } | ||
|
|
||
| // If the dup_func() returns 0 the whole CRYPTO_dup_ex_data() will fail. | ||
| // So, return 1 unless we returned 0 already. |
There was a problem hiding this comment.
Not sure what the "unless we returned 0 already" part refers to?
There was a problem hiding this comment.
Just theoretical future expansion. e.g. if we tried duplicating the g_x509_ocsp_index value it'd be something like *pptr = OCSP_RESPONSE_dup(*pptr); if (*pptr == NULL) { return 0; }
| { | ||
| // time_t granularity is seconds, so subtract 4 days worth of seconds. | ||
| // The 4 day policy is based on the CA/Browser Forum Baseline Requirements | ||
| // (version 1.6.3) section 4.9.10 (On-Line Revocation Checking Requirements) |
There was a problem hiding this comment.
These requirements have changed effective 30 Sep 2020. See https://cabforum.org/wp-content/uploads/CA-Browser-Forum-BR-1.8.2.pdf, Sec. 4.9.10.
There was a problem hiding this comment.
The new rules are certainly more complicated 😄. In the 1.8.2 version the longest period they allow before informing an OCSP server of a change is still 4 days (Subscriber item 4), so I think it's still a reasonable maximum age cache boundary. We could certainly look at making our window match their current policy later, if you like. (FWIW, this code just moved up. It was originally written Feb 2019)
ghost
locked as resolved and limited conversation to collaborators
bartonjs
added
os-linux
bartonjs
added
the
needs-further-triage
Based on (non-exhaustive) testing, chain builds from a Let's Encrypt issued certificate have
the following characteristics:
In both cached and stapled the revocation portion was about 39ms.
(The revocation mode was ExcludeRoot, the CRL pertaining to the intermediate was cached for all three measurements.)
The pattern of carrying the stapled response as extra (behind-the-scenes) data on the certificate is inspired by the Win32 behavior (CERT_OCSP_RESPONSE_PROP_ID).
Contributes to #33377.