From: Torsten Dreyer Date: Mon, 2 May 2016 14:21:42 +0000 (+0200) Subject: Hardening the DNSClient code X-Git-Url: https://git.mxchange.org/?a=commitdiff_plain;h=e8086fe07aeefc336a34925fe04d7e9e408d3e97;p=simgear.git Hardening the DNSClient code - add a timeout to avoid deadlock without a dns server - test against existing terrasync.flightgear.org RR's --- diff --git a/simgear/io/DNSClient.cxx b/simgear/io/DNSClient.cxx index 813f2c13..2c58edd5 100644 --- a/simgear/io/DNSClient.cxx +++ b/simgear/io/DNSClient.cxx @@ -31,23 +31,25 @@ namespace DNS { class Client::ClientPrivate { public: - ClientPrivate() { - if (dns_init(NULL, 0) < 0) - SG_LOG(SG_IO, SG_ALERT, "Can't init udns library" ); + ClientPrivate() { + if (dns_init(NULL, 0) < 0) + SG_LOG(SG_IO, SG_ALERT, "Can't init udns library" ); - if( dns_open(NULL) < 0 ) - SG_LOG(SG_IO, SG_ALERT, "Can't open udns context" ); - } + if( dns_open(NULL) < 0 ) + SG_LOG(SG_IO, SG_ALERT, "Can't open udns context" ); + } - ~ClientPrivate() { - dns_close(NULL); - } + ~ClientPrivate() { + dns_close(NULL); + } }; Request::Request( const std::string & dn ) : - _dn(dn), - _type(DNS_T_ANY), - _complete(false) + _dn(dn), + _type(DNS_T_ANY), + _complete(false), + _timeout_secs(5), + _start(0) { } @@ -55,40 +57,61 @@ Request::~Request() { } +bool Request::isTimeout() const +{ + return (time(NULL) - _start) > _timeout_secs; +} + NAPTRRequest::NAPTRRequest( const std::string & dn ) : - Request(dn) + Request(dn) +{ + _type = DNS_T_NAPTR; +} + +static bool sortNAPTR( const NAPTRRequest::NAPTR_ptr a, const NAPTRRequest::NAPTR_ptr b ) { - _type = DNS_T_NAPTR; + if( a->order > b->order ) return false; + if( a->order < b->order ) return true; + return a->preference < b->preference; } static void dnscbNAPTR(struct dns_ctx *ctx, struct dns_rr_naptr *result, void *data) { - NAPTRRequest * r = static_cast(data); - if (result) { - r->cname = result->dnsnaptr_cname; - r->qname = result->dnsnaptr_qname; - r->ttl = result->dnsnaptr_ttl; - for (int i = 0; i < result->dnsnaptr_nrr; i++) { - NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR); - r->entries.push_back(naptr); - naptr->order = result->dnsnaptr_naptr[i].order; - naptr->preference = result->dnsnaptr_naptr[i].preference; - naptr->flags = result->dnsnaptr_naptr[i].flags; - naptr->service = result->dnsnaptr_naptr[i].service; - naptr->regexp = result->dnsnaptr_naptr[i].regexp; - naptr->replacement = result->dnsnaptr_naptr[i].replacement; - } - free(result); - } - r->setComplete(); + NAPTRRequest * r = static_cast(data); + if (result) { + r->cname = result->dnsnaptr_cname; + r->qname = result->dnsnaptr_qname; + r->ttl = result->dnsnaptr_ttl; + for (int i = 0; i < result->dnsnaptr_nrr; i++) { + if( !r->qservice.empty() && r->qservice != result->dnsnaptr_naptr[i].service ) + return; + + //TODO: case ignore and result flags may have more than one flag + if( !r->qflags.empty() && r->qflags != result->dnsnaptr_naptr[i].flags ) + return; + + NAPTRRequest::NAPTR_ptr naptr(new NAPTRRequest::NAPTR); + r->entries.push_back(naptr); + naptr->order = result->dnsnaptr_naptr[i].order; + naptr->preference = result->dnsnaptr_naptr[i].preference; + naptr->flags = result->dnsnaptr_naptr[i].flags; + naptr->service = result->dnsnaptr_naptr[i].service; + naptr->regexp = result->dnsnaptr_naptr[i].regexp; + naptr->replacement = result->dnsnaptr_naptr[i].replacement; + } + std::sort( r->entries.begin(), r->entries.end(), sortNAPTR ); + free(result); + } + r->setComplete(); } void NAPTRRequest::submit() { - if (!dns_submit_naptr(NULL, getDn().c_str(), 0, dnscbNAPTR, this )) { - SG_LOG(SG_IO, SG_ALERT, "Can't submit dns request for " << getDn()); - return; - } + if (!dns_submit_naptr(NULL, getDn().c_str(), 0, dnscbNAPTR, this )) { + SG_LOG(SG_IO, SG_ALERT, "Can't submit dns request for " << getDn()); + return; + } + _start = time(NULL); } @@ -97,21 +120,21 @@ Client::~Client() } Client::Client() : - d(new ClientPrivate) + d(new ClientPrivate) { } void Client::makeRequest(const Request_ptr& r) { - r->submit(); + r->submit(); } void Client::update(int waitTimeout) { - time_t now = time(NULL); - if( dns_timeouts( NULL, waitTimeout, now ) < 0 ) - return; + time_t now = time(NULL); + if( dns_timeouts( NULL, waitTimeout, now ) < 0 ) + return; dns_ioevent(NULL, now); } diff --git a/simgear/io/DNSClient.hxx b/simgear/io/DNSClient.hxx index 5a7604dd..b23e6dc5 100644 --- a/simgear/io/DNSClient.hxx +++ b/simgear/io/DNSClient.hxx @@ -39,40 +39,47 @@ namespace DNS class Request : public SGReferenced { public: - Request( const std::string & dn ); - virtual ~Request(); - std::string getDn() const { return _dn; } - int getType() const { return _type; } - bool complete() const { return _complete; } - void setComplete( bool b = true ) { _complete = b; } - - virtual void submit() = 0; + Request( const std::string & dn ); + virtual ~Request(); + std::string getDn() const { return _dn; } + int getType() const { return _type; } + bool isComplete() const { return _complete; } + bool isTimeout() const; + void setComplete( bool b = true ) { _complete = b; } + + virtual void submit() = 0; + + std::string cname; + std::string qname; + unsigned ttl; protected: - std::string _dn; - int _type; - bool _complete; + std::string _dn; + int _type; + bool _complete; + time_t _timeout_secs; + time_t _start; }; class NAPTRRequest : public Request { public: - NAPTRRequest( const std::string & dn ); - virtual void submit(); - - struct NAPTR : SGReferenced { - int order; - int preference; - std::string flags; - std::string service; - std::string regexp; - std::string replacement; - }; - typedef SGSharedPtr NAPTR_ptr; - typedef std::vector NAPTR_list; - std::string cname; - std::string qname; - unsigned ttl; - NAPTR_list entries; + NAPTRRequest( const std::string & dn ); + virtual void submit(); + + struct NAPTR : SGReferenced { + int order; + int preference; + std::string flags; + std::string service; + std::string regexp; + std::string replacement; + }; + typedef SGSharedPtr NAPTR_ptr; + typedef std::vector NAPTR_list; + NAPTR_list entries; + + std::string qflags; + std::string qservice; }; typedef SGSharedPtr Request_ptr; diff --git a/simgear/io/test_DNS.cxx b/simgear/io/test_DNS.cxx index df2a10e8..0e9d17ff 100644 --- a/simgear/io/test_DNS.cxx +++ b/simgear/io/test_DNS.cxx @@ -14,6 +14,7 @@ #include "test_DNS.hxx" #include +#include using std::cout; using std::cerr; @@ -34,33 +35,80 @@ int main(int argc, char* argv[]) sglog().setLogLevels( SG_ALL, SG_DEBUG ); DNS::Client cl; +#define EXISTING_RECORD "terrasync.flightgear.org" // test existing NAPTR - // fgtest.t3r.de. 600 IN NAPTR 999 99 "U" "test" "!^.*$!http://dnstest.flightgear.org/!" . + // fgtest.t3r.de. 600 IN NAPTR 999 99 "U" "test" "!^.*$!http://dnstest.flightgear.org/!" . { - DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest("fgtest.t3r.de"); - DNS::Request_ptr r(naptrRequest); - cl.makeRequest(r); - while( !r->complete() ) - cl.update(0); - - COMPARE(naptrRequest->entries.size(), 1 ); - COMPARE(naptrRequest->entries[0]->order, 999 ); - COMPARE(naptrRequest->entries[0]->preference, 99 ); - COMPARE(naptrRequest->entries[0]->service, "test" ); - COMPARE(naptrRequest->entries[0]->regexp, "!^.*$!http://dnstest.flightgear.org/!" ); - COMPARE(naptrRequest->entries[0]->replacement, "" ); + DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest(EXISTING_RECORD); + DNS::Request_ptr r(naptrRequest); + cl.makeRequest(r); + while( !r->isComplete() && !r->isTimeout()) { + usleep(200000); + cl.update(0); + } + + if( r->isTimeout() ) { + cerr << "timeout testing existing record " EXISTING_RECORD << endl; + return EXIT_FAILURE; + } + if(naptrRequest->entries.empty()) { + cerr << "no results for " EXISTING_RECORD << endl; + return EXIT_FAILURE; + } + + // test for ascending preference/order + int order = -1, preference = -1; + for( DNS::NAPTRRequest::NAPTR_list::const_iterator it = naptrRequest->entries.begin(); it != naptrRequest->entries.end(); ++it ) { + // currently only support "U" which implies empty replacement + COMPARE((*it)->flags, "U" ); + COMPARE(naptrRequest->entries[0]->replacement, "" ); + + // currently only support ws20 + COMPARE((*it)->service, "ws20" ); + + if( (*it)->order < order ) { + cerr << "NAPTR entries not ascending for field 'order'" << endl; + return EXIT_FAILURE; + } else if( (*it)->order > order ) { + order = (*it)->order; + preference = (*it)->preference; + } else { + if( (*it)->preference < preference ) { + cerr << "NAPTR entries not ascending for field 'preference', order=" << order << endl; + return EXIT_FAILURE; + } + preference = (*it)->preference; + } + + if( false == simgear::strutils::starts_with( (*it)->regexp, "!^.*$!" ) ) { + cerr << "NAPTR entry with unsupported regexp: " << (*it)->regexp << endl; + return EXIT_FAILURE; + } + + if( false == simgear::strutils::ends_with( (*it)->regexp, "!" ) ) { + cerr << "NAPTR entry with unsupported regexp: " << (*it)->regexp << endl; + return EXIT_FAILURE; + } + + } } // test non-existing NAPTR { - DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest("jurkxkqdiufqzpfvzqok.prozhqrlcaavbxifkkhf"); - DNS::Request_ptr r(naptrRequest); - cl.makeRequest(r); - while( !r->complete() ) - cl.update(0); - - COMPARE(naptrRequest->entries.size(), 0 ); + DNS::NAPTRRequest * naptrRequest = new DNS::NAPTRRequest("jurkxkqdiufqzpfvzqok.prozhqrlcaavbxifkkhf"); + DNS::Request_ptr r(naptrRequest); + cl.makeRequest(r); + while( !r->isComplete() && !r->isTimeout()) { + usleep(200000); + cl.update(0); + } + + if( r->isTimeout() ) { + cerr << "timeout testing non-existing record." << endl; + return EXIT_FAILURE; + } + COMPARE(naptrRequest->entries.size(), 0 ); } cout << "all tests passed ok" << endl;