URL: https://github.com/SSSD/sssd/pull/5245 Author: thalman Title: #5245: RESOLV: Avoid DNS search to improve fail-over reaction Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5245/head:pr5245 git checkout pr5245
From 50d0bde2d594f37d22d48d78ab6eb92bfa88b1d5 Mon Sep 17 00:00:00 2001 From: Tomas Halman <thal...@redhat.com> Date: Tue, 14 Jul 2020 17:34:36 +0200 Subject: [PATCH 1/4] RESOLV: Avoid DNS search to improve fail-over reaction In case of unreachable DNS server or invalid hostname sssd/c-ares tries to search in multiple domains based on the search directive in resolv.conf But the hostnames in config file are fully qualified and this just extends the time spent with DNS resolution. This patch set the c-ares library flags to avoid DNS search Resolves: https://github.com/SSSD/sssd/issues/5390 --- src/config/SSSDConfig/sssdoptions.py | 1 + src/config/cfg_rules.ini | 1 + src/config/etc/sssd.api.conf | 2 +- src/providers/data_provider.h | 1 + src/providers/data_provider_fo.c | 3 +++ src/resolv/async_resolv.c | 17 +++++++++++++++-- src/resolv/async_resolv.h | 3 ++- 7 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/config/SSSDConfig/sssdoptions.py b/src/config/SSSDConfig/sssdoptions.py index f57ad4b41a..23bc457b81 100644 --- a/src/config/SSSDConfig/sssdoptions.py +++ b/src/config/SSSDConfig/sssdoptions.py @@ -191,6 +191,7 @@ def __init__(self): 'miliseconds)'), 'dns_resolver_op_timeout': _('How long should keep trying to resolve single DNS query (seconds)'), 'dns_resolver_timeout': _('How long to wait for replies from DNS when resolving servers (seconds)'), + 'dns_resolver_perform_dns_search': _('Should resolver perform DNS search'), 'dns_discovery_domain': _('The domain part of service discovery DNS query'), 'override_gid': _('Override GID value from the identity provider with this value'), 'case_sensitive': _('Treat usernames as case sensitive'), diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 2874ea048b..249cf4d9af 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -382,6 +382,7 @@ option = filter_groups option = dns_resolver_server_timeout option = dns_resolver_op_timeout option = dns_resolver_timeout +option = dns_resolver_perform_dns_search option = dns_discovery_domain option = override_gid option = case_sensitive diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 035c33cad8..1adb0e1409 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -179,6 +179,7 @@ filter_groups = list, str, false dns_resolver_server_timeout = int, None, false dns_resolver_op_timeout = int, None, false dns_resolver_timeout = int, None, false +dns_resolver_perform_dns_search = bool, None, true dns_discovery_domain = str, None, false override_gid = int, None, false case_sensitive = str, None, false @@ -226,4 +227,3 @@ dyndns_server = str, None, false [provider/deny] [provider/deny/access] - diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index 32215a0fa5..31eed0cafa 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -267,6 +267,7 @@ enum dp_res_opts { DP_RES_OPT_RESOLVER_TIMEOUT, DP_RES_OPT_RESOLVER_OP_TIMEOUT, DP_RES_OPT_RESOLVER_SERVER_TIMEOUT, + DP_RES_OPT_RESOLVER_PERFORM_DNS_SEARCH, DP_RES_OPT_DNS_DOMAIN, DP_RES_OPTS /* attrs counter */ diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c index afc6081afa..58a3681ad4 100644 --- a/src/providers/data_provider_fo.c +++ b/src/providers/data_provider_fo.c @@ -836,6 +836,7 @@ static struct dp_option dp_res_default_opts[] = { { "dns_resolver_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "dns_resolver_op_timeout", DP_OPT_NUMBER, { .number = 3 }, NULL_NUMBER }, { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER }, + { "dns_resolver_perform_dns_search", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, { "dns_discovery_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING }, DP_OPTION_TERMINATOR }; @@ -899,6 +900,8 @@ errno_t be_res_init(struct be_ctx *ctx) DP_RES_OPT_RESOLVER_OP_TIMEOUT), dp_opt_get_int(ctx->be_res->opts, DP_RES_OPT_RESOLVER_SERVER_TIMEOUT), + dp_opt_get_bool(ctx->be_res->opts, + DP_RES_OPT_RESOLVER_PERFORM_DNS_SEARCH), &ctx->be_res->resolv); if (ret != EOK) { talloc_zfree(ctx->be_res); diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c index 00b9531d49..ce61670ef7 100644 --- a/src/resolv/async_resolv.c +++ b/src/resolv/async_resolv.c @@ -81,6 +81,9 @@ struct resolv_ctx { /* Time in milliseconds before canceling a DNS request */ int timeout; + /* use search list from resolv.conf when translating name to ip address */ + bool domain_search; + /* Time in milliseconds for communication with single DNS server. */ int ares_timeout; @@ -428,11 +431,19 @@ recreate_ares_channel(struct resolv_ctx *ctx) /* Only affects ares_gethostbyname */ options.lookups = discard_const("f"); options.tries = 1; + + /* domain search */ + options.flags = 0; + if (!ctx->domain_search) { + options.flags |= ARES_FLAG_NOSEARCH; + } + ret = ares_init_options(&new_channel, &options, ARES_OPT_SOCK_STATE_CB | ARES_OPT_TIMEOUTMS | ARES_OPT_LOOKUPS | - ARES_OPT_TRIES); + ARES_OPT_TRIES | + ARES_OPT_FLAGS); if (ret != ARES_SUCCESS) { DEBUG(SSSDBG_CRIT_FAILURE, "Failed to initialize ares channel: %s\n", resolv_strerror(ret)); @@ -451,7 +462,8 @@ recreate_ares_channel(struct resolv_ctx *ctx) int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, int ares_timeout, struct resolv_ctx **ctxp) + int timeout, int ares_timeout, bool domain_search, + struct resolv_ctx **ctxp) { int ret; struct resolv_ctx *ctx; @@ -469,6 +481,7 @@ resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, ctx->ev_ctx = ev_ctx; ctx->timeout = timeout; ctx->ares_timeout = ares_timeout; + ctx->domain_search = domain_search; ret = recreate_ares_channel(ctx); if (ret != EOK) { diff --git a/src/resolv/async_resolv.h b/src/resolv/async_resolv.h index d83a7be447..2b1e9e2b03 100644 --- a/src/resolv/async_resolv.h +++ b/src/resolv/async_resolv.h @@ -52,7 +52,8 @@ struct resolv_ctx; int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, int ares_timeout, struct resolv_ctx **ctxp); + int timeout, int ares_timeout, bool domain_search, + struct resolv_ctx **ctxp); void resolv_reread_configuration(struct resolv_ctx *ctx); From 7425c3bc884baded5fd72b59e913fbc2255b8f06 Mon Sep 17 00:00:00 2001 From: Tomas Halman <thal...@redhat.com> Date: Mon, 12 Oct 2020 11:38:31 +0200 Subject: [PATCH 2/4] TEST: Update resolver tests Changing resolv_init call requires tests to be updated --- src/tests/cmocka/test_fo_srv.c | 5 +++-- src/tests/cmocka/test_resolv_fake.c | 2 +- src/tests/fail_over-tests.c | 2 +- src/tests/resolv-tests.c | 3 +-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c index c13cf3a69b..490ee34233 100644 --- a/src/tests/cmocka/test_fo_srv.c +++ b/src/tests/cmocka/test_fo_srv.c @@ -49,7 +49,8 @@ struct resolv_ctx { /* mock resolver interface. The resolver test is separate */ int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, int ares_timeout, struct resolv_ctx **ctxp) + int timeout, int ares_timeout, bool domain_search, + struct resolv_ctx **ctxp) { *ctxp = talloc(mem_ctx, struct resolv_ctx); return EOK; @@ -230,7 +231,7 @@ static int test_fo_setup(void **state) assert_non_null(test_ctx->ctx); ret = resolv_init(test_ctx, test_ctx->ctx->ev, - TEST_RESOLV_TIMEOUT, 2000, &test_ctx->resolv); + TEST_RESOLV_TIMEOUT, 2000, false, &test_ctx->resolv); assert_non_null(test_ctx->resolv); memset(&fopts, 0, sizeof(fopts)); diff --git a/src/tests/cmocka/test_resolv_fake.c b/src/tests/cmocka/test_resolv_fake.c index 0f4011a39f..88b52f52cd 100644 --- a/src/tests/cmocka/test_resolv_fake.c +++ b/src/tests/cmocka/test_resolv_fake.c @@ -240,7 +240,7 @@ static int test_resolv_fake_setup(void **state) assert_non_null(test_ctx->ctx); ret = resolv_init(test_ctx, test_ctx->ctx->ev, - TEST_DEFAULT_TIMEOUT, 2000, &test_ctx->resolv); + TEST_DEFAULT_TIMEOUT, 2000, false, &test_ctx->resolv); assert_int_equal(ret, EOK); *state = test_ctx; diff --git a/src/tests/fail_over-tests.c b/src/tests/fail_over-tests.c index b2269ef3b2..a95b8e5b9f 100644 --- a/src/tests/fail_over-tests.c +++ b/src/tests/fail_over-tests.c @@ -73,7 +73,7 @@ setup_test(void) fail("Could not init tevent context"); } - ret = resolv_init(ctx, ctx->ev, 5, 2000, &ctx->resolv); + ret = resolv_init(ctx, ctx->ev, 5, 2000, false, &ctx->resolv); if (ret != EOK) { talloc_free(ctx); fail("Could not init resolv context"); diff --git a/src/tests/resolv-tests.c b/src/tests/resolv-tests.c index bc4cd7cc18..7f906ff718 100644 --- a/src/tests/resolv-tests.c +++ b/src/tests/resolv-tests.c @@ -76,7 +76,7 @@ static int setup_resolv_test(int timeout, struct resolv_test_ctx **ctx) return EFAULT; } - ret = resolv_init(test_ctx, test_ctx->ev, timeout, 2000, &test_ctx->resolv); + ret = resolv_init(test_ctx, test_ctx->ev, timeout, 2000, false, &test_ctx->resolv); if (ret != EOK) { fail("Could not init resolv context"); talloc_free(test_ctx); @@ -1048,4 +1048,3 @@ int main(int argc, const char *argv[]) srunner_free(sr); return (failure_count==0 ? EXIT_SUCCESS : EXIT_FAILURE); } - From a63954f91dba3f02fbf8f7d1ca9fd37f78b516d5 Mon Sep 17 00:00:00 2001 From: Tomas Halman <thal...@redhat.com> Date: Mon, 12 Oct 2020 14:35:45 +0200 Subject: [PATCH 3/4] MAN: New resolver option man page --- src/man/include/failover.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index c9d2d3799c..6de503b29c 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -117,6 +117,23 @@ </para> </listitem> </varlistentry> + <varlistentry> + <term> + dns_resolver_perform_dns_search + </term> + <listitem> + <para> + The dns_resolver_perform_dns_search specifies whether + the DNS resolver uses DNS search during the name + resolution. Setting this option to True can slow + down the resolution in case that some DNS names + are not resolvable. + </para> + <para> + Default: True + </para> + </listitem> + </varlistentry> </variablelist> </para> <para> From 76dbc07af23d950cf1599fcaa8ec73672dce7a14 Mon Sep 17 00:00:00 2001 From: Tomas Halman <thal...@redhat.com> Date: Mon, 12 Oct 2020 16:04:15 +0200 Subject: [PATCH 4/4] RESOLV: Avoid DNS search by default This patch changes the default behaviour so DNS search is not performed by default. --- src/man/include/failover.xml | 2 +- src/providers/data_provider_fo.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index 6de503b29c..416a65ff4f 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -130,7 +130,7 @@ are not resolvable. </para> <para> - Default: True + Default: False </para> </listitem> </varlistentry> diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c index 58a3681ad4..a77649ba62 100644 --- a/src/providers/data_provider_fo.c +++ b/src/providers/data_provider_fo.c @@ -836,7 +836,7 @@ static struct dp_option dp_res_default_opts[] = { { "dns_resolver_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "dns_resolver_op_timeout", DP_OPT_NUMBER, { .number = 3 }, NULL_NUMBER }, { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER }, - { "dns_resolver_perform_dns_search", DP_OPT_BOOL, BOOL_TRUE, BOOL_TRUE }, + { "dns_resolver_perform_dns_search", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE }, { "dns_discovery_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING }, DP_OPTION_TERMINATOR };
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
