[1458] supported update ACL with TSIG.

on thinking about this in more detail, I deprecated UpdateSession's req_data.
the new assumption is that the caller should complete TSIG validation,
and for ACL check UpdateSession just uses the stored TSIG record in the
given message.

src/lib/python/isc/ddns/libddns_messages.mes

@@ -31,17 +31,27 @@ will simply return a response with an RCODE of NOTIMP to the client.
 The client's address and the zone name/class are logged.
 
 % LIBDDNS_UPDATE_NOTAUTH update client %1 for zone %2: not authoritative for update zone
-Debug message.  An update request for a zone for which the receiving
-server doesn't have authority.  In theory this is an unexpected event,
-but there are client implementations that could send update requests
-carelessly, so it may not necessarily be so uncommon in practice.  If
-possible, you may want to check the implementation or configuration of
-those clients to suppress the requests.  As specified in Section 3.1
-of RFC2136, the receiving server will return a response with an RCODE
-of NOTAUTH.
+Debug message.  An update request was received for a zone for which
+the receiving server doesn't have authority.  In theory this is an
+unexpected event, but there are client implementations that could send
+update requests carelessly, so it may not necessarily be so uncommon
+in practice.  If possible, you may want to check the implementation or
+configuration of those clients to suppress the requests.  As specified
+in Section 3.1 of RFC2136, the receiving server will return a response
+with an RCODE of NOTAUTH.
 
 % LIBDDNS_UPDATE_REFUSED update client %1 for zone %2 denied
-TBD
+Informational message.  An update request was denied because it was
+rejected by the zone's update ACL.  When this library is used by
+b10-ddns, the server will respond to the request with an RCODE of
+REFUSED as described in Section 3.3 of RFC2136.
 
 % LIBDDNS_UPDATE_DROPPED update client %1 for zone %2 dropped
-TBD
+Informational message.  An update request was denied because it was
+rejected by the zone's update ACL.  When this library is used by
+b10-ddns, the server will then completely ignore the request; no
+response will be sent.
+
+% LIBDDNS_UPDATE_APPROVED update client %1 for zone %2 approved
+Debug message.  An update request was approved in terms of the zone's
+update ACL.

src/lib/python/isc/ddns/logger.py

@@ -28,9 +28,11 @@ class ClientFormatter:
 
     This class is constructed with a Python standard socket address tuple.
     If it's 2-element tuple, it's assumed to be an IPv4 socket address
-    and will be converted to the form of '<addr>:<port>'.
+    and will be converted to the form of '<addr>:<port>(/key=<tsig-key>)'.
     If it's 4-element tuple, it's assumed to be an IPv6 socket address.
-    and will be converted to the form of '[<addr>]:<por>'.
+    and will be converted to the form of '[<addr>]:<por>(/key=<tsig-key>)'.
+    The optional key=<tsig-key> will be added if a TSIG record is given
+    on construction.  tsig-key is the TSIG key name in that case.
 
     This class is designed to delay the conversion until it's explicitly
     requested, so the conversion doesn't happen if the corresponding log
@@ -38,16 +40,23 @@ class ClientFormatter:
     for debug messages).
 
     """
-    def __init__(self, addr):
+    def __init__(self, addr, tsig_record=None):
         self.__addr = addr
+        self.__tsig_record = tsig_record
 
-    def __str__(self):
+    def __format_addr(self):
         if len(self.__addr) == 2:
             return self.__addr[0] + ':' + str(self.__addr[1])
         elif len(self.__addr) == 4:
             return '[' + self.__addr[0] + ']:' + str(self.__addr[1])
         return None
 
+    def __str__(self):
+        format = self.__format_addr()
+        if format is not None and self.__tsig_record is not None:
+            format += '/key=' + self.__tsig_record.get_name().to_text(True)
+        return format
+
 class ZoneFormatter:
     """A utility class to convert zone name and class to string.
 

src/lib/python/isc/ddns/session.py

@@ -67,30 +67,24 @@ class UpdateSession:
     class can use the message to send a response to the client.
 
     '''
-    def __init__(self, req_message, req_data, client_addr, zone_config):
+    def __init__(self, req_message, client_addr, zone_config):
         '''Constructor.
 
-        Note: req_data is not really used as of #1512 but is listed since
-        it's quite likely we need it in a subsequent task soon.  We'll
-        also need to get other parameters such as ACLs, for which, it's less
-        clear in which form we want to get the information, so it's left
-        open for now.
-
         Parameters:
         - req_message (isc.dns.Message) The request message.  This must be
-          in the PARSE mode.
-        - req_data (binary) Wire format data of the request message.
-          It will be used for TSIG verification if necessary.
+          in the PARSE mode, its Opcode must be UPDATE, and must have been
+          TSIG validatd if it's TSIG signed.
         - client_addr (socket address) The address/port of the update client
           in the form of Python socket address object.  This is mainly for
           logging and access control.
         - zone_config (ZoneConfig) A tentative container that encapsulates
           the server's zone configuration.  See zone_config.py.
-
-        (It'll soon need to be passed ACL in some way, too)
+        - req_data (binary) Wire format data of the request message.
+          It will be used for TSIG verification if necessary.
 
         '''
         self.__message = req_message
+        self.__tsig = req_message.get_tsig_record()
         self.__client_addr = client_addr
         self.__zone_config = zone_config
 
@@ -136,7 +130,7 @@ class UpdateSession:
         except UpdateError as e:
             if not e.nolog:
                 logger.debug(logger.DBGLVL_TRACE_BASIC, LIBDDNS_UPDATE_ERROR,
-                             ClientFormatter(self.__client_addr),
+                             ClientFormatter(self.__client_addr, self.__tsig),
                              ZoneFormatter(e.zname, e.zclass), e)
             # If RCODE is specified, create a corresponding resonse and return
             # ERROR; otherwise clear the message and return DROP.
@@ -177,30 +171,33 @@ class UpdateSession:
             # We are a secondary server; since we don't yet support update
             # forwarding, we return 'not implemented'.
             logger.debug(DBGLVL_TRACE_BASIC, LIBDDNS_UPDATE_FORWARD_FAIL,
-                         ClientFormatter(self.__client_addr),
+                         ClientFormatter(self.__client_addr, self.__tsig),
                          ZoneFormatter(zname, zclass))
             raise UpdateError('forward', zname, zclass, Rcode.NOTIMP(), True)
         # zone wasn't found
         logger.debug(DBGLVL_TRACE_BASIC, LIBDDNS_UPDATE_NOTAUTH,
-                     ClientFormatter(self.__client_addr),
+                     ClientFormatter(self.__client_addr, self.__tsig),
                      ZoneFormatter(zname, zclass))
         raise UpdateError('notauth', zname, zclass, Rcode.NOTAUTH(), True)
 
     def __check_update_acl(self, zname, zclass):
-        '''TBD'''
+        '''Apply update ACL for the zone to be updated.'''
         acl = self.__zone_config.get_update_acl(zname, zclass)
         action = acl.execute(isc.acl.dns.RequestContext(
-                (self.__client_addr[0], self.__client_addr[1])))
+                (self.__client_addr[0], self.__client_addr[1]), self.__tsig))
         if action == REJECT:
             logger.info(LIBDDNS_UPDATE_REFUSED,
-                        ClientFormatter(self.__client_addr),
+                        ClientFormatter(self.__client_addr, self.__tsig),
                         ZoneFormatter(zname, zclass))
             raise UpdateError('rejected', zname, zclass, Rcode.REFUSED(), True)
         if action == DROP:
             logger.info(LIBDDNS_UPDATE_DROPPED,
-                        ClientFormatter(self.__client_addr),
+                        ClientFormatter(self.__client_addr, self.__tsig),
                         ZoneFormatter(zname, zclass))
-            raise UpdateError('rejected', zname, zclass, None, True)
+            raise UpdateError('dropped', zname, zclass, None, True)
+        logger.debug(logger.DBGLVL_TRACE_BASIC, LIBDDNS_UPDATE_APPROVED,
+                     ClientFormatter(self.__client_addr, self.__tsig),
+                     ZoneFormatter(zname, zclass))
 
     def __make_response(self, rcode):
         '''Transform the internal message to the update response.

src/lib/python/isc/ddns/tests/session_tests.py

@@ -35,8 +35,10 @@ TEST_RRCLASS = RRClass.IN()
 TEST_ZONE_RECORD = Question(TEST_ZONE_NAME, TEST_RRCLASS, UPDATE_RRTYPE)
 TEST_CLIENT6 = ('2001:db8::1', 53, 0, 0)
 TEST_CLIENT4 = ('192.0.2.1', 53)
+# TSIG key for tests when needed.  The key name is TEST_ZONE_NAME.
+TEST_TSIG_KEY = TSIGKey("example.org:SFuWd/q99SzF8Yzd1QbB9g==")
 
-def create_update_msg(zones=[TEST_ZONE_RECORD]):
+def create_update_msg(zones=[TEST_ZONE_RECORD], tsig_key=None):
     msg = Message(Message.RENDER)
     msg.set_qid(5353)           # arbitrary chosen
     msg.set_opcode(Opcode.UPDATE())
@@ -45,35 +47,36 @@ def create_update_msg(zones=[TEST_ZONE_RECORD]):
         msg.add_question(z)
 
     renderer = MessageRenderer()
-    msg.to_wire(renderer)
+    if tsig_key is not None:
+        msg.to_wire(renderer, TSIGContext(tsig_key))
+    else:
+        msg.to_wire(renderer)
 
     # re-read the created data in the parse mode
     msg.clear(Message.PARSE)
     msg.from_wire(renderer.get_data())
 
-    return renderer.get_data(), msg
+    return msg
 
 class SesseionTestBase(unittest.TestCase):
     '''Base class for all sesion related tests.
 
-    It just initializes common test parameters in its setUp().
+    It just initializes common test parameters in its setUp() and defines
+    some common utility method(s).
 
     '''
     def setUp(self):
         shutil.copyfile(READ_ZONE_DB_FILE, WRITE_ZONE_DB_FILE)
         self._datasrc_client = DataSourceClient("sqlite3",
                                                 WRITE_ZONE_DB_CONFIG)
-        self._update_msgdata, self._update_msg = create_update_msg()
+        self._update_msg = create_update_msg()
         acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
                        REQUEST_LOADER.load([{"action": "ACCEPT"}])}
-        self._session = UpdateSession(self._update_msg,
-                                       self._update_msgdata, TEST_CLIENT4,
+        self._session = UpdateSession(self._update_msg, TEST_CLIENT4,
                                        ZoneConfig([], TEST_RRCLASS,
                                                   self._datasrc_client,
                                                   acl_map))
 
-class SessionTest(SesseionTestBase):
-    '''Basic session tests'''
     def check_response(self, msg, expected_rcode):
         '''Perform common checks on update resposne message.'''
         self.assertTrue(msg.get_header_flag(Message.HEADERFLAG_QR))
@@ -86,6 +89,9 @@ class SessionTest(SesseionTestBase):
         self.assertEqual(0, msg.get_rr_count(SECTION_UPDATE))
         self.assertEqual(0, msg.get_rr_count(Message.SECTION_ADDITIONAL))
 
+class SessionTest(SesseionTestBase):
+    '''Basic session tests'''
+
     def test_handle(self):
         '''Basic update case'''
         result, zname, zclass = self._session.handle()
@@ -99,8 +105,8 @@ class SessionTest(SesseionTestBase):
 
     def test_broken_request(self):
         # Zone section is empty
-        msg_data, msg = create_update_msg(zones=[])
-        session = UpdateSession(msg, msg_data, TEST_CLIENT6, None)
+        msg = create_update_msg(zones=[])
+        session = UpdateSession(msg, TEST_CLIENT6, None)
         result, zname, zclass = session.handle()
         self.assertEqual(UPDATE_ERROR, result)
         self.assertEqual(None, zname)
@@ -108,17 +114,15 @@ class SessionTest(SesseionTestBase):
         self.check_response(session.get_message(), Rcode.FORMERR())
 
         # Zone section contains multiple records
-        msg_data, msg = create_update_msg(zones=[TEST_ZONE_RECORD,
-                                                 TEST_ZONE_RECORD])
-        session = UpdateSession(msg, msg_data, TEST_CLIENT4, None)
+        msg = create_update_msg(zones=[TEST_ZONE_RECORD, TEST_ZONE_RECORD])
+        session = UpdateSession(msg, TEST_CLIENT4, None)
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.FORMERR())
 
         # Zone section's type is not SOA
-        msg_data, msg = create_update_msg(zones=[Question(TEST_ZONE_NAME,
-                                                          TEST_RRCLASS,
-                                                          RRType.A())])
-        session = UpdateSession(msg, msg_data, TEST_CLIENT4, None)
+        msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                                RRType.A())])
+        session = UpdateSession(msg, TEST_CLIENT4, None)
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.FORMERR())
 
@@ -126,24 +130,20 @@ class SessionTest(SesseionTestBase):
         # specified zone is configured as a secondary.  Since this
         # implementation doesn't support update forwarding, the result
         # should be NOTIMP.
-        msg_data, msg = create_update_msg(zones=[Question(TEST_ZONE_NAME,
-                                                          TEST_RRCLASS,
-                                                          RRType.SOA())])
-        session = UpdateSession(msg, msg_data, TEST_CLIENT4,
+        msg = create_update_msg(zones=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                                RRType.SOA())])
+        session = UpdateSession(msg, TEST_CLIENT4,
                                 ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
-                                           TEST_RRCLASS,
-                                           self._datasrc_client))
+                                           TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTIMP())
 
     def check_notauth(self, zname, zclass=TEST_RRCLASS):
         '''Common test sequence for the 'notauth' test'''
-        msg_data, msg = create_update_msg(zones=[Question(zname, zclass,
-                                                          RRType.SOA())])
-        session = UpdateSession(msg, msg_data, TEST_CLIENT4,
+        msg = create_update_msg(zones=[Question(zname, zclass, RRType.SOA())])
+        session = UpdateSession(msg, TEST_CLIENT4,
                                 ZoneConfig([(TEST_ZONE_NAME, TEST_RRCLASS)],
-                                           TEST_RRCLASS,
-                                           self._datasrc_client))
+                                           TEST_RRCLASS, self._datasrc_client))
         self.assertEqual(UPDATE_ERROR, session.handle()[0])
         self.check_response(session.get_message(), Rcode.NOTAUTH())
 
@@ -166,7 +166,7 @@ class SessionACLTest(SesseionTestBase):
 
         '''
         # create a separate session, with default (empty) ACL map.
-        session = UpdateSession(self._update_msg, self._update_msgdata,
+        session = UpdateSession(self._update_msg,
                                 TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
                                                          self._datasrc_client))
         # then the request should be rejected.
@@ -174,16 +174,43 @@ class SessionACLTest(SesseionTestBase):
 
         # recreate the request message, and test with an ACL that would result
         # in 'DROP'.  get_message() should return None.
-        msgdata, msg = create_update_msg()
+        msg = create_update_msg()
         acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
                        REQUEST_LOADER.load([{"action": "DROP", "from":
                                                  TEST_CLIENT4[0]}])}
-        session = UpdateSession(msg, msgdata, TEST_CLIENT4,
+        session = UpdateSession(msg, TEST_CLIENT4,
                                 ZoneConfig([], TEST_RRCLASS,
                                            self._datasrc_client, acl_map))
         self.assertEqual((UPDATE_DROP, None, None), session.handle())
         self.assertEqual(None, session.get_message())
 
+    def test_update_tsigacl_check(self):
+        '''Test for various ACL checks using TSIG.'''
+        # This ACL will accept requests from TEST_CLIENT4 (any port) *and*
+        # has TSIG signed by TEST_ZONE_NAME; all others will be rejected.
+        acl_map = {(TEST_ZONE_NAME, TEST_RRCLASS):
+                       REQUEST_LOADER.load([{"action": "ACCEPT",
+                                             "from": TEST_CLIENT4[0],
+                                             "key": TEST_ZONE_NAME.to_text()}])}
+
+        # If the message doesn't contain TSIG, it doesn't match the ACCEPT
+        # ACL entry, and the request should be rejected.
+        session = UpdateSession(self._update_msg,
+                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                                         self._datasrc_client,
+                                                         acl_map))
+        self.assertEqual((UPDATE_ERROR, None, None), session.handle())
+        self.check_response(session.get_message(), Rcode.REFUSED())
+
+        # If the message contains TSIG, it should match the ACCEPT
+        # ACL entry, and the request should be granted.
+        session = UpdateSession(create_update_msg(tsig_key=TEST_TSIG_KEY),
+                                TEST_CLIENT4, ZoneConfig([], TEST_RRCLASS,
+                                                         self._datasrc_client,
+                                                         acl_map))
+        self.assertEqual((UPDATE_SUCCESS, TEST_ZONE_NAME, TEST_RRCLASS),
+                         session.handle())
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()