From: Francis Dupont Date: Sat, 21 Dec 2024 21:40:24 +0000 (+0100) Subject: [#3398] Address src/bin UTs X-Git-Tag: Kea-2.7.6~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e129c7cd20d2f7d8bc63b56846e3a195ca7faaa9;p=thirdparty%2Fkea.git [#3398] Address src/bin UTs --- diff --git a/src/bin/agent/tests/get_config_unittest.cc b/src/bin/agent/tests/get_config_unittest.cc index 40d9c502f9..bbe01a9ab9 100644 --- a/src/bin/agent/tests/get_config_unittest.cc +++ b/src/bin/agent/tests/get_config_unittest.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -189,6 +190,9 @@ public: // update authentication directory dirReplacer(ca); + // redact passwords + ca = redactConfig(ca, { "*" }, "-----"); + // try AGENT configure ConstElementPtr status; try { diff --git a/src/bin/agent/tests/testdata/get_config.json b/src/bin/agent/tests/testdata/get_config.json index 4072ca3163..b7afd468e6 100644 --- a/src/bin/agent/tests/testdata/get_config.json +++ b/src/bin/agent/tests/testdata/get_config.json @@ -3,7 +3,7 @@ "authentication": { "clients": [ { - "password": "1234", + "password": "-----", "user": "admin", "user-context": { "comment": "admin is authorized" diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index feee4ba4bc..d0731b92c6 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -993,7 +993,7 @@ TEST_F(D2CfgMgrTest, comments) { " \"clients\": [ {" " \"comment\": \"admin is authorized\"," " \"user\": \"admin\"," - " \"password\": \"1234\"" + " \"password\": \"foobar\"" " } ]" " }" "}" diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index ced71238fe..ec868a1b3b 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -257,7 +257,7 @@ const char* PARSER_CONFIGS[] = { " \"clients\": [ {" " \"comment\": \"admin is authorized\"," " \"user\": \"admin\"," - " \"password\": \"1234\"" + " \"password\": \"foobar\"" " } ]" " }" " }" @@ -7069,7 +7069,7 @@ TEST_F(Dhcp4ParserTest, comments) { ASSERT_TRUE(client->get("user")); ASSERT_EQ("\"admin\"", client->get("user")->str()); ASSERT_TRUE(client->get("password")); - ASSERT_EQ("\"1234\"", client->get("password")->str()); + ASSERT_EQ("\"foobar\"", client->get("password")->str()); ConstElementPtr ctx_client = client->get("user-context"); ASSERT_TRUE(ctx_client); ASSERT_EQ(1, ctx_client->size()); diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 79d4da4617..297b686758 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -62,6 +63,7 @@ using namespace isc::data; using namespace isc::db; using namespace isc::dhcp; using namespace isc::dhcp::test; +using namespace isc::process; using namespace isc::util; using namespace std; @@ -2933,23 +2935,33 @@ Dhcpv4SrvTest::loadConfigFile(const string& path) { mutable_config->set(string("hooks-libraries"), Element::createList()); // Remove TLS parameters ConstElementPtr hosts = dhcp4->get("hosts-database"); - removeTlsParameters(hosts); + if (hosts) { + removeTlsParameters(hosts); + hosts = redactConfig(hosts, { "*" }, "keatest"); + mutable_config->set("hosts-database", hosts); + } hosts = dhcp4->get("hosts-databases"); if (hosts) { for (auto const& host : hosts->listValue()) { removeTlsParameters(host); } + hosts = redactConfig(hosts, { "*" }, "keatest"); + mutable_config->set("hosts-databases", hosts); } // Remove authentication clients using files. ConstElementPtr control_sockets = dhcp4->get("control-socket"); if (control_sockets) { removeAuthFiles(control_sockets); + control_sockets = redactConfig(control_sockets, { "*" }, "-----"); + mutable_config->set("control-socket", control_sockets); } control_sockets = dhcp4->get("control-sockets"); if (control_sockets) { for (int i = 0; i < control_sockets->size(); ++i) { removeAuthFiles(control_sockets->get(i)); } + control_sockets = redactConfig(control_sockets, { "*" }, "-----"); + mutable_config->set("control-sockets", control_sockets); } ASSERT_NO_THROW(Dhcpv4SrvTest::configure(dhcp4->str(), true, true, true, true)); diff --git a/src/bin/dhcp4/tests/get_config_unittest.cc b/src/bin/dhcp4/tests/get_config_unittest.cc index 3bb0907616..eca36f8112 100644 --- a/src/bin/dhcp4/tests/get_config_unittest.cc +++ b/src/bin/dhcp4/tests/get_config_unittest.cc @@ -2101,7 +2101,7 @@ const char* EXTRACTED_CONFIGS[] = { " \"authentication\": {\n" " \"clients\": [\n" " {\n" -" \"password\": \"1234\",\n" +" \"password\": \"foobar\",\n" " \"user\": \"admin\",\n" " \"user-context\": {\n" " \"comment\": \"admin is authorized\"\n" @@ -11602,7 +11602,7 @@ const char* UNPARSED_CONFIGS[] = { " \"authentication\": {\n" " \"clients\": [\n" " {\n" -" \"password\": \"1234\",\n" +" \"password\": \"foobar\",\n" " \"user\": \"admin\",\n" " \"user-context\": {\n" " \"comment\": \"admin is authorized\"\n" diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 7a8f60bc30..709bc9466a 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -338,7 +338,7 @@ const char* PARSER_CONFIGS[] = { " \"clients\": [ {" " \"comment\": \"admin is authorized\"," " \"user\": \"admin\"," - " \"password\": \"1234\"" + " \"password\": \"foobar\"" " } ]" " }" " }" @@ -7882,7 +7882,7 @@ TEST_F(Dhcp6ParserTest, comments) { ASSERT_TRUE(client->get("user")); ASSERT_EQ("\"admin\"", client->get("user")->str()); ASSERT_TRUE(client->get("password")); - ASSERT_EQ("\"1234\"", client->get("password")->str()); + ASSERT_EQ("\"foobar\"", client->get("password")->str()); ConstElementPtr ctx_client = client->get("user-context"); ASSERT_TRUE(ctx_client); ASSERT_EQ(1, ctx_client->size()); diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc index a81ee9d89e..dc3cca9231 100644 --- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc +++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -63,6 +64,7 @@ using namespace isc::data; using namespace isc::db; using namespace isc::dhcp; using namespace isc::dhcp::test; +using namespace isc::process; using namespace isc::util; using namespace std; @@ -304,23 +306,33 @@ Dhcpv6SrvTest::loadConfigFile(const string& path) { mutable_config->set(string("hooks-libraries"), Element::createList()); // Remove TLS parameters ConstElementPtr hosts = dhcp6->get("hosts-database"); - removeTlsParameters(hosts); + if (hosts) { + removeTlsParameters(hosts); + hosts = redactConfig(hosts, { "*" }, "keatest"); + mutable_config->set("hosts-database", hosts); + } hosts = dhcp6->get("hosts-databases"); if (hosts) { for (auto const& host : hosts->listValue()) { removeTlsParameters(host); } + hosts = redactConfig(hosts, { "*" }, "keatest"); + mutable_config->set("hosts-databases", hosts); } // Remove authentication clients using files. ConstElementPtr control_sockets = dhcp6->get("control-socket"); if (control_sockets) { removeAuthFiles(control_sockets); + control_sockets = redactConfig(control_sockets, { "*" }, "-----"); + mutable_config->set("control-socket", control_sockets); } control_sockets = dhcp6->get("control-sockets"); if (control_sockets) { for (int i = 0; i < control_sockets->size(); ++i) { removeAuthFiles(control_sockets->get(i)); } + control_sockets = redactConfig(control_sockets, { "*" }, "-----"); + mutable_config->set("control-sockets", control_sockets); } ASSERT_NO_THROW(Dhcpv6SrvTest::configure(dhcp6->str(), true, true, true, true)); diff --git a/src/bin/dhcp6/tests/get_config_unittest.cc b/src/bin/dhcp6/tests/get_config_unittest.cc index 156f60a908..09745e792d 100644 --- a/src/bin/dhcp6/tests/get_config_unittest.cc +++ b/src/bin/dhcp6/tests/get_config_unittest.cc @@ -1995,7 +1995,7 @@ const char* EXTRACTED_CONFIGS[] = { " \"authentication\": {\n" " \"clients\": [\n" " {\n" -" \"password\": \"1234\",\n" +" \"password\": \"foobar\",\n" " \"user\": \"admin\",\n" " \"user-context\": {\n" " \"comment\": \"admin is authorized\"\n" @@ -11398,7 +11398,7 @@ const char* UNPARSED_CONFIGS[] = { " \"authentication\": {\n" " \"clients\": [\n" " {\n" -" \"password\": \"1234\",\n" +" \"password\": \"foobar\",\n" " \"user\": \"admin\",\n" " \"user-context\": {\n" " \"comment\": \"admin is authorized\"\n" diff --git a/src/lib/process/redact_config.cc b/src/lib/process/redact_config.cc index 101084368c..9034b7128b 100644 --- a/src/lib/process/redact_config.cc +++ b/src/lib/process/redact_config.cc @@ -18,7 +18,7 @@ namespace { template ElementPtrType -redact(ElementPtrType const& element, list json_path) { +redact(ElementPtrType const& element, list json_path, string obscure) { if (!element) { isc_throw(BadValue, "redact() got a null pointer"); } @@ -36,7 +36,7 @@ redact(ElementPtrType const& element, list json_path) { // Then redact all children. result = Element::createList(); for (ElementPtr const& child : element->listValue()) { - result->add(redact(child, json_path)); + result->add(redact(child, json_path, obscure)); } return result; } @@ -53,7 +53,7 @@ redact(ElementPtrType const& element, list json_path) { if (boost::algorithm::ends_with(key, "password") || boost::algorithm::ends_with(key, "secret")) { // Sensitive data - result->set(key, Element::create(string("*****"))); + result->set(key, Element::create(obscure)); } else if (key == "user-context") { // Skip user contexts. result->set(key, value); @@ -64,7 +64,7 @@ redact(ElementPtrType const& element, list json_path) { result->set(key, value); } else { // We are looking for anything '*' so redact further. - result->set(key, redact(value, json_path)); + result->set(key, redact(value, json_path, obscure)); } } } @@ -74,7 +74,7 @@ redact(ElementPtrType const& element, list json_path) { if (child) { result = isc::data::copy(element, 1); json_path.pop_front(); - result->set(next_key, redact(child, json_path)); + result->set(next_key, redact(child, json_path, obscure)); return result; } } @@ -89,8 +89,9 @@ namespace isc { namespace process { ConstElementPtr -redactConfig(ConstElementPtr const& element, list const& json_path) { - return redact(element, json_path); +redactConfig(ConstElementPtr const& element, list const& json_path, + string obscure) { + return redact(element, json_path, obscure); } } // namespace process diff --git a/src/lib/process/redact_config.h b/src/lib/process/redact_config.h index a0d1d0a33e..bf2173fe6d 100644 --- a/src/lib/process/redact_config.h +++ b/src/lib/process/redact_config.h @@ -17,19 +17,22 @@ namespace process { /// /// This method walks on the configuration tree: /// - it copies only subtrees where a change was done. -/// - it replaces passwords and secrets by asterisks. +/// - it replaces passwords and secrets by obscure argument +/// (default 5 asterisks). /// - it skips user context. /// - if a not empty list of keywords is given it follows only them. /// /// @param element initially the Element tree structure that describe the /// configuration and smaller subtrees in recursive calls. /// @param json_path JSON path to redact +/// @param obscure new value of secrets / passwords /// /// @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 const& json_path = {"*"}); + std::list const& json_path = {"*"}, + std::string obscure = "*****"); } // namespace process } // namespace isc