From 5990344cb07858b4cd104eacb8d1b9bb38fa6ff7 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Thu, 27 Aug 2015 21:06:09 +0100 Subject: [PATCH 1/7] dns: make ctor private This ensures one can't instanciate a DNSResolver object by mistake, but uses the singleton. A separate create static function is added for cases where a new object is explicitely needed. --- src/common/dns_utils.cpp | 5 +++++ src/common/dns_utils.h | 11 ++++++++++- tests/unit_tests/dns_resolver.cpp | 12 ++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index e442d3d81..8b7c9b4d9 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -334,6 +334,11 @@ DNSResolver& DNSResolver::instance() return *staticInstance; } +DNSResolver DNSResolver::create() +{ + return DNSResolver(); +} + bool DNSResolver::check_address_syntax(const char *addr) { // if string doesn't contain a dot, we won't consider it a url for now. diff --git a/src/common/dns_utils.h b/src/common/dns_utils.h index 1e726c80c..50e8001f1 100644 --- a/src/common/dns_utils.h +++ b/src/common/dns_utils.h @@ -49,7 +49,7 @@ struct DNSResolverData; */ class DNSResolver { -public: +private: /** * @brief Constructs an instance of DNSResolver @@ -58,6 +58,8 @@ public: */ DNSResolver(); +public: + /** * @brief takes care of freeing C pointers and such */ @@ -119,6 +121,13 @@ public: */ static DNSResolver& instance(); + /** + * @brief Gets a new instance of DNSResolver + * + * @return returns a pointer to the new object + */ + static DNSResolver create(); + private: /** diff --git a/tests/unit_tests/dns_resolver.cpp b/tests/unit_tests/dns_resolver.cpp index 0709a773f..3285f7732 100644 --- a/tests/unit_tests/dns_resolver.cpp +++ b/tests/unit_tests/dns_resolver.cpp @@ -35,7 +35,7 @@ TEST(DNSResolver, IPv4Success) { - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; @@ -55,7 +55,7 @@ TEST(DNSResolver, IPv4Success) TEST(DNSResolver, IPv4Failure) { // guaranteed by IANA/ICANN/RFC to be invalid - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; @@ -70,7 +70,7 @@ TEST(DNSResolver, IPv4Failure) TEST(DNSResolver, DNSSECSuccess) { - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; @@ -86,7 +86,7 @@ TEST(DNSResolver, DNSSECSuccess) TEST(DNSResolver, DNSSECFailure) { - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; @@ -103,7 +103,7 @@ TEST(DNSResolver, DNSSECFailure) // It would be great to include an IPv6 test and assume it'll pass, but not every ISP / resolver plays nicely with IPv6;) /*TEST(DNSResolver, IPv6Success) { - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; @@ -123,7 +123,7 @@ TEST(DNSResolver, DNSSECFailure) TEST(DNSResolver, IPv6Failure) { // guaranteed by IANA/ICANN/RFC to be invalid - tools::DNSResolver resolver; + tools::DNSResolver resolver = tools::DNSResolver::create(); bool avail, valid; From f43d465da20864e9fc25fc3dd3f904c1a35bd8f9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Thu, 27 Aug 2015 21:08:03 +0100 Subject: [PATCH 2/7] dns_utils: lock access to the singleton This avoids races which could result in two objects being created --- src/common/dns_utils.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index 8b7c9b4d9..ce57151f5 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -38,6 +38,8 @@ using namespace epee; namespace bf = boost::filesystem; +static std::mutex instance_lock; + namespace { @@ -326,6 +328,8 @@ std::string DNSResolver::get_dns_format_from_oa_address(const std::string& oa_ad DNSResolver& DNSResolver::instance() { + std::lock_guard lock(instance_lock); + static DNSResolver* staticInstance = NULL; if (staticInstance == NULL) { From ae5f28cb51f51186998cf3c6b2dc1d56abe0174e Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Thu, 27 Aug 2015 21:08:40 +0100 Subject: [PATCH 3/7] dns_utils: add a const where possible --- src/common/dns_utils.cpp | 2 +- src/common/dns_utils.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index ce57151f5..d8b3c5308 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -343,7 +343,7 @@ DNSResolver DNSResolver::create() return DNSResolver(); } -bool DNSResolver::check_address_syntax(const char *addr) +bool DNSResolver::check_address_syntax(const char *addr) const { // if string doesn't contain a dot, we won't consider it a url for now. if (strchr(addr,'.') == NULL) diff --git a/src/common/dns_utils.h b/src/common/dns_utils.h index 50e8001f1..332bceafe 100644 --- a/src/common/dns_utils.h +++ b/src/common/dns_utils.h @@ -137,7 +137,7 @@ private: * * @return true if it looks enough like a URL, false if not */ - bool check_address_syntax(const char *addr); + bool check_address_syntax(const char *addr) const; DNSResolverData *m_data; }; // class DNSResolver From 4ef0da184d18f15e1a16c34f898483dc50b125d7 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Thu, 27 Aug 2015 22:21:07 +0100 Subject: [PATCH 4/7] dns_utils: simplify string handling and fix leak --- src/common/dns_utils.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index d8b3c5308..33e71c79d 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -301,11 +301,7 @@ std::vector DNSResolver::get_txt_record(const std::string& url, boo { for (size_t i=0; result->data[i] != NULL; i++) { - // plz fix this, but this does NOT work and spills over into parts of memory it shouldn't: records.push_back(result.ptr->data[i]); - char *restxt; - restxt = (char*) calloc(result->len[i]+1, 1); - memcpy(restxt, result->data[i]+1, result->len[i]-1); - records.push_back(restxt); + records.push_back(std::string(result->data[i]+1, result->len[i]-1)); } } } From f928468b9b9fa584c2da252f2263f0bc7d7485fc Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 29 Aug 2015 12:53:01 +0100 Subject: [PATCH 5/7] dns_utils: factor the fetching code for different DNS record types --- src/common/dns_utils.cpp | 80 +++++++++++----------------------------- src/common/dns_utils.h | 13 +++++++ 2 files changed, 34 insertions(+), 59 deletions(-) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index 33e71c79d..742e039fc 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -100,8 +100,10 @@ namespace tools { // fuck it, I'm tired of dealing with getnameinfo()/inet_ntop/etc -std::string ipv4_to_string(const char* src) +std::string ipv4_to_string(const char* src, size_t len) { + assert(memchr(src, 0, len)); + std::stringstream ss; unsigned int bytes[4]; for (int i = 0; i < 4; i++) @@ -118,8 +120,10 @@ std::string ipv4_to_string(const char* src) // this obviously will need to change, but is here to reflect the above // stop-gap measure and to make the tests pass at least... -std::string ipv6_to_string(const char* src) +std::string ipv6_to_string(const char* src, size_t len) { + assert(memchr(src, 0, len)); + std::stringstream ss; unsigned int bytes[8]; for (int i = 0; i < 8; i++) @@ -138,6 +142,11 @@ std::string ipv6_to_string(const char* src) return ss.str(); } +std::string txt_to_string(const char* src, size_t len) +{ + return std::string(src+1, len-1); +} + // custom smart pointer. // TODO: see if std::auto_ptr and the like support custom destructors template @@ -215,7 +224,7 @@ DNSResolver::~DNSResolver() } } -std::vector DNSResolver::get_ipv4(const std::string& url, bool& dnssec_available, bool& dnssec_valid) +std::vector DNSResolver::get_record(const std::string& url, int record_type, std::string (*reader)(const char *,size_t), bool& dnssec_available, bool& dnssec_valid) { std::vector addresses; dnssec_available = false; @@ -231,7 +240,7 @@ std::vector DNSResolver::get_ipv4(const std::string& url, bool& dns ub_result_ptr result; // call DNS resolver, blocking. if return value not zero, something went wrong - if (!ub_resolve(m_data->m_ub_context, urlC, DNS_TYPE_A, DNS_CLASS_IN, &result)) + if (!ub_resolve(m_data->m_ub_context, urlC, record_type, DNS_CLASS_IN, &result)) { dnssec_available = (result->secure || (!result->secure && result->bogus)); dnssec_valid = result->secure && !result->bogus; @@ -239,7 +248,7 @@ std::vector DNSResolver::get_ipv4(const std::string& url, bool& dns { for (size_t i=0; result->data[i] != NULL; i++) { - addresses.push_back(ipv4_to_string(result->data[i])); + addresses.push_back((*reader)(result->data[i], result->len[i])); } } } @@ -247,66 +256,19 @@ std::vector DNSResolver::get_ipv4(const std::string& url, bool& dns return addresses; } +std::vector DNSResolver::get_ipv4(const std::string& url, bool& dnssec_available, bool& dnssec_valid) +{ + return get_record(url, DNS_TYPE_A, ipv4_to_string, dnssec_available, dnssec_valid); +} + std::vector DNSResolver::get_ipv6(const std::string& url, bool& dnssec_available, bool& dnssec_valid) { - std::vector addresses; - dnssec_available = false; - dnssec_valid = false; - - string_ptr urlC(strdup(url.c_str())); - if (!check_address_syntax(urlC)) - { - return addresses; - } - - ub_result_ptr result; - - // call DNS resolver, blocking. if return value not zero, something went wrong - if (!ub_resolve(m_data->m_ub_context, urlC, DNS_TYPE_AAAA, DNS_CLASS_IN, &result)) - { - dnssec_available = (result->secure || (!result->secure && result->bogus)); - dnssec_valid = result->secure && !result->bogus; - if (result->havedata) - { - for (size_t i=0; result->data[i] != NULL; i++) - { - addresses.push_back(ipv6_to_string(result->data[i])); - } - } - } - - return addresses; + return get_record(url, DNS_TYPE_AAAA, ipv6_to_string, dnssec_available, dnssec_valid); } std::vector DNSResolver::get_txt_record(const std::string& url, bool& dnssec_available, bool& dnssec_valid) { - std::vector records; - dnssec_available = false; - dnssec_valid = false; - - string_ptr urlC(strdup(url.c_str())); - if (!check_address_syntax(urlC)) - { - return records; - } - - ub_result_ptr result; - - // call DNS resolver, blocking. if return value not zero, something went wrong - if (!ub_resolve(m_data->m_ub_context, urlC, DNS_TYPE_TXT, DNS_CLASS_IN, &result)) - { - dnssec_available = (result->secure || (!result->secure && result->bogus)); - dnssec_valid = result->secure && !result->bogus; - if (result->havedata) - { - for (size_t i=0; result->data[i] != NULL; i++) - { - records.push_back(std::string(result->data[i]+1, result->len[i]-1)); - } - } - } - - return records; + return get_record(url, DNS_TYPE_TXT, txt_to_string, dnssec_available, dnssec_valid); } std::string DNSResolver::get_dns_format_from_oa_address(const std::string& oa_addr) diff --git a/src/common/dns_utils.h b/src/common/dns_utils.h index 332bceafe..63bf25445 100644 --- a/src/common/dns_utils.h +++ b/src/common/dns_utils.h @@ -130,6 +130,19 @@ public: private: + /** + * @brief gets all records of a given type from a DNS query for the supplied URL; + * if no such record is present returns an empty vector. + * + * @param url A string containing a URL to query for + * @param record_type the record type to retrieve (DNS_TYPE_A, etc) + * @param reader a function that converts a record data to a string + * + * @return A vector of strings containing the requested record; or an empty vector + */ + // TODO: modify this to accomodate DNSSEC + std::vector get_record(const std::string& url, int record_type, std::string (*reader)(const char *,size_t), bool& dnssec_available, bool& dnssec_valid); + /** * @brief Checks a string to see if it looks like a URL * From 4e138a02dfe79734d74de6644ff6a0cb674448f3 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 29 Aug 2015 12:55:43 +0100 Subject: [PATCH 6/7] dns_utils: remove unnecessary string conversion --- src/common/dns_utils.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/common/dns_utils.cpp b/src/common/dns_utils.cpp index 742e039fc..5cb5ed2de 100644 --- a/src/common/dns_utils.cpp +++ b/src/common/dns_utils.cpp @@ -175,8 +175,6 @@ private: }; typedef class scoped_ptr ub_result_ptr; -static void freestring(char *ptr) { free(ptr); } -typedef class scoped_ptr string_ptr; struct DNSResolverData { @@ -230,8 +228,7 @@ std::vector DNSResolver::get_record(const std::string& url, int rec dnssec_available = false; dnssec_valid = false; - string_ptr urlC(strdup(url.c_str())); - if (!check_address_syntax(urlC)) + if (!check_address_syntax(url.c_str())) { return addresses; } @@ -240,7 +237,7 @@ std::vector DNSResolver::get_record(const std::string& url, int rec ub_result_ptr result; // call DNS resolver, blocking. if return value not zero, something went wrong - if (!ub_resolve(m_data->m_ub_context, urlC, record_type, DNS_CLASS_IN, &result)) + if (!ub_resolve(m_data->m_ub_context, url.c_str(), record_type, DNS_CLASS_IN, &result)) { dnssec_available = (result->secure || (!result->secure && result->bogus)); dnssec_valid = result->secure && !result->bogus; From 3c10239327b7495550c091d0789d9019c7bdf5b8 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 30 Aug 2015 15:21:24 +0100 Subject: [PATCH 7/7] unbound: use the mini event fallback implementation Using libevent seems to have high peaks of file descriptor use, which can cause failure to create fds in other parts of bitmonerod. The fallback implementation seems to run fine in a significantly tighter file descriptor limit. --- external/unbound/config.h.cmake.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/external/unbound/config.h.cmake.in b/external/unbound/config.h.cmake.in index d9bfebf33..377bcd97f 100644 --- a/external/unbound/config.h.cmake.in +++ b/external/unbound/config.h.cmake.in @@ -575,7 +575,8 @@ #cmakedefine USE_GOST /* Define if you want to use internal select based events */ -#cmakedefine USE_MINI_EVENT +/* #cmakedefine USE_MINI_EVENT */ +#define USE_MINI_EVENT 1 /* Define this to enable SHA256 and SHA512 support. */ #cmakedefine USE_SHA2