Skip to content
Merged
Prev Previous commit
Next Next commit
Optimize setting certname list in OpenSSL
  • Loading branch information
rzikm committed Feb 23, 2022
commit 23cfd08191589b5f3b1e6d6acf0e55c8365dd31f
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,21 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia
X509Certificate2Collection certList = (trust._trustList ?? trust._store!.Certificates);

Debug.Assert(certList != null, "certList != null");
foreach (X509Certificate2 cert in certList)
Span<IntPtr> handles = certList.Count <= 256
? stackalloc IntPtr[256]
: new IntPtr[certList.Count];

for (int i = 0; i < certList.Count; i++)
{
handles[i] = certList[i].Handle;
}

if (!Ssl.SslAddClientCAs(sslHandle, handles.Slice(0, certList.Count)))
{
if (!Ssl.SslAddClientCA(sslHandle, cert.Handle))
{
Debug.Fail("Failed to add issuer to trusted CA list.");
}
// The method can fail only when the number of cert names exceeds the maximum capacity
// supported by STACK_OF(X509_NAME) structure, which should not happen under normal
// operation.
Debug.Fail("Failed to add issuer to trusted CA list.");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,16 @@ internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, Span<byte> serial
[GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddExtraChainCert")]
internal static partial bool SslAddExtraChainCert(SafeSslHandle ssl, SafeX509Handle x509);

[GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddClientCA")]
internal static partial bool SslAddClientCA(SafeSslHandle ssl, IntPtr x509);
[GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddClientCAs")]
private static unsafe partial bool SslAddClientCAs(SafeSslHandle ssl, IntPtr* x509s, int count);

internal static unsafe bool SslAddClientCAs(SafeSslHandle ssl, Span<IntPtr> x509handles)
{
fixed (IntPtr* pHandles = &MemoryMarshal.GetReference(x509handles))
{
return SslAddClientCAs(ssl, pHandles, x509handles.Length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should throw here on failure to be consistent with SslSetCertificateAuthorities on macOS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can fail only when maximum number of STACK_OF(X509_NAME) items is reached (something around MAX_INT), so we would not be likely to throw anyway, I added the propagation of the return code for consistency.

}
}

internal static bool AddExtraChainCertificates(SafeSslHandle ssl, X509Certificate2[] chain)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public async Task SslStream_SendCertificateTrust_CertificateCollection()
SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions
{
TargetHost = "localhost",
EnabledSslProtocols = SslProtocols.Tls12,
RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true,
LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) =>
{
Expand Down Expand Up @@ -93,7 +92,6 @@ public async Task SslStream_SendCertificateTrust_CertificateStore()
SslClientAuthenticationOptions clientOptions = new SslClientAuthenticationOptions
{
TargetHost = "localhost",
EnabledSslProtocols = SslProtocols.Tls12,
RemoteCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true,
LocalCertificateSelectionCallback = (sender, targetHost, localCertificates, remoteCertificate, issuers) =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ static const Entry s_cryptoNative[] =
DllImportEntry(CryptoNative_SslCtxUseCertificate)
DllImportEntry(CryptoNative_SslCtxUsePrivateKey)
DllImportEntry(CryptoNative_SslAddExtraChainCert)
DllImportEntry(CryptoNative_SslAddClientCA)
DllImportEntry(CryptoNative_SslAddClientCAs)
DllImportEntry(CryptoNative_SslDestroy)
DllImportEntry(CryptoNative_SslDoHandshake)
DllImportEntry(CryptoNative_SslGetClientCAList)
Expand Down
15 changes: 12 additions & 3 deletions src/native/libs/System.Security.Cryptography.Native/pal_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -772,14 +772,23 @@ int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509)
return 0;
}

int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509)
int32_t CryptoNative_SslAddClientCAs(SSL* ssl, X509** x509s, uint32_t count)
{
if (!x509 || !ssl)
if (!x509s || !ssl)
{
return 0;
}

return SSL_add_client_CA(ssl, x509);
for (uint32_t i = 0; i < count; i++)
{
int res = SSL_add_client_CA(ssl, x509s[i]);
if (res != 1)
{
return res;
}
}

return 1;
}

void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void* arg)
Expand Down
4 changes: 2 additions & 2 deletions src/native/libs/System.Security.Cryptography.Native/pal_ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -406,13 +406,13 @@ Returns 1 if success and 0 in case of failure
PALEXPORT int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509);

/*
Adds the name of the given certificate to the list of acceptable issuers sent to
Adds the names of the given certificates to the list of acceptable issuers sent to
client when requesting a client certificate. Shims the SSL_add_client_CA function.

No transfer of ownership or refcount changes.
Returns 1 if success and 0 in case of failure
*/
PALEXPORT int32_t CryptoNative_SslAddClientCA(SSL* ssl, X509* x509);
PALEXPORT int32_t CryptoNative_SslAddClientCAs(SSL* ssl, X509** x509s, uint32_t count);

/*
Shims the ssl_ctx_set_alpn_select_cb method.
Expand Down