Enable using system certs on Android. (#543)

Disable in-memory cert store and loading certs from model.
  - TBD if it will be needed - need to know how reliable using the Android system certs will be and whether any scenarios need to have custom cert management.

operators/azure/curl_invoker.cc

@@ -5,24 +5,35 @@
 
 #include <sstream>
 
-#if defined(__ANDROID__)
-#define USE_IN_MEMORY_CURL_CERTS
-#endif
-
-#if defined(USE_IN_MEMORY_CURL_CERTS)
+// TODO: We were enabling this on Android but can now use the system certs.
+// TBD if there are user scenarios that require manual cert management where it would be beneficial for the user to
+// provide manage specific certs themselves. If nothing shows up in the next few months it can be removed.
+#if defined(ENABLE_USING_CERTS_FROM_MODEL)
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 #include <openssl/x509.h>
 #include <gsl/util>
+
+#include "narrow.h"
 #endif
 
 namespace ort_extensions {
 namespace {
-// need to do in-memory cert on Android pending finding a way to use the system certs.
-#if defined(USE_IN_MEMORY_CURL_CERTS)
+// build an in-memory cert store and populate with certs from the model
+#if defined(ENABLE_USING_CERTS_FROM_MODEL)
 // based on the approach from https://curl.se/libcurl/c/cacertinmem.html
-X509_STORE* CreateX509Store(const std::string& certs) {
+X509_STORE* CreateX509Store(std::optional<const std::string> certs) {
   bool success = false;
+
+  // Any calls to GetCertificateStore from the CurlInvoker ctor will have `certs` set, and the return result
+  // populates the static variable in GetCertificateStore on the first successful call.
+  // Calls to GetCertificateStore during execution do not provide certs, so if CreateX509Store is being called without
+  // certs we didn't end up having any nodes with the certs in the x509_certificates attribute in the model,
+  // and will not use the in-memory store.
+  if (!certs) {
+    return nullptr;
+  }
+
   X509_STORE* cts = X509_STORE_new();
   if (!cts) {
     ORTX_CXX_API_THROW("X509_STORE_new returned nullptr", ORT_RUNTIME_EXCEPTION);
@@ -34,7 +45,7 @@ X509_STORE* CreateX509Store(const std::string& certs) {
     }
   });
 
-  BIO* cbio = BIO_new_mem_buf(certs.data(), certs.length());
+  BIO* cbio = BIO_new_mem_buf(certs.value().data(), narrow<int>(certs.value().length()));
   if (!cbio) {
     ORTX_CXX_API_THROW("BIO_new_mem_buf returned nullptr", ORT_RUNTIME_EXCEPTION);
   }
@@ -69,7 +80,7 @@ X509_STORE* CreateX509Store(const std::string& certs) {
   return cts;
 }
 
-X509_STORE* GetCertificateStore(const std::string& certs) {
+X509_STORE* GetCertificateStore(std::optional<const std::string> certs) {
   // first call populates the store. `certs` is ignored after that.
   static std::unique_ptr<X509_STORE, decltype(&X509_STORE_free)> store{CreateX509Store(certs), &X509_STORE_free};
 
@@ -78,14 +89,13 @@ X509_STORE* GetCertificateStore(const std::string& certs) {
 
 CURLcode sslctx_function(CURL* /*curl*/, void* sslctx, void* /*parm*/) {
   // Need to use SSL_CTX_set1_cert_store so the ref count on the store gets incremented correctly.
-  SSL_CTX_set1_cert_store(static_cast<SSL_CTX*>(sslctx), GetCertificateStore(""));
+  SSL_CTX_set1_cert_store(static_cast<SSL_CTX*>(sslctx), GetCertificateStore(std::nullopt));
 
   return CURLE_OK;
 }
-#endif  // defined(USE_IN_MEMORY_CURL_CERTS)
+#endif  // defined(ENABLE_USING_CERTS_FROM_MODEL)
 }  // namespace
 
-// apply the callback only when response is for sure to be a '/0' terminated string
 /// <summary>
 /// Callback to add contents to a string
 /// </summary>
@@ -122,8 +132,11 @@ CurlHandler::CurlHandler() : curl_(curl_easy_init(), curl_easy_cleanup),
   curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L);
   curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteStringCallback);
 
-#if defined(USE_IN_MEMORY_CURL_CERTS)
-  curl_easy_setopt(curl, CURLOPT_SSL_CTX_FUNCTION, sslctx_function);
+#if defined(ENABLE_USING_CERTS_FROM_MODEL)
+  // using the in-memory store is optional so make sure we have one before we enable overriding the default
+  if (GetCertificateStore(std::nullopt) != nullptr) {
+    curl_easy_setopt(curl, CURLOPT_SSL_CTX_FUNCTION, sslctx_function);
+  }
 #endif
 }
 
@@ -131,18 +144,14 @@ CurlHandler::CurlHandler() : curl_(curl_easy_init(), curl_easy_cleanup),
 
 CurlInvoker::CurlInvoker(const OrtApi& api, const OrtKernelInfo& info)
     : CloudBaseKernel(api, info) {
-#if defined(USE_IN_MEMORY_CURL_CERTS)
+#if defined(ENABLE_USING_CERTS_FROM_MODEL)
   std::string x509_certs;
   if (TryToGetAttribute(kX509Certificates, x509_certs) && !x509_certs.empty()) {
     // populate certificate store
-    static_cast<void>(GetCertificateStore(x509_certs));
+    static_cast<void>(GetCertificateStore(std::move(x509_certs)));
   } else {
-    // attribute not present or empty. there could be other Azure operator nodes in the model though and we only need
-    // one to provide the certs.
-    KERNEL_LOG(GetLogger(), ORT_LOGGING_LEVEL_WARNING,
-               (std::string(kX509Certificates) +
-                " attribute is required on Android from at least one Azure custom operator in the model")
-                   .c_str());
+    // attribute not present or empty. in-memory store may not be required or there could be other Azure operator
+    // nodes in the model and any of them could provide the certs.
   }
 #endif
 }

prebuild/build_curl_for_android.sh

@@ -16,7 +16,8 @@ if [ ! -d "openssl_for_ios_and_android" ]; then
     cd openssl_for_ios_and_android
     git checkout ci-release-663da9e2
     # patch with fixes to build on linux with NDK 25 or later
-    git apply ../build_curl_for_android_on_linux.patch
+    echo "Applying patches to tools for curl and openssl builds"
+    git apply --verbose ../build_curl_for_android_on_linux.patch
 else
     echo "Skipping checkout and patch"
     cd openssl_for_ios_and_android
@@ -31,7 +32,6 @@ else
     export api=${ANDROID_API_LEVEL}
 fi
 
-echo $api
 # provide a specific architecture as an argument to the script to limit the build to that
 # default is to build all
 # valid architecture values: "arm" "arm64" "x86" "x86_64"

prebuild/build_curl_for_android_on_linux.patch

@@ -62,7 +62,7 @@ index 87df207..6f3ec66 100755
      export LDFLAGS="-march=x86-64 -Wl,--gc-sections -Os -ffunction-sections -fdata-sections $(get_common_linked_libraries ${api} ${arch})"
      export CPPFLAGS=${CFLAGS}
 diff --git a/tools/build-android-curl.sh b/tools/build-android-curl.sh
-index b82d2bd..1d1e03b 100755
+index b82d2bd..394e821 100755
 --- a/tools/build-android-curl.sh
 +++ b/tools/build-android-curl.sh
 @@ -84,29 +84,32 @@ function configure_make() {
@@ -81,31 +81,31 @@ index b82d2bd..1d1e03b 100755
  
 -        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} --with-nghttp2=${NGHTTP2_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
 +        #./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
-+        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
++        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --with-ca-path=/system/etc/security/cacerts --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
  
      elif [[ "${ARCH}" == "x86" ]]; then
  
 -        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} --with-nghttp2=${NGHTTP2_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
 +        #./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
-+        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
++        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --with-ca-path=/system/etc/security/cacerts --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
  
      elif [[ "${ARCH}" == "arm" ]]; then
  
 -        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} --with-nghttp2=${NGHTTP2_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
 +        #./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
-+        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
++        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --with-ca-path=/system/etc/security/cacerts --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
  
      elif [[ "${ARCH}" == "arm64" ]]; then
  
          # --enable-shared need nghttp2 cpp compile
 -        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} --with-nghttp2=${NGHTTP2_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
 +        #./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --enable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
-+        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
++        ./configure --host=$(android_get_build_host "${ARCH}") --prefix="${PREFIX_DIR}" --disable-shared --with-ca-path=/system/etc/security/cacerts --enable-ipv6 --with-ssl=${OPENSSL_OUT_DIR} >"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
  
      else
          log_error "not support" && exit 1
 diff --git a/tools/build-android-openssl.sh b/tools/build-android-openssl.sh
-index e13c314..b29829c 100755
+index e13c314..929932f 100755
 --- a/tools/build-android-openssl.sh
 +++ b/tools/build-android-openssl.sh
 @@ -16,7 +16,7 @@
@@ -117,7 +117,18 @@ index e13c314..b29829c 100755
  
  source ./build-android-common.sh
  
-@@ -87,20 +87,20 @@ function configure_make() {
+@@ -69,6 +69,10 @@ function configure_make() {
+     pushd .
+     cd "${LIB_NAME}"
+ 
++    log_info "patch to change android hash for cert lookup"
++    # https://stackoverflow.com/a/66926685
++    patch -p1 --verbose -i ${TOOLS_ROOT}/../../openssl_crypto_x509_android_hash.patch
++
+     PREFIX_DIR="${pwd_path}/../output/android/openssl-${ABI}"
+     if [ -d "${PREFIX_DIR}" ]; then
+         rm -fr "${PREFIX_DIR}"
+@@ -87,20 +91,20 @@ function configure_make() {
      android_printf_global_params "$ARCH" "$ABI" "$ABI_TRIPLE" "$PREFIX_DIR" "$OUTPUT_ROOT"
  
      if [[ "${ARCH}" == "x86_64" ]]; then
@@ -146,7 +157,7 @@ index e13c314..b29829c 100755
  
      else
          log_error "not support" && exit 1
-@@ -115,6 +115,9 @@ function configure_make() {
+@@ -115,6 +119,9 @@ function configure_make() {
      if [ $the_rc -eq 0 ] ; then
          make SHLIB_EXT='.so' install_sw >>"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1
          make install_ssldirs >>"${OUTPUT_ROOT}/log/${ABI}.log" 2>&1

prebuild/openssl_crypto_x509_android_hash.patch

@@ -0,0 +1,17 @@
+diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
+index 46a861e90d..be4f45f0ea 100644
+--- a/crypto/x509/by_dir.c
++++ b/crypto/x509/by_dir.c
+@@ -247,7 +247,12 @@ static int get_cert_by_subject(X509_LOOKUP *xl, X509_LOOKUP_TYPE type,
+ 
+     ctx = (BY_DIR *)xl->method_data;
+ 
++#if defined(__ANDROID__)
++    h = X509_NAME_hash_old(name);
++#else
+     h = X509_NAME_hash(name);
++#endif
++
+     for (i = 0; i < sk_BY_DIR_ENTRY_num(ctx->dirs); i++) {
+         BY_DIR_ENTRY *ent;
+         int idx;

test/data/azure/create_openai_whisper_transcriptions.py

@@ -6,7 +6,6 @@ import onnx
 import numpy as np
 import sys
 
-from get_certs_for_model import get_certs_from_url
 from onnx import helper, numpy_helper, TensorProto
 
 # ORT 1.14 only supports IR version 8 so if we're unit testing with the oldest version of ORT that can be used
@@ -34,10 +33,6 @@ def make_graph(*args, doc_string=None, **kwargs):
     return graph
 
 
-# need to include the certs for curl+openssl on Android in the model as a node attribute
-x509_certs = get_certs_from_url("https://curl.se/ca/cacert.pem")
-assert x509_certs
-
 # This creates a model that allows the prompt and filename to be optionally provided as inputs.
 # The filename can be specified to indicate a different audio type to the default value in the audio_format attribute.
 model = helper.make_model(
@@ -72,7 +67,6 @@ model = helper.make_model(
                 model_uri='https://api.openai.com/v1/audio/transcriptions',
                 model_name='whisper-1',
                 timeout_seconds=20,
-                x509_certificates=x509_certs,
                 verbose=0,
             ),
         ],

test/data/azure/get_certs_for_model.py

@@ -1,25 +1,17 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
-# Curl + openssl has issues reading the system certificates on Android.
-# Pending a better solution we create an in-memory certificate store from certificates included in the model.
+# If the user needs/wants to control the certificates used in HTTPS requests to the custom op's endpoint, an 
+# in-memory certificate store can be built from certificates included in the model.
 #
-# The certificates must be added to the first Azure operator in the model in an attribute called 'x509_certificates'.
+# The certificates should be added to the first Azure operator in the model in an attribute called 'x509_certificates'.
 # The user must determine the correct certificates for their scenario, and add them to the model.
 # The PEM file from https://curl.se/docs/caextract.html may be used.
 #
 # Include this file in the python script that is creating your model with Azure custom operators.
-# Get the value to use in the 'x509_certificates' attribute from either a file (call get_certs_from_file) or
-# a url (call get_certs_from_url)
-# 
-# See create_openai_whisper_transcriptions.py for example usage.
-#
-# Notes:
-#
-# - Supposedly if openssl uses md5 hashing for the certificates in /system/etc/security/cacerts it should work, but
-# a patched version of openssl with this change still failed.
-# - The 'better' solution might be to use boringssl instead of openssl as it handles the certificate format in
-# /system/etc/security/cacerts, although even that is potentially problematic as there's no versioning of boringssl.
+# Set the 'x509_certificates' attribute of the node to the value returned from calling either get_certs_from_file
+# with the path to a PEM file, or get_certs_from_url with a URL that returns certificates in PEM format.
+
 
 import io
 import pathlib