Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#21900: Fallback to local DNS when no other nameservers are known #273

Closed
wants to merge 12 commits into from
Closed
67 changes: 44 additions & 23 deletions src/feature/relay/dns.c
Expand Up @@ -1385,42 +1385,63 @@ configure_nameservers(int force)
evdns_set_log_fn(evdns_log_cb);
if (conf_fname) {
log_debug(LD_FS, "stat()ing %s", conf_fname);
if (stat(sandbox_intern_string(conf_fname), &st)) {
log_warn(LD_EXIT, "Unable to stat resolver configuration in '%s': %s",
int missing_resolv_conf = 0;
int stat_res = stat(sandbox_intern_string(conf_fname), &st);

if (stat_res) {
log_warn(LD_EXIT, "unable to stat resolver configuration in '%s': %s",
conf_fname, strerror(errno));
goto err;
}
if (!force && resolv_conf_fname && !strcmp(conf_fname,resolv_conf_fname)
missing_resolv_conf = 1;
} else if (!force && resolv_conf_fname &&
!strcmp(conf_fname,resolv_conf_fname)
&& st.st_mtime == resolv_conf_mtime) {
log_info(LD_EXIT, "No change to '%s'", conf_fname);
log_info(LD_EXIT, "no change to '%s'", conf_fname);
return 0;
}

if (st.st_size == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the stat() call failed above, then at this point, st.st_size will be uninitialized, and we're not allowed to look at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking if stat() succeeded in 770aee4.

missing_resolv_conf = 1;

if (nameservers_configured) {
evdns_base_search_clear(the_evdns_base);
evdns_base_clear_nameservers_and_suspend(the_evdns_base);
}
#if defined(DNS_OPTION_HOSTSFILE) && defined(USE_LIBSECCOMP)
if (flags & DNS_OPTION_HOSTSFILE) {
flags ^= DNS_OPTION_HOSTSFILE;
log_debug(LD_FS, "Loading /etc/hosts");
log_debug(LD_FS, "loading /etc/hosts");
evdns_base_load_hosts(the_evdns_base,
sandbox_intern_string("/etc/hosts"));
}
#endif /* defined(DNS_OPTION_HOSTSFILE) && defined(USE_LIBSECCOMP) */
log_info(LD_EXIT, "Parsing resolver configuration in '%s'", conf_fname);
if ((r = evdns_base_resolv_conf_parse(the_evdns_base, flags,
sandbox_intern_string(conf_fname)))) {
log_warn(LD_EXIT, "Unable to parse '%s', or no nameservers in '%s' (%d)",
conf_fname, conf_fname, r);
goto err;
}
if (evdns_base_count_nameservers(the_evdns_base) == 0) {
log_warn(LD_EXIT, "Unable to find any nameservers in '%s'.", conf_fname);
goto err;
#endif /* defined(dns_option_hostsfile) && defined(use_libseccomp) */

if (!missing_resolv_conf) {
log_info(LD_EXIT, "parsing resolver configuration in '%s'", conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this log message, and the messages after it, are still lower-cased in the latest version of the branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the last changes to lowercase in 917eeed.

if ((r = evdns_base_resolv_conf_parse(the_evdns_base, flags,
sandbox_intern_string(conf_fname)))) {
log_warn(LD_EXIT, "unable to parse '%s', or no nameservers "
"in '%s' (%d)", conf_fname, conf_fname, r);

if (r != 6) // "r = 6" means "no DNS servers were in resolv.conf" -
goto err; // in which case we expect libevent to add 127.0.0.1 as
// fallback.
}
if (evdns_base_count_nameservers(the_evdns_base) == 0) {
log_warn(LD_EXIT, "unable to find any nameservers in '%s'.",
conf_fname);
}
resolv_conf_fname = tor_strdup(conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If resolv_conf_fname was already set, this will leak memory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolv_conf_mtime = st.st_mtime;
} else {
log_warn(LD_EXIT, "Could not read your resolv.conf - "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message should say what actual filename it tried to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging filename in a1656c9.

"please investigate your DNS configuration. "
"This is possibly a problem. Meanwhile, falling"
" back to local DNS at 127.0.0.1.");
evdns_base_nameserver_ip_add(the_evdns_base, "127.0.0.1");
}

tor_free(resolv_conf_fname);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tor_free() here will clear the value of rsolv_conf_fname that you set above...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed - 4c1913a.

resolv_conf_fname = tor_strdup(conf_fname);
resolv_conf_mtime = st.st_mtime;

if (nameservers_configured)
evdns_base_resume(the_evdns_base);
}
Expand All @@ -1431,12 +1452,12 @@ configure_nameservers(int force)
evdns_base_clear_nameservers_and_suspend(the_evdns_base);
}
if (evdns_base_config_windows_nameservers(the_evdns_base)) {
log_warn(LD_EXIT,"Could not config nameservers.");
log_warn(LD_EXIT,"could not config nameservers.");
goto err;
}
if (evdns_base_count_nameservers(the_evdns_base) == 0) {
log_warn(LD_EXIT, "Unable to find any platform nameservers in "
"your Windows configuration.");
log_warn(LD_EXIT, "unable to find any platform nameservers in "
"your windows configuration.");
goto err;
}
if (nameservers_configured)
Expand Down