Merge branch 'trac357'

doc/guide/bind10-guide.xml

@@ -1633,6 +1633,17 @@ can use various data source backends.
               </simpara>
             </listitem>
           </varlistentry>
+          <varlistentry>
+            <term>tcp_recv_timeout</term>
+            <listitem>
+              <simpara>
+                <varname>tcp_recv_timeout</varname> is the timeout used on
+                incoming TCP connections, in milliseconds. If the query
+                is not sent within this time, the connection is closed.
+                Setting this to 0 will disable TCP timeouts completely.
+              </simpara>
+            </listitem>
+          </varlistentry>
         </variablelist>
 
       </para>

src/bin/auth/auth.spec.pre.in

@@ -90,6 +90,11 @@
             }
           ]
         }
+      },
+      { "item_name": "tcp_recv_timeout",
+        "item_type": "integer",
+        "item_optional": false,
+        "item_default": 5000
       }
     ],
     "commands": [

src/bin/auth/auth_config.cc

@@ -116,6 +116,29 @@ private:
      */
     AddrListPtr rollbackAddresses_;
 };
+
+/// \brief Configuration for TCP receive timeouts
+class TCPRecvTimeoutConfig : public AuthConfigParser {
+public:
+    TCPRecvTimeoutConfig(AuthSrv& server) : server_(server), timeout_(0)
+    {}
+
+    virtual void build(ConstElementPtr config) {
+        if (config->intValue() >= 0) {
+            timeout_ = config->intValue();
+        } else {
+            isc_throw(AuthConfigError, "tcp_recv_timeout must be 0 or higher");
+        }
+    }
+
+    virtual void commit() {
+        server_.setTCPRecvTimeout(timeout_);
+    }
+private:
+    AuthSrv& server_;
+    size_t timeout_;
+};
+
 } // end of unnamed namespace
 
 AuthConfigParser*
@@ -147,6 +170,8 @@ createAuthConfigParser(AuthSrv& server, const std::string& config_id) {
         // We need to return something. The VersionConfig is empty now,
         // so we may abuse that one, as it is a short-term solution only.
         return (new VersionConfig());
+    } else if (config_id == "tcp_recv_timeout") {
+        return (new TCPRecvTimeoutConfig(server));
     } else {
         isc_throw(AuthConfigError, "Unknown configuration identifier: " <<
                   config_id);

src/bin/auth/auth_srv.cc

@@ -927,3 +927,8 @@ AuthSrv::getClientListClasses() const {
     }
     return (result);
 }
+
+void
+AuthSrv::setTCPRecvTimeout(size_t timeout) {
+    dnss_->setTCPRecvTimeout(timeout);
+}

src/bin/auth/auth_srv.h

@@ -319,6 +319,16 @@ public:
     ///     has been set by setClientList.
     std::vector<isc::dns::RRClass> getClientListClasses() const;
 
+    /// \brief Sets the timeout for incoming TCP connections
+    ///
+    /// Incoming TCP connections that have not sent their data
+    /// withing this time are dropped.
+    ///
+    /// \param timeout The timeout (in milliseconds). If se to
+    /// zero, no timeouts are used, and the connection will remain
+    /// open forever.
+    void setTCPRecvTimeout(size_t timeout);
+
 private:
     AuthSrvImpl* impl_;
     isc::asiolink::SimpleCallback* checkin_;

src/bin/auth/b10-auth.xml

@@ -152,6 +152,13 @@
       The default is 60.
     </para>
 
+    <para>
+      <varname>tcp_recv_timeout</varname> is the timeout used on
+      incoming TCP connections, in milliseconds. If the query
+      is not sent within this time, the connection is closed.
+      Setting this to 0 will disable TCP timeouts completely.
+    </para>
+
 <!-- TODO: formating -->
     <para>
       The configuration commands are:

src/bin/auth/tests/config_syntax_unittest.cc

@@ -36,7 +36,8 @@ TEST_F(AuthConfigSyntaxTest, inmemoryDefaultFileType) {
     EXPECT_TRUE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"listen_on\": [], \"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"listen_on\": [], \"datasources\": "
                 "  [{\"type\": \"memory\", \"class\": \"IN\", "
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
@@ -48,7 +49,8 @@ TEST_F(AuthConfigSyntaxTest, inmemorySQLite3Backend) {
     EXPECT_TRUE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"datasources\": "
                 "  [{\"type\": \"memory\","
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
@@ -58,14 +60,30 @@ TEST_F(AuthConfigSyntaxTest, inmemorySQLite3Backend) {
 
 TEST_F(AuthConfigSyntaxTest, badInmemoryFileType) {
     // filetype must be a string
-    EXPECT_FALSE(
+    ASSERT_FALSE(
         mspec_.validateConfig(
             Element::fromJSON(
-                "{\"datasources\": "
+                "{\"tcp_recv_timeout\": 1000,"
+                " \"datasources\": "
                 "  [{\"type\": \"memory\","
                 "    \"zones\": [{\"origin\": \"example.com\","
                 "                 \"file\": \""
                 TEST_DATA_DIR "/example.zone\","
                 "                 \"filetype\": 42}]}]}"), false));
 }
+
+TEST_F(AuthConfigSyntaxTest, badTCPRecvTimeout) {
+    // tcp_recv_timeout must be int
+    EXPECT_FALSE(
+        mspec_.validateConfig(
+            Element::fromJSON(
+                "{\"tcp_recv_timeout\": \"foo\","
+                " \"datasources\": "
+                "  [{\"type\": \"memory\","
+                "    \"zones\": [{\"origin\": \"example.com\","
+                "                 \"file\": \""
+                TEST_DATA_DIR "/example.zone\","
+                "                 \"filetype\": \"sqlite3\"}]}]}"), false));
+}
+
 }

src/bin/auth/tests/config_unittest.cc

@@ -143,4 +143,17 @@ TEST_F(AuthConfigTest, listenAddressConfig) {
     EXPECT_EQ(DNSService::SERVER_SYNC_OK, dnss_.getUDPFdParams().at(1).options);
 }
 
+// Try setting tcp receive timeout through config
+TEST_F(AuthConfigTest, tcpRecvTimeoutConfig) {
+    configureAuthServer(server, Element::fromJSON(
+    "{ \"tcp_recv_timeout\": 123 }"));
+    EXPECT_EQ(123, dnss_.getTCPRecvTimeout());
+    configureAuthServer(server, Element::fromJSON(
+    "{ \"tcp_recv_timeout\": 2000 }"));
+    EXPECT_EQ(2000, dnss_.getTCPRecvTimeout());
+    EXPECT_THROW(configureAuthServer(server, Element::fromJSON(
+                    "{ \"tcp_recv_timeout\": -123 }")),
+                 AuthConfigError);
+}
+
 }

src/lib/asiodns/dns_server.h

@@ -81,7 +81,7 @@ public:
     /// \brief Stop current running server
     virtual void stop() { self_->stop();}
 
-    /// \brief Resume processing of the server coroutine after an 
+    /// \brief Resume processing of the server coroutine after an
     /// asynchronous call (e.g., to the DNS Lookup provider) has completed.
     ///
     /// \param done If true, this signals the system there is an answer
@@ -99,6 +99,18 @@ public:
     virtual DNSServer* clone() { return (self_->clone()); }
     //@}
 
+    /// \brief Set timeout for incoming TCP connections
+    ///
+    /// Since this value is not relevant for every type of DNSServer
+    /// (like UDPServer), it has a no-op default implementation.
+    /// It is in the base class because the AuthSrv's DNSService has
+    /// no direct access to the derived API's after initialization,
+    /// and it does need to update running servers if the timeout
+    /// setting is changed.
+    ///
+    /// \param timeout The timeout in milliseconds
+    virtual void setTCPRecvTimeout(size_t) {}
+
 protected:
     /// \brief Lookup handler object.
     ///

src/lib/asiodns/dns_service.cc

@@ -40,7 +40,7 @@ public:
     DNSServiceImpl(IOService& io_service, SimpleCallback* checkin,
                    DNSLookup* lookup, DNSAnswer* answer) :
             io_service_(io_service), checkin_(checkin), lookup_(lookup),
-            answer_(answer)
+            answer_(answer), tcp_recv_timeout_(5000)
     {}
 
     IOService& io_service_;
@@ -53,13 +53,25 @@ public:
     SimpleCallback* checkin_;
     DNSLookup* lookup_;
     DNSAnswer* answer_;
+    size_t tcp_recv_timeout_;
 
     template<class Ptr, class Server> void addServerFromFD(int fd, int af) {
         Ptr server(new Server(io_service_.get_io_service(), fd, af, checkin_,
                               lookup_, answer_));
+        server->setTCPRecvTimeout(tcp_recv_timeout_);
         (*server)();
         servers_.push_back(server);
     }
+
+    void setTCPRecvTimeout(size_t timeout) {
+        // Store it for future tcp connections
+        tcp_recv_timeout_ = timeout;
+        // Update existing (TCP) Servers
+        std::vector<DNSServerPtr>::iterator it = servers_.begin();
+        for (; it != servers_.end(); ++it) {
+            (*it)->setTCPRecvTimeout(timeout);
+        }
+    }
 };
 
 DNSService::DNSService(IOService& io_service, SimpleCallback* checkin,
@@ -99,5 +111,10 @@ DNSService::clearServers() {
     impl_->servers_.clear();
 }
 
+void
+DNSService::setTCPRecvTimeout(size_t timeout) {
+    impl_->setTCPRecvTimeout(timeout);
+}
+
 } // namespace asiodns
 } // namespace isc

src/lib/asiodns/dns_service.h

@@ -84,6 +84,21 @@ public:
                                     ServerFlag options = SERVER_DEFAULT) = 0;
     virtual void clearServers() = 0;
 
+    /// \brief Set the timeout for TCP DNS services
+    ///
+    /// The timeout is used for incoming TCP connections, so
+    /// that the connection is dropped if not all query data
+    /// is read.
+    ///
+    /// For existing DNSServer objects, where the timeout is
+    /// relevant (i.e. TCPServer instances), the timeout value
+    /// is updated.
+    /// The given value is also kept to use for DNSServer instances
+    /// which are created later
+    ///
+    /// \param timeout The timeout in milliseconds
+    virtual void setTCPRecvTimeout(size_t timeout) = 0;
+
     virtual asiolink::IOService& getIOService() = 0;
 };
 
@@ -187,6 +202,7 @@ public:
     /// \return IOService object for this DNS service.
     virtual asiolink::IOService& getIOService() { return (io_service_);}
 
+    virtual void setTCPRecvTimeout(size_t timeout);
 private:
     DNSServiceImpl* impl_;
     asiolink::IOService& io_service_;

src/lib/asiodns/tcp_server.cc

@@ -70,6 +70,23 @@ TCPServer::TCPServer(io_service& io_service, int fd, int af,
         // it
         isc_throw(IOError, exception.what());
     }
+    // Set it to some value. It should be set to the right one
+    // immediately, but set it to something non-zero just in case.
+    tcp_recv_timeout_.reset(new size_t(5000));
+}
+
+namespace {
+    // Called by the timeout_ deadline timer if the client takes too long.
+    // If not aborted, cancels the given socket
+    // (in which case TCPServer::operator() will be called to continue,
+    // with an 'aborted' error code
+    void do_timeout(asio::ip::tcp::socket& socket,
+                    const asio::error_code& error)
+    {
+        if (error != asio::error::operation_aborted) {
+            socket.cancel();
+        }
+    }
 }
 
 void
@@ -114,6 +131,15 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         /// asynchronous read call.
         data_.reset(new char[MAX_LENGTH]);
 
+        /// Start a timer to drop the connection if it is idle
+        if (*tcp_recv_timeout_ > 0) {
+            timeout_.reset(new asio::deadline_timer(io_));
+            timeout_->expires_from_now(
+                boost::posix_time::milliseconds(*tcp_recv_timeout_));
+            timeout_->async_wait(boost::bind(&do_timeout, boost::ref(*socket_),
+                                 asio::placeholders::error));
+        }
+
         /// Read the message, in two parts.  First, the message length:
         CORO_YIELD async_read(*socket_, asio::buffer(data_.get(),
                               TCP_MESSAGE_LENGTHSIZE), *this);
@@ -209,6 +235,9 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         // will simply exit at that time).
         CORO_YIELD async_write(*socket_, bufs, *this);
 
+        // All done, cancel the timeout timer
+        timeout_->cancel();
+
         // TODO: should we keep the connection open for a short time
         // to see if new requests come in?
         socket_->close();

src/lib/asiodns/tcp_server.h

@@ -34,7 +34,7 @@ namespace asiodns {
 /// \brief A TCP-specific \c DNSServer object.
 ///
 /// This class inherits from both \c DNSServer and from \c coroutine,
-/// defined in coroutine.h. 
+/// defined in coroutine.h.
 class TCPServer : public virtual DNSServer, public virtual coroutine {
 public:
     /// \brief Constructor
@@ -61,6 +61,16 @@ public:
         return (s);
     }
 
+    /// \brief Set the read timeout
+    ///
+    /// If the client does not send (all) query data within this
+    /// timeframe, the connection is dropped
+    ///
+    /// \param timeout in milliseconds
+    virtual void setTCPRecvTimeout(size_t timeout) {
+        *tcp_recv_timeout_ = timeout;
+    }
+
 private:
     enum { MAX_LENGTH = 65535 };
     static const size_t TCP_MESSAGE_LENGTHSIZE = 2;
@@ -122,6 +132,17 @@ private:
 
     boost::shared_ptr<isc::asiolink::IOEndpoint> peer_;
     boost::shared_ptr<isc::asiolink::IOSocket> iosock_;
+
+    // Timer used to timeout on tcp connections
+    // This is a shared pointer because we need to have something
+    // that outlives the operator() call and is copyable (for CORO_FORK)
+    // even though it is only set after fork
+    boost::shared_ptr<asio::deadline_timer> timeout_;
+
+    // Timeout value to use in the timer;
+    // this, too, is a pointer, so that it can be updated whithout restarting
+    // the server
+    boost::shared_ptr<size_t> tcp_recv_timeout_;
 };
 
 } // namespace asiodns

src/lib/asiodns/tests/dns_server_unittest.cc

@@ -258,7 +258,8 @@ class TCPClient : public SimpleClient {
     // this includes connect, send message and recevice message
     static const unsigned int SERVER_TIME_OUT = 2;
     TCPClient(asio::io_service& service, const ip::tcp::endpoint& server)
-        : SimpleClient(service, SERVER_TIME_OUT)
+        : SimpleClient(service, SERVER_TIME_OUT),
+          send_data_delay_(0), send_data_len_delay_(0)
     {
         server_ = server;
         socket_.reset(new ip::tcp::socket(service));
@@ -280,6 +281,20 @@ class TCPClient : public SimpleClient {
                                 std::string(received_data_ + 2));
     }
 
+    /// Set the delay before the data len is sent (in seconds)
+    /// If this is non-zero, the actual data is never sent
+    /// (it is used to test timeout, in which case the connection
+    /// should have been closed by the other side anyway)
+    void setSendDataLenDelay(size_t send_data_len_delay) {
+        send_data_len_delay_ = send_data_len_delay;
+    }
+
+    /// Set the delay before the packet data itself is sent
+    /// (in seconds)
+    void setSendDataDelay(size_t send_data_delay) {
+        send_data_delay_ = send_data_delay;
+    }
+
     private:
     void stopWaitingforResponse() {
         socket_->close();
@@ -288,6 +303,7 @@ class TCPClient : public SimpleClient {
     void connectHandler(const asio::error_code& error) {
         if (!error) {
             data_to_send_len_ = htons(data_to_send_len_);
+            sleep(send_data_len_delay_);
             socket_->async_send(buffer(&data_to_send_len_, 2),
                                 boost::bind(&TCPClient::sendMessageBodyHandler,
                                             this, _1, _2));
@@ -297,7 +313,8 @@ class TCPClient : public SimpleClient {
     void sendMessageBodyHandler(const asio::error_code& error,
                                 size_t send_bytes)
     {
-        if (!error && send_bytes == 2) {
+        if (!error && send_bytes == 2 && send_data_len_delay_ == 0) {
+            sleep(send_data_delay_);
             socket_->async_send(buffer(data_to_send_.c_str(),
                                        data_to_send_.size() + 1),
                     boost::bind(&TCPClient::finishSendHandler, this, _1, _2));
@@ -316,6 +333,9 @@ class TCPClient : public SimpleClient {
     ip::tcp::endpoint server_;
     std::string data_to_send_;
     uint16_t data_to_send_len_;
+
+    size_t send_data_delay_;
+    size_t send_data_len_delay_;
 };
 
 // \brief provide the context which including two clients and
@@ -565,6 +585,34 @@ TYPED_TEST(DNSServerTest, stopTCPServerAfterOneQuery) {
     EXPECT_TRUE(this->serverStopSucceed());
 }
 
+TYPED_TEST(DNSServerTest, TCPTimeoutOnLen) {
+    this->tcp_server_->setTCPRecvTimeout(100);
+    this->tcp_client_->setSendDataLenDelay(2);
+    this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
+                                  this->tcp_client_);
+    EXPECT_EQ("", this->tcp_client_->getReceivedData());
+    EXPECT_FALSE(this->serverStopSucceed());
+}
+
+TYPED_TEST(DNSServerTest, TCPTimeout) {
+    // set delay higher than timeout
+    this->tcp_server_->setTCPRecvTimeout(100);
+    this->tcp_client_->setSendDataDelay(2);
+    this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
+                                  this->tcp_client_);
+    EXPECT_EQ("", this->tcp_client_->getReceivedData());
+    EXPECT_TRUE(this->serverStopSucceed());
+}
+
+TYPED_TEST(DNSServerTest, TCPNoTimeout) {
+    // set delay lower than timeout
+    this->tcp_server_->setTCPRecvTimeout(3000);
+    this->tcp_client_->setSendDataDelay(1);
+    this->testStopServerByStopper(this->tcp_server_, this->tcp_client_,
+                                  this->tcp_client_);
+    EXPECT_EQ("BIND10 is awesome", this->tcp_client_->getReceivedData());
+    EXPECT_TRUE(this->serverStopSucceed());
+}
 
 // Test whether tcp server stopped successfully before server start to serve
 TYPED_TEST(DNSServerTest, stopTCPServerBeforeItStartServing) {

src/lib/testutils/mockups.h

@@ -138,9 +138,18 @@ public:
         return (udp_fd_params_);
     }
 
+    virtual void setTCPRecvTimeout(size_t timeout) {
+        tcp_recv_timeout_ = timeout;
+    }
+
+    size_t getTCPRecvTimeout() {
+        return tcp_recv_timeout_;
+    }
+
 private:
     std::vector<std::pair<int, int> > tcp_fd_params_;
     std::vector<UDPFdParams> udp_fd_params_;
+    size_t tcp_recv_timeout_;
 };
 
 // A nonoperative DNSServer object to be used in calls to processMessage().