Fix ssl on APIs 21-23

simonrozsival · simonrozsival · commit ec54d10ee17d · 2022-11-30T10:00:47.000+01:00
Code improvements
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
@@ -55,16 +55,18 @@ private static unsafe partial int SSLStreamInitializeImpl(
             IntPtr managedContextHandle,
             delegate* unmanaged<IntPtr, byte*, int*, PAL_SSLStreamStatus> streamRead,
             delegate* unmanaged<IntPtr, byte*, int, void> streamWrite,
-            int appBufferSize);
+            int appBufferSize,
+            [MarshalAs(UnmanagedType.LPUTF8Str)] string? peerHost);
         internal static unsafe void SSLStreamInitialize(
             SafeSslHandle sslHandle,
             bool isServer,
             IntPtr managedContextHandle,
             delegate* unmanaged<IntPtr, byte*, int*, PAL_SSLStreamStatus> streamRead,
             delegate* unmanaged<IntPtr, byte*, int, void> 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
@@ -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
@@ -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;
@@ -448,6 +456,7 @@ jmethodID g_SSLContextGetDefault;
 jmethodID g_SSLContextGetInstanceMethod;
 jmethodID g_SSLContextInitMethod;
 jmethodID g_SSLContextCreateSSLEngineMethod;
+jmethodID g_SSLContextCreateSSLEngineMethodWithHostAndPort;
 
 // javax/net/ssl/SSLSession
 jclass    g_SSLSession;
@@ -461,6 +470,7 @@ jmethodID g_SSLSessionGetProtocol;
 jclass    g_SSLEngineResult;
 jmethodID g_SSLEngineResultGetStatus;
 jmethodID g_SSLEngineResultGetHandshakeStatus;
+bool      g_SSLEngineResultStatusLegacyOrder;
 
 // javax/crypto/KeyAgreement
 jclass    g_KeyAgreementClass;
@@ -738,6 +748,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;");
@@ -1028,6 +1047,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");
@@ -1039,6 +1059,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
@@ -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;
@@ -462,7 +470,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;
@@ -476,6 +484,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
@@ -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;
@@ -115,6 +126,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();
@@ -134,6 +164,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:
@@ -208,6 +242,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:
@@ -425,7 +464,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");
@@ -435,10 +474,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);
@@ -476,19 +528,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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -221,7 +221,8 @@ private unsafe void InitializeSslContext(`
`221`	`221`	`// Make sure the class instance is associated to the session and is provided`
`222`	`222`	`// in the Read/Write callback connection parameter`
`223`	`223`	`IntPtr managedContextHandle = GCHandle.ToIntPtr(GCHandle.Alloc(this, GCHandleType.Weak));`
`224`		`- Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize);`
	`224`	`+ string? peerHost = !isServer && !string.IsNullOrEmpty(authOptions.TargetHost) ? authOptions.TargetHost : null;`
	`225`	`+ Interop.AndroidCrypto.SSLStreamInitialize(handle, isServer, managedContextHandle, &ReadFromConnection, &WriteToConnection, InitialBufferSize, peerHost);`
`225`	`226`
`226`	`227`	`if (authOptions.EnabledSslProtocols != SslProtocols.None)`
`227`	`228`	`{`