From 2b5b3ce8e161ead38598eb42c6bc702ff2c286b4 Mon Sep 17 00:00:00 2001 From: Jeff Date: Fri, 2 Oct 2020 18:01:26 -0400 Subject: [PATCH 1/7] default upstream dns to cloudflare --- llarp/config/config.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index af280bf45..ab25556cb 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -361,8 +361,11 @@ namespace llarp addr.setPort(53); return addr; }; + + constexpr auto DefaultUpstreamDNS = "1.1.1.1:53"; + conf.defineOption( - "dns", "upstream", false, true, std::nullopt, [=](std::string arg) { + "dns", "upstream", false, true, DefaultUpstreamDNS, [=](std::string arg) { m_upstreamDNS.push_back(parseDNSAddr(std::move(arg))); }); From 9d6dc40f818baa4aec052a403bbb0d86538424c4 Mon Sep 17 00:00:00 2001 From: Jeff Date: Fri, 2 Oct 2020 18:02:03 -0400 Subject: [PATCH 2/7] format --- llarp/config/config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index ab25556cb..3d9449ebc 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -363,7 +363,7 @@ namespace llarp }; constexpr auto DefaultUpstreamDNS = "1.1.1.1:53"; - + conf.defineOption( "dns", "upstream", false, true, DefaultUpstreamDNS, [=](std::string arg) { m_upstreamDNS.push_back(parseDNSAddr(std::move(arg))); From c97b9ef31b4e98ca0082a59e54aaa1ac7bf9117b Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 2 Oct 2020 19:38:58 -0300 Subject: [PATCH 3/7] Simplify k=v parsing code --- llarp/config/ini.cpp | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/llarp/config/ini.cpp b/llarp/config/ini.cpp index c62329abd..c81e50449 100644 --- a/llarp/config/ini.cpp +++ b/llarp/config/ini.cpp @@ -108,23 +108,18 @@ namespace llarp else if (kvDelim != std::string_view::npos) { // key value pair - std::string_view::size_type k_start = 0; - std::string_view::size_type k_end = kvDelim; - std::string_view::size_type v_start = kvDelim + 1; - std::string_view::size_type v_end = realLine.size() - 1; + std::string_view k = realLine.substr(0, kvDelim); + std::string_view v = realLine.substr(kvDelim + 1); + // clamp whitespaces - while (whitespace(realLine[k_start]) && k_start != kvDelim) - ++k_start; - while (whitespace(realLine[k_end - 1]) && k_end != k_start) - --k_end; - while (whitespace(realLine[v_start]) && v_start != v_end) - ++v_start; - while (whitespace(realLine[v_end])) - --v_end; - - // sect.k = v - std::string_view k = realLine.substr(k_start, k_end - k_start); - std::string_view v = realLine.substr(v_start, 1 + (v_end - v_start)); + for (auto* x : {&k, &v}) + { + while (!x->empty() && whitespace(x->front())) + x->remove_prefix(1); + while (!x->empty() && whitespace(x->back())) + x->remove_suffix(1); + } + if (k.size() == 0 || v.size() == 0) { LogError(m_FileName, " invalid line (", lineno, "): '", line, "'"); From d129b0432ae4e8a25c34b8114433e59fd5d00d74 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 2 Oct 2020 19:39:43 -0300 Subject: [PATCH 4/7] Allow empty values so that upstream= can disable the default --- llarp/config/config.cpp | 3 ++- llarp/config/ini.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index 3d9449ebc..f3f1a0833 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -366,7 +366,8 @@ namespace llarp conf.defineOption( "dns", "upstream", false, true, DefaultUpstreamDNS, [=](std::string arg) { - m_upstreamDNS.push_back(parseDNSAddr(std::move(arg))); + if (!arg.empty()) + m_upstreamDNS.push_back(parseDNSAddr(std::move(arg))); }); conf.defineOption("dns", "bind", false, "127.3.2.1:53", [=](std::string arg) { diff --git a/llarp/config/ini.cpp b/llarp/config/ini.cpp index c81e50449..4602394ec 100644 --- a/llarp/config/ini.cpp +++ b/llarp/config/ini.cpp @@ -120,7 +120,7 @@ namespace llarp x->remove_suffix(1); } - if (k.size() == 0 || v.size() == 0) + if (k.size() == 0) { LogError(m_FileName, " invalid line (", lineno, "): '", line, "'"); return false; From 01013c1963e0249d252bd1d960ff3101f557760a Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 2 Oct 2020 20:17:12 -0300 Subject: [PATCH 5/7] Make upstream= override work; reject non-default upstream port If you specify upstream= then you get no upstream, if you give one then you use that instead of the default, but you can still list multiple. unbound doesn't support an upstream port, so bail if the user gives a non-port-53 response. --- llarp/config/config.cpp | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index f3f1a0833..46a1c5942 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -355,23 +355,32 @@ namespace llarp DnsConfig::defineConfigOptions(ConfigDefinition& conf, const ConfigGenParameters& params) { (void)params; - auto parseDNSAddr = [](auto arg) { - IpAddress addr{arg}; - if (not addr.getPort()) - addr.setPort(53); - return addr; - }; - constexpr auto DefaultUpstreamDNS = "1.1.1.1:53"; + // Default, but if we get any upstream (including upstream=, i.e. empty string) we clear it + constexpr auto DefaultUpstreamDNS = "1.1.1.1"; + m_upstreamDNS.emplace_back(DefaultUpstreamDNS); conf.defineOption( - "dns", "upstream", false, true, DefaultUpstreamDNS, [=](std::string arg) { + "dns", "upstream", false, true, DefaultUpstreamDNS, [=, first=true](std::string arg) mutable { + if (first) + { + m_upstreamDNS.clear(); + first = false; + } if (!arg.empty()) - m_upstreamDNS.push_back(parseDNSAddr(std::move(arg))); + { + auto& addr = m_upstreamDNS.emplace_back(std::move(arg)); + if (auto p = addr.getPort(); p && *p != 53) + // unbound doesn't support specifying a port, so bail if the user gave a non-default port + throw std::invalid_argument(stringify("Invalid [dns] upstream setting: non-default DNS ports are not supported")); + addr.setPort(std::nullopt); + } }); conf.defineOption("dns", "bind", false, "127.3.2.1:53", [=](std::string arg) { - m_bind = parseDNSAddr(std::move(arg)); + m_bind = IpAddress{std::move(arg)}; + if (!m_bind.getPort()) + m_bind.setPort(53); }); // Ignored option (used by the systemd service file to disable resolvconf configuration). From 4faaf9082c46929706f11c159acc54a6db620f32 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 2 Oct 2020 20:19:53 -0300 Subject: [PATCH 6/7] Fix broken unbound resolver when including upstream port unbound breaks when given "1.2.3.4:53" as it expects only an IP. --- llarp/dns/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index b2c7ca58d..848e93ed5 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -124,9 +124,9 @@ namespace llarp } for (const auto& resolver : resolvers) { - if (not m_UnboundResolver->AddUpstreamResolver(resolver.toString())) + if (not m_UnboundResolver->AddUpstreamResolver(resolver.toHost())) { - llarp::LogError("Failed to add upstream DNS server: ", resolver.toString()); + llarp::LogError("Failed to add upstream DNS server: ", resolver.toHost()); m_UnboundResolver = nullptr; return false; } From 7aa4566016fc764d95bfaa08c619e1833fb2e6ad Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 2 Oct 2020 20:57:51 -0300 Subject: [PATCH 7/7] Make format --- llarp/config/config.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/llarp/config/config.cpp b/llarp/config/config.cpp index 46a1c5942..6f5af4807 100644 --- a/llarp/config/config.cpp +++ b/llarp/config/config.cpp @@ -361,7 +361,12 @@ namespace llarp m_upstreamDNS.emplace_back(DefaultUpstreamDNS); conf.defineOption( - "dns", "upstream", false, true, DefaultUpstreamDNS, [=, first=true](std::string arg) mutable { + "dns", + "upstream", + false, + true, + DefaultUpstreamDNS, + [=, first = true](std::string arg) mutable { if (first) { m_upstreamDNS.clear(); @@ -371,8 +376,9 @@ namespace llarp { auto& addr = m_upstreamDNS.emplace_back(std::move(arg)); if (auto p = addr.getPort(); p && *p != 53) - // unbound doesn't support specifying a port, so bail if the user gave a non-default port - throw std::invalid_argument(stringify("Invalid [dns] upstream setting: non-default DNS ports are not supported")); + // unbound doesn't support non-default ports so bail if the user gave one + throw std::invalid_argument( + "Invalid [dns] upstream setting: non-default DNS ports are not supported"); addr.setPort(std::nullopt); } });