Browse Source

[2003] supported quota on the nubmer of acceptable TCP clients.

right now, it's harcoded, but it should be made configurable in a later
task.
JINMEI Tatuya 13 years ago
parent
commit
d05e135bd5
3 changed files with 76 additions and 7 deletions
  1. 26 6
      src/bin/ddns/ddns.py.in
  2. 13 0
      src/bin/ddns/ddns_messages.mes
  3. 37 1
      src/bin/ddns/tests/ddns_test.py

+ 26 - 6
src/bin/ddns/ddns.py.in

@@ -152,6 +152,10 @@ def get_datasrc_client(cc_session):
         return HARDCODED_DATASRC_CLASS, DummyDataSourceClient(ex)
 
 class DDNSServer:
+    # The number of TCP clients that can be handled by the server at the same
+    # time (this should be configurable parameter).
+    TCP_CLIENTS = 10
+
     def __init__(self, cc_session=None):
         '''
         Initialize the DDNS Server.
@@ -409,7 +413,7 @@ class DDNSServer:
                 tcp_ctx = DNSTCPContext(sock)
                 send_result = tcp_ctx.send(data)
                 if send_result == DNSTCPContext.SENDING:
-                    self._tcp_ctxs[sock.fileno] = (tcp_ctx, dest)
+                    self._tcp_ctxs[sock.fileno()] = (tcp_ctx, dest)
                 elif send_result == DNSTCPContext.CLOSED:
                     raise socket.error("socket error in TCP send")
                 else:
@@ -457,19 +461,35 @@ class DDNSServer:
                          ZoneFormatter(zname, zclass), error_msg)
 
     def handle_session(self, fileno):
-        """
-        Handle incoming session on the socket with given fileno.
+        """Handle incoming session on the socket with given fileno.
+
+        Return True if a response (whether positive or negative) has been
+        sent; otherwise False.  The return value isn't expected to be used
+        for other purposes than testing.
+
         """
         logger.debug(TRACE_BASIC, DDNS_SESSION, fileno)
-        (socket, receiver) = self._socksession_receivers[fileno]
+        (session_socket, receiver) = self._socksession_receivers[fileno]
         try:
-            self.handle_request(receiver.pop())
+            req_session = receiver.pop()
+            (sock, remote_addr) = (req_session[0], req_session[2])
+
+            # If this is a TCP client, check the quota, and immediately reject
+            # it if we cannot accept more.
+            if sock.proto == socket.IPPROTO_TCP and \
+                    len(self._tcp_ctxs) >= self.TCP_CLIENTS:
+                logger.warn(DDNS_REQUEST_TCP_QUOTA,
+                            ClientFormatter(remote_addr), len(self._tcp_ctxs))
+                sock.close()
+                return False
+            return self.handle_request(req_session)
         except isc.util.cio.socketsession.SocketSessionError as se:
             # No matter why this failed, the connection is in unknown, possibly
             # broken state. So, we close the socket and remove the receiver.
             del self._socksession_receivers[fileno]
-            socket.close()
+            session_socket.close()
             logger.warn(DDNS_DROP_CONN, fileno, se)
+            return False
 
     def run(self):
         '''

+ 13 - 0
src/bin/ddns/ddns_messages.mes

@@ -149,3 +149,16 @@ place, it's more likely for the subsequent attempt to succeed.  In any
 case, there may not be able to do anything to fix it at the server
 side, but the administrator may want to check the general reachability
 with the client address.
+
+% DDNS_REQUEST_TCP_QUOTA reject TCP update client %1 (%2 running)
+b10-ddns received a new update request from a client over TCP, but
+the number of TCP clients being handled by the server already reached
+the configured quota, so the latest client was rejected by closing
+the connection.  The administrator may want to check the status of
+b10-ddns, and if this happens even if the server is not very busy,
+the quota may have to be increased.  Or, if it's more likely to be
+malicious or simply bogus clients that somehow keep the TCP connection
+open for a long period, maybe they should be rejected with an
+appropriate ACL configuration or some lower layer filtering.  The
+number of existing TCP clients are shown in the log, which should be
+identical to the current quota.

+ 37 - 1
src/bin/ddns/tests/ddns_test.py

@@ -901,7 +901,7 @@ class TestDDNSSession(unittest.TestCase):
         # data should be the expected response.
         s.make_send_ready()
         self.assertEqual(DNSTCPContext.SEND_DONE,
-                         self.server._tcp_ctxs[s.fileno][0].send_ready())
+                         self.server._tcp_ctxs[s.fileno()][0].send_ready())
         self.check_update_response(s._sent_data, Rcode.NOERROR(), tcp=True)
 
     def test_tcp_request_error(self):
@@ -916,6 +916,42 @@ class TestDDNSSession(unittest.TestCase):
         # the socket should have been closed.
         self.assertEqual(1, s._close_called)
 
+    def test_tcp_request_quota(self):
+        '''Test'''
+        # Originally the TCP context map should be empty.
+        self.assertEqual(0, len(self.server._tcp_ctxs))
+
+        class FakeReceiver:
+            '''Faked SessionReceiver, just returning given param by pop()'''
+            def __init__(self, param):
+                self.__param = param
+            def pop(self):
+                return self.__param
+
+        def check_tcp_ok(fd, expect_grant):
+            '''Supplemental checker to see if TCP request is handled.'''
+            s = FakeSocket(fd, proto=socket.IPPROTO_TCP)
+            s._send_buflen = 7
+            self.server._socksession_receivers[s.fileno()] = \
+                (None, FakeReceiver((s, TEST_SERVER6, TEST_CLIENT6,
+                                     create_msg())))
+            self.assertEqual(expect_grant,
+                             self.server.handle_session(s.fileno()))
+            self.assertEqual(0 if expect_grant else 1, s._close_called)
+
+        # By default up to 10 TCP clients can coexist (use hardcode
+        # intentionally so we can test the default value itself)
+        for i in range(0, 10):
+            check_tcp_ok(i, True)
+        self.assertEqual(10, len(self.server._tcp_ctxs))
+
+        # Beyond that, it should be rejected (by reset)
+        check_tcp_ok(11, False)
+
+        # If we remove one context from the server, new client can go in again.
+        self.server._tcp_ctxs.pop(5)
+        check_tcp_ok(12, True)
+
     def test_request_message(self):
         '''Test if the request message stores RRs separately.'''
         # Specify 'drop' so the passed message won't be modified.