From 0c3cdb2142f79b2be95e3fd389d3d0fc5a83daac Mon Sep 17 00:00:00 2001 From: Jonathan Reams Date: Fri, 17 Aug 2018 16:12:30 -0400 Subject: SERVER-36744 Fix and add tests for command-line redaction (cherry picked from commit 3a37add7837885d0fe31eb44480769969a330f77) --- src/mongo/client/mongo_uri_test.cpp | 30 ++++++++++++ src/mongo/shell/SConscript | 10 ++++ src/mongo/shell/dbshell.cpp | 14 +----- src/mongo/shell/shell_options.cpp | 44 ++++++++++++++++++ src/mongo/shell/shell_options.h | 2 + src/mongo/shell/shell_options_test.cpp | 83 ++++++++++++++++++++++++++++++++++ 6 files changed, 170 insertions(+), 13 deletions(-) create mode 100644 src/mongo/shell/shell_options_test.cpp diff --git a/src/mongo/client/mongo_uri_test.cpp b/src/mongo/client/mongo_uri_test.cpp index d934bcde049..4d8088ea015 100644 --- a/src/mongo/client/mongo_uri_test.cpp +++ b/src/mongo/client/mongo_uri_test.cpp @@ -871,5 +871,35 @@ TEST(MongoURI, srvRecordTest) { } } +/* + * Checks that redacting various secret info from URIs produces actually redacted URIs. + * Also checks that SRV URI's don't turn into non-SRV URIs after redaction. + */ +TEST(MongoURI, Redact) { + constexpr auto goodWithDBName = "mongodb://admin@localhost/admin"_sd; + constexpr auto goodWithoutDBName = "mongodb://admin@localhost"_sd; + constexpr auto goodWithOnlyDBAndHost = "mongodb://localhost/admin"_sd; + const std::initializer_list> testCases = { + {"mongodb://admin:password@localhost/admin"_sd, goodWithDBName}, + {"mongodb://admin@localhost/admin?secretConnectionOption=foo"_sd, goodWithDBName}, + {"mongodb://admin:password@localhost/admin?secretConnectionOptions"_sd, goodWithDBName}, + {"mongodb://admin@localhost/admin"_sd, goodWithDBName}, + {"mongodb://admin@localhost/admin?secretConnectionOptions", goodWithDBName}, + {"mongodb://admin:password@localhost"_sd, goodWithoutDBName}, + {"mongodb://admin@localhost", goodWithoutDBName}, + {"mongodb://localhost/admin?socketTimeoutMS=5", goodWithOnlyDBAndHost}, + {"mongodb://localhost/admin", goodWithOnlyDBAndHost}, + }; + + for (const auto& testCase : testCases) { + ASSERT_TRUE(MongoURI::isMongoURI(testCase.first)); + ASSERT_EQ(MongoURI::redact(testCase.first), testCase.second); + } + + const auto toRedactSRV = "mongodb+srv://admin:password@localhost/admin?secret=foo"_sd; + const auto redactedSRV = "mongodb+srv://admin@localhost/admin"_sd; + ASSERT_EQ(MongoURI::redact(toRedactSRV), redactedSRV); +} + } // namespace } // namespace mongo diff --git a/src/mongo/shell/SConscript b/src/mongo/shell/SConscript index f91c475762c..79e956ab06e 100644 --- a/src/mongo/shell/SConscript +++ b/src/mongo/shell/SConscript @@ -76,3 +76,13 @@ env.Library( "$BUILD_DIR/mongo/base", ], ) + +env.CppUnitTest( + target='shell_options_test', + source=[ + "shell_options_test.cpp", + ], + LIBDEPS=[ + "$BUILD_DIR/mongo/shell_core", + ], +) diff --git a/src/mongo/shell/dbshell.cpp b/src/mongo/shell/dbshell.cpp index 5ae0a512271..7c834f8835f 100644 --- a/src/mongo/shell/dbshell.cpp +++ b/src/mongo/shell/dbshell.cpp @@ -758,19 +758,7 @@ int _main(int argc, char* argv[], char** envp) { uassertStatusOK(tlPtr->start()); // hide password from ps output - for (int i = 0; i < (argc - 1); ++i) { - StringData arg(argv[i]); - if (arg == "-p"_sd || arg == "--password"_sd) { - char* arg = argv[i + 1]; - while (*arg) { - *arg++ = 'x'; - } - } else if (MongoURI::isMongoURI(arg)) { - auto reformedURI = MongoURI::redact(arg); - auto length = arg.size(); - ::strncpy(argv[i], reformedURI.data(), length); - } - } + redactPasswordOptions(argc, argv); if (!mongo::serverGlobalParams.quiet.load()) cout << mongoShellVersion(VersionInfoInterface::instance()) << endl; diff --git a/src/mongo/shell/shell_options.cpp b/src/mongo/shell/shell_options.cpp index 632874055ae..68c3888cbd2 100644 --- a/src/mongo/shell/shell_options.cpp +++ b/src/mongo/shell/shell_options.cpp @@ -489,4 +489,48 @@ Status storeMongoShellOptions(const moe::Environment& params, return Status::OK(); } + +void redactPasswordOptions(int argc, char** argv) { + constexpr auto kLongPasswordOption = "--password"_sd; + constexpr auto kShortPasswordOption = "-p"_sd; + for (int i = 0; i < argc; ++i) { + StringData arg(argv[i]); + if (arg.startsWith(kShortPasswordOption)) { + char* toRedact = nullptr; + // Handle -p password + if ((arg == kShortPasswordOption) && (i + 1 < argc)) { + toRedact = argv[++i]; + // Handle -ppassword + } else { + toRedact = argv[i] + kShortPasswordOption.size(); + } + + invariant(toRedact); + // The arg should be null-terminated, replace everything up to \0 to 'x' + while (*toRedact) { + *toRedact++ = 'x'; + } + } + if (arg.startsWith(kLongPasswordOption)) { + char* toRedact; + // Handle --password password + if ((arg == kLongPasswordOption) && (i + 1 < argc)) { + toRedact = argv[++i]; + // Handle --password=password + } else { + toRedact = argv[i] + kLongPasswordOption.size(); + // It's not valid to do --passwordpassword, make sure there's an = separator + invariant(*(toRedact++) == '='); + } + // The arg should be null-terminated, replace everything up to \0 to 'x' + while (*toRedact) { + *toRedact++ = 'x'; + } + } else if (MongoURI::isMongoURI(arg)) { + auto reformedURI = MongoURI::redact(arg); + auto length = arg.size(); + ::strncpy(argv[i], reformedURI.data(), length); + } + } +} } // namespace mongo diff --git a/src/mongo/shell/shell_options.h b/src/mongo/shell/shell_options.h index 87ce7ec48fb..c0a66fb35e2 100644 --- a/src/mongo/shell/shell_options.h +++ b/src/mongo/shell/shell_options.h @@ -95,4 +95,6 @@ bool handlePreValidationMongoShellOptions(const moe::Environment& params, const std::vector& args); Status storeMongoShellOptions(const moe::Environment& params, const std::vector& args); + +void redactPasswordOptions(int argc, char** argv); } diff --git a/src/mongo/shell/shell_options_test.cpp b/src/mongo/shell/shell_options_test.cpp new file mode 100644 index 00000000000..994e8a07060 --- /dev/null +++ b/src/mongo/shell/shell_options_test.cpp @@ -0,0 +1,83 @@ +/** + * Copyright (C) 2018 MongoDB Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the GNU Affero General Public License in all respects + * for all of the code used other than as permitted herein. If you modify + * file(s) with this exception, you may extend this exception to your + * version of the file(s), but you are not obligated to do so. If you do not + * wish to do so, delete this exception statement from your version. If you + * delete this exception statement from all source files in the program, + * then also delete it in the license file. + */ + +#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kDefault; + +#include "shell_options.h" + +#include +#include + +#include "mongo/unittest/unittest.h" +#include "mongo/util/log.h" + +namespace mongo { +namespace { + +TEST(ShellOptions, RedactPasswords) { + std::vector, const std::vector>> testData = { + // Check that just passwords get redacted correctly + {{"-u", "admin", "-p", "password", "--port", "27017"}, // NOLINT + {"-u"_sd, "admin"_sd, "-p"_sd, "xxxxxxxx"_sd, "--port"_sd, "27017"_sd}}, // NOLINT + // Check that passwords and URIs get redacted correctly + {{"-p", "password", "mongodb://admin:password@localhost"}, // NOLINT + {"-p"_sd, "xxxxxxxx", "mongodb://admin@localhost"_sd}}, // NOLINT + // Check that just URIs get redacted correctly + {{"mongodb://admin:password@localhost"}, // NOLINT + {"mongodb://admin@localhost"_sd}}, // NOLINT + // Sanity check that non-passwords don't get redacted + {{"localhost"}, // NOLINT + {"localhost"_sd}}, // NOLINT + // Sanity check that we don't overflow argv + {{"-p"}, // NOLINT + {"-p"_sd}}, // NOLINT + // Check for --passsword=foo + {{"--password=foo"}, // NOLINT + {"--password=xxx"_sd}}, // NOLINT + {{"-ppassword"}, // NOLINT + {"-pxxxxxxxx"}}, // NOLINT + }; + + for (auto& testCase : testData) { + // Sanity check for the test data + ASSERT_EQ(testCase.first.size(), testCase.second.size()); + std::vector argv; + for (const auto& arg : testCase.first) { + argv.push_back(const_cast(&arg.front())); + } + + redactPasswordOptions(argv.size(), &argv.front()); + for (size_t i = 0; i < testCase.first.size(); i++) { + auto shrunkArg = testCase.first[i]; + shrunkArg.resize(::strlen(shrunkArg.c_str())); + ASSERT_EQ(shrunkArg, testCase.second[i]); + } + } +} +} // namespace +} // namespace mongo -- cgit v1.2.1