From ec3b0ccbb1e30a51e98f098421cb253c18e0d4c3 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Mon, 6 Jun 2022 15:16:24 -0400 Subject: [PATCH] android: attempting to respect Android's cleartext policies (#2318) Rejecting requests which don't have an https scheme if Android disallows cleartext Risk Level: High Testing: TODO (https://github.com/envoyproxy/envoy-mobile/issues/2341) Docs Changes: n/a Release Notes: inline Fixes #1572 Signed-off-by: Alyssa Wilk Signed-off-by: JP Simard --- mobile/docs/root/intro/version_history.rst | 1 + mobile/library/common/http/BUILD | 1 + mobile/library/common/http/client.cc | 11 ++++++ mobile/library/common/jni/BUILD | 22 ++++++++++++ .../library/common/jni/android_jni_utility.cc | 34 +++++++++++++++++++ .../library/common/jni/android_jni_utility.h | 11 ++++++ 6 files changed, 80 insertions(+) create mode 100644 mobile/library/common/jni/android_jni_utility.cc create mode 100644 mobile/library/common/jni/android_jni_utility.h diff --git a/mobile/docs/root/intro/version_history.rst b/mobile/docs/root/intro/version_history.rst index fd1247bb5dfc..f2140102163e 100644 --- a/mobile/docs/root/intro/version_history.rst +++ b/mobile/docs/root/intro/version_history.rst @@ -8,6 +8,7 @@ Breaking changes: - api: replace the ``drainConnections()`` method with a broader ``resetConnectivityState()``. (:issue:`#2225 <2225>`). - api: disallow setting 'host' header directly (:issue:`#2275 <2275>`) +- android: respect Android's NetworkSecurityPolicy isCleartextTrafficPermitted APIs. - net: enable happy eyeballs by default (:issue:`#2272 <2272>`) - iOS: remove support for installing via CocoaPods, which had not worked since 2020 (:issue:`#2215 <2215>`) - iOS: enable usage of ``NWPathMonitor`` by default (:issue:`#2329 <2329>`) diff --git a/mobile/library/common/http/BUILD b/mobile/library/common/http/BUILD index a5e17fb7fcf6..d846333a9f54 100644 --- a/mobile/library/common/http/BUILD +++ b/mobile/library/common/http/BUILD @@ -18,6 +18,7 @@ envoy_cc_library( "//library/common/extensions/filters/http/local_error:local_error_filter_lib", "//library/common/extensions/filters/http/network_configuration:network_configuration_filter_lib", "//library/common/http:header_utility_lib", + "//library/common/jni:android_jni_utility_lib", "//library/common/network:configurator_lib", "//library/common/network:synthetic_address_lib", "//library/common/stream_info:extra_stream_info_lib", diff --git a/mobile/library/common/http/client.cc b/mobile/library/common/http/client.cc index 5bb80baace76..3cd006b84207 100644 --- a/mobile/library/common/http/client.cc +++ b/mobile/library/common/http/client.cc @@ -13,6 +13,7 @@ #include "library/common/data/utility.h" #include "library/common/http/header_utility.h" #include "library/common/http/headers.h" +#include "library/common/jni/android_jni_utility.h" #include "library/common/network/configurator.h" #include "library/common/stream_info/extra_stream_info.h" @@ -479,6 +480,16 @@ void Client::sendHeaders(envoy_stream_t stream, envoy_headers headers, bool end_ if (direct_stream) { ScopeTrackerScopeState scope(direct_stream.get(), scopeTracker()); RequestHeaderMapPtr internal_headers = Utility::toRequestHeaders(headers); + + // This is largely a check for the android platform: is_cleartext_permitted + // is a no-op for other platforms. + if (internal_headers->getSchemeValue() != "https" && + !is_cleartext_permitted(internal_headers->getHostValue())) { + direct_stream->request_decoder_->sendLocalReply( + Http::Code::BadRequest, "Cleartext is not permitted", nullptr, absl::nullopt, ""); + return; + } + setDestinationCluster(*internal_headers); // Set the x-forwarded-proto header to https because Envoy Mobile only has clusters with TLS // enabled. This is done here because the ApiListener's synthetic connection would make the diff --git a/mobile/library/common/jni/BUILD b/mobile/library/common/jni/BUILD index ac721be0639f..dcf22d6136b9 100644 --- a/mobile/library/common/jni/BUILD +++ b/mobile/library/common/jni/BUILD @@ -199,3 +199,25 @@ cc_library( "//library/common/jni/import:jni_import_lib", ], ) + +cc_library( + name = "android_jni_utility_lib", + srcs = [ + "android_jni_utility.cc", + ], + hdrs = [ + "android_jni_utility.h", + ], + deps = [ + "@envoy//source/common/common:assert_lib", + "//library/common/types:c_types_lib", + ] + select({ + "@envoy//bazel:android": [ + ":ndk_jni_support", + ":jni_utility_lib", + "//library/common/data:utility_lib", + "//library/common/jni/import:jni_import_lib", + ], + "//conditions:default": [], + }), +) diff --git a/mobile/library/common/jni/android_jni_utility.cc b/mobile/library/common/jni/android_jni_utility.cc new file mode 100644 index 000000000000..733cc3b9295a --- /dev/null +++ b/mobile/library/common/jni/android_jni_utility.cc @@ -0,0 +1,34 @@ +#include "library/common/jni/android_jni_utility.h" + +#include +#include + +#include "source/common/common/assert.h" + +#if defined(__ANDROID_API__) +#include "library/common/data/utility.h" +#include "library/common/jni/import/jni_import.h" +#include "library/common/jni/jni_support.h" +#include "library/common/jni/jni_version.h" +#include "library/common/jni/jni_utility.h" +#endif + +// NOLINT(namespace-envoy) + +bool is_cleartext_permitted(absl::string_view hostname) { +#if defined(__ANDROID_API__) + envoy_data host = Envoy::Data::Utility::copyToBridgeData(hostname); + JNIEnv* env = get_env(); + jstring java_host = native_data_to_string(env, host); + jclass jcls_Boolean = env->FindClass("org/chromium/net/AndroidNetworkLibrary"); + jmethodID jmid_isCleartextTrafficPermitted = + env->GetMethodID(jcls_Boolean, "isCleartextTrafficPermitted", "(Ljava/lang/String;)Z"); + jboolean result = env->CallBooleanMethod(java_host, jmid_isCleartextTrafficPermitted); + env->DeleteLocalRef(java_host); + release_envoy_data(host); + return result == JNI_TRUE; +#else + UNREFERENCED_PARAMETER(hostname); + return true; +#endif +} diff --git a/mobile/library/common/jni/android_jni_utility.h b/mobile/library/common/jni/android_jni_utility.h new file mode 100644 index 000000000000..e0a3feba3c55 --- /dev/null +++ b/mobile/library/common/jni/android_jni_utility.h @@ -0,0 +1,11 @@ +#pragma once + +#include "absl/strings/string_view.h" + +// NOLINT(namespace-envoy) + +/* For android, calls up through JNI to see if cleartext is permitted for this + * host. + * For other platforms simply returns true. + */ +bool is_cleartext_permitted(absl::string_view hostname);