diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index 28cc8b01e0..21cb23db0f 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -50,24 +50,19 @@ const dns::Name& TSIGKeyInfo::stringToAlgorithmName(const std::string& algorithm_id) { if (boost::iequals(algorithm_id, HMAC_MD5_STR)) { return (dns::TSIGKey::HMACMD5_NAME()); - } - if (boost::iequals(algorithm_id, HMAC_SHA1_STR)) { + } else if (boost::iequals(algorithm_id, HMAC_SHA1_STR)) { return (dns::TSIGKey::HMACSHA1_NAME()); - } - if (boost::iequals(algorithm_id, HMAC_SHA224_STR)) { + } else if (boost::iequals(algorithm_id, HMAC_SHA224_STR)) { return (dns::TSIGKey::HMACSHA224_NAME()); - } - if (boost::iequals(algorithm_id, HMAC_SHA256_STR)) { + } else if (boost::iequals(algorithm_id, HMAC_SHA256_STR)) { return (dns::TSIGKey::HMACSHA256_NAME()); - } - if (boost::iequals(algorithm_id, HMAC_SHA384_STR)) { + } else if (boost::iequals(algorithm_id, HMAC_SHA384_STR)) { return (dns::TSIGKey::HMACSHA384_NAME()); - } - if (boost::iequals(algorithm_id, HMAC_SHA512_STR)) { + } else if (boost::iequals(algorithm_id, HMAC_SHA512_STR)) { return (dns::TSIGKey::HMACSHA512_NAME()); } - isc_throw(BadValue, "Unknown TSIG Key algorithm:" << algorithm_id); + isc_throw(BadValue, "Unknown TSIG Key algorithm: " << algorithm_id); } void @@ -577,8 +572,8 @@ DdnsDomainParser::build(isc::data::ConstElementPtr domain_config) { } if (!tsig_key_info) { - isc_throw(D2CfgError, "DdnsDomain :" << name << - " specifies an undefined key:" << key_name); + isc_throw(D2CfgError, "DdnsDomain " << name << + " specifies an undefined key: " << key_name); } } diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index bca5625efb..26198d2c3d 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -175,9 +175,10 @@ public: /// -# "HMAC-SHA384" /// -# "HMAC-SHA512" /// - /// @param secret the base-64 encoded secret component for this key - /// such as one would typically see in a key file for the entry "Key" - /// in the example below: + /// @param secret The base-64 encoded secret component for this key. + /// (A suitable string for use here could be obtained by running the + /// BIND 9 dnssec-keygen program; the contents of resulting key file + /// will look similar to: /// @code /// Private-key-format: v1.3 /// Algorithm: 157 (HMAC_MD5) @@ -187,6 +188,7 @@ public: /// Publish: 20140515143700 /// Activate: 20140515143700 /// @endcode + /// where the value the "Key:" entry is the secret component of the key.) /// /// @throw D2CfgError if values supplied are invalid: /// name cannot be blank, algorithm must be a supported value, @@ -237,7 +239,7 @@ public: /// -# "HMAC-SHA384" /// -# "HMAC-SHA512" /// - /// @return const reference to a dns::Name containing the alogorithm name + /// @return const reference to a dns::Name containing the algorithm name /// @throw BadValue if ID isn't recognized. static const dns::Name& stringToAlgorithmName(const std::string& algorithm_id); @@ -419,7 +421,7 @@ public: return (name_); } - /// @brief Conveneince method which returns the domain's TSIG key name. + /// @brief Convenience method which returns the domain's TSIG key name. /// /// @return returns the key name in an std::string. If domain has no /// TSIG key, the string will empty. @@ -432,6 +434,10 @@ public: return (servers_); } + /// @brief Getter which returns the domain's TSIGKey info + /// + /// @return returns the pointer to the server storage. If the domain + /// is not configured to use TSIG the pointer will be empty. const TSIGKeyInfoPtr& getTSIGKeyInfo() { return (tsig_key_info_); } @@ -443,7 +449,8 @@ private: /// @brief The list of server(s) supporting this domain. DnsServerInfoStoragePtr servers_; - /// @brief + /// @brief Pointer to domain's the TSIGKeyInfo. + /// Value is empty if the domain is not configured for TSIG. TSIGKeyInfoPtr tsig_key_info_; }; diff --git a/src/bin/d2/d2_update_message.cc b/src/bin/d2/d2_update_message.cc index c52f5fb041..b572767637 100644 --- a/src/bin/d2/d2_update_message.cc +++ b/src/bin/d2/d2_update_message.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/bin/d2/d2_update_message.h b/src/bin/d2/d2_update_message.h index c20677d754..c52f2610e2 100644 --- a/src/bin/d2/d2_update_message.h +++ b/src/bin/d2/d2_update_message.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/bin/d2/dns_client.cc b/src/bin/d2/dns_client.cc index 1b38e92e48..86598cf268 100644 --- a/src/bin/d2/dns_client.cc +++ b/src/bin/d2/dns_client.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -81,14 +81,7 @@ public: const uint16_t ns_port, D2UpdateMessage& update, const unsigned int wait, - const dns::TSIGKey& tsig_key); - - // Starts asynchronous DNS Update. - void doUpdate(asiolink::IOService& io_service, - const asiolink::IOAddress& ns_addr, - const uint16_t ns_port, - D2UpdateMessage& update, - const unsigned int wait); + const dns::TSIGKeyPtr& tsig_key); // This function maps the IO error to the DNSClient error. DNSClient::Status getStatus(const asiodns::IOFetch::Result); @@ -194,17 +187,7 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service, const uint16_t ns_port, D2UpdateMessage& update, const unsigned int wait, - const dns::TSIGKey& tsig_key) { - tsig_context_.reset(new TSIGContext(tsig_key)); - doUpdate(io_service, ns_addr, ns_port, update, wait); -} - -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) { // 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. @@ -214,6 +197,15 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service, << ". Provided timeout value is '" << wait << "'"); } + // Create a TSIG context if we have a key, otherwise clear the context + // pointer. Message marshalling uses non-null context is the indicator + // that TSIG should be used. + if (tsig_key) { + tsig_context_.reset(new TSIGContext(*tsig_key)); + } else { + tsig_context_.reset(); + } + // A renderer is used by the toWire function which creates the on-wire data // from the DNS Update message. A renderer has its internal buffer where it // renders data by default. However, this buffer can't be directly accessed. @@ -244,7 +236,6 @@ DNSClientImpl::doUpdate(asiolink::IOService& io_service, io_service.post(io_fetch); } - DNSClient::DNSClient(D2UpdateMessagePtr& response_placeholder, Callback* callback, const DNSClient::Protocol proto) : impl_(new DNSClientImpl(response_placeholder, callback, proto)) { @@ -266,19 +257,9 @@ DNSClient::doUpdate(asiolink::IOService& io_service, const uint16_t ns_port, D2UpdateMessage& update, const unsigned int wait, - const dns::TSIGKey& tsig_key) { + const dns::TSIGKeyPtr& tsig_key) { impl_->doUpdate(io_service, ns_addr, ns_port, update, wait, tsig_key); } -void -DNSClient::doUpdate(asiolink::IOService& io_service, - const IOAddress& ns_addr, - const uint16_t ns_port, - D2UpdateMessage& update, - const unsigned int wait) { - impl_->doUpdate(io_service, ns_addr, ns_port, update, wait); -} - } // namespace d2 } // namespace isc - diff --git a/src/bin/d2/dns_client.h b/src/bin/d2/dns_client.h index 54d6a6a637..f45a78908e 100644 --- a/src/bin/d2/dns_client.h +++ b/src/bin/d2/dns_client.h @@ -146,41 +146,15 @@ 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 An @c isc::dns::TSIGKey object representing TSIG - /// context which will be used to render the DNS Update message. - /// - /// @todo Implement TSIG Support. Currently any attempt to call this - /// function will result in exception. + /// @param tsig_key A pointer to an @c isc::dns::TSIGKey object that will + /// (if not null) be used to sign the DNS Update message and verify the + /// response. void doUpdate(asiolink::IOService& io_service, const asiolink::IOAddress& ns_addr, const uint16_t ns_port, D2UpdateMessage& update, const unsigned int wait, - const dns::TSIGKey& tsig_key); - - /// @brief Start asynchronous DNS Update without TSIG. - /// - /// This function starts asynchronous DNS Update and returns. The DNS Update - /// will be executed by the specified IO service. Once the message exchange - /// with a DNS server is complete, timeout occurs or IO operation is - /// interrupted, the caller-supplied callback function will be invoked. - /// - /// An address and port of the DNS server is specified through the function - /// arguments so as the same instance of the @c DNSClient can be used to - /// initiate multiple message exchanges. - /// - /// @param io_service IO service to be used to run the message exchange. - /// @param ns_addr DNS server address. - /// @param ns_port DNS server port. - /// @param update A DNS Update message to be sent to the server. - /// @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. - void doUpdate(asiolink::IOService& io_service, - const asiolink::IOAddress& ns_addr, - const uint16_t ns_port, - D2UpdateMessage& update, - const unsigned int wait); + const dns::TSIGKeyPtr& tsig_key = dns::TSIGKeyPtr()); private: DNSClientImpl* impl_; ///< Pointer to DNSClient implementation. diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc index 6200d1c603..72e0096102 100644 --- a/src/bin/d2/nc_trans.cc +++ b/src/bin/d2/nc_trans.cc @@ -172,18 +172,11 @@ NameChangeTransaction::sendUpdate(const std::string& comment) { // @todo time out should ultimately be configurable, down to // server level? - 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); - } + dns_client_->doUpdate(*io_service_, current_server_->getIpAddress(), + current_server_->getPort(), + *dns_update_request_, + DNS_UPDATE_DEFAULT_TIMEOUT, + tsig_key_); // Message is on its way, so the next event should be NOP_EVT. postNextEvent(NOP_EVT); diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h index c093c00b8b..494b68458b 100644 --- a/src/bin/d2/nc_trans.h +++ b/src/bin/d2/nc_trans.h @@ -471,12 +471,6 @@ 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 diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index 91b4de093f..7f43f6552f 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -108,9 +108,6 @@ bool checkServer(DnsServerInfoPtr server, const char* hostname, /// @brief Convenience function which compares the contents of the given /// TSIGKeyInfo against the given set of values. /// -/// It is structured in such a way that each value is checked, and output -/// is generate for all that do not match. -/// /// @param key is a pointer to the key to check against. /// @param name is the value to compare against key's name_. /// @param algorithm is the string value to compare against key's algorithm. @@ -119,39 +116,13 @@ bool checkServer(DnsServerInfoPtr server, const char* hostname, /// @return returns true if there is a match across the board, otherwise it /// returns false. bool checkKey(TSIGKeyInfoPtr key, const std::string& name, - const std::string& algorithm, const std::string& secret) -{ + const std::string& algorithm, const std::string& secret) { // Return value, assume its a match. - bool result = true; - if (!key) { - EXPECT_TRUE(key); - return false; - } - - // Check name. - if (key->getName() != name) { - EXPECT_EQ(name, key->getName()); - result = false; - } - - // Check algorithm. - if (key->getAlgorithm() != algorithm) { - EXPECT_EQ(algorithm, key->getAlgorithm()); - result = false; - } - - // Check secret. - if (key->getSecret() != secret) { - EXPECT_EQ (secret, key->getSecret()); - result = false; - } - - if (!key->getTSIGKey()) { - EXPECT_TRUE (key->getTSIGKey()); - return false; - } - - return (result); + return (((key) && + (key->getName() == name) && + (key->getAlgorithm() == algorithm) && + (key->getSecret() == secret) && + (key->getTSIGKey()))); } /// @brief Test fixture class for testing DnsServerInfo parsing. diff --git a/src/bin/d2/tests/d2_process_unittests.cc b/src/bin/d2/tests/d2_process_unittests.cc index f984e3b84e..d697deedd9 100644 --- a/src/bin/d2/tests/d2_process_unittests.cc +++ b/src/bin/d2/tests/d2_process_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/bin/d2/tests/d2_update_message_unittests.cc b/src/bin/d2/tests/d2_update_message_unittests.cc index 460893d7d5..7fb9716ec0 100644 --- a/src/bin/d2/tests/d2_update_message_unittests.cc +++ b/src/bin/d2/tests/d2_update_message_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -591,7 +591,7 @@ TEST_F(D2UpdateMessageTest, toWireInvalidQRFlag) { // TSIG test TEST_F(D2UpdateMessageTest, validTSIG) { // Create a TSIG Key and context - std::string secret ("this key will match"); + std::string secret("this key will match"); TSIGKeyPtr right_key; ASSERT_NO_THROW(right_key.reset(new TSIGKey(Name("right.com"), diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc index 806629305e..67d32743dc 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/bin/d2/tests/dns_client_unittests.cc @@ -188,6 +188,29 @@ public: *remote); } + // @brief Request handler for testing clients using TSIG + // + // This callback handler is installed when performing async read on a + // socket to emulate reception of the DNS Update request with TSIG by a + // server. As a result, this handler will send an appropriate DNS Update + // response message back to the address from which the request has come. + // + // @param socket A pointer to a socket used to receive a query and send a + // response. + // @param remote A pointer to an object which specifies the host (address + // and port) from which a request has come. + // @param receive_length A length (in bytes) of the received data. + // @param corrupt_response A bool value which indicates that the server's + // response should be invalid (true) or valid (false) + // @param client_key TSIG key the server should use to verify the inbound + // request. If the pointer is NULL, the server will not attempt to + // verify the request. + // @param server_key TSIG key the server should use to sign the outbound + // request. If the pointer is NULL, the server will not sign the outbound + // response. If the pointer is not NULL and not the same value as the + // client_key, the server will use a new context to sign the response then + // the one used to verify it. This allows us to simulate the server + // signing with the wrong key. void TSIGReceiveHandler(udp::socket* socket, udp::endpoint* remote, size_t receive_length, TSIGKeyPtr client_key, @@ -428,13 +451,8 @@ public: // response. const int timeout = 500; expected_++; - if (client_key) { - dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT, - message, timeout, *client_key); - } else { - dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT, - message, timeout); - } + dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT, + message, timeout, client_key); // Kick of the message exchange by actually running the scheduled // "send" and "receive" operations. diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/bin/d2/tests/nc_test_utils.cc index 2ee28e17c1..95b58e65a8 100644 --- a/src/bin/d2/tests/nc_test_utils.cc +++ b/src/bin/d2/tests/nc_test_utils.cc @@ -88,11 +88,11 @@ FauxServer::requestHandler(const asio::error_code& error, std::size_t bytes_recvd, const ResponseMode& response_mode, const dns::Rcode& response_rcode) { + receive_pending_ = false; // If we encountered an error or received no data then fail. // We expect the client to send good requests. if (error.value() != 0 || bytes_recvd < 1) { ADD_FAILURE() << "FauxServer receive failed: " << error.message(); - receive_pending_ = false; return; } @@ -127,7 +127,6 @@ FauxServer::requestHandler(const asio::error_code& error, // If the request cannot be parsed, then fail the test. // We expect the client to send good requests. ADD_FAILURE() << "FauxServer request is corrupt:" << ex.what(); - receive_pending_ = false; return; } @@ -185,7 +184,6 @@ FauxServer::requestHandler(const asio::error_code& error, ADD_FAILURE() << "FauxServer send failed: " << ex.what(); } - receive_pending_ = false; if (perpetual_receive_) { // Schedule the next receive receive (response_mode, response_rcode); @@ -433,9 +431,9 @@ dhcp_ddns::NameChangeRequestPtr makeNcrFromString(const std::string& ncr_str) { DdnsDomainPtr makeDomain(const std::string& zone_name, const std::string& key_name) { - DdnsDomainPtr domain; DnsServerInfoStoragePtr servers(new DnsServerInfoStorage()); - domain.reset(new DdnsDomain(zone_name, servers, makeTSIGKeyInfo(key_name))); + DdnsDomainPtr domain(new DdnsDomain(zone_name, servers, + makeTSIGKeyInfo(key_name))); return (domain); } diff --git a/src/bin/d2/tests/nc_test_utils.h b/src/bin/d2/tests/nc_test_utils.h index 5fc0d662f3..752a210da6 100644 --- a/src/bin/d2/tests/nc_test_utils.h +++ b/src/bin/d2/tests/nc_test_utils.h @@ -118,11 +118,11 @@ public: return receive_pending_; } - /// @breif Sets the TSIG key to the given value. + /// @brief 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) { + void setTSIGKey (const dns::TSIGKeyPtr& tsig_key) { tsig_key_ = tsig_key; } }; diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/bin/d2/tests/nc_trans_unittests.cc index 87f9d63175..b8c89b5cb8 100644 --- a/src/bin/d2/tests/nc_trans_unittests.cc +++ b/src/bin/d2/tests/nc_trans_unittests.cc @@ -1120,6 +1120,7 @@ TEST_F(NameChangeTransactionTest, tsigAllValid) { algorithms.push_back(TSIGKeyInfo::HMAC_SHA512_STR); for (int i = 0; i < algorithms.size(); ++i) { + SCOPED_TRACE (algorithms[i]); TSIGKeyInfoPtr key; ASSERT_NO_THROW(key.reset(new TSIGKeyInfo("test_key", algorithms[i],