From 1d83352dadf600f9acd777ca7530920204d78700 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 3 Jan 2017 16:57:15 +0100 Subject: [PATCH] Merge #8877: Qt RPC console: history sensitive-data filter, and saving input line when browsing history 8562792 GUI/RPCConsole: Include importmulti in history sensitive-command filter (Luke Dashjr) ff77faf Qt/RPCConsole: Use RPCParseCommandLine to perform command filtering (Luke Dashjr) a79598d Qt/Test: Make sure filtering sensitive data works correctly in nested commands (Luke Dashjr) 629cd42 Qt/RPCConsole: Teach RPCParseCommandLine how to filter out arguments to sensitive commands (Luke Dashjr) e2d9213 Qt/RPCConsole: Make it possible to parse a command without executing it (Luke Dashjr) 1755c04 Qt/RPCConsole: Truncate filtered commands to just the command name, rather than skip it entirely in history (Luke Dashjr) d80a006 Qt/RPCConsole: Add signmessagewithprivkey to list of commands filtered from history (Luke Dashjr) afde12f Qt/RPCConsole: Refactor command_may_contain_sensitive_data function out of RPCConsole::on_lineEdit_returnPressed (Luke Dashjr) de8980d Bugfix: Do not add sensitive information to history for real (Luke Dashjr) 9044908 Qt/RPCConsole: Don't store commands with potentially sensitive information in the history (Jonas Schnelli) fc95daa Qt/RPCConsole: Save current command entry when browsing history (Jonas Schnelli) --- src/qt/rpcconsole.cpp | 117 ++++++++++++++++++++++++++++----- src/qt/rpcconsole.h | 6 +- src/qt/test/rpcnestedtests.cpp | 28 +++++++- 3 files changed, 131 insertions(+), 20 deletions(-) diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index fe096d444..054d5252a 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -73,6 +74,20 @@ const struct { {NULL, NULL} }; +namespace { + +// don't add private key handling cmd's to the history +const QStringList historyFilter = QStringList() + << "importprivkey" + << "importmulti" + << "signmessagewithprivkey" + << "signrawtransaction" + << "walletpassphrase" + << "walletpassphrasechange" + << "encryptwallet"; + +} + /* Object for executing console RPC commands in a separate thread. */ class RPCExecutor : public QObject @@ -123,10 +138,10 @@ public: #include "rpcconsole.moc" /** - * Split shell command line into a list of arguments and execute the command(s). + * Split shell command line into a list of arguments and optionally execute the command(s). * Aims to emulate \c bash and friends. * - * - Command nesting is possible with brackets [example: validateaddress(getnewaddress())] + * - Command nesting is possible with parenthesis; for example: validateaddress(getnewaddress()) * - Arguments are delimited with whitespace or comma * - Extra whitespace at the beginning and end and between arguments will be ignored * - Text can be "double" or 'single' quoted @@ -137,9 +152,11 @@ public: * * @param[out] result stringified Result from the executed command(chain) * @param[in] strCommand Command line to split + * @param[in] fExecute set true if you want the command to be executed + * @param[out] pstrFilteredOut Command line, filtered to remove any sensitive data */ -bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string &strCommand) +bool RPCConsole::RPCParseCommandLine(std::string &strResult, const std::string &strCommand, const bool fExecute, std::string * const pstrFilteredOut) { std::vector< std::vector > stack; stack.push_back(std::vector()); @@ -159,12 +176,35 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string } state = STATE_EATING_SPACES; std::string curarg; UniValue lastResult; + unsigned nDepthInsideSensitive = 0; + size_t filter_begin_pos = 0, chpos; + std::vector> filter_ranges; + + auto add_to_current_stack = [&](const std::string& curarg) { + if (stack.back().empty() && (!nDepthInsideSensitive) && historyFilter.contains(QString::fromStdString(curarg), Qt::CaseInsensitive)) { + nDepthInsideSensitive = 1; + filter_begin_pos = chpos; + } + stack.back().push_back(curarg); + }; + + auto close_out_params = [&]() { + if (nDepthInsideSensitive) { + if (!--nDepthInsideSensitive) { + assert(filter_begin_pos); + filter_ranges.push_back(std::make_pair(filter_begin_pos, chpos)); + filter_begin_pos = 0; + } + } + stack.pop_back(); + }; std::string strCommandTerminated = strCommand; if (strCommandTerminated.back() != '\n') strCommandTerminated += "\n"; - for(char ch: strCommandTerminated) + for (chpos = 0; chpos < strCommandTerminated.size(); ++chpos) { + char ch = strCommandTerminated[chpos]; switch(state) { case STATE_COMMAND_EXECUTED_INNER: @@ -183,7 +223,7 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string curarg += ch; break; } - if (curarg.size()) + if (curarg.size() && fExecute) { // if we have a value query, query arrays with index and objects with a string key UniValue subelement; @@ -208,7 +248,7 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string breakParsing = false; // pop the stack and return the result to the current command arguments - stack.pop_back(); + close_out_params(); // don't stringify the json in case of a string to avoid doublequotes if (lastResult.isStr()) @@ -220,7 +260,7 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string if (curarg.size()) { if (stack.size()) - stack.back().push_back(curarg); + add_to_current_stack(curarg); else strResult = curarg; } @@ -246,25 +286,31 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string if (state == STATE_ARGUMENT) { if (ch == '(' && stack.size() && stack.back().size() > 0) + { + if (nDepthInsideSensitive) { + ++nDepthInsideSensitive; + } stack.push_back(std::vector()); + } // don't allow commands after executed commands on baselevel if (!stack.size()) throw std::runtime_error("Invalid Syntax"); - stack.back().push_back(curarg); + add_to_current_stack(curarg); curarg.clear(); state = STATE_EATING_SPACES_IN_BRACKETS; } if ((ch == ')' || ch == '\n') && stack.size() > 0) { - std::string strPrint; - // Convert argument list to JSON objects in method-dependent way, - // and pass it along with the method name to the dispatcher. - JSONRPCRequest req; - req.params = RPCConvertValues(stack.back()[0], std::vector(stack.back().begin() + 1, stack.back().end())); - req.strMethod = stack.back()[0]; - lastResult = tableRPC.execute(req); + if (fExecute) { + // Convert argument list to JSON objects in method-dependent way, + // and pass it along with the method name to the dispatcher. + JSONRPCRequest req; + req.params = RPCConvertValues(stack.back()[0], std::vector(stack.back().begin() + 1, stack.back().end())); + req.strMethod = stack.back()[0]; + lastResult = tableRPC.execute(req); + } state = STATE_COMMAND_EXECUTED; curarg.clear(); @@ -276,7 +322,7 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string else if(state == STATE_ARGUMENT) // Space ends argument { - stack.back().push_back(curarg); + add_to_current_stack(curarg); curarg.clear(); } if ((state == STATE_EATING_SPACES_IN_BRACKETS || state == STATE_ARGUMENT) && ch == ',') @@ -313,6 +359,16 @@ bool RPCConsole::RPCExecuteCommandLine(std::string &strResult, const std::string break; } } + if (pstrFilteredOut) { + if (STATE_COMMAND_EXECUTED == state) { + assert(!stack.empty()); + close_out_params(); + } + *pstrFilteredOut = strCommand; + for (auto i = filter_ranges.rbegin(); i != filter_ranges.rend(); ++i) { + pstrFilteredOut->replace(i->first, i->second - i->first, "(…)"); + } + } switch(state) // final state { case STATE_COMMAND_EXECUTED: @@ -841,12 +897,30 @@ void RPCConsole::setMempoolSize(long numberOfTxs, size_t dynUsage) void RPCConsole::on_lineEdit_returnPressed() { QString cmd = ui->lineEdit->text(); - ui->lineEdit->clear(); if(!cmd.isEmpty()) { + std::string strFilteredCmd; + try { + std::string dummy; + if (!RPCParseCommandLine(dummy, cmd.toStdString(), false, &strFilteredCmd)) { + // Failed to parse command, so we cannot even filter it for the history + throw std::runtime_error("Invalid command line"); + } + } catch (const std::exception& e) { + QMessageBox::critical(this, "Error", QString("Error: ") + QString::fromStdString(e.what())); + return; + } + + ui->lineEdit->clear(); + + cmdBeforeBrowsing = QString(); + message(CMD_REQUEST, cmd); Q_EMIT cmdRequest(cmd); + + cmd = QString::fromStdString(strFilteredCmd); + // Remove command, if already in history history.removeOne(cmd); // Append command to history @@ -856,6 +930,7 @@ void RPCConsole::on_lineEdit_returnPressed() history.removeFirst(); // Set pointer to end of history historyPtr = history.size(); + // Scroll console view to end scrollToEnd(); } @@ -863,6 +938,11 @@ void RPCConsole::on_lineEdit_returnPressed() void RPCConsole::browseHistory(int offset) { + // store current text when start browsing through the history + if (historyPtr == history.size()) { + cmdBeforeBrowsing = ui->lineEdit->text(); + } + historyPtr += offset; if(historyPtr < 0) historyPtr = 0; @@ -871,6 +951,9 @@ void RPCConsole::browseHistory(int offset) QString cmd; if(historyPtr < history.size()) cmd = history.at(historyPtr); + else if (!cmdBeforeBrowsing.isNull()) { + cmd = cmdBeforeBrowsing; + } ui->lineEdit->setText(cmd); } diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 83e4f7558..d31762e03 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -37,7 +37,10 @@ public: explicit RPCConsole(const PlatformStyle *platformStyle, QWidget *parent); ~RPCConsole(); - static bool RPCExecuteCommandLine(std::string &strResult, const std::string &strCommand); + static bool RPCParseCommandLine(std::string &strResult, const std::string &strCommand, bool fExecute, std::string * const pstrFilteredOut = NULL); + static bool RPCExecuteCommandLine(std::string &strResult, const std::string &strCommand, std::string * const pstrFilteredOut = NULL) { + return RPCParseCommandLine(strResult, strCommand, true, pstrFilteredOut); + } void setClientModel(ClientModel *model); @@ -157,6 +160,7 @@ private: ClientModel *clientModel; QStringList history; int historyPtr; + QString cmdBeforeBrowsing; QList cachedNodeids; const PlatformStyle *platformStyle; RPCTimerInterface *rpcTimerInterface; diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index a87643038..66572979c 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -61,8 +61,10 @@ void RPCNestedTests::rpcNestedTests() std::string result; std::string result2; - RPCConsole::RPCExecuteCommandLine(result, "getblockchaininfo()[chain]"); //simple result filtering with path + std::string filtered; + RPCConsole::RPCExecuteCommandLine(result, "getblockchaininfo()[chain]", &filtered); //simple result filtering with path QVERIFY(result=="main"); + QVERIFY(filtered == "getblockchaininfo()[chain]"); RPCConsole::RPCExecuteCommandLine(result, "getblock(getbestblockhash())"); //simple 2 level nesting RPCConsole::RPCExecuteCommandLine(result, "getblock(getblock(getbestblockhash())[hash], true)"); @@ -87,8 +89,30 @@ void RPCNestedTests::rpcNestedTests() (RPCConsole::RPCExecuteCommandLine(result2, "createrawtransaction( [], {} , 0 )")); //whitespace between parametres is allowed QVERIFY(result == result2); - RPCConsole::RPCExecuteCommandLine(result, "getblock(getbestblockhash())[tx][0]"); + RPCConsole::RPCExecuteCommandLine(result, "getblock(getbestblockhash())[tx][0]", &filtered); QVERIFY(result == "e0028eb9648db56b1ac77cf090b99048a8007e2bb64b68f092c03c7f56a662c7"); + QVERIFY(filtered == "getblock(getbestblockhash())[tx][0]"); + + RPCConsole::RPCParseCommandLine(result, "importprivkey", false, &filtered); + QVERIFY(filtered == "importprivkey(…)"); + RPCConsole::RPCParseCommandLine(result, "signmessagewithprivkey abc", false, &filtered); + QVERIFY(filtered == "signmessagewithprivkey(…)"); + RPCConsole::RPCParseCommandLine(result, "signmessagewithprivkey abc,def", false, &filtered); + QVERIFY(filtered == "signmessagewithprivkey(…)"); + RPCConsole::RPCParseCommandLine(result, "signrawtransaction(abc)", false, &filtered); + QVERIFY(filtered == "signrawtransaction(…)"); + RPCConsole::RPCParseCommandLine(result, "walletpassphrase(help())", false, &filtered); + QVERIFY(filtered == "walletpassphrase(…)"); + RPCConsole::RPCParseCommandLine(result, "walletpassphrasechange(help(walletpassphrasechange(abc)))", false, &filtered); + QVERIFY(filtered == "walletpassphrasechange(…)"); + RPCConsole::RPCParseCommandLine(result, "help(encryptwallet(abc, def))", false, &filtered); + QVERIFY(filtered == "help(encryptwallet(…))"); + RPCConsole::RPCParseCommandLine(result, "help(importprivkey())", false, &filtered); + QVERIFY(filtered == "help(importprivkey(…))"); + RPCConsole::RPCParseCommandLine(result, "help(importprivkey(help()))", false, &filtered); + QVERIFY(filtered == "help(importprivkey(…))"); + RPCConsole::RPCParseCommandLine(result, "help(importprivkey(abc), walletpassphrase(def))", false, &filtered); + QVERIFY(filtered == "help(importprivkey(…), walletpassphrase(…))"); RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest"); QVERIFY(result == "[]");