[3432] Added basic TSIG support to NameChangeTransaction

Added TSIGKeyPtr instance member to NameChangeTransaction.
Modified NameChangeTransaction::sendUpdate() to do a TSIG updates if its
TSIGKeyPtr is not null.

Extended FauxServer test class to support TSIG if given a key.
Added round trip TSIG tests to NameChangeTransaction base class tests.

This does not address how the transaction's key is determined. That
requires changes to configuration classes in D2Config.

src/bin/d2/nc_trans.cc

@@ -54,7 +54,7 @@ NameChangeTransaction(IOServicePtr& io_service,
      dns_update_status_(DNSClient::OTHER), dns_update_response_(),
      forward_change_completed_(false), reverse_change_completed_(false),
      current_server_list_(), current_server_(), next_server_pos_(0),
-     update_attempts_(0) {
+     update_attempts_(0), tsig_key_() {
     // @todo if io_service is NULL we are multi-threading and should
     // instantiate our own
     if (!io_service_) {
@@ -163,8 +163,7 @@ NameChangeTransaction::transactionOutcomeString() const {
 
 
 void
-NameChangeTransaction::sendUpdate(const std::string& comment,
-                                  bool /* use_tsig_ */) {
+NameChangeTransaction::sendUpdate(const std::string& comment) {
     try {
         ++update_attempts_;
         // @todo add logic to add/replace TSIG key info in request if
@@ -173,9 +172,18 @@ NameChangeTransaction::sendUpdate(const std::string& comment,
 
         // @todo time out should ultimately be configurable, down to
         // server level?
-        dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(),
-                              current_server_->getPort(), *dns_update_request_,
-                              DNS_UPDATE_DEFAULT_TIMEOUT);
+        if (tsig_key_) {
+            dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(),
+                                  current_server_->getPort(),
+                                  *dns_update_request_,
+                                  DNS_UPDATE_DEFAULT_TIMEOUT,
+                                  *tsig_key_);
+        } else {
+            dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(),
+                                  current_server_->getPort(),
+                                  *dns_update_request_,
+                                  DNS_UPDATE_DEFAULT_TIMEOUT);
+        }
 
         // Message is on its way, so the next event should be NOP_EVT.
         postNextEvent(NOP_EVT);
@@ -420,6 +428,7 @@ NameChangeTransaction::initServerSelection(const DdnsDomainPtr& domain) {
         isc_throw(NameChangeTransactionError,
                   "initServerSelection called with an empty domain");
     }
+
     current_server_list_ = domain->getServers();
     next_server_pos_ = 0;
     current_server_.reset();
@@ -446,6 +455,11 @@ NameChangeTransaction::selectNextServer() {
     return (false);
 }
 
+void
+NameChangeTransaction::setTSIGKey(const dns::TSIGKeyPtr& tsig_key) {
+    tsig_key_ = tsig_key;
+}
+
 const DNSClientPtr&
 NameChangeTransaction::getDNSClient() const {
     return (dns_client_);

src/bin/d2/nc_trans.h

@@ -23,6 +23,7 @@
 #include <d2/dns_client.h>
 #include <d2/state_model.h>
 #include <dhcp_ddns/ncr_msg.h>
+#include <dns/tsig.h>
 
 #include <boost/shared_ptr.hpp>
 #include <map>
@@ -207,14 +208,15 @@ protected:
     /// currently selected server.  Since the send is asynchronous, the method
     /// posts NOP_EVT as the next event and then returns.
     ///
+    /// If tsig_key_ is not NULL, then the update will be conducted using
+    /// the key to sign the request and verify the response, otherwise it
+    /// will be conducted without TSIG.
+    ///
     /// @param comment text to include in log detail
-    /// @param use_tsig True if the update should be include a TSIG key. This
-    /// is not yet implemented.
     ///
     /// If an exception occurs it will be logged and and the transaction will
     /// be failed.
-    virtual void sendUpdate(const std::string& comment = "",
-                            bool use_tsig = false);
+    virtual void sendUpdate(const std::string& comment = "");
 
     /// @brief Adds events defined by NameChangeTransaction to the event set.
     ///
@@ -347,6 +349,12 @@ protected:
     /// servers from which to select.
     bool selectNextServer();
 
+    /// @brief Sets the TSIG key to be used.
+    ///
+    /// @param tsig_key TSIG Key value to be used for signing and verifying
+    /// messages.  If set to an emtpy pointer, TSIG will not be used.
+    void setTSIGKey(const dns::TSIGKeyPtr& tsig_key);
+
     /// @brief Sets the update attempt count to the given value.
     ///
     /// @param value is the new value to assign.
@@ -469,6 +477,12 @@ public:
     /// @return A const pointer reference to the DNSClient
     const DNSClientPtr& getDNSClient() const;
 
+    /// @brief Pointer to the TSIG key which should be used (if any).
+    ///
+    /// @return A const pointer reference to the current TSIG key. Pointer
+    /// will be empty if TSIG is not being used.
+    const dns::TSIGKeyPtr& getTSIGKey() const;
+
     /// @brief Fetches the current DNS update request packet.
     ///
     /// @return A const pointer reference to the current D2UpdateMessage
@@ -570,6 +584,9 @@ private:
 
     /// @brief Number of transmit attempts for the current request.
     size_t update_attempts_;
+
+    /// @brief Pointer to the TSIG key which should be used (if any).
+    dns::TSIGKeyPtr tsig_key_;
 };
 
 /// @brief Defines a pointer to a NameChangeTransaction.

src/bin/d2/tests/nc_add_unittests.cc

@@ -55,10 +55,8 @@ public:
     /// the simulate_send_exception_ flag is true.
     ///
     /// @param comment Parameter is unused, but present in base class method.
-    /// @param use_tsig_ Parameter is unused, but present in base class method.
     ///
-    virtual void sendUpdate(const std::string& /*comment*/,
-                            bool /* use_tsig_ = false */) {
+    virtual void sendUpdate(const std::string& /*comment*/) {
         if (simulate_send_exception_) {
             // Make the flag a one-shot by resetting it.
             simulate_send_exception_ = false;

src/bin/d2/tests/nc_remove_unittests.cc

@@ -56,10 +56,8 @@ public:
     /// the simulate_send_exception_ flag is true.
     ///
     /// @param comment Parameter is unused, but present in base class method
-    /// @param use_tsig Parameter is unused, but present in base class method.
     ///
-    virtual void sendUpdate(const std::string& /* comment */,
-                            bool /* use_tsig = false */) {
+    virtual void sendUpdate(const std::string& /* comment */) {
         if (simulate_send_exception_) {
             // Make the flag a one-shot by resetting it.
             simulate_send_exception_ = false;

src/bin/d2/tests/nc_test_utils.cc

@@ -31,7 +31,6 @@ namespace d2 {
 const char* TEST_DNS_SERVER_IP = "127.0.0.1";
 size_t TEST_DNS_SERVER_PORT = 5301;
 
-const bool HAS_RDATA = true;
 const bool NO_RDATA = false;
 
 //*************************** FauxServer class ***********************
@@ -39,7 +38,8 @@ const bool NO_RDATA = false;
 FauxServer::FauxServer(asiolink::IOService& io_service,
                        asiolink::IOAddress& address, size_t port)
     :io_service_(io_service), address_(address), port_(port),
-     server_socket_(), receive_pending_(false), perpetual_receive_(true) {
+     server_socket_(), receive_pending_(false), perpetual_receive_(true),
+     tsig_key_() {
 
     server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
                                                    asio::ip::udp::v4()));
@@ -53,7 +53,7 @@ FauxServer::FauxServer(asiolink::IOService& io_service,
                        DnsServerInfo& server)
     :io_service_(io_service), address_(server.getIpAddress()),
      port_(server.getPort()), server_socket_(), receive_pending_(false),
-     perpetual_receive_(true) {
+     perpetual_receive_(true), tsig_key_() {
     server_socket_.reset(new asio::ip::udp::socket(io_service_.get_io_service(),
                                                    asio::ip::udp::v4()));
     server_socket_->set_option(asio::socket_base::reuse_address(true));
@@ -95,6 +95,13 @@ FauxServer::requestHandler(const asio::error_code& error,
         return;
     }
 
+    // If TSIG key isn't NULL, create a context and use to verify the
+    // request and sign the response.
+    dns::TSIGContextPtr context;
+    if (tsig_key_) {
+        context.reset(new dns::TSIGContext(*tsig_key_));
+    }
+
     // We have a successfully received data. We need to turn it into
     // a request in order to build a proper response.
     // Note D2UpdateMessage is geared towards making requests and
@@ -104,6 +111,17 @@ FauxServer::requestHandler(const asio::error_code& error,
     util::InputBuffer request_buf(receive_buffer_, bytes_recvd);
     try {
         request.fromWire(request_buf);
+
+        // If contex is not NULL, then we need to verify the message.
+        if (context) {
+            dns::TSIGError error = context->verify(request.getTSIGRecord(),
+                                                   receive_buffer_,
+                                                   bytes_recvd);
+            if (error != dns::TSIGError::NOERROR()) {
+                isc_throw(TSIGVerifyError, "TSIG verification failed: "
+                          << error.toText());
+            }
+        }
     } catch (const std::exception& ex) {
         // If the request cannot be parsed, then fail the test.
         // We expect the client to send good requests.
@@ -131,7 +149,19 @@ FauxServer::requestHandler(const asio::error_code& error,
     dns::MessageRenderer renderer;
     util::OutputBuffer response_buf(TEST_MSG_MAX);
     renderer.setBuffer(&response_buf);
-    response.toWire(renderer);
+
+    if (response_mode == INVALID_TSIG) {
+        // Create a different key to sign the response.
+        std::string secret ("key that doesn't match");
+        dns::TSIGKeyPtr key;
+        ASSERT_NO_THROW(key.reset(new
+                                  dns::TSIGKey(dns::Name("badkey"),
+                                               dns::TSIGKey::HMACMD5_NAME(),
+                                               secret.c_str(), secret.size())));
+        context.reset(new dns::TSIGContext(*key));
+    }
+
+    response.toWire(renderer, context.get());
 
     // If mode is to ship garbage, then stomp on part of the rendered
     // message.
@@ -162,6 +192,7 @@ FauxServer::requestHandler(const asio::error_code& error,
 }
 
 
+
 //********************** TimedIO class ***********************
 
 TimedIO::TimedIO()

src/bin/d2/tests/nc_test_utils.h

@@ -40,8 +40,9 @@ typedef boost::shared_ptr<asio::ip::udp::socket> SocketPtr;
 class FauxServer {
 public:
     enum  ResponseMode {
-        USE_RCODE,   // Generate a response with a given RCODE
-        CORRUPT_RESP  // Generate a corrupt response
+        USE_RCODE,    // Generate a response with a given RCODE
+        CORRUPT_RESP, // Generate a corrupt response
+        INVALID_TSIG  // Generate a repsonse with the wrong TSIG key
     };
 
     // Reference to IOService to use for IO processing.
@@ -63,6 +64,9 @@ public:
     // a receive has been completed, a new one will be automatically
     // initiated.
     bool perpetual_receive_;
+    // TSIG Key to use to verify requests and sign responses.  If its
+    // NULL TSIG is not used.
+    dns::TSIGKeyPtr tsig_key_;
 
     /// @brief Constructor
     ///
@@ -96,7 +100,9 @@ public:
     /// @brief Socket IO Completion callback
     ///
     /// This method servers as the Server's UDP socket receive callback handler.
-    /// When the receive completes the handler is invoked with the
+    /// When the receive completes the handler is invoked with the parameters
+    /// listed.
+    ///
     /// @param error result code of the receive (determined by asio layer)
     /// @param bytes_recvd number of bytes received, if any
     /// @param response_mode type of response the handler should produce
@@ -111,6 +117,14 @@ public:
     bool isReceivePending() {
         return receive_pending_;
     }
+
+    /// @breif Sets the TSIG key to the given value.
+    ///
+    /// @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) {
+        tsig_key_ = tsig_key;
+    }
 };
 
 /// @brief Provides a means to process IOService IO for a finite amount of time.

src/bin/d2/tests/nc_trans_unittests.cc

@@ -264,6 +264,7 @@ public:
     using NameChangeTransaction::addPtrRdata;
     using NameChangeTransaction::responseString;
     using NameChangeTransaction::transactionOutcomeString;
+    using NameChangeTransaction::setTSIGKey;
 };
 
 // Declare them so Gtest can see them.
@@ -1004,6 +1005,154 @@ TEST_F(NameChangeTransactionTest, sendUpdate) {
     EXPECT_EQ("response.example.com.", zone->getName().toText());
 }
 
+/// @brief Tests that an unsigned response to a signed request is an error
+TEST_F(NameChangeTransactionTest, tsigUnsignedResponse) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a server and start it listening.
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
+    server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Create a key and manually set it as the transaction's TSIG key
+    std::string secret ("key number one");
+    dns::TSIGKeyPtr key_one;
+    ASSERT_NO_THROW(key_one.reset(new
+                                  dns::TSIGKey(dns::Name("one.com"),
+                                               dns::TSIGKey::HMACMD5_NAME(),
+                                               secret.c_str(), secret.size())));
+    name_change->setTSIGKey(key_one);
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is
+    // INVALID_RESPONSE.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+
+    ASSERT_EQ(DNSClient::INVALID_RESPONSE, name_change->getDnsUpdateStatus());
+
+    // When TSIG errors occur, only the message header (including Rcode) is
+    // unpacked.  In this case, it should be NOERROR but have no other
+    // information.
+    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+    ASSERT_TRUE(response);
+    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
+    EXPECT_FALSE(response->getZone());
+}
+
+/// @brief Tests that a response signed with the wrong key is an error
+TEST_F(NameChangeTransactionTest, tsigInvalidResponse) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a server, tell it to sign responses with a "random" key,
+    // then start it listening.
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
+    server.receive (FauxServer::INVALID_TSIG, dns::Rcode::NOERROR());
+
+    // Create a key and manually set it as the transaction's TSIG key
+    std::string secret ("key number one");
+    dns::TSIGKeyPtr key_one;
+    ASSERT_NO_THROW(key_one.reset(new
+                                  dns::TSIGKey(dns::Name("one.com"),
+                                               dns::TSIGKey::HMACMD5_NAME(),
+                                               secret.c_str(), secret.size())));
+    name_change->setTSIGKey(key_one);
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is
+    // INVALID_RESPONSE.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+
+    ASSERT_EQ(DNSClient::INVALID_RESPONSE, name_change->getDnsUpdateStatus());
+
+    // When TSIG errors occur, only the message header (including Rcode) is
+    // unpacked.  In this case, it should be NOERROR but have no other
+    // information.
+    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+    ASSERT_TRUE(response);
+    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
+    EXPECT_FALSE(response->getZone());
+}
+
+/// @brief Tests that a signed response to an unsigned request is ok.
+/// Currently our policy is to accept a signed response to an unsigned request
+/// even though the spec says a server MUST not do that.
+TEST_F(NameChangeTransactionTest, tsigUnexpectedSignedResponse) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a server, tell it to sign responses with a "random" key,
+    // then start it listening.
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
+    server.receive (FauxServer::INVALID_TSIG, dns::Rcode::NOERROR());
+
+    // Perform an update without TSIG.
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+
+    ASSERT_EQ(DNSClient::SUCCESS, name_change->getDnsUpdateStatus());
+
+    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+    ASSERT_TRUE(response);
+    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
+    D2ZonePtr zone = response->getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ("response.example.com.", zone->getName().toText());
+}
+
+
+/// @brief Tests that a TSIG udpate succeeds when client and server both use
+/// the right key.
+TEST_F(NameChangeTransactionTest, tsigValidExchange) {
+    NameChangeStubPtr name_change;
+    ASSERT_NO_THROW(name_change = makeCannedTransaction());
+    ASSERT_NO_THROW(name_change->initDictionaries());
+    ASSERT_TRUE(name_change->selectFwdServer());
+
+    // Create a key
+    std::string secret ("key number one");
+    dns::TSIGKeyPtr key_one;
+    ASSERT_NO_THROW(key_one.reset(new
+                                  dns::TSIGKey(dns::Name("one.com"),
+                                               dns::TSIGKey::HMACMD5_NAME(),
+                                               secret.c_str(), secret.size())));
+
+    // Create a server, set its TSIG key, and then start it listening.
+    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
+    server.setTSIGKey(key_one);
+    server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+    // Manually set the transaction's key to the same key as the server.
+    name_change->setTSIGKey(key_one);
+
+    // Do the update.
+    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
+
+    // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
+    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+              name_change->getNextEvent());
+
+    ASSERT_EQ(DNSClient::SUCCESS, name_change->getDnsUpdateStatus());
+
+    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+    ASSERT_TRUE(response);
+    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
+    D2ZonePtr zone = response->getZone();
+    EXPECT_TRUE(zone);
+    EXPECT_EQ("response.example.com.", zone->getName().toText());
+}
+
 /// @brief Tests the prepNewRequest method
 TEST_F(NameChangeTransactionTest, prepNewRequest) {
     NameChangeStubPtr name_change;