Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix build with old openssl
  • Loading branch information
wfurt committed Aug 9, 2021
commit b6b40d753a11177e66e13c5fd3c8b22478f024ce
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,6 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia
Ssl.SslSetVerifyPeer(context);
}
}

// Sets policy and security level
// if (!Ssl.SetEncryptionPolicy(context, sslAuthenticationOptions.EncryptionPolicy))
// {
// throw new SslException(
// SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy));
// }
}
catch
{
Expand All @@ -285,12 +278,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia
if (innerContext != null && cacheSslContext)
{
// We allocated new context
if (sslAuthenticationOptions.CertificateContext?.contexts != null &&
sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext))
{
//Console.WriteLine("Added {0} to CTX cache", sslCtx.DangerousGetHandle());
}
else
if (sslAuthenticationOptions.CertificateContext?.contexts == null ||
!sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext))
{
innerContext.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

In the event that we're not caching, what happens here? Seems like we'll have created an SSL_CTX, from that we created an SSL, then without doing anything yet we call dispose on the SSL_CTX handle, which might call SSL_shutdown.

I feel like I must be missing something.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think disposing the context would call SSL_shutdown. It does not have data to do so and it would only call Interop.Ssl.SslCtxDestroy().

We already do that 100% in existing code:

internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX509Handle? certHandle, SafeEvpPKeyHandle? certKeyHandle, EncryptionPolicy policy, SslAuthenticationOptions sslAuthenticationOptions)
{
SafeSslHandle? context = null;
// Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest
// mutually supported version and can thus handle any of the set protocols, and we then use
// SetProtocolOptions to ensure we only allow the ones requested.
using (SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method))

This logic is there to skip it if we put the context to cache.

}
Expand Down Expand Up @@ -519,7 +508,6 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte
*outlen = 0;
IntPtr sslData = Ssl.SslGetData(ssl);

Console.WriteLine("AlpnServerSelectCallback called for {0} and {1}", ssl, sslData);
if (sslData == IntPtr.Zero)
{
// We did not set ALPN list.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ internal static partial class Ssl
[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetOpenSslCipherSuiteName")]
private static extern IntPtr GetOpenSslCipherSuiteName(SafeSslHandle ssl, int cipherSuite, out int isTls12OrLower);

//[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetEncryptionPolicy")]
//internal static extern bool SetEncryptionPolicy(SafeSslHandle ssl, EncryptionPolicy policy);

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetCiphers")]
internal static extern unsafe bool SslSetCiphers(SafeSslHandle ssl, byte* cipherList, byte* cipherSuites);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ static const Entry s_cryptoNative[] =
DllImportEntry(CryptoNative_SslCtxSetCiphers)
DllImportEntry(CryptoNative_SslCtxSetEncryptionPolicy)
DllImportEntry(CryptoNative_SetCiphers)
DllImportEntry(CryptoNative_SetEncryptionPolicy)
DllImportEntry(CryptoNative_SetProtocolOptions)
DllImportEntry(CryptoNative_SslAddExtraChainCert)
DllImportEntry(CryptoNative_SslCreate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,23 +513,6 @@ int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy po
return false;
}

int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy)
{
switch (policy)
{
case AllowNoEncryption:
case NoEncryption:
// No minimum security policy, same as OpenSSL 1.0
SSL_set_security_level(ssl, 0);
//ResetCtxProtocolRestrictions(ctx);
return true;
case RequireEncryption:
return true;
}

return false;
}

int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites)
{
int32_t ret = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,19 +326,11 @@ PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl);
PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr);
PALEXPORT void* CryptoNative_SslGetData(SSL* ssl);


/*
Shims the SSL_CTX_set_cert_verify_callback method.
*/
//PALEXPORT void
//CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxSetCertVerifyCallbackCallback callback, void* arg);

/*

Sets the specified encryption policy on the SSL_CTX.
*/
PALEXPORT int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy);
PALEXPORT int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy);

/*
Sets ciphers (< TLS 1.3) and cipher suites (TLS 1.3) on the SSL_CTX
Expand Down