From: Yoshitaka Aharen <aharen@jprs.co.jp>
Date: Thu, 31 Jan 2013 13:31:49 +0000 (+0900)
Subject: [2157] increment opcode counter if message header parse succeeded
X-Git-Tag: bind10-1.1.0beta1-release~85^2~5^2~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dc6a6a9ef65b75bdb4cc79a67a83c04645657055;p=thirdparty%2Fkea.git

[2157] increment opcode counter if message header parse succeeded
---

diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 6a58caebab..81ae05d22d 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -525,6 +525,9 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         return;
     }
 
+    const Opcode opcode = message.getOpcode();
+    stats_attrs.setRequestOpCode(opcode);
+
     try {
         // Parse the message.
         message.fromWire(request_buffer);
@@ -573,7 +576,6 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         return;
     }
 
-    const Opcode opcode = message.getOpcode();
     bool send_answer = true;
     try {
         // note: This can only be reliable after TSIG check succeeds.
@@ -584,8 +586,6 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         }
 
         // note: This can only be reliable after TSIG check succeeds.
-        stats_attrs.setRequestOpCode(opcode);
-
         if (opcode == Opcode::NOTIFY()) {
             send_answer = impl_->processNotify(io_message, message, buffer,
                                                tsig_context, stats_attrs);
diff --git a/src/bin/auth/b10-auth.xml.pre b/src/bin/auth/b10-auth.xml.pre
index 0ecc49cf2f..45fb68472c 100644
--- a/src/bin/auth/b10-auth.xml.pre
+++ b/src/bin/auth/b10-auth.xml.pre
@@ -196,21 +196,6 @@
 
 <!-- ### STATISTICS DATA PLACEHOLDER ### -->
 
-    <note><para>
-      DNS parameters (e.g. opcode) in the request message will not be
-      considered if <command>b10-auth</command> failed to parse message,
-      including but not limited to the following circumstance:
-      <itemizedlist>
-        <listitem><para>
-          EDNS version of a message is unknown to
-          <command>b10-auth</command>.
-        </para></listitem>
-        <listitem><para>
-          TSIG signature verification fails.
-        </para></listitem>
-      </itemizedlist>
-    </para></note>
-
   </refsect1>
 
   <refsect1>
diff --git a/src/bin/auth/statistics.cc.pre b/src/bin/auth/statistics.cc.pre
index 199068e719..7df39de07d 100644
--- a/src/bin/auth/statistics.cc.pre
+++ b/src/bin/auth/statistics.cc.pre
@@ -155,7 +155,8 @@ Counters::incRequest(const MessageAttributes& msgattrs) {
     // OPCODE
     const boost::optional<isc::dns::Opcode>& opcode =
         msgattrs.getRequestOpCode();
-    // Increment opcode counter only if the opcode exists.
+    // Increment opcode counter only if the opcode exists; it can happen if
+    // short message which does not contain DNS header received.
     if (opcode) {
         server_msg_counter_.inc(opcode_to_msgcounter[opcode.get().getCode()]);
     }
@@ -199,7 +200,13 @@ Counters::incResponse(const MessageAttributes& msgattrs,
 
     const boost::optional<isc::dns::Opcode>& opcode =
         msgattrs.getRequestOpCode();
-    if (opcode && opcode.get() == Opcode::QUERY()) {
+    if (!opcode) {
+        isc_throw(isc::Unexpected, "Opcode of the request is empty while it is"
+                                   " responded");
+    }
+    if (!msgattrs.getRequestSigBadSig() &&
+               opcode.get() == Opcode::QUERY())
+    {
         // compound attributes
         const unsigned int answer_rrs =
             response.getRRCount(Message::SECTION_ANSWER);
diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h
index 18d2f83f09..c845a6eee2 100644
--- a/src/bin/auth/statistics.h
+++ b/src/bin/auth/statistics.h
@@ -286,7 +286,7 @@ public:
     /// \param msgattrs DNS message attributes.
     /// \param response DNS response message.
     /// \param done DNS response was sent to the client.
-    /// \throw None
+    /// \throw isc::Unexpected Internal condition check failed.
     void inc(const MessageAttributes& msgattrs,
              const isc::dns::Message& response, const bool done);
 
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 70f98fb03d..aede681e81 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -309,13 +309,31 @@ TEST_F(AuthSrvTest, response) {
 // Query with a broken question
 TEST_F(AuthSrvTest, shortQuestion) {
     shortQuestion();
-    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
+    ConstElementPtr stats_after = server.getStatistics()->get("zones")->
+        get("_SERVER_");
+    std::map<std::string, int> expect;
+    expect["request.v4"] = 1;
+    expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
+    expect["responses"] = 1;
+    expect["rcode.formerr"] = 1;
+    expect["qrynoauthans"] = 1;
+    checkStatisticsCounters(stats_after, expect);
 }
 
 // Query with a broken answer section
 TEST_F(AuthSrvTest, shortAnswer) {
     shortAnswer();
-    checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
+    ConstElementPtr stats_after = server.getStatistics()->get("zones")->
+        get("_SERVER_");
+    std::map<std::string, int> expect;
+    expect["request.v4"] = 1;
+    expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
+    expect["responses"] = 1;
+    expect["rcode.formerr"] = 1;
+    expect["qrynoauthans"] = 1;
+    checkStatisticsCounters(stats_after, expect);
 }
 
 // Query with unsupported version of EDNS.
@@ -328,8 +346,10 @@ TEST_F(AuthSrvTest, ednsBadVers) {
     expect["request.v4"] = 1;
     expect["request.badednsver"] = 1;
     expect["request.udp"] = 1;
+    expect["opcode.query"] = 1;
     expect["responses"] = 1;
     expect["rcode.badvers"] = 1;
+    expect["qrynoauthans"] = 1;
     checkStatisticsCounters(stats_after, expect);
 }