Browse Source

[2524] Add method to redact password from access string

The access string is logged: as it includes the password, this
is removed from the message logged.
Stephen Morris 12 years ago
parent
commit
8ea9bddd35

+ 33 - 6
src/lib/dhcpsrv/lease_mgr_factory.cc

@@ -43,17 +43,17 @@ LeaseMgrFactory::getLeaseMgrPtr() {
 }
 
 LeaseMgr::ParameterMap
-LeaseMgrFactory::parse(const std::string& dbconfig) {
+LeaseMgrFactory::parse(const std::string& dbaccess) {
     LeaseMgr::ParameterMap mapped_tokens;
 
-    if (!dbconfig.empty()) {
+    if (!dbaccess.empty()) {
         vector<string> tokens;
 
         // We need to pass a string to is_any_of, not just char*. Otherwise
         // there are cryptic warnings on Debian6 running g++ 4.4 in
         // /usr/include/c++/4.4/bits/stl_algo.h:2178 "array subscript is above
         // array bounds"
-        boost::split(tokens, dbconfig, boost::is_any_of( string("\t ") ));
+        boost::split(tokens, dbaccess, boost::is_any_of( string("\t ") ));
         BOOST_FOREACH(std::string token, tokens) {
             size_t pos = token.find("=");
             if (pos != string::npos) {
@@ -70,12 +70,39 @@ LeaseMgrFactory::parse(const std::string& dbconfig) {
     return (mapped_tokens);
 }
 
+std::string
+LeaseMgrFactory::redactedAccessString(const LeaseMgr::ParameterMap& parameters) {
+    // Reconstruct the access string
+    std::string access = "";
+    for (LeaseMgr::ParameterMap::const_iterator i = parameters.begin();
+         i != parameters.end(); ++i) {
+
+        // Separate second and subsequent tokens.
+        if (!access.empty()) {
+            access += " ";
+        }
+
+        // Append name of parameter
+        access += i->first;
+        access += "=";
+
+        // Redact the password
+        if (i->first == std::string("password")) {
+            access += "*****";
+        } else {
+            access += i->second;
+        }
+    }
+
+    return (access);
+}
+
 void
-LeaseMgrFactory::create(const std::string& dbconfig) {
+LeaseMgrFactory::create(const std::string& dbaccess) {
     const std::string type = "type";
 
     // Is "type" present?
-    LeaseMgr::ParameterMap parameters = parse(dbconfig);
+    LeaseMgr::ParameterMap parameters = parse(dbaccess);
     if (parameters.find(type) == parameters.end()) {
         isc_throw(InvalidParameter, "Database configuration parameters do not "
                   "contain the 'type' keyword");
@@ -94,7 +121,7 @@ LeaseMgrFactory::create(const std::string& dbconfig) {
     }
 
     // Get here on no match
-    isc_throw(InvalidType, "Database configuration parameter 'type' does "
+    isc_throw(InvalidType, "Database access parameter 'type' does "
               "not specify a supported database backend");
 }
 

+ 23 - 12
src/lib/dhcpsrv/lease_mgr_factory.h

@@ -66,21 +66,21 @@ public:
     /// @note When called, the current lease manager is <b>always</b> destroyed
     ///       and a new one created - even if the parameters are the same.
     ///
-    /// dbconfig is a generic way of passing parameters. Parameters are passed
+    /// dbaccess is a generic way of passing parameters. Parameters are passed
     /// in the "name=value" format, separated by spaces.  The data MUST include
     /// a keyword/value pair of the form "type=dbtype" giving the database
     /// type, e.q. "mysql" or "sqlite3".
     ///
-    /// @param dbconfig Database configuration parameters.  These are in
-    ///        the form of "keyword=value" pairs, separated by spaces. These
-    ///        are back-end specific, although must include the "type" keyword
-    ///        which gives the backend in use.
+    /// @param dbaccess Database access parameters.  These are in the form of
+    ///        "keyword=value" pairs, separated by spaces. They are backend-
+    ///        -end specific, although must include the "type" keyword which
+    ///        gives the backend in use.
     ///
-    /// @throw isc::InvalidParameter dbconfig string does not contain the "type"
+    /// @throw isc::InvalidParameter dbaccess string does not contain the "type"
     ///        keyword.
-    /// @throw isc::dhcp::InvalidType The "type" keyword in dbconfig does not
+    /// @throw isc::dhcp::InvalidType The "type" keyword in dbaccess does not
     ///        identify a supported backend.
-    static void create(const std::string& dbconfig);
+    static void create(const std::string& dbaccess);
 
     /// @brief Destroy lease manager
     ///
@@ -89,7 +89,7 @@ public:
     /// lease manager is available.
     static void destroy();
 
-    /// @brief Return Current Lease Manager
+    /// @brief Return current lease manager
     ///
     /// Returns an instance of the "current" lease manager.  An exception
     /// will be thrown if none is available.
@@ -98,15 +98,26 @@ public:
     ///        create() to create one before calling this method.
     static LeaseMgr& instance();
 
-    /// @brief Parse Database Parameters
+    /// @brief Parse database access string
     ///
     /// Parses the string of "keyword=value" pairs and separates them
     /// out into the map.
     ///
-    /// @param dbconfig Database configuration string
+    /// @param dbaccess Database access string.
     ///
     /// @return std::map<std::string, std::string> Map of keyword/value pairs.
-    static LeaseMgr::ParameterMap parse(const std::string& dbconfig);
+    static LeaseMgr::ParameterMap parse(const std::string& dbaccess);
+
+    /// @brief Redact database access string
+    ///
+    /// Takes the database parameters and returns a database access string
+    /// passwords replaced by asterisks. This string is used in log messages.
+    ///
+    /// @param dbaccess Database access parameters (output of "parse").
+    ///
+    /// @return Redacted database access string.
+    static std::string redactedAccessString(
+            const LeaseMgr::ParameterMap& parameters);
 
 private:
     /// @brief Hold pointer to lease manager

+ 78 - 7
src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc

@@ -16,6 +16,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <exceptions/exceptions.h>
 
 #include <gtest/gtest.h>
 
@@ -37,16 +38,86 @@ public:
     }
 };
 
-// This test checks if the LeaseMgr can be instantiated and that it
-// parses parameters string properly.
+// This test checks that a database access string can be parsed correctly.
 TEST_F(LeaseMgrFactoryTest, parse) {
 
-    std::map<std::string, std::string> parameters = LeaseMgrFactory::parse(
-        "param1=value1 param2=value2 param3=value3");
+    LeaseMgr::ParameterMap parameters = LeaseMgrFactory::parse(
+        "user=me password=forbidden name=kea somethingelse= type=mysql");
 
-    EXPECT_EQ("value1", parameters["param1"]);
-    EXPECT_EQ("value2", parameters["param2"]);
-    EXPECT_TRUE(parameters.find("type") == parameters.end());
+    EXPECT_EQ(5, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("forbidden", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+    EXPECT_EQ("", parameters["somethingelse"]);
+}
+
+// This test checks that an invalid database access string behaves as expected.
+TEST_F(LeaseMgrFactoryTest, parseInvalid) {
+
+    // No tokens in the string, so we expect no parameters
+    std::string invalid = "";
+    LeaseMgr::ParameterMap parameters = LeaseMgrFactory::parse(invalid);
+    EXPECT_EQ(0, parameters.size());
+
+    // With spaces, there are some tokens so we expect invalid parameter
+    // as there are no equals signs.
+    invalid = "   \t  ";
+    EXPECT_THROW(LeaseMgrFactory::parse(invalid), isc::InvalidParameter);
+
+    invalid = "   noequalshere  ";
+    EXPECT_THROW(LeaseMgrFactory::parse(invalid), isc::InvalidParameter);
+
+    // A single "=" is valid string, but is placed here as the result is
+    // expected to be nothing.
+    invalid = "=";
+    parameters = LeaseMgrFactory::parse(invalid);
+    EXPECT_EQ(1, parameters.size());
+    EXPECT_EQ("", parameters[""]);
+}
+
+/// @brief redactConfigString test
+///
+/// Checks that the redacted configuration string includes the password only
+/// as a set of asterisks.
+TEST(LeaseMgr, redactAccessString) {
+
+    // To check the redacted string, break it down into its component
+    // parameters and check the results.
+    LeaseMgr::ParameterMap parameters = LeaseMgrFactory::parse(
+        "user=me password=forbidden name=kea type=mysql");
+    std::string redacted = LeaseMgrFactory::redactedAccessString(parameters);
+
+    // ... and break the redacted string down into its components.
+    parameters = LeaseMgrFactory::parse(redacted);
+
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("*****", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
+    // Do the same, but check it works for an empty password.
+    parameters = LeaseMgrFactory::parse(
+        "user=me password=forbidden name=kea type=mysql");
+    redacted = LeaseMgrFactory::redactedAccessString(parameters);
+    parameters = LeaseMgrFactory::parse(redacted);
+
+    EXPECT_EQ(4, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("*****", parameters["password"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
+
+    // Do the same, but check it works in the absence of a password token
+    parameters = LeaseMgrFactory::parse("user=me name=kea type=mysql");
+    redacted = LeaseMgrFactory::redactedAccessString(parameters);
+    parameters = LeaseMgrFactory::parse(redacted);
+
+    EXPECT_EQ(3, parameters.size());
+    EXPECT_EQ("me", parameters["user"]);
+    EXPECT_EQ("kea", parameters["name"]);
+    EXPECT_EQ("mysql", parameters["type"]);
 }
 
 }; // end of anonymous namespace