Merge pull request #2555

a4b4651c dns_utils: allow an optional DNS server IP in DNS_PUBLIC (moneromooo-monero)

4 files changed, 56 insertions, 7 deletions

diff --git a/README.md b/README.md
index a907bad0a..db629394d 100644
--- a/README.md
+++ b/README.md
@@ -414,7 +414,7 @@ See [README.i18n.md](README.i18n.md).
 
 ## Using Tor
 
-While Monero isn't made to integrate with Tor, it can be used wrapped with torsocks, if you add --p2p-bind-ip 127.0.0.1 to the monerod command line. You also want to set DNS requests to go over TCP, so they'll be routed through Tor, by setting DNS_PUBLIC=tcp. You may also disable IGD (UPnP port forwarding negotiation), which is pointless with Tor. To allow local connections from the wallet, you might have to add TORSOCKS_ALLOW_INBOUND=1, some OSes need it and some don't. Example:
+While Monero isn't made to integrate with Tor, it can be used wrapped with torsocks, if you add --p2p-bind-ip 127.0.0.1 to the monerod command line. You also want to set DNS requests to go over TCP, so they'll be routed through Tor, by setting DNS_PUBLIC=tcp or use a particular DNS server with DNS_PUBLIC=tcp://a.b.c.d (default is 8.8.4.4, which is Google DNS). You may also disable IGD (UPnP port forwarding negotiation), which is pointless with Tor. To allow local connections from the wallet, you might have to add TORSOCKS_ALLOW_INBOUND=1, some OSes need it and some don't. Example:
 
 `DNS_PUBLIC=tcp torsocks monerod --p2p-bind-ip 127.0.0.1 --no-igd`
 
diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp
index 1310b8bfd..f549218cb 100644
--- a/src/common/dns_utils.cpp
+++ b/src/common/dns_utils.cpp
@@ -40,6 +40,8 @@ namespace bf = boost::filesystem;
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "net.dns"
 
+#define DEFAULT_DNS_PUBLIC_ADDR "8.8.4.4"
+
 static boost::mutex instance_lock;
 
 namespace
@@ -197,16 +199,19 @@ public:
 DNSResolver::DNSResolver() : m_data(new DNSResolverData())
 {
   int use_dns_public = 0;
-  const char* dns_public_addr = "8.8.4.4";
+  std::string dns_public_addr = DEFAULT_DNS_PUBLIC_ADDR;
   if (auto res = getenv("DNS_PUBLIC"))
   {
-    std::string dns_public(res);
-    // TODO: could allow parsing of IP and protocol: e.g. DNS_PUBLIC=tcp:8.8.8.8
-    if (dns_public == "tcp")
+    dns_public_addr = tools::dns_utils::parse_dns_public(res);
+    if (!dns_public_addr.empty())
     {
-      LOG_PRINT_L0("Using public DNS server: " << dns_public_addr << " (TCP)");
+      MGINFO("Using public DNS server: " << dns_public_addr << " (TCP)");
       use_dns_public = 1;
     }
+    else
+    {
+      MERROR("Failed to parse DNS_PUBLIC");
+    }
   }
 
   // init libunbound context
@@ -214,7 +219,7 @@ DNSResolver::DNSResolver() : m_data(new DNSResolverData())
 
   if (use_dns_public)
   {
-    ub_ctx_set_fwd(m_data->m_ub_context, string_copy(dns_public_addr));
+    ub_ctx_set_fwd(m_data->m_ub_context, dns_public_addr.c_str());
     ub_ctx_set_option(m_data->m_ub_context, string_copy("do-udp:"), string_copy("no"));
     ub_ctx_set_option(m_data->m_ub_context, string_copy("do-tcp:"), string_copy("yes"));
   }
@@ -519,6 +524,34 @@ bool load_txt_records_from_dns(std::vector<std::string> &good_records, const std
   return true;
 }
 
+std::string parse_dns_public(const char *s)
+{
+  unsigned ip0, ip1, ip2, ip3;
+  char c;
+  std::string dns_public_addr;
+  if (!strcmp(s, "tcp"))
+  {
+    dns_public_addr = DEFAULT_DNS_PUBLIC_ADDR;
+    LOG_PRINT_L0("Using default public DNS server: " << dns_public_addr << " (TCP)");
+  }
+  else if (sscanf(s, "tcp://%u.%u.%u.%u%c", &ip0, &ip1, &ip2, &ip3, &c) == 4)
+  {
+    if (ip0 > 255 || ip1 > 255 || ip2 > 255 || ip3 > 255)
+    {
+      MERROR("Invalid IP: " << s << ", using default");
+    }
+    else
+    {
+      dns_public_addr = std::string(s + strlen("tcp://"));
+    }
+  }
+  else
+  {
+    MERROR("Invalid DNS_PUBLIC contents, ignored");
+  }
+  return dns_public_addr;
+}
+
 }  // namespace tools::dns_utils
 
 }  // namespace tools
diff --git a/src/common/dns_utils.h b/src/common/dns_utils.h
index f19584516..c0a2dbf2b 100644
--- a/src/common/dns_utils.h
+++ b/src/common/dns_utils.h
@@ -167,6 +167,8 @@ std::string get_account_address_as_str_from_url(const std::string& url, bool& dn
 
 bool load_txt_records_from_dns(std::vector<std::string> &records, const std::vector<std::string> &dns_urls);
 
+std::string parse_dns_public(const char *s);
+
 }  // namespace tools::dns_utils
 
 }  // namespace tools
diff --git a/tests/unit_tests/dns_resolver.cpp b/tests/unit_tests/dns_resolver.cpp
index aca74a93f..8ac86146e 100644
--- a/tests/unit_tests/dns_resolver.cpp
+++ b/tests/unit_tests/dns_resolver.cpp
@@ -157,3 +157,17 @@ TEST(DNSResolver, GetTXTRecord)
   addr = tools::DNSResolver::instance().get_dns_format_from_oa_address("donate.getmonero.org");
   EXPECT_STREQ("donate.getmonero.org", addr.c_str());
 }
+
+TEST(DNS_PUBLIC, empty) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("").c_str()); }
+TEST(DNS_PUBLIC, default) { EXPECT_STREQ("8.8.4.4", tools::dns_utils::parse_dns_public("tcp").c_str()); }
+TEST(DNS_PUBLIC, invalid_scheme) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("invalid").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_alpha) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://invalid").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_num1) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_num3) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4.5").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_num4_extra) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4.5.6x").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_num4_range) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4.542.6").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_dot) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4.5.6.").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_num5) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4.5.6.7").c_str()); }
+TEST(DNS_PUBLIC, invalid_ip_4_missing) { EXPECT_STREQ("", tools::dns_utils::parse_dns_public("tcp://3.4..7").c_str()); }
+TEST(DNS_PUBLIC, valid_ip_lo) { EXPECT_STREQ("127.0.0.1", tools::dns_utils::parse_dns_public("tcp://127.0.0.1").c_str()); }
+TEST(DNS_PUBLIC, valid_ip) { EXPECT_STREQ("3.4.5.6", tools::dns_utils::parse_dns_public("tcp://3.4.5.6").c_str()); }