[#4288] Updated redact code

diff --git a/src/lib/process/redact_config.cc b/src/lib/process/redact_config.cc
index 2127c724d4b3e3653dde3b9a6ed392cdd0d770d3..79de2cad02e327c763baf246385491312ca74542 100644 (file)
--- a/src/lib/process/redact_config.cc
+++ b/src/lib/process/redact_config.cc
@@ -18,10 +18,14 @@ namespace {
 
 template <typename ElementPtrType>
 ElementPtrType
-redact(ElementPtrType const& element, list<string> json_path, string obscure) {
+    redact(ElementPtrType const& element, list<string> json_path,
+           string obscure, unsigned level) {
     if (!element) {
         isc_throw(BadValue, "redact() got a null pointer");
     }
+    if (level == 0) {
+        isc_throw(BadValue, "redact() got too deep recursion");
+    }
 
     string const next_key(json_path.empty() ? string() : json_path.front());
     ElementPtr result;
@@ -36,9 +40,9 @@ redact(ElementPtrType const& element, list<string> json_path, string obscure) {
             // Then redact all children.
             result = Element::createList();
             for (ElementPtr const& child : element->listValue()) {
-                result->add(redact(child, json_path, obscure));
+                result->add(redact(child, json_path, obscure, level - 1));
             }
-            return result;
+            return (result);
         }
     } else if (element->getType() == Element::map) {
         // If we are looking for anything or if we have reached the end of a
@@ -64,23 +68,25 @@ redact(ElementPtrType const& element, list<string> json_path, string obscure) {
                         result->set(key, value);
                     } else {
                         // We are looking for anything '*' so redact further.
-                        result->set(key, redact(value, json_path, obscure));
+                        result->set(key, redact(value, json_path, obscure,
+                                                level - 1));
                     }
                 }
             }
-            return result;
+            return (result);
         } else {
             ConstElementPtr child(element->get(next_key));
             if (child) {
                 result = isc::data::copy(element, 1);
                 json_path.pop_front();
-                result->set(next_key, redact(child, json_path, obscure));
-                return result;
+                result->set(next_key,
+                            redact(child, json_path, obscure, level - 1));
+                return (result);
             }
         }
     }
 
-    return element;
+    return (element);
 }
 
 }  // namespace
@@ -90,8 +96,8 @@ namespace process {
 
 ConstElementPtr
 redactConfig(ConstElementPtr const& element, list<string> const& json_path,
-             string obscure) {
-    return redact(element, json_path, obscure);
+             string obscure, unsigned max_nesting_depth) {
+    return (redact(element, json_path, obscure, max_nesting_depth));
 }
 
 }  // namespace process

diff --git a/src/lib/process/redact_config.h b/src/lib/process/redact_config.h
index 0b66e229707c8fcacd8db2da68fa4712a6cd3e5b..6ea5d3a06d6765a4da2945084281f9852b7648d9 100644 (file)
--- a/src/lib/process/redact_config.h
+++ b/src/lib/process/redact_config.h
@@ -26,13 +26,15 @@ namespace process {
 /// configuration and smaller subtrees in recursive calls.
 /// @param json_path JSON path to redact
 /// @param obscure new value of secrets / passwords
+/// @param max_nesting_depth maximum nesting depth
 ///
 /// @return a copy of the config where passwords and secrets were replaced by
 /// asterisks so it can be safely logged to an unprivileged place.
 isc::data::ConstElementPtr
 redactConfig(isc::data::ConstElementPtr const& element,
              std::list<std::string> const& json_path = {"*"},
-             std::string obscure = "*****");
+             std::string obscure = "*****",
+             unsigned max_nesting_depth = isc::data::Element::MAX_NESTING_LEVEL);
 
 } // namespace process
 } // namespace isc

diff --git a/src/lib/process/tests/d_cfg_mgr_unittests.cc b/src/lib/process/tests/d_cfg_mgr_unittests.cc
index 82fadcc2c9e9b9b2b90771d8286528aa26e9148b..467cace83dccf15be5ffbd106ddeb4e67add0ea3 100644 (file)
--- a/src/lib/process/tests/d_cfg_mgr_unittests.cc
+++ b/src/lib/process/tests/d_cfg_mgr_unittests.cc
@@ -313,6 +313,14 @@ TEST_F(DStubCfgMgrTest, redactConfig) {
     expected = "{ \"foo\": { \"password\": \"*****\" }, ";
     expected += "\"next\": { \"secret\": \"bar\" } }";
     EXPECT_EQ(expected, ret->str());
+
+    // Verify that it throws on cycles.
+    ElementPtr cycle = Element::createList();
+    cycle->add(cycle);
+    EXPECT_THROW(redactConfig(cycle), BadValue);
+    cycle = Element::createMap();
+    cycle->set("loop", cycle);
+    EXPECT_THROW(redactConfig(cycle), BadValue);
 }
 
 // Test that user context is not touched when configuration is redacted.