Skip to content

Commit

Permalink
Pull request #519: Move timeout from UpstreamOptions to UpstreamFacto…
Browse files Browse the repository at this point in the history
…ryConfig (exposed through DnsProxySettings)

Merge in ADGUARD-CORE-LIBS/dns-libs from fix/AG-17041 to dev-2.2

Squashed commit of the following:

commit 4b4135a37985f87cf37e304b97fb15d2b517aac0
Author: Boris <[email protected]>
Date:   Thu Mar 16 16:46:06 2023 +0200

    issue #AG-17041, 42

commit 9348f489cf8b1291e26fbc8040c08b4e896d4dbc
Author: Boris <[email protected]>
Date:   Thu Mar 16 16:17:50 2023 +0200

    issue #AG-17041, win part 2

commit 88f49ab379506428a71cac116ddc84b5f3c8ca9e
Author: Boris <[email protected]>
Date:   Thu Mar 16 16:05:15 2023 +0200

    issue #AG-17041, win part

commit f56b79934e32df862761c8cf9af3e77304dc9458
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 21:13:31 2023 +0300

    Fix build (Android), update changelog.

commit 5081bc5e59c5a25bd9f8fe3b81eb9807441fd47a
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 17:43:53 2023 +0300

    Fix build (mac)

commit 31db10d325f078f8ca9701a3967411c7acf1d086
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 17:07:31 2023 +0300

    Make standalone listener not require root on linux

commit c149f65fcb990cfc39f1ac477cec0e23e02e5d5a
Merge: d961ec4c 52bbe7c
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 16:57:36 2023 +0300

    Merge remote-tracking branch 'origin/dev-2.2' into fix/AG-17041

    # Conflicts:
    #	platform/windows/capi/src/ag_dns_h_hash.inc

commit d961ec4c1c60d8c73b32785fd790640e2dc1b855
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 16:51:40 2023 +0300

    Fix build

commit 1a05c5a31038e3136476a8a6542b9b113f6cadf8
Author: Nikita Gorskikh <[email protected]>
Date:   Wed Mar 15 13:35:18 2023 +0300

    Move timeout from UpstreamOptions to UpstreamFactoryConfig (exposed through DnsProxySettings)
  • Loading branch information
ngorskikh committed Mar 16, 2023
1 parent 52bbe7c commit be468e4
Show file tree
Hide file tree
Showing 47 changed files with 290 additions and 247 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

* [Feature] Upstream exchange timeout is now specified separately from the upstream options.
The signature of some functions is changed as a result.
* [Android] See `com.adguard.dnslibs.proxy.DnsProxySettings#setUpstreamTimeoutMs`/`com.adguard.dnslibs.proxy.DnsProxySettings#getUpstreamTimeoutMs`
See `com.adguard.dnslibs.proxy.DnsProxy#testUpstream`.
* [Apple] See `AGDnsProxyConfig.upstreamTimeoutMs`.
See `[AGDnsUtils testUpstream:]`.
* [C API] See `ag_dnsproxy_settings::upstream_timeout_ms`.
See `ag_test_upstream`.

* [Feature] Implement fingerprints verification for two types of fingerprints for encrypted DNS protocols.
1) SPKI fingerprint, set separately in the upstream options, it compared with the sha256 hash of the `SubjectPublicKeyInfo` certificate part. It is possible to transfer several such fingerprints, they will try to get matched with one of the certificates in the chain.
2) The fingerprint of the certificate in full, which is passed as one of the DNS Stamp fields. Compared with sha256 hashes of the entire certificate.
Expand Down
2 changes: 1 addition & 1 deletion dnscrypt/include/dns/dnscrypt/dns_crypt_server_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ struct ServerInfo {
AE_CERTIFICATE_EXPIRED,
AE_SHARED_KEY_CALCULATION,
};
friend class ag::ErrorCodeToString<TxtToCertInfoError>;
friend struct ag::ErrorCodeToString<TxtToCertInfoError>;
using TxtToCertInfoResult = Result<CertInfo, TxtToCertInfoError>;

TxtToCertInfoResult txt_to_cert_info(const ldns_rr &answer_rr) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ public void testEventsMultithreaded() {
final DnsProxySettings settings = getDefaultSettings();
settings.getUpstreams().clear();
settings.getUpstreams().add(new UpstreamSettings(
"1.1.1.1", Collections.emptyList(), 10000, new byte[]{}, 42));
"1.1.1.1", Collections.emptyList(), new byte[]{}, 42));
settings.setUpstreamTimeoutMs(10000);

final List<DnsRequestProcessedEvent> eventList =
Collections.synchronizedList(new ArrayList<DnsRequestProcessedEvent>());
Expand Down Expand Up @@ -218,7 +219,6 @@ public void testSettingsMarshalling() {
dot.setAddress("tls://dns.adguard.com");
dot.getBootstrap().add("8.8.8.8");
dot.setServerIp(new byte[]{8, 8, 8, 8});
dot.setTimeoutMs(10000);
dot.setId(42);
dot.setOutboundInterfaceName("whtvr0");
settings.getUpstreams().add(dot);
Expand Down Expand Up @@ -251,9 +251,10 @@ public void testSettingsMarshalling() {
fallbackUpstream.setAddress("https://fall.back/up/stream");
fallbackUpstream.setBootstrap(Collections.singletonList("1.1.1.1"));
fallbackUpstream.setServerIp(new byte[]{8, 8, 8, 8});
fallbackUpstream.setTimeoutMs(4200);
settings.getFallbacks().add(fallbackUpstream);

settings.setUpstreamTimeoutMs(4200);

settings.setOutboundProxy(
new OutboundProxySettings(OutboundProxySettings.Protocol.SOCKS5_UDP, "::", 1234,
null, new OutboundProxySettings.AuthInfo("1", "2"), true, false));
Expand Down Expand Up @@ -291,10 +292,10 @@ private void testCertificateVerification(String upstreamAddr) {
final UpstreamSettings us = new UpstreamSettings();
us.setAddress(upstreamAddr);
us.getBootstrap().add("8.8.8.8");
us.setTimeoutMs(10000);
final DnsProxySettings settings = getDefaultSettings();
settings.getUpstreams().clear();
settings.getUpstreams().add(us);
settings.setUpstreamTimeoutMs(10000);
settings.setIpv6Available(false); // DoT times out trying to reach dns.adguard.com over IPv6

final DnsProxyEvents events = new DnsProxyEvents() {
Expand Down Expand Up @@ -339,11 +340,11 @@ private void testCertificateVerificationWithFingerprint(String upstreamAddr, Str
final UpstreamSettings us = new UpstreamSettings();
us.setAddress(upstreamAddr);
us.getBootstrap().add("8.8.8.8");
us.setTimeoutMs(10000);
us.getFingerprints().add(certFingerprint);
final DnsProxySettings settings = getDefaultSettings();
settings.getUpstreams().clear();
settings.getUpstreams().add(us);
settings.setUpstreamTimeoutMs(10000);
settings.setIpv6Available(false); // DoT times out trying to reach dns.adguard.com over IPv6

final DnsProxyEvents events = new DnsProxyEvents() {
Expand Down Expand Up @@ -467,27 +468,27 @@ void put(String stampStr, DnsStamp dnsStamp, String prettyUrl, String prettierUr

@Test
public void testTestUpstream() {
final long timeout = 500; // ms
final int timeout = 500; // ms
IllegalArgumentException e0 = null;
try {
DnsProxy.testUpstream(new UpstreamSettings("123.12.32.1:1493", new ArrayList<String>(), timeout, null, 42),
false, false);
DnsProxy.testUpstream(new UpstreamSettings("123.12.32.1:1493", new ArrayList<String>(), null, 42),
timeout, false, false);
} catch (IllegalArgumentException e) {
e0 = e;
}
assertNotNull(e0);
try {
DnsProxy.testUpstream(new UpstreamSettings("8.8.8.8:53", new ArrayList<String>(), 10 * timeout, null, 42),
false, false);
DnsProxy.testUpstream(new UpstreamSettings("8.8.8.8:53", new ArrayList<String>(), null, 42),
10 * timeout, false, false);
} catch (IllegalArgumentException e) {
fail(e.toString());
}
try {
ArrayList<String> bootstrap = new ArrayList<>();
bootstrap.add("1.2.3.4");
bootstrap.add("8.8.8.8");
DnsProxy.testUpstream(new UpstreamSettings("tls://dns.adguard-dns.com", bootstrap, 10 * timeout, null, 42),
false, false);
DnsProxy.testUpstream(new UpstreamSettings("tls://dns.adguard-dns.com", bootstrap, null, 42),
10 * timeout, false, false);
} catch (IllegalArgumentException e) {
fail(e.toString());
}
Expand Down
19 changes: 9 additions & 10 deletions platform/android/dnsproxy/lib/src/main/cpp/android_dnsproxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ extern "C" JNIEXPORT jboolean JNICALL Java_com_adguard_dnslibs_proxy_DnsProxy_is
}

extern "C" JNIEXPORT jstring JNICALL Java_com_adguard_dnslibs_proxy_DnsProxy_testUpstreamNative(JNIEnv *env,
jclass clazz, jlong native_ptr, jobject upstream_settings, jboolean ipv6, jobject events_adapter,
jboolean offline) {
jclass clazz, jlong native_ptr, jobject upstream_settings, jint timeout_ms,
jboolean ipv6, jobject events_adapter, jboolean offline) {
auto *proxy = (AndroidDnsProxy *) native_ptr;
return proxy->test_upstream(env, upstream_settings, ipv6, events_adapter, offline);
return proxy->test_upstream(env, upstream_settings, timeout_ms, ipv6, events_adapter, offline);
}

extern "C" JNIEXPORT jobject JNICALL Java_com_adguard_dnslibs_proxy_DnsProxy_init(
Expand Down Expand Up @@ -104,7 +104,6 @@ UpstreamOptions AndroidDnsProxy::marshal_upstream(JNIEnv *env, jobject java_upst

auto dns_server_field = env->GetFieldID(clazz, "address", "Ljava/lang/String;");
auto bootstrap_field = env->GetFieldID(clazz, "bootstrap", "Ljava/util/List;");
auto timeout_field = env->GetFieldID(clazz, "timeoutMs", "J");
auto server_ip_field = env->GetFieldID(clazz, "serverIp", "[B");
auto id_field = env->GetFieldID(clazz, "id", "I");
auto if_field = env->GetFieldID(clazz, "outboundInterfaceName", "Ljava/lang/String;");
Expand All @@ -127,8 +126,6 @@ UpstreamOptions AndroidDnsProxy::marshal_upstream(JNIEnv *env, jobject java_upst
});
}

upstream.timeout = std::chrono::milliseconds(env->GetLongField(java_upstream_settings, timeout_field));

if (LocalRef server_ip{env, (jbyteArray) env->GetObjectField(java_upstream_settings, server_ip_field)}) {

assert(env->IsInstanceOf(server_ip.get(), env->FindClass("[B")));
Expand Down Expand Up @@ -166,7 +163,6 @@ LocalRef<jobject> AndroidDnsProxy::marshal_upstream(JNIEnv *env, const UpstreamO
auto ctor = env->GetMethodID(clazz, "<init>", "()V");
auto dns_server_field = env->GetFieldID(clazz, "address", "Ljava/lang/String;");
auto bootstrap_field = env->GetFieldID(clazz, "bootstrap", "Ljava/util/List;");
auto timeout_field = env->GetFieldID(clazz, "timeoutMs", "J");
auto server_ip_field = env->GetFieldID(clazz, "serverIp", "[B");
auto id_field = env->GetFieldID(clazz, "id", "I");
auto if_field = env->GetFieldID(clazz, "outboundInterfaceName", "Ljava/lang/String;");
Expand All @@ -175,7 +171,6 @@ LocalRef<jobject> AndroidDnsProxy::marshal_upstream(JNIEnv *env, const UpstreamO
auto java_upstream = env->NewObject(clazz, ctor);

env->SetObjectField(java_upstream, dns_server_field, m_utils.marshal_string(env, settings.address).get());
env->SetLongField(java_upstream, timeout_field, settings.timeout.count());
env->SetIntField(java_upstream, id_field, settings.id);

if (std::holds_alternative<Ipv4Address>(settings.resolved_server_ip)) {
Expand Down Expand Up @@ -449,6 +444,7 @@ DnsProxySettings AndroidDnsProxy::marshal_settings(JNIEnv *env, jobject java_dns
auto fallback_on_failure_field = env->GetFieldID(clazz, "enableFallbackOnUpstreamsFailure", "Z");
auto servfail_on_failure_field = env->GetFieldID(clazz, "enableServfailOnUpstreamsFailure", "Z");
auto enable_http3_field = env->GetFieldID(clazz, "enableHttp3", "Z");
auto timeout_field = env->GetFieldID(clazz, "upstreamTimeoutMs", "I");

DnsProxySettings settings{};

Expand Down Expand Up @@ -521,6 +517,7 @@ DnsProxySettings AndroidDnsProxy::marshal_settings(JNIEnv *env, jobject java_dns
settings.enable_fallback_on_upstreams_failure = env->GetBooleanField(java_dnsproxy_settings, fallback_on_failure_field);
settings.enable_servfail_on_upstreams_failure = env->GetBooleanField(java_dnsproxy_settings, servfail_on_failure_field);
settings.enable_http3 = env->GetBooleanField(java_dnsproxy_settings, enable_http3_field);
settings.upstream_timeout = Millis{env->GetIntField(java_dnsproxy_settings, timeout_field)};

return settings;
}
Expand Down Expand Up @@ -552,6 +549,7 @@ LocalRef<jobject> AndroidDnsProxy::marshal_settings(JNIEnv *env, const DnsProxyS
auto fallback_on_failure_field = env->GetFieldID(clazz, "enableFallbackOnUpstreamsFailure", "Z");
auto servfail_on_failure_field = env->GetFieldID(clazz, "enableServfailOnUpstreamsFailure", "Z");
auto enable_http3_field = env->GetFieldID(clazz, "enableHttp3", "Z");
auto timeout_field = env->GetFieldID(clazz, "upstreamTimeoutMs", "I");

auto java_settings = env->NewObject(clazz, ctor);

Expand Down Expand Up @@ -616,6 +614,7 @@ LocalRef<jobject> AndroidDnsProxy::marshal_settings(JNIEnv *env, const DnsProxyS
env->SetBooleanField(java_settings, fallback_on_failure_field, (jboolean) settings.enable_fallback_on_upstreams_failure);
env->SetBooleanField(java_settings, servfail_on_failure_field, (jboolean) settings.enable_servfail_on_upstreams_failure);
env->SetBooleanField(java_settings, enable_http3_field, (jboolean) settings.enable_http3);
env->SetIntField(java_settings, timeout_field, (jint) settings.upstream_timeout.count());

return LocalRef(env, java_settings);
}
Expand Down Expand Up @@ -853,9 +852,9 @@ jobject AndroidDnsProxy::get_settings(JNIEnv *env) {
}

jstring AndroidDnsProxy::test_upstream(
JNIEnv *env, jobject upstream_settings, jboolean ipv6, jobject events_adapter, jboolean offline) {
JNIEnv *env, jobject upstream_settings, jint timeout_ms, jboolean ipv6, jobject events_adapter, jboolean offline) {
m_events = GlobalRef(get_vm(env), events_adapter);
auto err = ag::dns::test_upstream(marshal_upstream(env, upstream_settings), ipv6,
auto err = ag::dns::test_upstream(marshal_upstream(env, upstream_settings), Millis{timeout_ms}, ipv6,
marshal_events(env, events_adapter).on_certificate_verification, offline);
if (err) {
return m_utils.marshal_string(env, err->str()).release();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class AndroidDnsProxy {
* Checks if upstream is valid and available.
* @return Null or error string marshalled to Java.
*/
jstring test_upstream(JNIEnv *env, jobject upstream_settings, jboolean ipv6, jobject events_adapter, jboolean offline);
jstring test_upstream(JNIEnv *env, jobject upstream_settings, jint timeout_ms, jboolean ipv6, jobject events_adapter, jboolean offline);

/**
* Suggest an action for filtering log event.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,24 +242,26 @@ public static void setLogLevel(LogLevel level) {
/**
* Checks if upstream is valid and available
* @param upstreamSettings Upstream settings
* @param timeoutMs Maximum amount of time, in milliseconds, allowed for upstream exchange
* @param ipv6Available Whether IPv6 is available (bootstrapper is allowed to make AAAA queries)
* @param offline Don't perform online upstream check
* @throws IllegalArgumentException with an explanation if check failed
*/
public static void testUpstream(UpstreamSettings upstreamSettings, boolean ipv6Available,
boolean offline) throws IllegalArgumentException {
public static void testUpstream(UpstreamSettings upstreamSettings,
int timeoutMs, boolean ipv6Available, boolean offline)
throws IllegalArgumentException {
String error;
try (final DnsProxy proxy = new DnsProxy()) {
error = testUpstreamNative(proxy.nativePtr, upstreamSettings, ipv6Available,
new EventsAdapter(null), false);
error = testUpstreamNative(proxy.nativePtr, upstreamSettings, timeoutMs, ipv6Available,
new EventsAdapter(null), offline);
}
if (error != null) {
throw new IllegalArgumentException(error);
}
}

private static native String testUpstreamNative(long nativePtr, Object upstreamSettings, boolean ipv6,
Object eventsAdapter, boolean offline);
private static native String testUpstreamNative(long nativePtr, Object upstreamSettings, int timeoutMs,
boolean ipv6, Object eventsAdapter, boolean offline);

/**
* Events adapter implementation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public static BlockingMode fromCode(int code) {
private boolean enableFallbackOnUpstreamsFailure;
private boolean enableServfailOnUpstreamsFailure;
private boolean enableHttp3;
private int upstreamTimeoutMs;

/**
* @return Maximum number of cached responses
Expand Down Expand Up @@ -434,6 +435,20 @@ public void setEnableHttp3(boolean enableHttp3) {
this.enableHttp3 = enableHttp3;
}

/**
* @return Maximum amount of time, in milliseconds, allowed for upstream exchange.
*/
public int getUpstreamTimeoutMs() {
return upstreamTimeoutMs;
}

/**
* @param upstreamTimeoutMs Maximum amount of time, in milliseconds, allowed for upstream exchange.
*/
public void setUpstreamTimeoutMs(int upstreamTimeoutMs) {
this.upstreamTimeoutMs = upstreamTimeoutMs;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
Expand Down Expand Up @@ -461,7 +476,8 @@ public boolean equals(Object o) {
blockEch == that.blockEch &&
enableParallelUpstreamQueries == that.enableParallelUpstreamQueries &&
enableFallbackOnUpstreamsFailure == that.enableFallbackOnUpstreamsFailure &&
enableServfailOnUpstreamsFailure == that.enableServfailOnUpstreamsFailure;
enableServfailOnUpstreamsFailure == that.enableServfailOnUpstreamsFailure &&
upstreamTimeoutMs == that.upstreamTimeoutMs;
}

@Override
Expand All @@ -470,7 +486,8 @@ public int hashCode() {
filterParams, listeners, outboundProxy, ipv6Available, blockIpv6, adblockRulesBlockingMode, hostsRulesBlockingMode,
customBlockingIpv4, customBlockingIpv6,
dnsCacheSize, optimisticCache, enableDNSSECOK, enableRetransmissionHandling, blockEch,
enableParallelUpstreamQueries, enableFallbackOnUpstreamsFailure, enableServfailOnUpstreamsFailure);
enableParallelUpstreamQueries, enableFallbackOnUpstreamsFailure, enableServfailOnUpstreamsFailure,
upstreamTimeoutMs);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
public class UpstreamSettings {
private String address;
private List<String> bootstrap = new ArrayList<>();
private long timeoutMs;
private byte[] serverIp;
private int id;
private String outboundInterfaceName;
Expand All @@ -20,16 +19,12 @@ public UpstreamSettings() {}
* Creates UpstreamSettings
* @param address The DNS server's address.
* @param bootstrap List of plain DNS servers to be used to resolve DOH/DOT hostnames (if any).
* @param timeoutMs Default upstream timeout in milliseconds. Also, it is used as a timeout for bootstrap DNS requests.
* {@code timeout = 0} means infinite timeout.
* @param serverIp Resolver's IP address. In the case if it's specified, bootstrap DNS servers won't be used at all.
* @param id User-provided ID
*/
public UpstreamSettings(String address, List<String> bootstrap,
long timeoutMs, byte[] serverIp, int id) {
public UpstreamSettings(String address, List<String> bootstrap, byte[] serverIp, int id) {
setAddress(address);
setBootstrap(bootstrap);
setTimeoutMs(timeoutMs);
setServerIp(serverIp);
setId(id);
}
Expand Down Expand Up @@ -66,22 +61,6 @@ public void setBootstrap(List<String> bootstrap) {
this.bootstrap = new ArrayList<>(bootstrap);
}

/**
* @return Default upstream timeout in milliseconds. Also, it is used as a timeout for bootstrap DNS requests.
* {@code timeout = 0} means infinite timeout.
*/
public long getTimeoutMs() {
return timeoutMs;
}

/**
* @param timeoutMs Default upstream timeout in milliseconds. Also, it is used as a timeout for bootstrap DNS requests.
* {@code timeout = 0} means infinite timeout.
*/
public void setTimeoutMs(long timeoutMs) {
this.timeoutMs = timeoutMs;
}

/**
* @return Resolver's IP address. In the case if it's specified, bootstrap DNS servers won't be used at all.
*/
Expand Down Expand Up @@ -138,8 +117,7 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
UpstreamSettings that = (UpstreamSettings) o;
return timeoutMs == that.timeoutMs &&
id == that.id &&
return id == that.id &&
Objects.equals(address, that.address) &&
bootstrap.equals(that.bootstrap) &&
Arrays.equals(serverIp, that.serverIp) &&
Expand All @@ -149,6 +127,6 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
return Objects.hash(timeoutMs, id, address, bootstrap, serverIp, outboundInterfaceName, fingerprints);
return Objects.hash(id, address, bootstrap, Arrays.hashCode(serverIp), outboundInterfaceName, fingerprints);
}
}
Loading

0 comments on commit be468e4

Please sign in to comment.