Deprecate commands that perform remote tx signing (RIPD-1649):

In order to facilitate transaction signing, `rippled` offers the `sign` and
`sign_for` and `submit` commands, which, given a seed, can be used to sign or
sign-and-submit transactions. These commands are accessible from the command
line, as well as over the WebSocket and RPC interfaces that `rippled` can be
configured to provide.

These commands, unfortunately, have significant security implications:

  1. They require divulging an account's seed (commonly known as a "secret
     key") to the server.
  2. When executing these commands against remote servers, the seeds can be
     transported over clear-text links.
  3. When executing these commands over the command line, the account
     seed may be visible using common tools that show running processes
     and may potentially be inadvertently stored by system monitoring
     tools or facilities designed to maintain a history of previously
     typed commands.

While this commit cannot prevent users from issuing these commands to a
server, whether locally or remotely, it restricts the `sign` and `sign_for`
commands, as well as the `submit` command when used to sign-and-submit,
so that they require administrative privileges on the server.

Server operators that want to allow unrestricted signing can do so by
adding the following stanza to their configuration file:

    [signing_support]
    true

Ripple discourages server operators from doing so and advises against using
these commands, which will be removed in a future release. If you rely on
these commands for signing, please migrate to a standalone signing solution
as soon as possible. One option is to use `ripple-lib`; documentation is
available at https://developers.ripple.com/rippleapi-reference.html#sign.

If the commands are administratively enabled, the server includes a warning
on startup and adds a new field in the resulting JSON, informing the caller
that the commands are deprecated and may become unavailable at any time.

Acknowledgements:
Jesper Wallin for reporting this issue to Ripple.

Bug Bounties and Responsible Disclosures:
We welcome reviews of the rippled code and urge researchers to responsibly
disclose any issues that they may find. For more on Ripple's Bug Bounty
program, please visit: https://ripple.com/bug-bounty

cfg/rippled-example.cfg

@@ -1002,7 +1002,32 @@
 #
 #-------------------------------------------------------------------------------
 #
-# 8. Example Settings
+# 8. Misc Settings
+#
+#----------
+#
+# [signing_support]
+#
+#   Specifies whether the server will accept "sign" and "sign_for" commands
+#   from remote users. Even if the commands are sent over a secure protocol
+#   like secure websocket, this should generally be discouraged, because it
+#   requires sending the secret to use for signing to the server. In order
+#   to sign transactions, users should prefer to use a standalone signing
+#   tool instead.
+#
+#   This flag has no effect on the "sign" and "sign_for" command line options
+#   that rippled makes available.
+#
+#   The default value of this field is "false"
+#
+#   Example:
+#
+#   [signing_support]
+#   true
+#
+#-------------------------------------------------------------------------------
+#
+# 9. Example Settings
 #
 #--------------------
 #

src/ripple/app/main/Application.cpp

@@ -1399,6 +1399,26 @@ bool ApplicationImp::setup()
         m_networkOPs->setStandAlone ();
     }
 
+    if (config_->canSign())
+    {
+        JLOG(m_journal.warn()) <<
+            "*** The server is configured to allow the 'sign' and 'sign_for'";
+        JLOG(m_journal.warn()) <<
+            "*** commands. These commands have security implications and have";
+        JLOG(m_journal.warn()) <<
+            "*** been deprecated. They will be removed in a future release of";
+        JLOG(m_journal.warn()) <<
+            "*** rippled.";
+        JLOG(m_journal.warn()) <<
+            "*** If you do not use them to sign transactions please edit your";
+        JLOG(m_journal.warn()) <<
+            "*** configuration file and remove the [enable_signing] stanza.";
+        JLOG(m_journal.warn()) <<
+            "*** If you do use them to sign transactions please migrate to a";
+        JLOG(m_journal.warn()) <<
+            "*** standalone signing solution as soon as possible.";
+    }
+
     //
     // Execute start up rpc commands.
     //

src/ripple/core/Config.h

@@ -107,6 +107,14 @@ private:
     */
     bool                        RUN_STANDALONE = false;
 
+    /** Determines if the server will sign a tx, given an account's secret seed.
+
+        In the past, this was allowed, but this functionality can have security
+        implications. The new default is to not allow this functionality, but
+        a config option is included to enable this.
+    */
+    bool signingEnabled_ = false;
+
 public:
     bool doImport = false;
     bool nodeToShard = false;
@@ -196,6 +204,8 @@ public:
     bool quiet() const { return QUIET; }
     bool silent() const { return SILENT; }
     bool standalone() const { return RUN_STANDALONE; }
+
+    bool canSign() const { return signingEnabled_; }
 };
 
 } // ripple

src/ripple/core/ConfigSections.h

@@ -58,6 +58,7 @@ struct ConfigSection
 #define SECTION_PEER_PRIVATE            "peer_private"
 #define SECTION_PEERS_MAX               "peers_max"
 #define SECTION_RPC_STARTUP             "rpc_startup"
+#define SECTION_SIGNING_SUPPORT         "signing_support"
 #define SECTION_SNTP                    "sntp_servers"
 #define SECTION_SSL_VERIFY              "ssl_verify"
 #define SECTION_SSL_VERIFY_FILE         "ssl_verify_file"

src/ripple/core/impl/Config.cpp

@@ -345,6 +345,9 @@ void Config::loadFromString (std::string const& fileContents)
         }
     }
 
+    if (getSingleSection (secConfig, SECTION_SIGNING_SUPPORT, strTemp, j_))
+        signingEnabled_     = beast::lexicalCastThrow <bool> (strTemp);
+
     if (getSingleSection (secConfig, SECTION_ELB_SUPPORT, strTemp, j_))
         ELB_SUPPORT         = beast::lexicalCastThrow <bool> (strTemp);
 

src/ripple/rpc/handlers/SignFor.cpp

@@ -33,6 +33,12 @@ namespace ripple {
 // }
 Json::Value doSignFor (RPC::Context& context)
 {
+    if (context.role != Role::ADMIN && !context.app.config().canSign())
+    {
+        return RPC::make_error (rpcNOT_SUPPORTED,
+            "Signing is not supported by this server.");
+    }
+
     // Bail if multisign is not enabled.
     if (! context.app.getLedgerMaster().getValidatedRules().
         enabled (featureMultiSign))
@@ -44,12 +50,14 @@ Json::Value doSignFor (RPC::Context& context)
     auto const failHard = context.params[jss::fail_hard].asBool();
     auto const failType = NetworkOPs::doFailHard (failHard);
 
-    return RPC::transactionSignFor (
-        context.params,
-        failType,
-        context.role,
-        context.ledgerMaster.getValidatedLedgerAge(),
-        context.app);
+    auto ret = RPC::transactionSignFor (
+        context.params, failType, context.role,
+        context.ledgerMaster.getValidatedLedgerAge(), context.app);
+
+    ret[jss::deprecated] = "This command has been deprecated and will be "
+                           "removed in a future version of the server. Please "
+                           "migrate to a standalone signing tool.";
+    return ret;
 }
 
 } // ripple

src/ripple/rpc/handlers/SignHandler.cpp

@@ -31,18 +31,27 @@ namespace ripple {
 // }
 Json::Value doSign (RPC::Context& context)
 {
+    if (context.role != Role::ADMIN && !context.app.config().canSign())
+    {
+        return RPC::make_error (rpcNOT_SUPPORTED,
+            "Signing is not supported by this server.");
+    }
+
     context.loadType = Resource::feeHighBurdenRPC;
     NetworkOPs::FailHard const failType =
         NetworkOPs::doFailHard (
             context.params.isMember (jss::fail_hard)
             && context.params[jss::fail_hard].asBool ());
 
-    return RPC::transactionSign (
-        context.params,
-        failType,
-        context.role,
-        context.ledgerMaster.getValidatedLedgerAge(),
-        context.app);
+    auto ret = RPC::transactionSign (
+        context.params, failType, context.role,
+        context.ledgerMaster.getValidatedLedgerAge(), context.app);
+
+    ret[jss::deprecated] = "This command has been deprecated and will be "
+                           "removed in a future version of the server. Please "
+                           "migrate to a standalone signing tool.";
+
+    return ret;
 }
 
 } // ripple

src/ripple/rpc/handlers/Submit.cpp

@@ -48,13 +48,21 @@ Json::Value doSubmit (RPC::Context& context)
     {
         auto const failType = getFailHard (context);
 
-        return RPC::transactionSubmit (
-        context.params,
-        failType,
-        context.role,
-        context.ledgerMaster.getValidatedLedgerAge(),
-        context.app,
-        RPC::getProcessTxnFn (context.netOps));
+        if (context.role != Role::ADMIN && !context.app.config().canSign())
+            return RPC::make_error (rpcNOT_SUPPORTED,
+                "Signing is not supported by this server.");
+
+        auto ret = RPC::transactionSubmit (
+            context.params, failType, context.role,
+            context.ledgerMaster.getValidatedLedgerAge(),
+            context.app, RPC::getProcessTxnFn (context.netOps));
+
+        ret[jss::deprecated] = "Signing support in the 'submit' command has been "
+                               "deprecated and will be removed in a future version "
+                               "of the server. Please migrate to a standalone "
+                               "signing tool.";
+
+        return ret;
     }
 
     Json::Value jvResult;

src/ripple/rpc/impl/RPCHandler.cpp

@@ -235,7 +235,22 @@ void getResult (
     {
         JLOG (context.j.debug()) << "rpcError: " << status.toString();
         result[jss::status] = jss::error;
-        result[jss::request] = context.params;
+
+        auto rq = context.params;
+
+        if (rq.isObject())
+        {
+            if (rq.isMember(jss::passphrase.c_str()))
+                rq[jss::passphrase.c_str()] = "<masked>";
+            if (rq.isMember(jss::secret.c_str()))
+                rq[jss::secret.c_str()] = "<masked>";
+            if (rq.isMember(jss::seed.c_str()))
+                rq[jss::seed.c_str()] = "<masked>";
+            if (rq.isMember(jss::seed_hex.c_str()))
+                rq[jss::seed_hex.c_str()] = "<masked>";
+        }
+
+        result[jss::request] = rq;
     }
     else
     {

src/ripple/rpc/impl/ServerHandlerImp.cpp

@@ -469,7 +469,22 @@ ServerHandlerImp::processSession(
     {
         jr = jr[jss::result];
         jr[jss::status] = jss::error;
-        jr[jss::request] = jv;
+
+        auto rq = jv;
+
+        if (rq.isObject())
+        {
+            if (rq.isMember(jss::passphrase.c_str()))
+                rq[jss::passphrase.c_str()] = "<masked>";
+            if (rq.isMember(jss::secret.c_str()))
+                rq[jss::secret.c_str()] = "<masked>";
+            if (rq.isMember(jss::seed.c_str()))
+                rq[jss::seed.c_str()] = "<masked>";
+            if (rq.isMember(jss::seed_hex.c_str()))
+                rq[jss::seed_hex.c_str()] = "<masked>";
+        }
+
+        jr[jss::request] = rq;
     }
     else
     {
@@ -780,8 +795,23 @@ ServerHandlerImp::processRequest (Port const& port,
             // Always report "status".  On an error report the request as received.
             if (result.isMember (jss::error))
             {
+                auto rq = params;
+
+                if (rq.isObject())
+                { // But mask potentially sensitive information.
+                    if (rq.isMember(jss::passphrase.c_str()))
+                        rq[jss::passphrase.c_str()] = "<masked>";
+                    if (rq.isMember(jss::secret.c_str()))
+                        rq[jss::secret.c_str()] = "<masked>";
+                    if (rq.isMember(jss::seed.c_str()))
+                        rq[jss::seed.c_str()] = "<masked>";
+                    if (rq.isMember(jss::seed_hex.c_str()))
+                        rq[jss::seed_hex.c_str()] = "<masked>";
+                }
+
                 result[jss::status] = jss::error;
-                result[jss::request] = params;
+                result[jss::request] = rq;
+
                 JLOG (m_journal.debug())  <<
                     "rpcError: " << result [jss::error] <<
                     ": " << result [jss::error_message];

src/test/app/MultiSign_test.cpp

@@ -15,6 +15,7 @@
 */
 //==============================================================================
 
+#include <ripple/core/ConfigSections.h>
 #include <ripple/protocol/JsonFields.h>     // jss:: definitions
 #include <ripple/protocol/Feature.h>
 #include <test/jtx.h>
@@ -190,7 +191,12 @@ public:
     test_enablement()
     {
         using namespace jtx;
-        Env env(*this, FeatureBitset{});
+        Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
+            {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
+                return cfg;
+            }), FeatureBitset{});
+
         Account const alice {"alice", KeyType::ed25519};
         env.fund(XRP(1000), alice);
         env.close();
@@ -414,7 +420,11 @@ public:
     void test_regularSignersUsingSubmitMulti()
     {
         using namespace jtx;
-        Env env(*this);
+        Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
+            {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
+                return cfg;
+            }));
         Account const alice {"alice", KeyType::secp256k1};
         Account const becky {"becky", KeyType::ed25519};
         Account const cheri {"cheri", KeyType::secp256k1};
@@ -1142,7 +1152,11 @@ public:
         using namespace jtx;
         Account const alice {"alice", KeyType::ed25519};
 
-        Env env(*this);
+        Env env(*this, envconfig([](std::unique_ptr<Config> cfg)
+            {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
+                return cfg;
+            }));
         env.fund (XRP(1000), alice);
         env.close();
 

src/test/rpc/AmendmentBlocked_test.cpp

@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include <test/jtx.h>
+#include <ripple/core/ConfigSections.h>
 #include <ripple/protocol/JsonFields.h>
 #include <ripple/app/misc/NetworkOPs.h>
 #include <test/jtx/WSClient.h>
@@ -29,7 +30,11 @@ class AmendmentBlocked_test : public beast::unit_test::suite
     void testBlockedMethods()
     {
         using namespace test::jtx;
-        Env env {*this};
+        Env env {*this, envconfig([](std::unique_ptr<Config> cfg)
+            {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
+                return cfg;
+            })};
         auto const gw = Account {"gateway"};
         auto const USD = gw["USD"];
         auto const alice = Account {"alice"};

src/test/rpc/JSONRPC_test.cpp

@@ -20,6 +20,7 @@
 #include <ripple/app/misc/LoadFeeTrack.h>
 #include <ripple/app/misc/TxQ.h>
 #include <ripple/basics/contract.h>
+#include <ripple/core/ConfigSections.h>
 #include <ripple/json/json_reader.h>
 #include <ripple/protocol/ErrorCodes.h>
 #include <ripple/protocol/Feature.h>
@@ -1948,6 +1949,7 @@ public:
         using namespace test::jtx;
         Env env {*this, envconfig([](std::unique_ptr<Config> cfg)
             {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
                 cfg->section("transaction_queue")
                     .set("minimum_txn_in_ledger_standalone", "3");
                 return cfg;

src/test/rpc/RPCOverload_test.cpp

@@ -15,6 +15,7 @@
 */
 //==============================================================================
 
+#include <ripple/core/ConfigSections.h>
 #include <ripple/protocol/JsonFields.h>
 #include <test/jtx/WSClient.h>
 #include <test/jtx/JSONRPCClient.h>
@@ -31,7 +32,11 @@ public:
     {
         testcase << "Overload " << (useWS ? "WS" : "HTTP") << " RPC client";
         using namespace jtx;
-        Env env {*this, envconfig(no_admin)};
+        Env env {*this, envconfig([](std::unique_ptr<Config> cfg)
+            {
+                cfg->loadFromString ("[" SECTION_SIGNING_SUPPORT "]\ntrue");
+                return no_admin(std::move(cfg));
+            })};
 
         Account const alice {"alice"};
         Account const bob {"bob"};