BIND 10 trac1351, updated. 0cf505f04be53e6f198cb0d091bd90d74e597cbe [1351] Use TSIGKeyRing class in xfrin

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Dec 10 21:40:22 UTC 2012


The branch, trac1351 has been updated
       via  0cf505f04be53e6f198cb0d091bd90d74e597cbe (commit)
       via  669a650ec38562f5459a5144cf8549df1b3f1cc4 (commit)
      from  b73262553443c404f3211913272a2ff89be904da (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 0cf505f04be53e6f198cb0d091bd90d74e597cbe
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Dec 10 22:39:52 2012 +0100

    [1351] Use TSIGKeyRing class in xfrin
    
    Also updated said class (added a find(name) without algorithm)

commit 669a650ec38562f5459a5144cf8549df1b3f1cc4
Author: Jelte Jansen <jelte at isc.org>
Date:   Mon Dec 10 21:49:23 2012 +0100

    [1351] Update docstring of set_tsig_key_name()

-----------------------------------------------------------------------

Summary of changes:
 src/bin/xfrin/tests/xfrin_test.py               |    8 +++--
 src/bin/xfrin/xfrin.py.in                       |   39 ++++++++---------------
 src/lib/dns/python/tests/tsigkey_python_test.py |    6 ++++
 src/lib/dns/python/tsigkey_python.cc            |   17 ++++++----
 src/lib/dns/tests/tsigkey_unittest.cc           |    9 ++++++
 src/lib/dns/tsigkey.cc                          |   12 ++++++-
 src/lib/dns/tsigkey.h                           |   22 +++++++++++++
 7 files changed, 79 insertions(+), 34 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index fcfe986..b7fe056 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -26,6 +26,7 @@ from xfrin import *
 import xfrin
 from isc.xfrin.diff import Diff
 import isc.log
+from isc.server_common.tsig_keyring import init_keyring, get_keyring
 # If we use any python library that is basically a wrapper for
 # a library we use as well (like sqlite3 in our datasources),
 # we must make sure we import ours first; If we have special
@@ -139,10 +140,12 @@ class MockCC(MockModuleCCSession):
         if identifier == "zones/use_ixfr":
             return False
 
+    def add_remote_config_by_name(self, name, callback):
+        pass
+
     def get_remote_config_value(self, module, identifier):
         if module == 'tsig_keys' and identifier == 'keys':
-            return (['example.com.key.:EvAAsfU2h7uofnmqaTCrhHunGsc=',
-                     'bad.key.:EvAAsfU2h7uofnmqaTCrhHu' ], True)
+            return (['example.com.key.:EvAAsfU2h7uofnmqaTCrhHunGsc='], True)
         else:
             raise Exception('MockCC requested for unknown config value ' +
                             + module + "/" + identifier)
@@ -237,6 +240,7 @@ class MockXfrin(Xfrin):
     def _cc_setup(self):
         self._tsig_key = None
         self._module_cc = MockCC()
+        init_keyring(self._module_cc)
         pass
 
     def _get_db_file(self):
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index d3621e0..811869d 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -34,6 +34,7 @@ from isc.datasrc import DataSourceClient, ZoneFinder
 import isc.net.parse
 from isc.xfrin.diff import Diff
 from isc.server_common.auth_command import auth_loadzone_command
+from isc.server_common.tsig_keyring import init_keyring, get_keyring
 from isc.log_messages.xfrin_messages import *
 
 isc.log.init("b10-xfrin")
@@ -72,8 +73,6 @@ XFROUT_MODULE_NAME = 'Xfrout'
 
 # Remote module and identifiers (according to their spec files)
 ZONE_MANAGER_MODULE_NAME = 'Zonemgr'
-TSIG_KEYS_MODULE_NAME = 'tsig_keys'
-TSIG_KEYS_IDENTIFIER = 'keys'
 
 REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr'
 
@@ -1246,10 +1245,12 @@ class ZoneInfo:
                 raise XfrinZoneInfoException(errmsg)
 
     def set_tsig_key_name(self, tsig_key_str):
-        """Set the tsig_key for this zone, given a TSIG key string
-           representation. If tsig_key_str is None, no TSIG key will
-           be set. Raises XfrinZoneInfoException if tsig_key_str cannot
-           be parsed. TODO UPDATE"""
+        """Set the name of the tsig_key for this zone. If tsig_key_str
+           is None, no TSIG key will be used. This name is used to
+           find the TSIG key to use for transfers in the global TSIG
+           key ring.
+           Raises XfrinZoneInfoException if tsig_key_str is not a valid
+           (dns) name."""
         if tsig_key_str is None:
             self.tsig_key_name = None
         else:
@@ -1263,24 +1264,12 @@ class ZoneInfo:
     def get_tsig_key(self):
         if self.tsig_key_name is None:
             return None
-        key_strings, _ = self._module_cc.get_remote_config_value(
-                                  TSIG_KEYS_MODULE_NAME,
-                                  TSIG_KEYS_IDENTIFIER)
-        # Find the TSIG key in the keyring
-        for possible_key_string in key_strings:
-            try:
-                key = TSIGKey(possible_key_string)
-            except InvalidParameter as ipe:
-                # In theory, bad keys should never end up here (but be refused
-                # by the validator for TSIG keys config. However,
-                # should they do, we simply log and ignore them
-                logger.error(XFRIN_BAD_TSIG_KEY_STRING, possible_key_string)
-            if key.get_key_name() == self.tsig_key_name:
-                return key
-        logger.error(XFRIN_TSIG_KEY_NOT_FOUND, self.tsig_key_name.to_text())
-        errmsg = "TSIG key not found in keyring: " +\
-                 self.tsig_key_name.to_text()
-        raise XfrinZoneInfoException(errmsg)
+        result, key = get_keyring().find(self.tsig_key_name)
+        if result != isc.dns.TSIGKeyRing.SUCCESS:
+            raise XfrinZoneInfoException("TSIG key not found in keyring: " +
+                                         self.tsig_key_name.to_text())
+        else:
+            return key
 
     def set_use_ixfr(self, use_ixfr):
         """Set use_ixfr. If set to True, it will use
@@ -1335,7 +1324,7 @@ class Xfrin:
         self.config_handler(config_data)
         self._module_cc.add_remote_config(AUTH_SPECFILE_LOCATION,
                                           self._auth_config_handler)
-        self._module_cc.add_remote_config_by_name(TSIG_KEYS_MODULE_NAME)
+        init_keyring(self._module_cc)
 
     def _cc_check_command(self):
         '''This is a straightforward wrapper for cc.check_command,
diff --git a/src/lib/dns/python/tests/tsigkey_python_test.py b/src/lib/dns/python/tests/tsigkey_python_test.py
index 516bea4..ca7a61e 100644
--- a/src/lib/dns/python/tests/tsigkey_python_test.py
+++ b/src/lib/dns/python/tests/tsigkey_python_test.py
@@ -170,6 +170,12 @@ class TSIGKeyRingTest(unittest.TestCase):
         self.assertEqual(TSIGKey.HMACSHA256_NAME, key.get_algorithm_name())
         self.assertEqual(self.secret, key.get_secret())
 
+        (code, key) = self.keyring.find(self.key_name)
+        self.assertEqual(TSIGKeyRing.SUCCESS, code)
+        self.assertEqual(self.key_name, key.get_key_name())
+        self.assertEqual(TSIGKey.HMACSHA256_NAME, key.get_algorithm_name())
+        self.assertEqual(self.secret, key.get_secret())
+
         (code, key) = self.keyring.find(Name('different-key.example'),
                                         self.sha256_name)
         self.assertEqual(TSIGKeyRing.NOTFOUND, code)
diff --git a/src/lib/dns/python/tsigkey_python.cc b/src/lib/dns/python/tsigkey_python.cc
index cf79c1a..bcab59c 100644
--- a/src/lib/dns/python/tsigkey_python.cc
+++ b/src/lib/dns/python/tsigkey_python.cc
@@ -287,7 +287,9 @@ PyMethodDef TSIGKeyRing_methods[] = {
       METH_VARARGS,
       "Remove a TSIGKey for the given name from the TSIGKeyRing." },
     { "find", reinterpret_cast<PyCFunction>(TSIGKeyRing_find), METH_VARARGS,
-      "Find a TSIGKey for the given name in the TSIGKeyRing. "
+      "Find a TSIGKey for the given name in the TSIGKeyRing. Optional "
+      "second argument is an algorithm, in which case it only returns "
+      "a key if both match.\n"
       "It returns a tuple of (result_code, key)." },
     { NULL, NULL, 0, NULL }
 };
@@ -362,13 +364,16 @@ TSIGKeyRing_remove(const s_TSIGKeyRing* self, PyObject* args) {
 PyObject*
 TSIGKeyRing_find(const s_TSIGKeyRing* self, PyObject* args) {
     PyObject* key_name;
-    PyObject* algorithm_name;
+    PyObject* algorithm_name = NULL;
 
-    if (PyArg_ParseTuple(args, "O!O!", &name_type, &key_name,
+    if (PyArg_ParseTuple(args, "O!|O!", &name_type, &key_name,
                          &name_type, &algorithm_name)) {
-        const TSIGKeyRing::FindResult result =
-            self->cppobj->find(PyName_ToName(key_name),
-                               PyName_ToName(algorithm_name));
+        // Can't init TSIGKeyRing::FindResult without actual result,
+        // so use ternary operator
+        TSIGKeyRing::FindResult result = (algorithm_name == NULL) ?
+                    self->cppobj->find(PyName_ToName(key_name)) :
+                    self->cppobj->find(PyName_ToName(key_name),
+                                       PyName_ToName(algorithm_name));
         if (result.key != NULL) {
             s_TSIGKey* key = PyObject_New(s_TSIGKey, &tsigkey_type);
             if (key == NULL) {
diff --git a/src/lib/dns/tests/tsigkey_unittest.cc b/src/lib/dns/tests/tsigkey_unittest.cc
index 20ee802..f58da1c 100644
--- a/src/lib/dns/tests/tsigkey_unittest.cc
+++ b/src/lib/dns/tests/tsigkey_unittest.cc
@@ -251,6 +251,15 @@ TEST_F(TSIGKeyRingTest, find) {
     const TSIGKeyRing::FindResult result3 = keyring.find(key_name, md5_name);
     EXPECT_EQ(TSIGKeyRing::NOTFOUND, result3.code);
     EXPECT_EQ(static_cast<const TSIGKey*>(NULL), result3.key);
+
+    // But with just the name it should work
+    const TSIGKeyRing::FindResult result4(keyring.find(key_name));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, result1.code);
+    EXPECT_EQ(key_name, result1.key->getKeyName());
+    EXPECT_EQ(TSIGKey::HMACSHA256_NAME(), result1.key->getAlgorithmName());
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret, secret_len,
+                        result1.key->getSecret(),
+                        result1.key->getSecretLength());
 }
 
 TEST_F(TSIGKeyRingTest, findFromSome) {
diff --git a/src/lib/dns/tsigkey.cc b/src/lib/dns/tsigkey.cc
index d7d60eb..e55cce3 100644
--- a/src/lib/dns/tsigkey.cc
+++ b/src/lib/dns/tsigkey.cc
@@ -51,7 +51,7 @@ namespace {
         if (name == TSIGKey::HMACSHA512_NAME()) {
             return (isc::cryptolink::SHA512);
         }
- 
+
         return (isc::cryptolink::UNKNOWN_HASH);
     }
 }
@@ -270,6 +270,16 @@ TSIGKeyRing::remove(const Name& key_name) {
 }
 
 TSIGKeyRing::FindResult
+TSIGKeyRing::find(const Name& key_name) const {
+    TSIGKeyRingImpl::TSIGKeyMap::const_iterator found =
+        impl_->keys.find(key_name);
+    if (found == impl_->keys.end()) {
+        return (FindResult(NOTFOUND, NULL));
+    }
+    return (FindResult(SUCCESS, &((*found).second)));
+}
+
+TSIGKeyRing::FindResult
 TSIGKeyRing::find(const Name& key_name, const Name& algorithm_name) const {
     TSIGKeyRingImpl::TSIGKeyMap::const_iterator found =
         impl_->keys.find(key_name);
diff --git a/src/lib/dns/tsigkey.h b/src/lib/dns/tsigkey.h
index 1bbd3fe..b10660c 100644
--- a/src/lib/dns/tsigkey.h
+++ b/src/lib/dns/tsigkey.h
@@ -327,6 +327,27 @@ public:
     /// Find a \c TSIGKey for the given name in the \c TSIGKeyRing.
     ///
     /// It searches the internal storage for a \c TSIGKey whose name is
+    /// \c key_name.
+    /// It returns the result in the form of a \c FindResult
+    /// object as follows:
+    /// - \c code: \c SUCCESS if a key is found; otherwise \c NOTFOUND.
+    /// - \c key: A pointer to the found \c TSIGKey object if one is found;
+    /// otherwise \c NULL.
+    ///
+    /// The pointer returned in the \c FindResult object is only valid until
+    /// the corresponding key is removed from the key ring.
+    /// The caller must ensure that the key is held in the key ring while
+    /// it needs to refer to it, or it must make a local copy of the key.
+    ///
+    /// This method never throws an exception.
+    ///
+    /// \param key_name The name of the key to be found.
+    /// \return A \c FindResult object enclosing the search result (see above).
+    FindResult find(const Name& key_name) const;
+
+    /// Find a \c TSIGKey for the given name in the \c TSIGKeyRing.
+    ///
+    /// It searches the internal storage for a \c TSIGKey whose name is
     /// \c key_name and that uses the hash algorithm identified by
     /// \c algorithm_name.
     /// It returns the result in the form of a \c FindResult
@@ -346,6 +367,7 @@ public:
     /// \param algorithm_name The name of the algorithm of the found key.
     /// \return A \c FindResult object enclosing the search result (see above).
     FindResult find(const Name& key_name, const Name& algorithm_name) const;
+
 private:
     struct TSIGKeyRingImpl;
     TSIGKeyRingImpl* impl_;



More information about the bind10-changes mailing list