BIND 10 trac2157_2, updated. 9430b068ddc62ed1a31ec72b3012aa87d8a743f4 [2157] fix opcode handling and added some documentation
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Feb 2 02:53:43 UTC 2013
The branch, trac2157_2 has been updated
via 9430b068ddc62ed1a31ec72b3012aa87d8a743f4 (commit)
via cddeabfcf7ae30d053f82952310377186b91d2e4 (commit)
via ba2d4df266cd44d44175260cf16bf2d22a4e1f12 (commit)
from 7f69bab21e2bfc6656a1926348f2ea4920301b61 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 9430b068ddc62ed1a31ec72b3012aa87d8a743f4
Author: Yoshitaka Aharen <aharen at jprs.co.jp>
Date: Sat Feb 2 11:52:23 2013 +0900
[2157] fix opcode handling and added some documentation
commit cddeabfcf7ae30d053f82952310377186b91d2e4
Author: Yoshitaka Aharen <aharen at jprs.co.jp>
Date: Sat Feb 2 11:03:18 2013 +0900
[2157] make opcode a reference
commit ba2d4df266cd44d44175260cf16bf2d22a4e1f12
Author: Yoshitaka Aharen <aharen at jprs.co.jp>
Date: Sat Feb 2 10:55:56 2013 +0900
[2157] correct method naming of MessageAttributes
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_srv.cc | 4 ++-
src/bin/auth/b10-auth.xml.pre | 19 +++++++++++
src/bin/auth/statistics.cc.pre | 44 +++++++++++++------------
src/bin/auth/statistics.h | 14 ++++----
src/bin/auth/tests/auth_srv_unittest.cc | 3 ++
src/bin/auth/tests/statistics_unittest.cc.pre | 2 +-
6 files changed, 56 insertions(+), 30 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index 81ae05d..bb9c86d 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -525,7 +525,9 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
return;
}
- const Opcode opcode = message.getOpcode();
+ const Opcode& opcode = message.getOpcode();
+ // Get opcode at this point; for all requests regardless of message body
+ // sanity check.
stats_attrs.setRequestOpCode(opcode);
try {
diff --git a/src/bin/auth/b10-auth.xml.pre b/src/bin/auth/b10-auth.xml.pre
index 45fb684..933d82b 100644
--- a/src/bin/auth/b10-auth.xml.pre
+++ b/src/bin/auth/b10-auth.xml.pre
@@ -196,6 +196,25 @@
<!-- ### STATISTICS DATA PLACEHOLDER ### -->
+ <note>
+ <para>
+ Opcode of a request message will not be counted if:
+ <itemizedlist>
+ <listitem><para>
+ The request message is too short to parse the message header
+ </para></listitem>
+ <listitem><para>
+ The request message is a response (i.e. QR bit is set)
+ </para></listitem>
+ </itemizedlist>
+ </para>
+
+ <para>
+ Request attributes except for opcode will not be counted if signature
+ validation failed as they are not reliable.
+ </para>
+ </note>
+
</refsect1>
<refsect1>
diff --git a/src/bin/auth/statistics.cc.pre b/src/bin/auth/statistics.cc.pre
index 7df39de..af3b44f 100644
--- a/src/bin/auth/statistics.cc.pre
+++ b/src/bin/auth/statistics.cc.pre
@@ -131,35 +131,37 @@ Counters::incRequest(const MessageAttributes& msgattrs) {
server_msg_counter_.inc(MSG_REQUEST_TCP);
}
- // request TSIG
- if (msgattrs.getRequestSigTSIG()) {
+ // Opcode
+ const boost::optional<isc::dns::Opcode>& opcode =
+ msgattrs.getRequestOpCode();
+ // Increment opcode counter only if the opcode exists; opcode can be empty
+ // if a short message which does not contain DNS header is received, or
+ // a response message (i.e. QR bit is set) is received.
+ if (opcode) {
+ server_msg_counter_.inc(opcode_to_msgcounter[opcode.get().getCode()]);
+ }
+
+ // TSIG
+ if (msgattrs.requestHasTSIG()) {
server_msg_counter_.inc(MSG_REQUEST_TSIG);
}
- if (msgattrs.getRequestSigBadSig()) {
+ if (msgattrs.requestHasBadSig()) {
server_msg_counter_.inc(MSG_REQUEST_BADSIG);
- // If signature validation is failed, no other query attributes are
- // reliable. Skip processing of the rest of query counters.
+ // If signature validation failed, no other request attributes (except
+ // for opcode) are reliable. Skip processing of the rest of request
+ // counters.
return;
}
- // request EDNS
- if (msgattrs.getRequestEDNS0()) {
+ // EDNS0
+ if (msgattrs.requestHasEDNS0()) {
server_msg_counter_.inc(MSG_REQUEST_EDNS0);
}
- // request DNSSEC
- if (msgattrs.getRequestDO()) {
+ // DNSSEC OK bit
+ if (msgattrs.requestHasDO()) {
server_msg_counter_.inc(MSG_REQUEST_DNSSEC_OK);
}
-
- // OPCODE
- const boost::optional<isc::dns::Opcode>& opcode =
- msgattrs.getRequestOpCode();
- // 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()]);
- }
}
void
@@ -170,7 +172,7 @@ Counters::incResponse(const MessageAttributes& msgattrs,
server_msg_counter_.inc(MSG_RESPONSE);
// response truncated
- if (msgattrs.getResponseTruncated()) {
+ if (msgattrs.responseIsTruncated()) {
server_msg_counter_.inc(MSG_RESPONSE_TRUNCATED);
}
@@ -181,7 +183,7 @@ Counters::incResponse(const MessageAttributes& msgattrs,
}
// response TSIG
- if (msgattrs.getResponseTSIG()) {
+ if (msgattrs.responseHasTSIG()) {
server_msg_counter_.inc(MSG_RESPONSE_TSIG);
}
@@ -204,7 +206,7 @@ Counters::incResponse(const MessageAttributes& msgattrs,
isc_throw(isc::Unexpected, "Opcode of the request is empty while it is"
" responded");
}
- if (!msgattrs.getRequestSigBadSig() &&
+ if (!msgattrs.requestHasBadSig() &&
opcode.get() == Opcode::QUERY())
{
// compound attributes
diff --git a/src/bin/auth/statistics.h b/src/bin/auth/statistics.h
index c845a6e..1097416 100644
--- a/src/bin/auth/statistics.h
+++ b/src/bin/auth/statistics.h
@@ -60,7 +60,7 @@ public:
};
private:
// request attributes
- int req_address_family_; // IP version
+ int req_address_family_; // IP version
int req_transport_protocol_; // Transport layer protocol
boost::optional<isc::dns::Opcode> req_opcode_; // OpCode
enum BitAttributes {
@@ -142,7 +142,7 @@ public:
///
/// \return true if EDNS version of the request is 0
/// \throw None
- bool getRequestEDNS0() const {
+ bool requestHasEDNS0() const {
return (bit_attributes_[REQ_WITH_EDNS_0]);
}
@@ -158,7 +158,7 @@ public:
///
/// \return true if DNSSEC OK (DO) bit of the request is set
/// \throw None
- bool getRequestDO() const {
+ bool requestHasDO() const {
return (bit_attributes_[REQ_WITH_DNSSEC_OK]);
}
@@ -174,7 +174,7 @@ public:
///
/// \return true if the request is TSIG signed
/// \throw None
- bool getRequestSigTSIG() const {
+ bool requestHasTSIG() const {
return (bit_attributes_[REQ_TSIG_SIGNED]);
}
@@ -182,7 +182,7 @@ public:
///
/// \return true if the signature of the request is bad
/// \throw None
- bool getRequestSigBadSig() const {
+ bool requestHasBadSig() const {
return (bit_attributes_[REQ_BADSIG]);
}
@@ -206,7 +206,7 @@ public:
///
/// \return true if the response is truncated
/// \throw None
- bool getResponseTruncated() const {
+ bool responseIsTruncated() const {
return (bit_attributes_[RES_IS_TRUNCATED]);
}
@@ -222,7 +222,7 @@ public:
///
/// \return true if the response is signed with TSIG
/// \throw None
- bool getResponseTSIG() const {
+ bool responseHasTSIG() const {
return (bit_attributes_[RES_TSIG_SIGNED]);
}
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index aede681..fec4613 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -419,6 +419,7 @@ TEST_F(AuthSrvTest, TSIGSignedBadKey) {
expect["request.tsig"] = 1;
expect["request.badsig"] = 1;
expect["request.udp"] = 1;
+ expect["opcode.query"] = 1;
expect["responses"] = 1;
expect["response.tsig"] = 1;
expect["rcode.notauth"] = 1;
@@ -464,6 +465,7 @@ TEST_F(AuthSrvTest, TSIGBadSig) {
expect["request.tsig"] = 1;
expect["request.badsig"] = 1;
expect["request.udp"] = 1;
+ expect["opcode.query"] = 1;
expect["responses"] = 1;
expect["response.tsig"] = 1;
expect["rcode.notauth"] = 1;
@@ -512,6 +514,7 @@ TEST_F(AuthSrvTest, TSIGCheckFirst) {
expect["request.tsig"] = 1;
expect["request.badsig"] = 1;
expect["request.udp"] = 1;
+ expect["opcode.other"] = 1;
expect["responses"] = 1;
expect["response.tsig"] = 1;
expect["rcode.notauth"] = 1;
diff --git a/src/bin/auth/tests/statistics_unittest.cc.pre b/src/bin/auth/tests/statistics_unittest.cc.pre
index 536767e..cf6f29a 100644
--- a/src/bin/auth/tests/statistics_unittest.cc.pre
+++ b/src/bin/auth/tests/statistics_unittest.cc.pre
@@ -344,7 +344,7 @@ TEST_F(CountersTest, incrementTSIG) {
expect.clear();
expect["request.v4"] = i+1;
expect["request.udp"] = i+1;
- expect["opcode.query"] = i+1 - count_badsig;
+ expect["opcode.query"] = i+1;
expect["request.edns0"] = i+1 - count_badsig;
expect["request.badednsver"] = 0;
expect["request.dnssec_ok"] = i+1 - count_badsig;
More information about the bind10-changes
mailing list