From 2b1d11dd50429f54bc46f83b6c68d1d0b79e3d8d Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 6 Dec 2022 15:32:37 +0100 Subject: [PATCH] [Android] Fix SslStream on APIs 21-23 (#78918) --- .../Interop.Ssl.cs | 8 +- .../Pal.Android/SafeDeleteSslContext.cs | 3 +- .../pal_jni.c | 21 ++++ .../pal_jni.h | 11 ++- .../pal_sslstream.c | 97 +++++++++++++++++-- .../pal_sslstream.h | 2 +- 6 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Ssl.cs index ba9fbd297c93b..addb8d540e43c 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Ssl.cs @@ -55,16 +55,18 @@ private static unsafe partial int SSLStreamInitializeImpl( IntPtr managedContextHandle, delegate* unmanaged streamRead, delegate* unmanaged streamWrite, - int appBufferSize); + int appBufferSize, + [MarshalAs(UnmanagedType.LPUTF8Str)] string? peerHost); internal static unsafe void SSLStreamInitialize( SafeSslHandle sslHandle, bool isServer, IntPtr managedContextHandle, delegate* unmanaged streamRead, delegate* unmanaged streamWrite, - int appBufferSize) + int appBufferSize, + string? peerHost) { - int ret = SSLStreamInitializeImpl(sslHandle, isServer, managedContextHandle, streamRead, streamWrite, appBufferSize); + int ret = SSLStreamInitializeImpl(sslHandle, isServer, managedContextHandle, streamRead, streamWrite, appBufferSize, peerHost); if (ret != SUCCESS) throw new SslException(); } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs index e4a2ee35c53d8..a07f84a1440e0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/Pal.Android/SafeDeleteSslContext.cs @@ -221,7 +221,8 @@ private unsafe void InitializeSslContext( // Make sure the class instance is associated to the session and is provided // in the Read/Write callback connection parameter IntPtr managedContextHandle = GCHandle.ToIntPtr(GCHandle.Alloc(this, GCHandleType.Weak)); - Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize); + string? peerHost = !isServer && !string.IsNullOrEmpty(authOptions.TargetHost) ? authOptions.TargetHost : null; + Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize, peerHost); if (authOptions.EnabledSslProtocols != SslProtocols.None) { diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c index 2c2857b8b20b1..9ec692cb11b63 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.c @@ -83,6 +83,14 @@ jmethodID g_SSLParametersGetProtocols; jmethodID g_SSLParametersSetApplicationProtocols; jmethodID g_SSLParametersSetServerNames; +// com/android/org/conscrypt/OpenSSLEngineImpl +jclass g_ConscryptOpenSSLEngineImplClass; +jfieldID g_ConscryptOpenSSLEngineImplSslParametersField; + +// com/android/org/conscrypt/SSLParametersImpl +jclass g_ConscryptSSLParametersImplClass; +jmethodID g_ConscryptSSLParametersImplSetUseSni; + // javax/net/ssl/SSLContext jclass g_sslCtxClass; jmethodID g_sslCtxGetDefaultMethod; @@ -449,6 +457,7 @@ jmethodID g_SSLContextGetDefault; jmethodID g_SSLContextGetInstanceMethod; jmethodID g_SSLContextInitMethod; jmethodID g_SSLContextCreateSSLEngineMethod; +jmethodID g_SSLContextCreateSSLEngineMethodWithHostAndPort; // javax/net/ssl/SSLSession jclass g_SSLSession; @@ -462,6 +471,7 @@ jmethodID g_SSLSessionGetProtocol; jclass g_SSLEngineResult; jmethodID g_SSLEngineResultGetStatus; jmethodID g_SSLEngineResultGetHandshakeStatus; +bool g_SSLEngineResultStatusLegacyOrder; // javax/crypto/KeyAgreement jclass g_KeyAgreementClass; @@ -739,6 +749,15 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_SSLParametersGetProtocols = GetMethod(env, false, g_SSLParametersClass, "getProtocols", "()[Ljava/lang/String;"); g_SSLParametersSetApplicationProtocols = GetOptionalMethod(env, false, g_SSLParametersClass, "setApplicationProtocols", "([Ljava/lang/String;)V"); + g_ConscryptOpenSSLEngineImplClass = GetOptionalClassGRef(env, "com/android/org/conscrypt/OpenSSLEngineImpl"); + if (g_ConscryptOpenSSLEngineImplClass != NULL) + { + g_ConscryptOpenSSLEngineImplSslParametersField = GetField(env, false, g_ConscryptOpenSSLEngineImplClass, "sslParameters", "Lcom/android/org/conscrypt/SSLParametersImpl;"); + + g_ConscryptSSLParametersImplClass = GetClassGRef(env, "com/android/org/conscrypt/SSLParametersImpl"); + g_ConscryptSSLParametersImplSetUseSni = GetMethod(env, false, g_ConscryptSSLParametersImplClass, "setUseSni", "(Z)V"); + } + g_sslCtxClass = GetClassGRef(env, "javax/net/ssl/SSLContext"); g_sslCtxGetDefaultMethod = GetMethod(env, true, g_sslCtxClass, "getDefault", "()Ljavax/net/ssl/SSLContext;"); g_sslCtxGetDefaultSslParamsMethod = GetMethod(env, false, g_sslCtxClass, "getDefaultSSLParameters", "()Ljavax/net/ssl/SSLParameters;"); @@ -1030,6 +1049,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_SSLContextGetInstanceMethod = GetMethod(env, true, g_SSLContext, "getInstance", "(Ljava/lang/String;)Ljavax/net/ssl/SSLContext;"); g_SSLContextInitMethod = GetMethod(env, false, g_SSLContext, "init", "([Ljavax/net/ssl/KeyManager;[Ljavax/net/ssl/TrustManager;Ljava/security/SecureRandom;)V"); g_SSLContextCreateSSLEngineMethod = GetMethod(env, false, g_SSLContext, "createSSLEngine", "()Ljavax/net/ssl/SSLEngine;"); + g_SSLContextCreateSSLEngineMethodWithHostAndPort = GetMethod(env, false, g_SSLContext, "createSSLEngine", "(Ljava/lang/String;I)Ljavax/net/ssl/SSLEngine;"); g_SSLSession = GetClassGRef(env, "javax/net/ssl/SSLSession"); g_SSLSessionGetApplicationBufferSize = GetMethod(env, false, g_SSLSession, "getApplicationBufferSize", "()I"); @@ -1041,6 +1061,7 @@ JNI_OnLoad(JavaVM *vm, void *reserved) g_SSLEngineResult = GetClassGRef(env, "javax/net/ssl/SSLEngineResult"); g_SSLEngineResultGetStatus = GetMethod(env, false, g_SSLEngineResult, "getStatus", "()Ljavax/net/ssl/SSLEngineResult$Status;"); g_SSLEngineResultGetHandshakeStatus = GetMethod(env, false, g_SSLEngineResult, "getHandshakeStatus", "()Ljavax/net/ssl/SSLEngineResult$HandshakeStatus;"); + g_SSLEngineResultStatusLegacyOrder = android_get_device_api_level() < 24; g_KeyAgreementClass = GetClassGRef(env, "javax/crypto/KeyAgreement"); g_KeyAgreementGetInstance = GetMethod(env, true, g_KeyAgreementClass, "getInstance", "(Ljava/lang/String;)Ljavax/crypto/KeyAgreement;"); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h index fd615219de4b0..ccdd8d27c3bc9 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_jni.h @@ -95,6 +95,14 @@ extern jmethodID g_SSLParametersGetProtocols; extern jmethodID g_SSLParametersSetApplicationProtocols; extern jmethodID g_SSLParametersSetServerNames; +// com/android/org/conscrypt/OpenSSLEngineImpl +extern jclass g_ConscryptOpenSSLEngineImplClass; +extern jfieldID g_ConscryptOpenSSLEngineImplSslParametersField; + +// com/android/org/conscrypt/SSLParametersImpl +extern jclass g_ConscryptSSLParametersImplClass; +extern jmethodID g_ConscryptSSLParametersImplSetUseSni; + // javax/net/ssl/SSLContext extern jclass g_sslCtxClass; extern jmethodID g_sslCtxGetDefaultMethod; @@ -463,7 +471,7 @@ extern jmethodID g_SSLContextGetDefault; extern jmethodID g_SSLContextGetInstanceMethod; extern jmethodID g_SSLContextInitMethod; extern jmethodID g_SSLContextCreateSSLEngineMethod; -extern jmethodID g_SSLContextCreateSSLEngineWithPeer; +extern jmethodID g_SSLContextCreateSSLEngineMethodWithHostAndPort; // javax/net/ssl/SSLSession extern jclass g_SSLSession; @@ -477,6 +485,7 @@ extern jmethodID g_SSLSessionGetProtocol; extern jclass g_SSLEngineResult; extern jmethodID g_SSLEngineResultGetStatus; extern jmethodID g_SSLEngineResultGetHandshakeStatus; +extern bool g_SSLEngineResultStatusLegacyOrder; // javax/crypto/KeyAgreement extern jclass g_KeyAgreementClass; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c index 436ee9f8f7b65..b73e2c75fac1e 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c @@ -15,6 +15,7 @@ enum }; // javax/net/ssl/SSLEngineResult$Status +// Android API 24+ enum { STATUS__BUFFER_UNDERFLOW = 0, @@ -23,6 +24,16 @@ enum STATUS__CLOSED = 3, }; +// javax/net/ssl/SSLEngineResult$Status +// Android API 21-23 +enum +{ + LEGACY__STATUS__BUFFER_OVERFLOW = 0, + LEGACY__STATUS__BUFFER_UNDERFLOW = 1, + LEGACY__STATUS__OK = 3, + LEGACY__STATUS__CLOSED = 2, +}; + struct ApplicationProtocolData_t { uint8_t* data; @@ -136,6 +147,25 @@ ARGS_NON_NULL_ALL static jobject EnsureRemaining(JNIEnv* env, jobject oldBuffer, } } +// There has been a change in the SSLEngineResult.Status enum between API 23 and 24 that changed +// the order/interger values of the enum options. +static int MapLegacySSLEngineResultStatus(int legacyStatus) { + switch (legacyStatus) { + case LEGACY__STATUS__BUFFER_OVERFLOW: + return STATUS__BUFFER_OVERFLOW; + case LEGACY__STATUS__BUFFER_UNDERFLOW: + return STATUS__BUFFER_UNDERFLOW; + case LEGACY__STATUS__CLOSED: + return STATUS__CLOSED; + case LEGACY__STATUS__OK: + return STATUS__OK; + default: + LOG_ERROR("Unknown legacy SSLEngineResult status: %d", legacyStatus); + assert(false && "Unknown SSLEngineResult status"); + return -1; + } +} + ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoWrap(JNIEnv* env, SSLStream* sslStream, int* handshakeStatus) { // appOutBuffer.flip(); @@ -155,6 +185,10 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoWrap(JNIEnv* env, SSLStream* sslS int status = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus)); (*env)->DeleteLocalRef(env, result); + if (g_SSLEngineResultStatusLegacyOrder) { + status = MapLegacySSLEngineResultStatus(status); + } + switch (status) { case STATUS__OK: @@ -229,6 +263,11 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss *handshakeStatus = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus)); int status = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus)); (*env)->DeleteLocalRef(env, result); + + if (g_SSLEngineResultStatusLegacyOrder) { + status = MapLegacySSLEngineResultStatus(status); + } + switch (status) { case STATUS__OK: @@ -446,7 +485,7 @@ SSLStream* AndroidCryptoNative_SSLStreamCreateWithCertificates(uint8_t* pkcs8Pri } int32_t AndroidCryptoNative_SSLStreamInitialize( - SSLStream* sslStream, bool isServer, ManagedContextHandle managedContextHandle, STREAM_READER streamReader, STREAM_WRITER streamWriter, int32_t appBufferSize) + SSLStream* sslStream, bool isServer, ManagedContextHandle managedContextHandle, STREAM_READER streamReader, STREAM_WRITER streamWriter, int32_t appBufferSize, char* peerHost) { abort_if_invalid_pointer_argument (sslStream); abort_unless(sslStream->sslContext != NULL, "sslContext is NULL in SSL stream"); @@ -456,10 +495,23 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( int32_t ret = FAIL; JNIEnv* env = GetJNIEnv(); - // SSLEngine sslEngine = sslContext.createSSLEngine(); + jobject sslEngine = NULL; + if (peerHost) + { + // SSLEngine sslEngine = sslContext.createSSLEngine(peerHost, -1); + jstring peerHostStr = make_java_string(env, peerHost); + sslEngine = (*env)->CallObjectMethod(env, sslStream->sslContext, g_SSLContextCreateSSLEngineMethodWithHostAndPort, peerHostStr, -1); + ReleaseLRef(env, peerHostStr); + ON_EXCEPTION_PRINT_AND_GOTO(exit); + } + else + { + // SSLEngine sslEngine = sslContext.createSSLEngine(); + sslEngine = (*env)->CallObjectMethod(env, sslStream->sslContext, g_SSLContextCreateSSLEngineMethod); + ON_EXCEPTION_PRINT_AND_GOTO(exit); + } + // sslEngine.setUseClientMode(!isServer); - jobject sslEngine = (*env)->CallObjectMethod(env, sslStream->sslContext, g_SSLContextCreateSSLEngineMethod); - ON_EXCEPTION_PRINT_AND_GOTO(exit); sslStream->sslEngine = ToGRef(env, sslEngine); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetUseClientMode, !isServer); ON_EXCEPTION_PRINT_AND_GOTO(exit); @@ -497,19 +549,48 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( return ret; } +// This method calls internal Android APIs that are specific to Android API 21-23 and it won't work +// on newer API levels. By calling the sslEngine.sslParameters.useSni(true) method, the SSLEngine +// will include the peerHost that was passed in to the SSLEngine factory method in the client hello +// message. +ARGS_NON_NULL_ALL static int32_t ApplyLegacyAndroidSNIWorkaround(JNIEnv* env, SSLStream* sslStream) +{ + if (g_ConscryptOpenSSLEngineImplClass == NULL || !(*env)->IsInstanceOf(env, sslStream->sslEngine, g_ConscryptOpenSSLEngineImplClass)) + return FAIL; + + int32_t ret = FAIL; + INIT_LOCALS(loc, sslParameters); + + loc[sslParameters] = (*env)->GetObjectField(env, sslStream->sslEngine, g_ConscryptOpenSSLEngineImplSslParametersField); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + if (!loc[sslParameters]) + goto cleanup; + + (*env)->CallVoidMethod(env, loc[sslParameters], g_ConscryptSSLParametersImplSetUseSni, true); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = SUCCESS; + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; +} + int32_t AndroidCryptoNative_SSLStreamSetTargetHost(SSLStream* sslStream, char* targetHost) { abort_if_invalid_pointer_argument (sslStream); abort_if_invalid_pointer_argument (targetHost); + JNIEnv* env = GetJNIEnv(); + if (g_SNIHostName == NULL || g_SSLParametersSetServerNames == NULL) { - // SSL not supported below API Level 24 - return UNSUPPORTED_API_LEVEL; + // SNIHostName is only available since API 24 + // on APIs 21-23 we use a workaround to force the SSLEngine to use SNI + return ApplyLegacyAndroidSNIWorkaround(env, sslStream); } - JNIEnv* env = GetJNIEnv(); - int32_t ret = FAIL; INIT_LOCALS(loc, hostStr, nameList, hostName, params); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.h b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.h index cc7a7b52b6f25..db20069eb843c 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.h +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.h @@ -67,7 +67,7 @@ Initialize an SSL context Returns 1 on success, 0 otherwise */ PALEXPORT int32_t AndroidCryptoNative_SSLStreamInitialize( - SSLStream* sslStream, bool isServer, ManagedContextHandle managedContextHandle, STREAM_READER streamReader, STREAM_WRITER streamWriter, int32_t appBufferSize); + SSLStream* sslStream, bool isServer, ManagedContextHandle managedContextHandle, STREAM_READER streamReader, STREAM_WRITER streamWriter, int32_t appBufferSize, char* peerHost); /* Set target host