From: Thomas Markwalder Date: Wed, 10 Sep 2025 19:10:25 +0000 (-0400) Subject: [#4086] Redact schema init parameters X-Git-Tag: Kea-3.1.2~21 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d3adbffc0c5f9f06443e5e0b8948660b1db91591;p=thirdparty%2Fkea.git [#4086] Redact schema init parameters new file: changelog_unreleased/4086-password-leaked-to-logs /src/lib/asiolink/process_spawn.* ProcessSpawnImpl::getCommandLine() - modified to accept ProcessSpawn::getCommandLine() - modified to accept a list of arguments whose values should be redacted /src/lib/asiolink/tests/process_spawn_unittest.cc TEST_F(ProcessSpawnTest, getCommandLineRedacted) - new test /src/lib/mysql/mysql_connection.cc MySqlConnection::initializeSchema() - add list of args to redact to getCommandLine() call /src/lib/pgsql/pgsql_connection.cc PgSqlConnection::initializeSchema() - add list of args to redact to getCommandLine() call --- diff --git a/changelog_unreleased/4086-password-leaked-to-logs b/changelog_unreleased/4086-password-leaked-to-logs new file mode 100644 index 0000000000..386a631dd2 --- /dev/null +++ b/changelog_unreleased/4086-password-leaked-to-logs @@ -0,0 +1,5 @@ +[sec] tmark + Removed logging of database password as clear + text when kea-dhcp4 or kea-dhcp6 initialize + the schema. + (Gitlab #4086) diff --git a/src/lib/asiolink/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index 1d91075cef..5bc868d01e 100644 --- a/src/lib/asiolink/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -93,7 +93,8 @@ public: ~ProcessSpawnImpl(); /// @brief Returns full command line, including arguments, for the process. - std::string getCommandLine() const; + /// @param redact_args list of arguments to redact. + std::string getCommandLine(std::unordered_set redact_args = {}) const; /// @brief Spawn the new process. /// @@ -316,15 +317,26 @@ ProcessSpawnImpl::~ProcessSpawnImpl() { } std::string -ProcessSpawnImpl::getCommandLine() const { +ProcessSpawnImpl::getCommandLine(std::unordered_set redact_args /* = {} */) const { std::ostringstream s; s << executable_; // Start with index 1, because the first argument duplicates the // path to the executable. Note, that even if there are no parameters // the minimum size of the table is 2. int i = 1; + bool redact_next = false; while (args_[i] != NULL) { - s << " " << args_[i]; + if (redact_next) { + s << " " << "*****"; + redact_next = false; + } else { + if (redact_args.contains(args_[i])) { + redact_next = true; + } + + s << " " << args_[i]; + } + ++i; } return (s.str()); @@ -489,8 +501,8 @@ ProcessSpawn::ProcessSpawn(const SpawnMode mode, } std::string -ProcessSpawn::getCommandLine() const { - return (impl_->getCommandLine()); +ProcessSpawn::getCommandLine(std::unordered_set redact_args /* = {} */) const { + return (impl_->getCommandLine(redact_args)); } pid_t diff --git a/src/lib/asiolink/process_spawn.h b/src/lib/asiolink/process_spawn.h index 394dc95728..aabdaad09d 100644 --- a/src/lib/asiolink/process_spawn.h +++ b/src/lib/asiolink/process_spawn.h @@ -15,6 +15,8 @@ #include #include +#include + namespace isc { namespace asiolink { @@ -85,7 +87,9 @@ public: ~ProcessSpawn() = default; /// @brief Returns full command line, including arguments, for the process. - std::string getCommandLine() const; + /// + /// @param redact_args list of arguments to redact + std::string getCommandLine(std::unordered_set redact_args = {}) const; /// @brief Spawn the new process. /// diff --git a/src/lib/asiolink/tests/process_spawn_unittest.cc b/src/lib/asiolink/tests/process_spawn_unittest.cc index 6ea7e69627..10a898c21b 100644 --- a/src/lib/asiolink/tests/process_spawn_unittest.cc +++ b/src/lib/asiolink/tests/process_spawn_unittest.cc @@ -579,4 +579,30 @@ TEST_F(ProcessSpawnTest, inheritEnvWithParentVarSync) { EXPECT_EQ(34, process.getExitStatus(pid)); } +// This test verifies that the full command line for the process is +// returned with specific arguments redacted. +TEST_F(ProcessSpawnTest, getCommandLineRedacted) { + { + // Case 1: arguments present. + ProcessArgs args; + args.push_back("db-init"); + args.push_back("mysql"); + args.push_back("--host"); + args.push_back("example.com"); + args.push_back("--user"); + args.push_back("someone"); + args.push_back("--password"); + args.push_back("sesame"); + args.push_back("--other"); + args.push_back("stuff"); + args.push_back("foo"); + + ProcessSpawn process(ProcessSpawn::ASYNC, TEST_SCRIPT_SH, args); + std::string expected = TEST_SCRIPT_SH; + expected += " db-init mysql --host example.com --user ***** --password ***** --other stuff foo"; + std::unordered_set redact_args = { "--user", "--password", "--not-there" }; + EXPECT_EQ(expected, process.getCommandLine(redact_args)); + } +} + } // end of anonymous namespace diff --git a/src/lib/mysql/mysql_connection.cc b/src/lib/mysql/mysql_connection.cc index ba7006d64d..8dc0c315a3 100644 --- a/src/lib/mysql/mysql_connection.cc +++ b/src/lib/mysql/mysql_connection.cc @@ -477,7 +477,8 @@ MySqlConnection::initializeSchema(const ParameterMap& parameters) { // Run. ProcessSpawn kea_admin(ProcessSpawn::SYNC, KEA_ADMIN_, kea_admin_parameters, vars, /* inherit_env = */ true); - DB_LOG_INFO(MYSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine()); + DB_LOG_INFO(MYSQL_INITIALIZE_SCHEMA) + .arg(kea_admin.getCommandLine(std::unordered_set{"--user", "--password"})); pid_t const pid(kea_admin.spawn()); if (kea_admin.isRunning(pid)) { isc_throw(SchemaInitializationFailed, "kea-admin still running"); diff --git a/src/lib/pgsql/pgsql_connection.cc b/src/lib/pgsql/pgsql_connection.cc index 65284f12c3..cf8d82410e 100644 --- a/src/lib/pgsql/pgsql_connection.cc +++ b/src/lib/pgsql/pgsql_connection.cc @@ -233,7 +233,8 @@ PgSqlConnection::initializeSchema(const ParameterMap& parameters) { // Run. ProcessSpawn kea_admin(ProcessSpawn::SYNC, KEA_ADMIN_, kea_admin_parameters, vars, /* inherit_env = */ true); - DB_LOG_INFO(PGSQL_INITIALIZE_SCHEMA).arg(kea_admin.getCommandLine()); + DB_LOG_INFO(PGSQL_INITIALIZE_SCHEMA) + .arg(kea_admin.getCommandLine(std::unordered_set{"--user", "--password"})); pid_t const pid(kea_admin.spawn()); if (kea_admin.isRunning(pid)) { isc_throw(SchemaInitializationFailed, "kea-admin still running");