Android's certificate verification JNI layer

library/common/jni/BUILD

@@ -21,6 +21,7 @@ cc_library(
     ],
     deps = [
         "//library/common/jni/import:jni_import_lib",
+        "//library/common/strings:string_conversions_lib",
         "//library/common/types:c_types_lib",
         "@envoy//source/common/common:assert_lib",
     ],
@@ -160,6 +161,7 @@ cc_library(
     ],
     deps = [
         ":java_jni_support",
+        "//library/common/strings:string_conversions_lib",
         "//library/common/jni/import:jni_import_lib",
         "//library/common:envoy_main_interface_lib",
         "//library/common/types:c_types_lib",

library/common/jni/jni_interface.cc

@@ -1057,6 +1057,12 @@ jobjectArray jvm_cert_get_certificate_chain_encoded(JNIEnv* env, jobject result)
       env->CallObjectMethod(jcls_AndroidCertVerifyResult, jmid_getCertificateChainEncoded, result));
 }
 
+// Once we have a better picture of how Android's certificate verification will
+// be plugged into EM, we should decide where this function should really live.
+// Context: as of now JNI functions declared in this file are not exported through any
+// header files, instead they are stored as callbacks into plain function
+// tables. For this reason, this function, which would ideally be defined in
+// jni_utility.cc, is currently defined here.
 void ExtractCertVerifyResult(JNIEnv* env, jobject result,
                              envoy_cert_verify_status_android_t* status,
                              bool* is_issued_by_known_root,

library/common/jni/jni_utility.cc

@@ -3,12 +3,11 @@
 #include <stdlib.h>
 #include <string.h>
 
-#include <codecvt>
-
 #include "source/common/common/assert.h"
 
 #include "library/common/jni/jni_support.h"
 #include "library/common/jni/jni_version.h"
+#include "library/common/strings/string_conversions.h"
 
 // NOLINT(namespace-envoy)
 
@@ -259,8 +258,9 @@ envoy_map to_native_map(JNIEnv* env, jobjectArray entries) {
 }
 
 jstring ConvertUTF8ToJavaString(JNIEnv* env, const std::string& str) {
-  std::u16string utf16 =
-      std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.from_bytes(str);
+  // JNI's NewStringUTF expects "modified" UTF8 so instead convert the string to UTF16 and pass it
+  // to NewString.
+  std::u16string utf16 = UTF8ToUTF16(str.data(), str.size());
   const jchar* jutf16 = reinterpret_cast<const jchar*>(utf16.data());
   return env->NewString(jutf16, utf16.length());
 }
@@ -292,8 +292,8 @@ void JavaArrayOfByteArrayToStringVector(JNIEnv* env, jobjectArray array,
   for (size_t i = 0; i < len; ++i) {
     jbyteArray bytes_array = static_cast<jbyteArray>(env->GetObjectArrayElement(array, i));
     jsize bytes_len = env->GetArrayLength(bytes_array);
-    // We don't care whether the returned array is a copy or not as we're simply
-    // copying it into C++ owned memory.
+    // It doesn't matter if the array returned by GetByteArrayElements is a copy
+    // or not, as the data will be simply be copied into C++ owned memory below.
     jbyte* bytes = env->GetByteArrayElements(bytes_array, /*isCopy=*/nullptr);
     (*out)[i].assign(reinterpret_cast<const char*>(bytes), bytes_len);
     // There is nothing to write back, it is always safe to JNI_ABORT.
@@ -307,8 +307,8 @@ void JavaArrayOfByteToBytesVector(JNIEnv* env, jbyteArray array, std::vector<uin
   const size_t len = env->GetArrayLength(array);
   out->resize(len);
 
-  // We don't care whether the returned array is a copy or not as we're simply
-  // copying it into C++ owned memory.
+  // It doesn't matter if the array returned by GetByteArrayElements is a copy
+  // or not, as the data will be simply be copied into C++ owned memory below.
   jbyte* jbytes = env->GetByteArrayElements(array, /*isCopy=*/nullptr);
   uint8_t* bytes = reinterpret_cast<uint8_t*>(jbytes);
   std::copy(bytes, bytes + len, out->begin());
@@ -317,11 +317,13 @@ void JavaArrayOfByteToBytesVector(JNIEnv* env, jbyteArray array, std::vector<uin
 }
 
 void ConvertJavaStringToUTF8(JNIEnv* env, jstring str, std::string* result) {
+  // JNI's GetStringUTFChars() returns strings in Java "modified" UTF8, so
+  // instead get the String in UTF16 and manually convert that to UTF8.
   // We don't care whether the returned string is a copy or not as we're simply
   // copying it into C++ owned memory.
   const jchar* utf16 = env->GetStringChars(str, /*isCopy=*/nullptr);
-  std::string utf8 = std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.to_bytes(
-      reinterpret_cast<const char16_t*>(utf16));
+  size_t len = env->GetStringLength(str);
+  std::string utf8 = UTF16ToUTF8(reinterpret_cast<const char16_t*>(utf16), len);
   *result = utf8;
   env->ReleaseStringChars(str, utf16);
 }

library/common/strings/BUILD

@@ -0,0 +1,10 @@
+load("@rules_cc//cc:defs.bzl", "cc_library")
+
+licenses(["notice"])  # Apache 2
+
+cc_library(
+    name = "string_conversions_lib",
+    srcs = ["string_conversions.cc"],
+    hdrs = ["string_conversions.h"],
+    visibility = ["//visibility:public"],
+)

library/common/strings/string_conversions.cc

@@ -0,0 +1,14 @@
+#include "library/common/strings/string_conversions.h"
+
+#include <codecvt>
+#include <locale>
+
+std::string UTF16ToUTF8(const char16_t* utf16, size_t len) {
+  return std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.to_bytes(utf16,
+                                                                                      utf16 + len);
+}
+
+std::u16string UTF8ToUTF16(const char* utf8, size_t len) {
+  return std::wstring_convert<std::codecvt_utf8_utf16<char16_t>, char16_t>{}.from_bytes(utf8,
+                                                                                        utf8 + len);
+}

library/common/strings/string_conversions.h

@@ -0,0 +1,7 @@
+#pragma once
+
+#include <string>
+
+std::string UTF16ToUTF8(const char16_t* utf16, size_t len);
+
+std::u16string UTF8ToUTF16(const char* utf8, size_t len);

library/java/org/chromium/net/FakeX509Util.java

@@ -9,24 +9,30 @@
  * Fake utility functions to verify X.509 certificates.
  *
  * FakeX509Util is not particularly clever: from its perspective a certificate is just a string and
- * its contents have no particular meaning. For a verification to succeed all certificates in a
- * chain must have been previously registered as root certificates. This doesn't make much sense
- * w.r.t. how X.509 certificates are really validated, but we're not interested in mimicking that,
- * we just want something to confirm that JNI calls have taken place.
+ * its contents have no particular meaning. For a verification to succeed:
+ * - all certificates in a chain must have been previously registered as root certificates
+ * - host and authentication type must match the expected hardcoded values
+ * This doesn't make much sense w.r.t. how X.509 certificates are really validated, but we're not
+ * interested in mimicking that, we just want something to confirm that JNI calls have taken place.
  */
 public final class FakeX509Util {
   private static final Set<String> validFakeCerts = new HashSet<String>();
 
+  public static final String expectedAuthType = "RSA";
+  public static final String expectedHost = "www.example.com";
+
   public static void addTestRootCertificate(byte[] rootCertBytes) {
     String fakeCertificate = new String(rootCertBytes);
     validFakeCerts.add(fakeCertificate);
   }
 
   public static void clearTestRootCertificates() { validFakeCerts.clear(); }
 
-  /* Fake certificate chain verification. Returns CertVerifyStatusAndroid.OK only if all
-   * certificates in the chain have been previously registered as root certificates,
-   * CertVerifyStatusAndroid.NO_TRUSTED_ROOT otherwise.
+  /**
+   * Performs fake certificate chain verification. Returns CertVerifyStatusAndroid.NO_TRUSTED_ROOT
+   * if at least one of the certificates in the chain has not been previously registered as a root.
+   * Returns CertVerifyStatusAndroid.OK if authType and host match respectively expectedAuthType and
+   * expectedHost; CertVerifyStatusAndroid.FAILED otherwise.
    */
   public static AndroidCertVerifyResult verifyServerCertificates(byte[][] certChain,
                                                                  String authType, String host) {
@@ -43,6 +49,8 @@ public static AndroidCertVerifyResult verifyServerCertificates(byte[][] certChai
       }
     }
 
-    return new AndroidCertVerifyResult(CertVerifyStatusAndroid.OK);
+    return authType.equals(expectedAuthType) && host.equals(expectedHost)
+        ? new AndroidCertVerifyResult(CertVerifyStatusAndroid.OK)
+        : new AndroidCertVerifyResult(CertVerifyStatusAndroid.FAILED);
   }
 }

test/common/strings/BUILD

@@ -0,0 +1,14 @@
+load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_package")
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+envoy_cc_test(
+    name = "string_conversions_test",
+    srcs = ["string_conversions_test.cc"],
+    repository = "@envoy",
+    deps = [
+        "//library/common/strings:string_conversions_lib",
+    ],
+)

test/common/strings/string_conversions_test.cc

@@ -0,0 +1,11 @@
+#include <string>
+
+#include "gtest/gtest.h"
+#include "library/common/strings/string_conversions.h"
+
+TEST(StringConversions, SameEncoding) {
+  std::string utf8 = u8"z\u00df\u6c34\U0001f34c";
+  std::u16string utf16 = u"z\u00df\u6c34\U0001f34c";
+
+  EXPECT_EQ(utf16, UTF8ToUTF16(utf8.data(), utf8.size()));
+}

test/java/org/chromium/net/CertificateVerificationTest.java

@@ -24,6 +24,9 @@ public final class CertificateVerificationTest {
     JniLibrary.load();
   }
 
+  private static final String authType = FakeX509Util.expectedAuthType;
+  private static final String host = FakeX509Util.expectedHost;
+
   @Before
   public void setUp() throws Exception {
     AndroidNetworkLibrary.setFakeCertificateVerificationForTesting(true);
@@ -39,8 +42,6 @@ public void tearDown() throws Exception {
   public void testChainWithNonRootCertificate() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert"};
     final byte[][] certChain = new byte[][] {fakeCertChain[0].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     AndroidCertVerifyResult result =
         (AndroidCertVerifyResult)JniLibrary.callCertificateVerificationFromNative(certChain,
@@ -52,8 +53,6 @@ public void testChainWithNonRootCertificate() throws Exception {
   public void testChainWithRootCertificate() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert"};
     final byte[][] certChain = new byte[][] {fakeCertChain[0].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
     AndroidCertVerifyResult result =
@@ -62,12 +61,36 @@ public void testChainWithRootCertificate() throws Exception {
     assertEquals(result.getStatus(), CertVerifyStatusAndroid.OK);
   }
 
+  @Test
+  public void testChainWithRootCertificateWrongHostname() throws Exception {
+    final String[] fakeCertChain = new String[] {"fake cert"};
+    final byte[][] certChain = new byte[][] {fakeCertChain[0].getBytes()};
+    final String host = "wrong host";
+
+    JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
+    AndroidCertVerifyResult result =
+        (AndroidCertVerifyResult)JniLibrary.callCertificateVerificationFromNative(certChain,
+                                                                                  authType, host);
+    assertEquals(result.getStatus(), CertVerifyStatusAndroid.FAILED);
+  }
+
+  @Test
+  public void testChainWithRootCertificateWrongAuthType() throws Exception {
+    final String[] fakeCertChain = new String[] {"fake cert"};
+    final byte[][] certChain = new byte[][] {fakeCertChain[0].getBytes()};
+    final String authType = "wrong auth type";
+
+    JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
+    AndroidCertVerifyResult result =
+        (AndroidCertVerifyResult)JniLibrary.callCertificateVerificationFromNative(certChain,
+                                                                                  authType, host);
+    assertEquals(result.getStatus(), CertVerifyStatusAndroid.FAILED);
+  }
+
   @Test
   public void testClearTestRootCertificate() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert"};
     final byte[][] certChain = new byte[][] {fakeCertChain[0].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
     JniLibrary.callClearTestRootCertificateFromNative();
@@ -82,8 +105,6 @@ public void testChainWithMultipleNonRootCertificates() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert", "another fake cert"};
     final byte[][] certChain =
         new byte[][] {fakeCertChain[0].getBytes(), fakeCertChain[1].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     AndroidCertVerifyResult result =
         (AndroidCertVerifyResult)JniLibrary.callCertificateVerificationFromNative(certChain,
@@ -96,8 +117,6 @@ public void testChainWithMixedCertificates() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert", "another fake cert"};
     final byte[][] certChain =
         new byte[][] {fakeCertChain[0].getBytes(), fakeCertChain[1].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
     AndroidCertVerifyResult result =
@@ -111,8 +130,6 @@ public void testChainWithMultipleRootCertificates() throws Exception {
     final String[] fakeCertChain = new String[] {"fake cert", "another fake cert"};
     final byte[][] certChain =
         new byte[][] {fakeCertChain[0].getBytes(), fakeCertChain[1].getBytes()};
-    final String authType = "";
-    final String host = "";
 
     JniLibrary.callAddTestRootCertificateFromNative(certChain[0]);
     JniLibrary.callAddTestRootCertificateFromNative(certChain[1]);