Skip to content

Commit 85c2772

Browse files
rzikmbartonjs
rzikm
and
bartonjs
authored
[release/8.0] Fix server-side OCSP stapling on Linux (#96838)
* Add entire issuer chain to trusted X509_STORE when stapling OCSP_Response (#96792) * Add entire issuer chain to trusted X509_STORE when validating OCSP_Response * Code review feedback * More code review feedback * Update src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs Co-authored-by: Jeremy Barton <[email protected]> * Fix compilation * Always include root certificate --------- Co-authored-by: Jeremy Barton <[email protected]> * Recover from failed OCSP download. (#96448) * Recover from failed OCSP check. * Add 5s back-off after failed OCSP querry * Don't shorten OCSP expriation on failed server OCSP fetch (#96972) * Don't shorten OCSP expriation on failed server OCSP fetch * Code review feedback --------- Co-authored-by: Jeremy Barton <[email protected]>
1 parent 683da71 commit 85c2772

File tree

4 files changed

+65
-22
lines changed

4 files changed

+65
-22
lines changed

‎src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OCSP.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,30 @@ private static unsafe partial int CryptoNative_X509DecodeOcspToExpiration(
2929
int len,
3030
SafeOcspRequestHandle req,
3131
IntPtr subject,
32-
IntPtr issuer,
32+
IntPtr* issuers,
33+
int issuersLen,
3334
ref long expiration);
3435

3536
internal static unsafe bool X509DecodeOcspToExpiration(
3637
ReadOnlySpan<byte> buf,
3738
SafeOcspRequestHandle request,
3839
IntPtr x509Subject,
39-
IntPtr x509Issuer,
40+
ReadOnlySpan<IntPtr> x509Issuers,
4041
out DateTimeOffset expiration)
4142
{
4243
long timeT = 0;
4344
int ret;
4445

4546
fixed (byte* pBuf = buf)
47+
fixed (IntPtr* pIssuers = x509Issuers)
4648
{
4749
ret = CryptoNative_X509DecodeOcspToExpiration(
4850
pBuf,
4951
buf.Length,
5052
request,
5153
x509Subject,
52-
x509Issuer,
54+
pIssuers,
55+
x509Issuers.Length,
5356
ref timeT);
5457
}
5558

‎src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs‎

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,30 @@ public partial class SslStreamCertificateContext
2626
private byte[]? _ocspResponse;
2727
private DateTimeOffset _ocspExpiration;
2828
private DateTimeOffset _nextDownload;
29+
// Private copy of the intermediate certificates, in case the user decides to dispose the
30+
// instances reachable through IntermediateCertificates property.
31+
private X509Certificate2[] _privateIntermediateCertificates;
32+
private X509Certificate2? _rootCertificate;
2933
private Task<byte[]?>? _pendingDownload;
3034
private List<string>? _ocspUrls;
31-
private X509Certificate2? _ca;
3235

3336
private SslStreamCertificateContext(X509Certificate2 target, ReadOnlyCollection<X509Certificate2> intermediates, SslCertificateTrust? trust)
3437
{
3538
IntermediateCertificates = intermediates;
39+
if (intermediates.Count > 0)
40+
{
41+
_privateIntermediateCertificates = new X509Certificate2[intermediates.Count];
42+
43+
for (int i = 0; i < intermediates.Count; i++)
44+
{
45+
_privateIntermediateCertificates[i] = new X509Certificate2(intermediates[i]);
46+
}
47+
}
48+
else
49+
{
50+
_privateIntermediateCertificates = Array.Empty<X509Certificate2>();
51+
}
52+
3653
TargetCertificate = target;
3754
Trust = trust;
3855
SslContexts = new ConcurrentDictionary<SslProtocols, SafeSslContextHandle>();
@@ -76,15 +93,8 @@ partial void SetNoOcspFetch(bool noOcspFetch)
7693

7794
partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool transferredOwnership)
7895
{
79-
if (IntermediateCertificates.Count == 0)
80-
{
81-
_ca = rootCertificate;
82-
transferredOwnership = true;
83-
}
84-
else
85-
{
86-
_ca = IntermediateCertificates[0];
87-
}
96+
_rootCertificate = rootCertificate;
97+
transferredOwnership = rootCertificate != null;
8898

8999
if (!_staplingForbidden)
90100
{
@@ -149,7 +159,7 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
149159
return new ValueTask<byte[]?>(pending);
150160
}
151161

152-
if (_ocspUrls is null && _ca is not null)
162+
if (_ocspUrls is null && _rootCertificate is not null)
153163
{
154164
foreach (X509Extension ext in TargetCertificate.Extensions)
155165
{
@@ -192,7 +202,9 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
192202

193203
private async Task<byte[]?> FetchOcspAsync()
194204
{
195-
X509Certificate2? caCert = _ca;
205+
Debug.Assert(_rootCertificate != null);
206+
X509Certificate2? caCert = _privateIntermediateCertificates.Length > 0 ? _privateIntermediateCertificates[0] : _rootCertificate;
207+
196208
Debug.Assert(_ocspUrls is not null);
197209
Debug.Assert(_ocspUrls.Count > 0);
198210
Debug.Assert(caCert is not null);
@@ -211,6 +223,13 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
211223
return null;
212224
}
213225

226+
IntPtr[] issuerHandles = ArrayPool<IntPtr>.Shared.Rent(_privateIntermediateCertificates.Length + 1);
227+
for (int i = 0; i < _privateIntermediateCertificates.Length; i++)
228+
{
229+
issuerHandles[i] = _privateIntermediateCertificates[i].Handle;
230+
}
231+
issuerHandles[_privateIntermediateCertificates.Length] = _rootCertificate.Handle;
232+
214233
using (SafeOcspRequestHandle ocspRequest = Interop.Crypto.X509BuildOcspRequest(subject, issuer))
215234
{
216235
byte[] rentedBytes = ArrayPool<byte>.Shared.Rent(Interop.Crypto.GetOcspRequestDerSize(ocspRequest));
@@ -227,7 +246,7 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
227246

228247
if (ret is not null)
229248
{
230-
if (!Interop.Crypto.X509DecodeOcspToExpiration(ret, ocspRequest, subject, issuer, out DateTimeOffset expiration))
249+
if (!Interop.Crypto.X509DecodeOcspToExpiration(ret, ocspRequest, subject, issuerHandles.AsSpan(0, _privateIntermediateCertificates.Length + 1), out DateTimeOffset expiration))
231250
{
232251
ret = null;
233252
continue;
@@ -247,15 +266,27 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
247266
_ocspResponse = ret;
248267
_ocspExpiration = expiration;
249268
_nextDownload = nextCheckA < nextCheckB ? nextCheckA : nextCheckB;
250-
_pendingDownload = null;
251269
break;
252270
}
253271
}
254272

273+
issuerHandles.AsSpan().Clear();
274+
ArrayPool<IntPtr>.Shared.Return(issuerHandles);
255275
ArrayPool<byte>.Shared.Return(rentedBytes);
256276
ArrayPool<char>.Shared.Return(rentedChars.Array!);
257277
GC.KeepAlive(TargetCertificate);
278+
GC.KeepAlive(_privateIntermediateCertificates);
279+
GC.KeepAlive(_rootCertificate);
258280
GC.KeepAlive(caCert);
281+
282+
_pendingDownload = null;
283+
if (ret == null)
284+
{
285+
// All download attempts failed, don't try again for 5 seconds.
286+
// This backoff will be applied only if the OCSP staple is not expired.
287+
// If it is expired, we will force-refresh it during next GetOcspResponseAsync call.
288+
_nextDownload = DateTimeOffset.UtcNow.AddSeconds(5);
289+
}
259290
return ret;
260291
}
261292
}

‎src/native/libs/System.Security.Cryptography.Native/pal_x509.c‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,11 +1302,11 @@ CryptoNative_X509ChainVerifyOcsp(X509_STORE_CTX* storeCtx, OCSP_REQUEST* req, OC
13021302
return X509ChainVerifyOcsp(storeCtx, subject, issuer, req, resp, cachePath);
13031303
}
13041304

1305-
int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len, OCSP_REQUEST* req, X509* subject, X509* issuer, int64_t* expiration)
1305+
int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len, OCSP_REQUEST* req, X509* subject, X509** issuers, int issuersLen, int64_t* expiration)
13061306
{
13071307
ERR_clear_error();
13081308

1309-
if (buf == NULL || len == 0)
1309+
if (buf == NULL || len == 0 || issuersLen == 0)
13101310
{
13111311
return 0;
13121312
}
@@ -1329,7 +1329,16 @@ int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len,
13291329

13301330
if (bag != NULL)
13311331
{
1332-
if (X509_STORE_add_cert(store, issuer) && sk_X509_push(bag, issuer))
1332+
int i;
1333+
for (i = 0; i < issuersLen; i++)
1334+
{
1335+
if (!X509_STORE_add_cert(store, issuers[i]) || !sk_X509_push(bag, issuers[i]))
1336+
{
1337+
break;
1338+
}
1339+
}
1340+
1341+
if (i == issuersLen)
13331342
{
13341343
ctx = X509_STORE_CTX_new();
13351344
}
@@ -1343,7 +1352,7 @@ int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len,
13431352
{
13441353
int canCache = 0;
13451354
time_t expiration_t = 0;
1346-
X509VerifyStatusCode code = CheckOcspGetExpiry(req, resp, subject, issuer, ctx, &canCache, &expiration_t);
1355+
X509VerifyStatusCode code = CheckOcspGetExpiry(req, resp, subject, issuers[0], ctx, &canCache, &expiration_t);
13471356

13481357
if (sizeof(time_t) == sizeof(int64_t))
13491358
{

‎src/native/libs/System.Security.Cryptography.Native/pal_x509.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,4 @@ PALEXPORT int32_t CryptoNative_X509ChainVerifyOcsp(X509_STORE_CTX* storeCtx,
412412
Decode len bytes of buf into an OCSP response, process it against the OCSP request, and return if the bytes were valid.
413413
If the bytes were valid, and the OCSP response had a nextUpdate value, assign it to expiration.
414414
*/
415-
PALEXPORT int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len, OCSP_REQUEST* req, X509* subject, X509* issuer, int64_t* expiration);
415+
PALEXPORT int32_t CryptoNative_X509DecodeOcspToExpiration(const uint8_t* buf, int32_t len, OCSP_REQUEST* req, X509* subject, X509** issuers, int issuersLen, int64_t* expiration);

0 commit comments

Comments
 (0)