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

@@ -32,6 +32,7 @@
 #include <QDir>
 #include <QKeyEvent>
 #include <QMenu>
+#include <QMessageBox>
 #include <QScrollBar>
 #include <QSettings>
 #include <QSignalMapper>
@@ -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<std::string> > stack;
     stack.push_back(std::vector<std::string>());
@@ -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<std::pair<size_t, size_t>> 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<std::string>());
+                        }
 
                         // 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;
+                        if (fExecute) {
-                        // Convert argument list to JSON objects in method-dependent way,
+                            // Convert argument list to JSON objects in method-dependent way,
-                        // and pass it along with the method name to the dispatcher.
+                            // and pass it along with the method name to the dispatcher.
-                        JSONRPCRequest req;
+                            JSONRPCRequest req;
-                        req.params = RPCConvertValues(stack.back()[0], std::vector<std::string>(stack.back().begin() + 1, stack.back().end()));
+                            req.params = RPCConvertValues(stack.back()[0], std::vector<std::string>(stack.back().begin() + 1, stack.back().end()));
-                        req.strMethod = stack.back()[0];
+                            req.strMethod = stack.back()[0];
-                        lastResult = tableRPC.execute(req);
+                            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);
 }
 

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<NodeId> cachedNodeids;
     const PlatformStyle *platformStyle;
     RPCTimerInterface *rpcTimerInterface;

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 == "[]");