Skip to content

Commit

Permalink
dns: stop using cares DNS resolver (#2618)
Browse files Browse the repository at this point in the history
Description: Remove the usage of cares DNS resolver, use `getaddrinfo` based DNS resolver instead. Fixes #2534.
Risk Level: Low, Envoy Mobile default was not to use cares.
Testing: Existing unit/integration tests.
Docs Changes: N/A
Release Notes: Done

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
  • Loading branch information
Augustyniak authored Oct 21, 2022
1 parent 232df84 commit c461cda
Show file tree
Hide file tree
Showing 15 changed files with 16 additions and 259 deletions.
30 changes: 0 additions & 30 deletions docs/root/api/starting_envoy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,6 @@ The configuration is expected as a JSON list.
// Swift
builder.addDNSPreresolveHostnames("[{\"address\": \"foo.com", \"port_value\": 443}]")

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``addDNSFallbackNameservers``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. attention::

This API is only available for Kotlin.

Add a list of IP addresses to use as fallback DNS name servers.
See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/network/dns_resolver/cares/v3/cares_dns_resolver.proto#extensions-network-dns-resolver-cares-v3-caresdnsresolverconfig>`__
for further information.

// Kotlin
builder.addDNSFallbackNameservers(listOf<String>("8.8.8.8"))

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
``enableDNSFilterUnroutableFamilies``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. attention::

This API is only available for Kotlin.

Specify whether to filter unroutable IP families during DNS resolution or not.
See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/network/dns_resolver/cares/v3/cares_dns_resolver.proto#extensions-network-dns-resolver-cares-v3-caresdnsresolverconfig>`__
for further information.

// Kotlin
builder.enableDNSFilterUnroutableFamilies(true)

~~~~~~~~~~~~~~~
``addLogLevel``
~~~~~~~~~~~~~~~
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Breaking changes:

- ios/android: remove ``addH2RawDomains`` method. (:issue: `#2590 <2590>`)
- build: building on macOS now requires Xcode 14.0. (:issue:`#2544 <2544>`)
- kotlin: always use ``getaddrinfo`` DNS resolver. Remove ``addDNSFallbackNameservers``, ``enableDNSFilterUnroutableFamilies``, and ``enableDNSUseSystemResolver`` methods from the Kotlin engine builder. (:issue:`#2618 <2618>`)

Bugfixes:

Expand Down
22 changes: 0 additions & 22 deletions library/cc/engine_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,27 +208,6 @@ EngineBuilder& EngineBuilder::addStringAccessor(const std::string& name,
}

std::string EngineBuilder::generateConfigStr() const {
#if defined(__APPLE__)
std::string dns_resolver_name = "envoy.network.dns_resolver.apple";
std::string dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\"}";
#else
std::string dns_resolver_name = "";
std::string dns_resolver_config = "";
if (this->use_system_resolver_) {
dns_resolver_name = "envoy.network.dns_resolver.getaddrinfo";
dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig\"}";
} else {
dns_resolver_name = "envoy.network.dns_resolver.cares";
dns_resolver_config =
"{\"@type\":\"type.googleapis.com/"
"envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig\"}";
}
#endif

std::vector<std::pair<std::string, std::string>> replacements {
{"connect_timeout", fmt::format("{}s", this->connect_timeout_seconds_)},
{"dns_fail_base_interval", fmt::format("{}s", this->dns_failure_refresh_seconds_base_)},
Expand All @@ -239,7 +218,6 @@ std::string EngineBuilder::generateConfigStr() const {
{"dns_preresolve_hostnames", this->dns_preresolve_hostnames_},
{"dns_refresh_rate", fmt::format("{}s", this->dns_refresh_seconds_)},
{"dns_query_timeout", fmt::format("{}s", this->dns_query_timeout_seconds_)},
{"dns_resolver_name", dns_resolver_name}, {"dns_resolver_config", dns_resolver_config},
{"enable_drain_post_dns_refresh", enable_drain_post_dns_refresh_ ? "true" : "false"},
{"enable_interface_binding", enable_interface_binding_ ? "true" : "false"},
{"h2_connection_keepalive_idle_interval",
Expand Down
4 changes: 2 additions & 2 deletions library/common/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ R"(- &dns_resolver_name envoy.network.dns_resolver.apple
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig"}
)"
#else
R"(- &dns_resolver_name envoy.network.dns_resolver.cares
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig"}
R"(- &dns_resolver_name envoy.network.dns_resolver.getaddrinfo
- &dns_resolver_config {"@type":"type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig"}
)"
#endif
R"(- &enable_drain_post_dns_refresh false
Expand Down
19 changes: 2 additions & 17 deletions library/common/jni/android_jni_interface.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#include <ares.h>

#include "library/common/jni/android_network_utility.h"
#include "library/common/jni/import/jni_import.h"
#include "library/common/jni/jni_support.h"
Expand All @@ -13,21 +11,8 @@
extern "C" JNIEXPORT jint JNICALL
Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
jclass, // class
jobject class_loader,
jobject connectivity_manager) {
jobject class_loader) {
set_class_loader(env->NewGlobalRef(class_loader));
// At this point, we know Android APIs are available. Register cert chain validation JNI calls.
envoy_status_t result =
register_platform_api(cert_validator_name, get_android_cert_validator_api());
if (result == ENVOY_FAILURE) {
return ENVOY_FAILURE;
}

// See note above about c-ares.
// c-ares jvm init is necessary in order to let c-ares perform DNS resolution in Envoy.
// More information can be found at:
// https://c-ares.haxx.se/ares_library_init_android.html
ares_library_init_jvm(get_vm());

return ares_library_init_android(connectivity_manager);
return register_platform_api(cert_validator_name, get_android_cert_validator_api());
}
3 changes: 1 addition & 2 deletions library/common/jni/android_test_jni_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
extern "C" JNIEXPORT jint JNICALL
Java_io_envoyproxy_envoymobile_engine_AndroidJniLibrary_initialize(JNIEnv* env,
jclass, // class
jobject class_loader,
jobject connectivity_manager) {
jobject class_loader) {
set_class_loader(env->NewGlobalRef(class_loader));
// At this point, we know Android APIs are available. Register cert chain validation JNI calls.
return register_platform_api(cert_validator_name, get_android_cert_validator_api());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,15 @@ public static void load(Context context) {
// its dependencies are loaded and initialized at most once.
private static class AndroidLoader {
private AndroidLoader(Context context) {
AndroidJniLibrary.initialize(
context.getClassLoader(),
(ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE));
AndroidJniLibrary.initialize(context.getClassLoader());
}
}

/**
* Native binding to register the ConnectivityManager to C-Ares.
*
* @param classLoader Application's class loader.
* @param connectivityManager Android's ConnectivityManager.
* @return The resulting status of the initialization.
*/
protected static native int initialize(ClassLoader classLoader,
ConnectivityManager connectivityManager);
protected static native int initialize(ClassLoader classLoader);
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ public enum TrustChainVerification {
public final Integer dnsQueryTimeoutSeconds;
public final Integer dnsMinRefreshSeconds;
public final String dnsPreresolveHostnames;
public final List<String> dnsFallbackNameservers;
public final Boolean dnsFilterUnroutableFamilies;
public final Boolean dnsUseSystemResolver;
public final Boolean enableDrainPostDnsRefresh;
public final Boolean enableHttp3;
public final Boolean enableGzip;
Expand Down Expand Up @@ -85,13 +82,6 @@ public enum TrustChainVerification {
* refresh DNS.
* @param dnsPreresolveHostnames hostnames to preresolve on Envoy Client
* construction.
* @param dnsFallbackNameservers addresses to use as DNS name server
* fallback.
* @param dnsFilterUnroutableFamilies whether to filter unroutable IP families
* or not.
* @param dnsUseSystemResolver whether to use the getaddrinfo-based
* system resolver or
* c-ares.
* @param enableDrainPostDnsRefresh whether to drain connections after soft
* DNS refresh.
* @param enableHttp3 whether to enable experimental support for
Expand Down Expand Up @@ -133,13 +123,12 @@ public EnvoyConfiguration(
boolean adminInterfaceEnabled, String grpcStatsDomain, int connectTimeoutSeconds,
int dnsRefreshSeconds, int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax,
int dnsQueryTimeoutSeconds, int dnsMinRefreshSeconds, String dnsPreresolveHostnames,
List<String> dnsFallbackNameservers, boolean dnsFilterUnroutableFamilies,
boolean dnsUseSystemResolver, boolean enableDrainPostDnsRefresh, boolean enableHttp3,
boolean enableGzip, boolean enableBrotli, boolean enableSocketTagging,
boolean enableHappyEyeballs, boolean enableInterfaceBinding,
int h2ConnectionKeepaliveIdleIntervalMilliseconds, int h2ConnectionKeepaliveTimeoutSeconds,
boolean h2ExtendKeepaliveTimeout, int maxConnectionsPerHost, int statsFlushSeconds,
int streamIdleTimeoutSeconds, int perTryIdleTimeoutSeconds, String appVersion, String appId,
boolean enableDrainPostDnsRefresh, boolean enableHttp3, boolean enableGzip,
boolean enableBrotli, boolean enableSocketTagging, boolean enableHappyEyeballs,
boolean enableInterfaceBinding, int h2ConnectionKeepaliveIdleIntervalMilliseconds,
int h2ConnectionKeepaliveTimeoutSeconds, boolean h2ExtendKeepaliveTimeout,
int maxConnectionsPerHost, int statsFlushSeconds, int streamIdleTimeoutSeconds,
int perTryIdleTimeoutSeconds, String appVersion, String appId,
TrustChainVerification trustChainVerification, String virtualClusters,
List<EnvoyNativeFilterConfig> nativeFilterChain,
List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories,
Expand All @@ -155,9 +144,6 @@ public EnvoyConfiguration(
this.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds;
this.dnsMinRefreshSeconds = dnsMinRefreshSeconds;
this.dnsPreresolveHostnames = dnsPreresolveHostnames;
this.dnsFallbackNameservers = dnsFallbackNameservers;
this.dnsFilterUnroutableFamilies = dnsFilterUnroutableFamilies;
this.dnsUseSystemResolver = dnsUseSystemResolver;
this.enableDrainPostDnsRefresh = enableDrainPostDnsRefresh;
this.enableHttp3 = enableHttp3;
this.enableGzip = enableGzip;
Expand Down Expand Up @@ -235,33 +221,6 @@ String resolveTemplate(final String configTemplate, final String platformFilterT
String processedTemplate =
configTemplate.replace("#{custom_filters}", customFiltersBuilder.toString());

String dnsFallbackNameserversAsString = "[]";
if (!dnsFallbackNameservers.isEmpty()) {
StringBuilder sb = new StringBuilder("[");
String separator = "";
for (String nameserver : dnsFallbackNameservers) {
sb.append(separator);
separator = ",";
sb.append(String.format("{\"socket_address\":{\"address\":\"%s\"}}", nameserver));
}
sb.append("]");
dnsFallbackNameserversAsString = sb.toString();
}

String dnsResolverName = "";
String dnsResolverConfig = "";
if (dnsUseSystemResolver) {
dnsResolverName = "envoy.network.dns_resolver.getaddrinfo";
dnsResolverConfig =
"{\"@type\":\"type.googleapis.com/envoy.extensions.network.dns_resolver.getaddrinfo.v3.GetAddrInfoDnsResolverConfig\"}";
} else {
dnsResolverName = "envoy.network.dns_resolver.cares";
dnsResolverConfig = String.format(
"{\"@type\":\"type.googleapis.com/envoy.extensions.network.dns_resolver.cares.v3.CaresDnsResolverConfig\",\"resolvers\":%s,\"use_resolvers_as_fallback\": %s, \"filter_unroutable_families\": %s}",
dnsFallbackNameserversAsString, !dnsFallbackNameservers.isEmpty() ? "true" : "false",
dnsFilterUnroutableFamilies ? "true" : "false");
}

StringBuilder configBuilder = new StringBuilder("!ignore platform_defs:\n");
configBuilder.append(String.format("- &connect_timeout %ss\n", connectTimeoutSeconds))
.append(String.format("- &dns_fail_base_interval %ss\n", dnsFailureRefreshSecondsBase))
Expand All @@ -275,9 +234,7 @@ String resolveTemplate(final String configTemplate, final String platformFilterT
String.format("- &dns_multiple_addresses %s\n", enableHappyEyeballs ? "true" : "false"))
.append(String.format("- &h2_delay_keepalive_timeout %s\n",
h2ExtendKeepaliveTimeout ? "true" : "false"))
.append(String.format("- &dns_resolver_name %s\n", dnsResolverName))
.append(String.format("- &dns_refresh_rate %ss\n", dnsRefreshSeconds))
.append(String.format("- &dns_resolver_config %s\n", dnsResolverConfig))
.append(String.format("- &enable_drain_post_dns_refresh %s\n",
enableDrainPostDnsRefresh ? "true" : "false"))
.append(String.format("- &enable_interface_binding %s\n",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ private EnvoyConfiguration createEnvoyConfiguration() {
return new EnvoyConfiguration(
mAdminInterfaceEnabled, mGrpcStatsDomain, mConnectTimeoutSeconds, mDnsRefreshSeconds,
mDnsFailureRefreshSecondsBase, mDnsFailureRefreshSecondsMax, mDnsQueryTimeoutSeconds,
mDnsMinRefreshSeconds, mDnsPreresolveHostnames, mDnsFallbackNameservers,
mEnableDnsFilterUnroutableFamilies, mDnsUseSystemResolver, mEnableDrainPostDnsRefresh,
quicEnabled(), mEnableGzip, brotliEnabled(), mEnableSocketTag, mEnableHappyEyeballs,
mDnsMinRefreshSeconds, mDnsPreresolveHostnames, mEnableDrainPostDnsRefresh, quicEnabled(),
mEnableGzip, brotliEnabled(), mEnableSocketTag, mEnableHappyEyeballs,
mEnableInterfaceBinding, mH2ConnectionKeepaliveIdleIntervalMilliseconds,
mH2ConnectionKeepaliveTimeoutSeconds, mH2ExtendKeepaliveTimeout, mMaxConnectionsPerHost,
mStatsFlushSeconds, mStreamIdleTimeoutSeconds, mPerTryIdleTimeoutSeconds, mAppVersion,
Expand Down
47 changes: 0 additions & 47 deletions library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ open class EngineBuilder(
private var dnsRefreshSeconds = 60
private var dnsFailureRefreshSecondsBase = 2
private var dnsFailureRefreshSecondsMax = 10
private var dnsFallbackNameservers = listOf<String>()
private var dnsFilterUnroutableFamilies = true
private var dnsUseSystemResolver = true
private var dnsQueryTimeoutSeconds = 25
private var dnsMinRefreshSeconds = 60
private var dnsPreresolveHostnames = "[]"
Expand Down Expand Up @@ -196,47 +193,6 @@ open class EngineBuilder(
return this
}

/**
* Add a list of IP addresses to use as fallback DNS name servers.
*
* @param dnsFallbackNameservers addresses to use.
*
* @return this builder.
*/
fun addDNSFallbackNameservers(dnsFallbackNameservers: List<String>): EngineBuilder {
this.dnsFallbackNameservers = dnsFallbackNameservers
return this
}

/**
* Specify whether to filter unroutable IP families during DNS resolution or not.
* Defaults to true.
*
* @param dnsFilterUnroutableFamilies whether to filter or not.
*
* @return this builder.
*/
fun enableDNSFilterUnroutableFamilies(dnsFilterUnroutableFamilies: Boolean): EngineBuilder {
this.dnsFilterUnroutableFamilies = dnsFilterUnroutableFamilies
return this
}

/**
* Specify whether to use the getaddrinfo-based system DNS resolver or the c-ares resolver.
* Defaults to true.
*
* Note that if this is set, the values of `dnsFallbackNameservers` and
* `dnsFilterUnroutableFamilies` will be ignored.
*
* @param dnsUseSystemResolver whether to use the system DNS resolver.
*
* @return this builder.
*/
fun enableDNSUseSystemResolver(dnsUseSystemResolver: Boolean): EngineBuilder {
this.dnsUseSystemResolver = dnsUseSystemResolver
return this
}

/**
* Specify whether to drain connections after the resolution of a soft DNS refresh. A refresh may
* be triggered directly via the Engine API, or as a result of a network status update provided by
Expand Down Expand Up @@ -628,9 +584,6 @@ open class EngineBuilder(
dnsQueryTimeoutSeconds,
dnsMinRefreshSeconds,
dnsPreresolveHostnames,
dnsFallbackNameservers,
dnsFilterUnroutableFamilies,
dnsUseSystemResolver,
enableDrainPostDnsRefresh,
enableHttp3,
enableGzip,
Expand Down
5 changes: 0 additions & 5 deletions library/objective-c/EnvoyConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ - (nullable NSString *)resolveTemplate:(NSString *)templateYAML {
[definitions appendFormat:@"- &dns_refresh_rate %lus\n", (unsigned long)self.dnsRefreshSeconds];
[definitions appendFormat:@"- &enable_drain_post_dns_refresh %@\n",
self.enableDrainPostDnsRefresh ? @"true" : @"false"];
// No additional values are currently needed for Apple-based DNS resolver.
[definitions
appendFormat:@"- &dns_resolver_config "
@"{\"@type\":\"type.googleapis.com/"
@"envoy.extensions.network.dns_resolver.apple.v3.AppleDnsResolverConfig\"}\n"];
[definitions appendFormat:@"- &enable_interface_binding %@\n",
self.enableInterfaceBinding ? @"true" : @"false"];
[definitions appendFormat:@"- &trust_chain_verification %@\n", self.enforceTrustChainVerification
Expand Down
13 changes: 0 additions & 13 deletions test/cc/unit/envoy_config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,6 @@ TEST(TestConfig, ConfigIsValid) {
#endif
}

#if !defined(__APPLE__)
TEST(TestConfig, SetUseDnsCAresResolver) {
EngineBuilder engine_builder;
engine_builder.useDnsSystemResolver(false);
std::string config_str = engine_builder.generateConfigStr();
envoy::config::bootstrap::v3::Bootstrap bootstrap;
TestUtility::loadFromYaml(absl::StrCat(config_header, config_str), bootstrap);

ASSERT_THAT(bootstrap.DebugString(), HasSubstr("envoy.network.dns_resolver.cares"));
ASSERT_THAT(bootstrap.DebugString(), Not(HasSubstr("envoy.network.dns_resolver.getaddrinfo")));
}
#endif

TEST(TestConfig, SetGzip) {
EngineBuilder engine_builder;

Expand Down
Loading

0 comments on commit c461cda

Please sign in to comment.