Merge #16115: On bitcoind startup, write config args to debug.log

MarcoFalke · MarcoFalke · commit 1d1f8bbf5711 · 2020-01-31T11:10:56.000+13:00
b951b09 on startup, write config options to debug.log (Larry Ruane) Pull request description: When a developer is examining `debug.log` after something goes wrong, it's often useful to know the exact options the failing instance of `bitcoind` was started with. Sometimes the `debug.log` file is all that's available for the analysis. This PR logs the `bitcoin.conf` entries and command-line arguments to `debug.log` on startup. ACKs for top commit: MarcoFalke: ACK b951b09 🐪 jonatack: ACK b951b09 reviewed diff, re-code review, built, ran tests, launched bitcoind and reviewed debug log output, verified value of `str` debug log in the added unit test. Tree-SHA512: bbca4fb3d49f99261758302bde0b8b67300ccc72e7380b01f1f66a146ae8a008a045df0ca5ca9664caff034d0ee38ea7ef38a50f38374525608c07ba52790358
diff --git a/src/init.cpp b/src/init.cpp
@@ -436,7 +436,7 @@ void SetupServerArgs()
     gArgs.AddArg("-timeout=<n>", strprintf("Specify connection timeout in milliseconds (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-peertimeout=<n>", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    gArgs.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION);
     gArgs.AddArg("-asmap=<file>", "Specify asn mapping used for bucketing of the peers. Path should be relative to the -datadir path.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
 #ifdef USE_UPNP
 #if USE_UPNP
@@ -537,15 +537,15 @@ void SetupServerArgs()
 
     gArgs.AddArg("-rest", strprintf("Accept public REST requests (default: %u)", DEFAULT_REST_ENABLE), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     gArgs.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
-    gArgs.AddArg("-rpcauth=<userpw>", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field <userpw> comes in the format: <USERNAME>:<SALT>$<HASH>. A canonical python script is included in share/rpcauth. The client then connects normally using the rpcuser=<USERNAME>/rpcpassword=<PASSWORD> pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
-    gArgs.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC);
+    gArgs.AddArg("-rpcauth=<userpw>", "Username and HMAC-SHA-256 hashed password for JSON-RPC connections. The field <userpw> comes in the format: <USERNAME>:<SALT>$<HASH>. A canonical python script is included in share/rpcauth. The client then connects normally using the rpcuser=<USERNAME>/rpcpassword=<PASSWORD> pair of arguments. This option can be specified multiple times", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC);
+    gArgs.AddArg("-rpcbind=<addr>[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY | ArgsManager::SENSITIVE, OptionsCategory::RPC);
     gArgs.AddArg("-rpccookiefile=<loc>", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
-    gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC);
     gArgs.AddArg("-rpcport=<port>", strprintf("Listen for JSON-RPC connections on <port> (default: %u, testnet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC);
     gArgs.AddArg("-rpcserialversion", strprintf("Sets the serialization of raw transaction or block hex returned in non-verbose mode, non-segwit(0) or segwit(1) (default: %d)", DEFAULT_RPC_SERIALIZE_VERSION), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     gArgs.AddArg("-rpcservertimeout=<n>", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     gArgs.AddArg("-rpcthreads=<n>", strprintf("Set the number of threads to service RPC calls (default: %d)", DEFAULT_HTTP_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
-    gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
+    gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC);
     gArgs.AddArg("-rpcwhitelist=<whitelist>", "Set a whitelist to filter incoming RPC calls for a specific user. The field <whitelist> comes in the format: <USERNAME>:<rpc 1>,<rpc 2>,...,<rpc n>. If multiple whitelists are set for a given user, they are set-intersected. See -rpcwhitelistdefault documentation for information on default whitelist behavior.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
     gArgs.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_BOOL, OptionsCategory::RPC);
     gArgs.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
@@ -1230,6 +1230,9 @@ bool AppInitMain(NodeContext& node)
         LogPrintf("Config file: %s (not found, skipping)\n", config_file_path.string());
     }
 
+    // Log the config arguments to debug.log
+    gArgs.LogArgs();
+
     LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, nFD);
 
     // Warn about relative -datadir path.
diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
@@ -183,4 +183,32 @@ BOOST_AUTO_TEST_CASE(boolargno)
     BOOST_CHECK(gArgs.GetBoolArg("-foo", false));
 }
 
+BOOST_AUTO_TEST_CASE(logargs)
+{
+    const auto okaylog_bool = std::make_pair("-okaylog-bool", ArgsManager::ALLOW_BOOL);
+    const auto okaylog_negbool = std::make_pair("-okaylog-negbool", ArgsManager::ALLOW_BOOL);
+    const auto okaylog = std::make_pair("-okaylog", ArgsManager::ALLOW_ANY);
+    const auto dontlog = std::make_pair("-dontlog", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE);
+    SetupArgs({okaylog_bool, okaylog_negbool, okaylog, dontlog});
+    ResetArgs("-okaylog-bool -nookaylog-negbool -okaylog=public -dontlog=private");
+
+    // Everything logged to debug.log will also append to str
+    std::string str;
+    auto print_connection = LogInstance().PushBackCallback(
+        [&str](const std::string& s) {
+            str += s;
+        });
+
+    // Log the arguments
+    gArgs.LogArgs();
+
+    LogInstance().DeleteCallback(print_connection);
+    // Check that what should appear does, and what shouldn't doesn't.
+    BOOST_CHECK(str.find("Command-line arg: okaylog-bool=\"\"") != std::string::npos);
+    BOOST_CHECK(str.find("Command-line arg: okaylog-negbool=false") != std::string::npos);
+    BOOST_CHECK(str.find("Command-line arg: okaylog=\"public\"") != std::string::npos);
+    BOOST_CHECK(str.find("dontlog=****") != std::string::npos);
+    BOOST_CHECK(str.find("private") == std::string::npos);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util/system.cpp b/src/util/system.cpp
@@ -864,6 +864,32 @@ std::vector<util::SettingsValue> ArgsManager::GetSettingsList(const std::string&
     return util::GetSettingsList(m_settings, m_network, SettingName(arg), !UseDefaultSection(arg));
 }
 
+void ArgsManager::logArgsPrefix(
+    const std::string& prefix,
+    const std::string& section,
+    const std::map<std::string, std::vector<util::SettingsValue>>& args) const
+{
+    std::string section_str = section.empty() ? "" : "[" + section + "] ";
+    for (const auto& arg : args) {
+        for (const auto& value : arg.second) {
+            Optional<unsigned int> flags = GetArgFlags('-' + arg.first);
+            if (flags) {
+                std::string value_str = (*flags & SENSITIVE) ? "****" : value.write();
+                LogPrintf("%s %s%s=%s\n", prefix, section_str, arg.first, value_str);
+            }
+        }
+    }
+}
+
+void ArgsManager::LogArgs() const
+{
+    LOCK(cs_args);
+    for (const auto& section : m_settings.ro_config) {
+        logArgsPrefix("Config file arg:", section.first, section.second);
+    }
+    logArgsPrefix("Command-line arg:", "", m_settings.command_line_options);
+}
+
 bool RenameOver(fs::path src, fs::path dest)
 {
 #ifdef WIN32
diff --git a/src/util/system.h b/src/util/system.h
@@ -145,6 +145,8 @@ class ArgsManager
          * between mainnet and regtest/testnet won't cause problems due to these
          * parameters by accident. */
         NETWORK_ONLY = 0x200,
+        // This argument's value is sensitive (such as a password).
+        SENSITIVE = 0x400,
     };
 
 protected:
@@ -318,6 +320,19 @@ class ArgsManager
      * Return nullopt for unknown arg.
      */
     Optional<unsigned int> GetArgFlags(const std::string& name) const;
+
+    /**
+     * Log the config file options and the command line arguments,
+     * useful for troubleshooting.
+     */
+    void LogArgs() const;
+
+private:
+    // Helper function for LogArgs().
+    void logArgsPrefix(
+        const std::string& prefix,
+        const std::string& section,
+        const std::map<std::string, std::vector<util::SettingsValue>>& args) const;
 };
 
 extern ArgsManager gArgs;
diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
@@ -83,10 +83,40 @@ def test_log_buffer(self):
             self.start_node(0, extra_args=['-noconnect=0'])
         self.stop_node(0)
 
+    def test_args_log(self):
+        self.log.info('Test config args logging')
+        with self.nodes[0].assert_debug_log(
+                expected_msgs=[
+                    'Command-line arg: addnode="some.node"',
+                    'Command-line arg: rpcauth=****',
+                    'Command-line arg: rpcbind=****',
+                    'Command-line arg: rpcpassword=****',
+                    'Command-line arg: rpcuser=****',
+                    'Command-line arg: torpassword=****',
+                    'Config file arg: regtest="1"',
+                    'Config file arg: [regtest] server="1"',
+                ],
+                unexpected_msgs=[
+                    'alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0',
+                    '127.1.1.1',
+                    'secret-rpcuser',
+                    'secret-torpassword',
+                ]):
+            self.start_node(0, extra_args=[
+                '-addnode=some.node',
+                '-rpcauth=alice:f7efda5c189b999524f151318c0c86$d5b51b3beffbc0',
+                '-rpcbind=127.1.1.1',
+                '-rpcpassword=',
+                '-rpcuser=secret-rpcuser',
+                '-torpassword=secret-torpassword',
+            ])
+        self.stop_node(0)
+
     def run_test(self):
         self.stop_node(0)
 
         self.test_log_buffer()
+        self.test_args_log()
 
         self.test_config_file_parser()
 
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
@@ -298,7 +298,9 @@ def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT):
         wait_until(self.is_node_stopped, timeout=timeout)
 
     @contextlib.contextmanager
-    def assert_debug_log(self, expected_msgs, timeout=2):
+    def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2):
+        if unexpected_msgs is None:
+            unexpected_msgs = []
         time_end = time.time() + timeout
         debug_log = os.path.join(self.datadir, self.chain, 'debug.log')
         with open(debug_log, encoding='utf-8') as dl:
@@ -313,6 +315,9 @@ def assert_debug_log(self, expected_msgs, timeout=2):
                 dl.seek(prev_size)
                 log = dl.read()
             print_log = " - " + "\n - ".join(log.splitlines())
+            for unexpected_msg in unexpected_msgs:
+                if re.search(re.escape(unexpected_msg), log, flags=re.MULTILINE):
+                    self._raise_assertion_error('Unexpected message "{}" partially matches log:\n\n{}\n\n'.format(unexpected_msg, print_log))
             for expected_msg in expected_msgs:
                 if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None:
                     found = False
