From 32d47188a5034c0ed3973099ebfe28ffe21ff1ce Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 16 Mar 2011 15:01:45 +0100 Subject: [trac697] Catch DNSProtocolError (such as parse errors) If so, for now treat these as a timeout and resend if there are retries left, fail otherwise --- src/lib/resolve/recursive_query.cc | 50 +++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 14 deletions(-) (limited to 'src/lib') diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc index b68d80f1e9..e59f796e28 100644 --- a/src/lib/resolve/recursive_query.cc +++ b/src/lib/resolve/recursive_query.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -621,20 +622,41 @@ public: Message incoming(Message::PARSE); InputBuffer ibuf(buffer_->getData(), buffer_->getLength()); - incoming.fromWire(ibuf); - - buffer_->clear(); - if (recursive_mode() && - incoming.getRcode() == Rcode::NOERROR()) { - done_ = handleRecursiveAnswer(incoming); - } else { - isc::resolve::copyResponseMessage(incoming, answer_message_); - done_ = true; - } - - if (done_) { - callCallback(true); - stop(); + try { + incoming.fromWire(ibuf); + + buffer_->clear(); + if (recursive_mode() && + incoming.getRcode() == Rcode::NOERROR()) { + done_ = handleRecursiveAnswer(incoming); + } else { + isc::resolve::copyResponseMessage(incoming, answer_message_); + done_ = true; + } + if (done_) { + callCallback(true); + stop(); + } + } catch (const isc::dns::DNSProtocolError& dpe) { + dlog("DNS Protocol error in answer for " + + question_.toText() + " " + + question_.getType().toText() + ": " + + dpe.what()); + // Right now, we treat this similar to timeouts + // (except we don't store RTT) + // We probably want to make this an integral part + // of the fetch data process. (TODO) + if (retries_--) { + dlog("Retrying"); + send(); + } else { + dlog("Giving up"); + if (!callback_called_) { + makeSERVFAIL(); + callCallback(true); + } + stop(); + } } } else if (!done_ && retries_--) { // Query timed out, but we have some retries, so send again -- cgit v1.2.3 From 8d638e07f2347d5e713493ffed9659ea5048e872 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 16 Mar 2011 15:27:13 +0100 Subject: [trac697] add test for dns parse error in response --- .../resolve/tests/recursive_query_unittest_2.cc | 27 +++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'src/lib') diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc index 7d2d1501a1..b94d4e8693 100644 --- a/src/lib/resolve/tests/recursive_query_unittest_2.cc +++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc @@ -101,11 +101,13 @@ public: /// Set before the query and then by each "server" when responding. enum QueryStatus { NONE = 0, ///< Default - UDP_ROOT = 1, ///< Query root server over UDP - UDP_ORG = 2, ///< Query ORG server over UDP - TCP_ORG = 3, ///< Query ORG server over TCP - UDP_EXAMPLE_ORG = 4, ///< Query EXAMPLE.ORG server over UDP - COMPLETE = 5 ///< Query is complete + UDP_ROOT, ///< Query root server over UDP + UDP_ORG, ///< Query ORG server over UDP + TCP_ORG, ///< Query ORG server over TCP + UDP_EXAMPLE_ORG_BAD, ///< Query EXAMPLE.ORG server over UDP + ///< (return malformed packet) + UDP_EXAMPLE_ORG, ///< Query EXAMPLE.ORG server over UDP + COMPLETE ///< Query is complete }; // Common stuff @@ -305,6 +307,12 @@ public: expected_ = TCP_ORG; break; + case UDP_EXAMPLE_ORG_BAD: + // Return the answer to the question. + setAnswerWwwExampleOrg(msg); + // Set new expected in check below + break; + case UDP_EXAMPLE_ORG: // Return the answer to the question. setAnswerWwwExampleOrg(msg); @@ -320,6 +328,13 @@ public: MessageRenderer renderer(*udp_send_buffer_); msg.toWire(renderer); + if (expected_ == UDP_EXAMPLE_ORG_BAD) { + // mangle the packet a bit + // set additional to one more + udp_send_buffer_->writeUint8At(3, 11); + expected_ = UDP_EXAMPLE_ORG; + } + // Return a message back to the IOFetch object (after setting the // expected length of data for the check in the send handler). udp_length_ = udp_send_buffer_->getLength(); @@ -450,7 +465,7 @@ public: // readiness for the next read. (If any - at present, there is only // one read in the test, although extensions to this test suite could // change that.) - expected_ = UDP_EXAMPLE_ORG; + expected_ = UDP_EXAMPLE_ORG_BAD; tcp_cumulative_ = 0; // Unless we go through a callback loop we cannot simply use -- cgit v1.2.3 From f2256e0e6f8260cf8addd1511fe49eaacf22d2bf Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Thu, 17 Mar 2011 10:07:31 +0100 Subject: [trac697] move decls to instide try block --- src/lib/resolve/recursive_query.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/lib') diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc index e59f796e28..65545a4420 100644 --- a/src/lib/resolve/recursive_query.cc +++ b/src/lib/resolve/recursive_query.cc @@ -620,9 +620,10 @@ public: dlog("RTT: " + boost::lexical_cast(rtt)); current_ns_address.updateRTT(rtt); - Message incoming(Message::PARSE); - InputBuffer ibuf(buffer_->getData(), buffer_->getLength()); try { + Message incoming(Message::PARSE); + InputBuffer ibuf(buffer_->getData(), buffer_->getLength()); + incoming.fromWire(ibuf); buffer_->clear(); -- cgit v1.2.3 From c34be1b6a604ffb0a6ef34abfcf9c2fa42451d8c Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Thu, 17 Mar 2011 11:41:34 +0100 Subject: [trac697] address review comments --- .../resolve/tests/recursive_query_unittest_2.cc | 23 +++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'src/lib') diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc index b94d4e8693..ce960c0b7c 100644 --- a/src/lib/resolve/tests/recursive_query_unittest_2.cc +++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc @@ -101,13 +101,13 @@ public: /// Set before the query and then by each "server" when responding. enum QueryStatus { NONE = 0, ///< Default - UDP_ROOT, ///< Query root server over UDP - UDP_ORG, ///< Query ORG server over UDP - TCP_ORG, ///< Query ORG server over TCP - UDP_EXAMPLE_ORG_BAD, ///< Query EXAMPLE.ORG server over UDP + UDP_ROOT = 1, ///< Query root server over UDP + UDP_ORG = 2, ///< Query ORG server over UDP + TCP_ORG = 3, ///< Query ORG server over TCP + UDP_EXAMPLE_ORG_BAD = 4, ///< Query EXAMPLE.ORG server over UDP ///< (return malformed packet) - UDP_EXAMPLE_ORG, ///< Query EXAMPLE.ORG server over UDP - COMPLETE ///< Query is complete + UDP_EXAMPLE_ORG = 5, ///< Query EXAMPLE.ORG server over UDP + COMPLETE = 6 ///< Query is complete }; // Common stuff @@ -287,6 +287,10 @@ public: Message msg(Message::RENDER); setCommonMessage(msg, qid); + // In the case of UDP_EXAMPLE_ORG_BAD, we shall mangle the + // response + bool mangle_response = false; + // Set up state-dependent bits: switch (expected_) { case UDP_ROOT: @@ -310,7 +314,9 @@ public: case UDP_EXAMPLE_ORG_BAD: // Return the answer to the question. setAnswerWwwExampleOrg(msg); - // Set new expected in check below + // Mangle the response to enfore another query + mangle_response = true; + expected_ = UDP_EXAMPLE_ORG; break; case UDP_EXAMPLE_ORG: @@ -328,11 +334,10 @@ public: MessageRenderer renderer(*udp_send_buffer_); msg.toWire(renderer); - if (expected_ == UDP_EXAMPLE_ORG_BAD) { + if (mangle_response) { // mangle the packet a bit // set additional to one more udp_send_buffer_->writeUint8At(3, 11); - expected_ = UDP_EXAMPLE_ORG; } // Return a message back to the IOFetch object (after setting the -- cgit v1.2.3