[#2040] Checkpoint (todo: add stat UTs)

src/bin/d2/d2_hooks.dox

@@ -69,7 +69,7 @@ to the end of this list.
  @subsection d2HooksSelectKey select_key
  - @b Arguments:
    - name: @b current_server, type: isc::d2::DnsServerInfoPtr, direction: <b>in</b>
-   - name: @b tsig_key, type: isc::dns::TSIGKeyPtr, direction: <b>in/out</b>
+   - name: @b tsig_key, type: isc::d2::D2TsigKeyPtr, direction: <b>in/out</b>
 
  - @b Description: this callout is executed when the D2 is selecting for
    a TSIG key to protect the next DNS update to the already chosen DNS

src/bin/d2/d2_process.cc

@@ -12,6 +12,7 @@
 #include <d2/d2_process.h>
 #include <d2srv/d2_cfg_mgr.h>
 #include <d2srv/d2_log.h>
+#include <d2srv/d2_tsig_key.h>
 #include <hooks/hooks.h>
 #include <hooks/hooks_manager.h>
 #include <stats/stats_mgr.h>
@@ -67,6 +68,9 @@ D2Process::D2Process(const char* name, const asiolink::IOServicePtr& io_service)
     // Instantiate stats manager.
     isc::stats::StatsMgr& stats_mgr = isc::stats::StatsMgr::instance();
     stats_mgr.setMaxSampleCountDefault(0);
+    for (const auto& name : D2TsigKey::globalStats) {
+        stats_mgr.setValue("global." + name, static_cast<int64_t>(0));
+    }
 };
 
 void

src/bin/d2/dns_client.cc

@@ -8,6 +8,7 @@
 #include <d2/dns_client.h>
 #include <d2srv/d2_log.h>
 #include <dns/messagerenderer.h>
+#include <stats/stats_mgr.h>
 #include <limits>
 
 namespace isc {
@@ -26,6 +27,7 @@ using namespace isc::util;
 using namespace isc::asiolink;
 using namespace isc::asiodns;
 using namespace isc::dns;
+using namespace isc::stats;
 
 // This class provides the implementation for the DNSClient. This allows for
 // the separation of the DNSClient interface from the implementation details.
@@ -55,6 +57,8 @@ public:
     DNSClient::Protocol proto_;
     // TSIG context used to sign outbound and verify inbound messages.
     dns::TSIGContextPtr tsig_context_;
+    // TSIG key name for stats.
+    std::string tsig_key_name_;
 
     // Constructor and Destructor
     DNSClientImpl(D2UpdateMessagePtr& response_placeholder,
@@ -74,10 +78,13 @@ public:
                   const uint16_t ns_port,
                   D2UpdateMessage& update,
                   const unsigned int wait,
-                  const dns::TSIGKeyPtr& tsig_key);
+                  const D2TsigKeyPtr& tsig_key);
 
     // This function maps the IO error to the DNSClient error.
     DNSClient::Status getStatus(const asiodns::IOFetch::Result);
+
+    // This function updates statistics.
+    void incrStats(const std::string& stat, bool update_key = true);
 };
 
 DNSClientImpl::DNSClientImpl(D2UpdateMessagePtr& response_placeholder,
@@ -137,17 +144,22 @@ DNSClientImpl::operator()(asiodns::IOFetch::Result result) {
         try {
             response_->fromWire(in_buf_->getData(), in_buf_->getLength(),
                                 tsig_context_.get());
+            incrStats("success");
         } catch (const isc::Exception& ex) {
             status = DNSClient::INVALID_RESPONSE;
             LOG_DEBUG(d2_to_dns_logger, isc::log::DBGLVL_TRACE_DETAIL,
                       DHCP_DDNS_INVALID_RESPONSE).arg(ex.what());
-
+            incrStats("error");
         }
 
         if (tsig_context_) {
             // Context is a one-shot deal, get rid of it.
             tsig_context_.reset();
         }
+    } else if (status == DNSClient::TIMEOUT) {
+        incrStats("timeout");
+    } else {
+        incrStats("error");
     }
 
     // Once we are done with internal business, let's call a callback supplied
@@ -174,13 +186,14 @@ DNSClientImpl::getStatus(const asiodns::IOFetch::Result result) {
     }
     return (DNSClient::OTHER);
 }
+
 void
 DNSClientImpl::doUpdate(asiolink::IOService& io_service,
                         const IOAddress& ns_addr,
                         const uint16_t ns_port,
                         D2UpdateMessage& update,
                         const unsigned int wait,
-                        const dns::TSIGKeyPtr& tsig_key) {
+                        const D2TsigKeyPtr& tsig_key) {
     // The underlying implementation which we use to send DNS Updates uses
     // signed integers for timeout. If we want to avoid overflows we need to
     // respect this limitation here.
@@ -195,8 +208,10 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service,
     // that TSIG should be used.
     if (tsig_key) {
         tsig_context_.reset(new TSIGContext(*tsig_key));
+        tsig_key_name_ = tsig_key->getKeyName().toText();
     } else {
         tsig_context_.reset();
+        tsig_key_name_.clear();
     }
 
     // A renderer is used by the toWire function which creates the on-wire data
@@ -227,6 +242,24 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service,
     // Post the task to the task queue in the IO service. Caller will actually
     // run these tasks by executing IOService::run.
     io_service.post(io_fetch);
+
+    // Update sent statistics.
+    incrStats("sent");
+    if (tsig_key) {
+        incrStats("signed", false);
+    } else {
+        incrStats("unsigned", false);
+    }
+}
+
+void
+DNSClientImpl::incrStats(const std::string& stat, bool update_key) {
+    StatsMgr& mgr = StatsMgr::instance();
+    mgr.addValue("global." + stat, static_cast<int64_t>(1));
+    if (update_key && !tsig_key_name_.empty()) {
+        mgr.addValue(StatsMgr::generateName("key", tsig_key_name_, stat),
+                     static_cast<int64_t>(1));
+    }
 }
 
 DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder,
@@ -250,7 +283,7 @@ DNSClient::doUpdate(asiolink::IOService& io_service,
                     const uint16_t ns_port,
                     D2UpdateMessage& update,
                     const unsigned int wait,
-                    const dns::TSIGKeyPtr& tsig_key) {
+                    const D2TsigKeyPtr& tsig_key) {
     impl_->doUpdate(io_service, ns_addr, ns_port, update, wait, tsig_key);
 }
 

src/bin/d2/dns_client.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2020 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -13,7 +13,7 @@
 #include <util/buffer.h>
 
 #include <asiodns/io_fetch.h>
-#include <dns/tsig.h>
+#include <d2srv/d2_tsig_key.h>
 
 namespace isc {
 namespace d2 {
@@ -138,7 +138,7 @@ public:
     /// @param wait A timeout (in milliseconds) for the response. If a response
     /// is not received within the timeout, exchange is interrupted. This value
     /// must not exceed maximal value for 'int' data type.
-    /// @param tsig_key A pointer to an @c isc::dns::TSIGKey object that will
+    /// @param tsig_key A pointer to an @c D2TsigKeyPtr object that will
     /// (if not null) be used to sign the DNS Update message and verify the
     /// response.
     void doUpdate(asiolink::IOService& io_service,
@@ -146,7 +146,7 @@ public:
                   const uint16_t ns_port,
                   D2UpdateMessage& update,
                   const unsigned int wait,
-                  const dns::TSIGKeyPtr& tsig_key = dns::TSIGKeyPtr());
+                  const D2TsigKeyPtr& tsig_key = D2TsigKeyPtr());
 
 private:
     DNSClientImpl* impl_;  ///< Pointer to DNSClient implementation.

src/bin/d2/nc_trans.h

@@ -12,8 +12,8 @@
 #include <asiolink/io_service.h>
 #include <d2/dns_client.h>
 #include <d2srv/d2_cfg_mgr.h>
+#include <d2srv/d2_tsig_key.h>
 #include <dhcp_ddns/ncr_msg.h>
-#include <dns/tsig.h>
 #include <exceptions/exceptions.h>
 #include <util/state_model.h>
 
@@ -589,7 +589,7 @@ private:
     D2CfgMgrPtr cfg_mgr_;
 
     /// @brief Pointer to the TSIG key which should be used (if any).
-    dns::TSIGKeyPtr tsig_key_;
+    D2TsigKeyPtr tsig_key_;
 };
 
 /// @brief Defines a pointer to a NameChangeTransaction.

src/bin/d2/tests/d2_simple_parser_unittest.cc

@@ -641,7 +641,7 @@ TEST_F(TSIGKeyInfoParserTest, invalidEntry) {
               " \"digest-bits\": 120 , "
               " \"secret\": \"bogus\" "
               "}";
-    PARSE_FAIL(config, "Cannot make TSIGKey: Incomplete input for base64:"
+    PARSE_FAIL(config, "Cannot make D2TsigKey: Incomplete input for base64:"
                        " bogus (<string>:1:1)");
 }
 

src/bin/d2/tests/d2_update_message_unittests.cc

@@ -583,18 +583,18 @@ TEST_F(D2UpdateMessageTest, toWireInvalidQRFlag) {
 TEST_F(D2UpdateMessageTest, validTSIG) {
     // Create a TSIG Key and context
     std::string secret("this key will match");
-    TSIGKeyPtr right_key;
+    D2TsigKeyPtr right_key;
     ASSERT_NO_THROW(right_key.reset(new
-                                    TSIGKey(Name("right.com"),
+                                    D2TsigKey(Name("right.com"),
-                                            TSIGKey::HMACMD5_NAME(),
+                                              TSIGKey::HMACMD5_NAME(),
-                                            secret.c_str(), secret.size())));
+                                              secret.c_str(), secret.size())));
 
-    TSIGKeyPtr wrong_key;
+    D2TsigKeyPtr wrong_key;
     secret = "this key will not match";
     ASSERT_NO_THROW(wrong_key.reset(new
-                                    TSIGKey(Name("wrong.com"),
+                                    D2TsigKey(Name("wrong.com"),
-                                            TSIGKey::HMACMD5_NAME(),
+                                              TSIGKey::HMACMD5_NAME(),
-                                            secret.c_str(), secret.size())));
+                                              secret.c_str(), secret.size())));
 
 
     // Build a request message
@@ -658,9 +658,9 @@ TEST_F(D2UpdateMessageTest, allValidTSIG) {
     dns::Name key_name("test_key");
     std::string secret("random text for secret");
     for (int i = 0; i < algorithms.size(); ++i) {
-        dns::TSIGKey key(key_name,
+        D2TsigKey key(key_name,
-                         TSIGKeyInfo::stringToAlgorithmName(algorithms[i]),
+                      TSIGKeyInfo::stringToAlgorithmName(algorithms[i]),
-                         secret.c_str(), secret.size());
+                      secret.c_str(), secret.size());
 
         // Build a request message
         D2UpdateMessage msg;

src/bin/d2/tests/dns_client_unittests.cc

@@ -205,8 +205,8 @@ public:
     // signing with the wrong key.
     void TSIGReceiveHandler(udp::socket* socket, udp::endpoint* remote,
                             size_t receive_length,
-                            TSIGKeyPtr client_key,
+                            D2TsigKeyPtr client_key,
-                            TSIGKeyPtr server_key) {
+                            D2TsigKeyPtr server_key) {
 
         TSIGContextPtr context;
         if (client_key) {
@@ -413,7 +413,7 @@ public:
     // @param server_key TSIG key the "server" should use to sign the response.
     // If this is NULL, then client_key is used.
     // @param should_pass indicates if the test should pass.
-    void runTSIGTest(TSIGKeyPtr client_key, TSIGKeyPtr server_key,
+    void runTSIGTest(D2TsigKeyPtr client_key, D2TsigKeyPtr server_key,
                      bool should_pass = true) {
         // Tell operator() method if we expect an invalid response.
         corrupt_response_ = !should_pass;
@@ -485,18 +485,18 @@ TEST_F(DNSClientTest, invalidTimeout) {
 // Verifies that TSIG can be used to sign requests and verify responses.
 TEST_F(DNSClientTest, runTSIGTest) {
     std::string secret ("key number one");
-    TSIGKeyPtr key_one;
+    D2TsigKeyPtr key_one;
     ASSERT_NO_THROW(key_one.reset(new
-                                    TSIGKey(Name("one.com"),
+                                    D2TsigKey(Name("one.com"),
-                                            TSIGKey::HMACMD5_NAME(),
+                                              TSIGKey::HMACMD5_NAME(),
-                                            secret.c_str(), secret.size())));
+                                              secret.c_str(), secret.size())));
     secret = "key number two";
-    TSIGKeyPtr key_two;
+    D2TsigKeyPtr key_two;
     ASSERT_NO_THROW(key_two.reset(new
-                                    TSIGKey(Name("two.com"),
+                                    D2TsigKey(Name("two.com"),
-                                            TSIGKey::HMACMD5_NAME(),
+                                              TSIGKey::HMACMD5_NAME(),
-                                            secret.c_str(), secret.size())));
+                                              secret.c_str(), secret.size())));
-    TSIGKeyPtr nokey;
+    D2TsigKeyPtr nokey;
 
     // Should be able to send and receive with no keys.
     // Neither client nor server will attempt to sign or verify.

src/bin/d2/tests/nc_test_utils.cc

@@ -173,11 +173,11 @@ FauxServer::requestHandler(const boost::system::error_code& error,
     if (response_mode == INVALID_TSIG) {
         // Create a different key to sign the response.
         std::string secret ("key that doesn't match");
-        dns::TSIGKeyPtr key;
+        D2TsigKeyPtr key;
         ASSERT_NO_THROW(key.reset(new
-                                  dns::TSIGKey(dns::Name("badkey"),
+                                  D2TsigKey(dns::Name("badkey"),
-                                               dns::TSIGKey::HMACMD5_NAME(),
+                                            dns::TSIGKey::HMACMD5_NAME(),
-                                               secret.c_str(), secret.size())));
+                                            secret.c_str(), secret.size())));
         context.reset(new dns::TSIGContext(*key));
     }
 

src/bin/d2/tests/nc_test_utils.h

@@ -61,7 +61,7 @@ public:
     bool perpetual_receive_;
     // TSIG Key to use to verify requests and sign responses.  If its
     // NULL TSIG is not used.
-    dns::TSIGKeyPtr tsig_key_;
+    D2TsigKeyPtr tsig_key_;
 
     /// @brief Constructor
     ///
@@ -117,7 +117,7 @@ public:
     ///
     /// @param tsig_key Pointer to the TSIG key to use.  If the pointer is
     /// empty, TSIG will not be used.
-    void setTSIGKey (const dns::TSIGKeyPtr& tsig_key) {
+    void setTSIGKey(const D2TsigKeyPtr& tsig_key) {
         tsig_key_ = tsig_key;
     }
 };

src/bin/d2/tests/testdata/d2_cfg_tests.json

@@ -702,7 +702,7 @@
 #-----
 ,{
 "description" : "D2.tsig-keys, invalid secret",
-"logic-error" : "Cannot make TSIGKey: Incomplete input for base64: bogus (<string>:1:62)",
+"logic-error" : "Cannot make D2TsigKey: Incomplete input for base64: bogus (<string>:1:62)",
 "data" :
     {
     "forward-ddns" : {},

src/lib/d2srv/d2_config.cc

@@ -165,7 +165,7 @@ void
 TSIGKeyInfo::remakeKey() {
     try {
         // Since our secret value is base64 encoded already, we need to
-        // build the input string for the appropriate TSIGKey constructor.
+        // build the input string for the appropriate D2TsigKey constructor.
         // If secret isn't a valid base64 value, the constructor will throw.
         std::ostringstream stream;
         stream << dns::Name(name_).toText() << ":"
@@ -175,9 +175,9 @@ TSIGKeyInfo::remakeKey() {
             stream << ":" << digestbits_;
         }
 
-        tsig_key_.reset(new dns::TSIGKey(stream.str()));
+        tsig_key_.reset(new D2TsigKey(stream.str()));
     } catch (const std::exception& ex) {
-        isc_throw(D2CfgError, "Cannot make TSIGKey: " << ex.what());
+        isc_throw(D2CfgError, "Cannot make D2TsigKey: " << ex.what());
     }
 }
 
@@ -439,7 +439,7 @@ TSIGKeyInfoParser::parse(ConstElementPtr key_config) {
         }
 
     // Everything should be valid, so create the key instance.
-    // It is possible for the asiodns::dns::TSIGKey create to fail such as
+    // It is possible for the D2TsigKey constructor to fail such as
     // with an invalid secret content.
     TSIGKeyInfoPtr key_info;
     try {

src/lib/d2srv/d2_config.h

@@ -12,8 +12,8 @@
 #include <cc/simple_parser.h>
 #include <cc/cfg_to_element.h>
 #include <cc/user_context.h>
+#include <d2srv/d2_tsig_key.h>
 #include <dhcpsrv/parsers/dhcp_parsers.h>
-#include <dns/tsig.h>
 #include <exceptions/exceptions.h>
 #include <process/d_cfg_mgr.h>
 
@@ -343,8 +343,8 @@ public:
     /// @brief Getter which returns the TSIG key used to sign and verify
     /// messages
     ///
-    /// @return const pointer reference to dns::TSIGKey.
+    /// @return const pointer reference to @c D2TsigKeyPtr
-    const dns::TSIGKeyPtr& getTSIGKey() const {
+    const D2TsigKeyPtr& getTSIGKey() const {
         return (tsig_key_);
     }
 
@@ -397,7 +397,7 @@ private:
     uint32_t digestbits_;
 
     /// @brief The actual TSIG key.
-    dns::TSIGKeyPtr tsig_key_;
+    D2TsigKeyPtr tsig_key_;
 };
 
 /// @brief Defines a pointer for TSIGKeyInfo instances.

src/lib/d2srv/d2_tsig_key.cc

@@ -21,8 +21,6 @@ namespace d2 {
 set<string>
 D2TsigKey::keyStats = {
     "sent",
-    "signed",
-//  "unsigned",
     "success",
     "timeout",
     "error"
@@ -38,7 +36,18 @@ D2TsigKey::globalStats = {
     "error"
 };
 
-D2TsigKey::D2TsigKey(const std::string& key_name) : TSIGKey(key_name) {
+D2TsigKey::D2TsigKey(const std::string& key_spec) : TSIGKey(key_spec) {
+    initStats();
+}
+
+D2TsigKey::D2TsigKey(const Name& key_name, const Name& algorithm_name,
+                     const void* secret, size_t secret_len, size_t digestbits)
+    : TSIGKey(key_name, algorithm_name, secret, secret_len, digestbits) {
+    initStats();
+}
+
+void
+D2TsigKey::initStats() {
     StatsMgr& stats_mgr = StatsMgr::instance();
     const string& kname = getKeyName().toText();
     for (const auto& name : keyStats) {

src/lib/d2srv/d2_tsig_key.h

@@ -26,8 +26,24 @@ public:
     ///
     /// Initialize the key statistics.
     ///
-    /// @param key_name Domain name of the key.
+    /// @param key_spec Specification of the key
-    D2TsigKey(const std::string& key_name);
+    /// (name:secret[:algorithm][:digestbits])
+    explicit D2TsigKey(const std::string& key_spec);
+
+    /// @brief Constructor.
+    ///
+    /// Initialize the key statistics.
+    ///
+    /// @param key_name The name of the key as a domain name.
+    /// @param algorithm_name The hash algorithm used for this key in the
+    /// form of domain name.
+    /// @param secret Point to a binary sequence of the shared secret to be
+    /// used for this key.
+    /// @param secret_len The size of the binary %data (@c secret) in bytes.
+    /// @param digestbits The number of bits to include in the digest
+    /// (0 means to include all)
+    D2TsigKey(const dns::Name& key_name, const dns::Name& algorithm_name,
+              const void* secret, size_t secret_len, size_t digestbits = 0);
 
     /// @brief Destructor.
     ///
@@ -47,6 +63,10 @@ public:
     ///
     /// The list of global statistic names.
     static std::set<std::string> globalStats;
+
+private:
+    /// @brief Initialize key statistics.
+    void initStats();
 };
 
 /// @brief Type of pointer to a D2 TSIG key.

src/lib/d2srv/tests/d2_tsig_key_unittest.cc

@@ -42,7 +42,7 @@ public:
 /// @brief Check TSIG key life.
 TEST_F(D2TsigKeyTest, key) {
     // Statistics names.
-    ASSERT_EQ(5, D2TsigKey::keyStats.size());
+    ASSERT_EQ(4, D2TsigKey::keyStats.size());
     ASSERT_EQ(6, D2TsigKey::globalStats.size());
 
     // Get the statistics manager.
@@ -52,7 +52,7 @@ TEST_F(D2TsigKeyTest, key) {
     // Create a key.
     const string& key_spec = "foo.bar.::test";
     D2TsigKeyPtr key(new D2TsigKey(key_spec));
-    EXPECT_EQ(5, stat_mgr.count());
+    EXPECT_EQ(4, stat_mgr.count());
 
     // Get the 'sent' statistics.
     const string& stat_name = "key[foo.bar.].sent";