Improve detection & handling of duplicate Node ID:

Each node on the network is supposed to have a unique cryptographic
identity. Typically, this identity is generated randomly at startup
and stored for later reuse in the (poorly named) file `wallet.db`.

If the file is copied, it is possible for two nodes to share the
same node identity. This is generally not desirable and existing
servers will detect and reject connections to other servers that
have the same key.

This commit achives three things:

1. It improves the detection code to pinpoint instances where two
   distinct servers with the same key connect with each other. In
   that case, servers will log an appropriate error and shut down
   pending intervention by the server's operator.
2. It makes it possible for server administrators to securely and
   easily generate new cryptographic identities for servers using
   the new `--newnodeid` command line arguments. When a server is
   started using this command, it will generate and save a random
   secure identity.
3. It makes it possible to configure the identity using a command
   line option, which makes it possible to derive it from data or
   parameters associated with the container or hardware where the
   instance is running by passing the `--nodeid` option, followed
   by a single argument identifying the infomation from which the
   node's identity is derived. For example, the following command
   will result in nodes with different hostnames having different
   node identities: `rippled --nodeid $HOSTNAME`

The last option is particularly useful for automated cloud-based
deployments that minimize the need for storing state and provide
unique deployment identifiers.

**Important note for server operators:**
Depending on variables outside of the the control of this code,
such as operating system version or configuration, permissions,
and more, it may be possible for other users or programs to be
able to access the command line arguments of other processes
on the system.

If you are operating in a shared environment, you should avoid
using this option, preferring instead to use the `[node_seed]`
option in the configuration file, and use permissions to limit
exposure of the node seed.

A user who gains access to the value used to derive the node's
unique identity could impersonate that node.

The commit also updates the minimum supported server protocol
version to `XRPL/2.1`, which has been supported since version
1.5.0 and eliminates support for `XPRL/2.0`.

src/ripple/app/consensus/RCLConsensus.cpp

@@ -86,9 +86,11 @@ RCLConsensus::Adaptor::Adaptor(
     , inboundTransactions_{inboundTransactions}
     , j_(journal)
     , validatorKeys_(validatorKeys)
-    , valCookie_{rand_int<std::uint64_t>(
-          1,
-          std::numeric_limits<std::uint64_t>::max())}
+    , valCookie_(
+          1 +
+          rand_int(
+              crypto_prng(),
+              std::numeric_limits<std::uint64_t>::max() - 1))
     , nUnlVote_(validatorKeys_.nodeID, j_)
 {
     assert(valCookie_ != 0);

src/ripple/app/main/Application.cpp

@@ -52,10 +52,12 @@
 #include <ripple/basics/ByteUtilities.h>
 #include <ripple/basics/PerfLog.h>
 #include <ripple/basics/ResolverAsio.h>
+#include <ripple/basics/random.h>
 #include <ripple/basics/safe_cast.h>
 #include <ripple/beast/asio/io_latency_probe.h>
 #include <ripple/beast/core/LexicalCast.h>
 #include <ripple/core/DatabaseCon.h>
+#include <ripple/crypto/csprng.h>
 #include <ripple/json/json_reader.h>
 #include <ripple/nodestore/DatabaseShard.h>
 #include <ripple/nodestore/DummyScheduler.h>
@@ -165,6 +167,8 @@ class ApplicationImp : public Application, public BasicApp
     std::unique_ptr<Logs> logs_;
     std::unique_ptr<TimeKeeper> timeKeeper_;
 
+    std::uint64_t const instanceCookie_;
+
     beast::Journal m_journal;
     std::unique_ptr<perf::PerfLog> perfLog_;
     Application::MutexType m_masterMutex;
@@ -274,6 +278,11 @@ class ApplicationImp : public Application, public BasicApp
         , config_(std::move(config))
         , logs_(std::move(logs))
         , timeKeeper_(std::move(timeKeeper))
+        , instanceCookie_(
+              1 +
+              rand_int(
+                  crypto_prng(),
+                  std::numeric_limits<std::uint64_t>::max() - 1))
         , m_journal(logs_->journal("Application"))
 
         // PerfLog must be started before any other threads are launched.
@@ -508,13 +517,13 @@ class ApplicationImp : public Application, public BasicApp
     //--------------------------------------------------------------------------
 
     bool
-    setup() override;
+    setup(boost::program_options::variables_map const& cmdline) override;
     void
     start(bool withTimers) override;
     void
     run() override;
     void
-    signalStop() override;
+    signalStop(std::string msg = "") override;
     bool
     checkSigs() const override;
     void
@@ -526,6 +535,12 @@ class ApplicationImp : public Application, public BasicApp
 
     //--------------------------------------------------------------------------
 
+    std::uint64_t
+    instanceID() const override
+    {
+        return instanceCookie_;
+    }
+
     Logs&
     logs() override
     {
@@ -1108,7 +1123,7 @@ class ApplicationImp : public Application, public BasicApp
 
 // TODO Break this up into smaller, more digestible initialization segments.
 bool
-ApplicationImp::setup()
+ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
 {
     // We want to intercept CTRL-C and the standard termination signal SIGTERM
     // and terminate the process. This handler will NEVER be invoked twice.
@@ -1146,8 +1161,10 @@ ApplicationImp::setup()
         if (logs_->threshold() > kDebug)
             logs_->threshold(kDebug);
     }
-    JLOG(m_journal.info()) << "process starting: "
-                           << BuildInfo::getFullVersionString();
+
+    JLOG(m_journal.info()) << "Process starting: "
+                           << BuildInfo::getFullVersionString()
+                           << ", Instance Cookie: " << instanceCookie_;
 
     if (numberOfThreads(*config_) < 2)
     {
@@ -1265,7 +1282,7 @@ ApplicationImp::setup()
     if (!config().reporting())
         m_orderBookDB.setup(getLedgerMaster().getCurrentLedger());
 
-    nodeIdentity_ = getNodeIdentity(*this);
+    nodeIdentity_ = getNodeIdentity(*this, cmdline);
 
     if (!cluster_->load(config().section(SECTION_CLUSTER_NODES)))
     {
@@ -1627,10 +1644,17 @@ ApplicationImp::run()
 }
 
 void
-ApplicationImp::signalStop()
+ApplicationImp::signalStop(std::string msg)
 {
     if (!isTimeToStop.exchange(true))
+    {
+        if (msg.empty())
+            JLOG(m_journal.warn()) << "Server stopping";
+        else
+            JLOG(m_journal.warn()) << "Server stopping: " << msg;
+
         stoppingCondition_.notify_all();
+    }
 }
 
 bool

src/ripple/app/main/Application.h

@@ -28,6 +28,7 @@
 #include <ripple/shamap/FullBelowCache.h>
 #include <ripple/shamap/TreeNodeCache.h>
 #include <boost/asio.hpp>
+#include <boost/program_options.hpp>
 #include <memory>
 #include <mutex>
 
@@ -136,13 +137,14 @@ class Application : public beast::PropertyStream::Source
     virtual ~Application() = default;
 
     virtual bool
-    setup() = 0;
+    setup(boost::program_options::variables_map const& options) = 0;
+
     virtual void
     start(bool withTimers) = 0;
     virtual void
     run() = 0;
     virtual void
-    signalStop() = 0;
+    signalStop(std::string msg = "") = 0;
     virtual bool
     checkSigs() const = 0;
     virtual void
@@ -154,6 +156,10 @@ class Application : public beast::PropertyStream::Source
     // ---
     //
 
+    /** Returns a 64-bit instance identifier, generated at startup */
+    virtual std::uint64_t
+    instanceID() const = 0;
+
     virtual Logs&
     logs() = 0;
     virtual Config&

src/ripple/app/main/Main.cpp

@@ -372,6 +372,10 @@ run(int argc, char** argv)
         "conf", po::value<std::string>(), "Specify the configuration file.")(
         "debug", "Enable normally suppressed debug logging")(
         "help,h", "Display this message.")(
+        "newnodeid", "Generate a new node identity for this server.")(
+        "nodeid",
+        po::value<std::string>(),
+        "Specify the node identity for this server.")(
         "quorum",
         po::value<std::size_t>(),
         "Override the minimum validation quorum.")(
@@ -756,7 +760,7 @@ run(int argc, char** argv)
         auto app = make_Application(
             std::move(config), std::move(logs), std::move(timeKeeper));
 
-        if (!app->setup())
+        if (!app->setup(vm))
             return -1;
 
         // With our configuration parsed, ensure we have

src/ripple/app/main/NodeIdentity.cpp

@@ -20,34 +20,49 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/app/main/NodeIdentity.h>
 #include <ripple/app/rdb/Wallet.h>
-#include <ripple/basics/Log.h>
 #include <ripple/core/Config.h>
 #include <ripple/core/ConfigSections.h>
-#include <boost/format.hpp>
 #include <boost/optional.hpp>
 
 namespace ripple {
 
 std::pair<PublicKey, SecretKey>
-getNodeIdentity(Application& app)
+getNodeIdentity(
+    Application& app,
+    boost::program_options::variables_map const& cmdline)
 {
-    // If a seed is specified in the configuration file use that directly.
-    if (app.config().exists(SECTION_NODE_SEED))
+    std::optional<Seed> seed;
+
+    if (cmdline.count("nodeid"))
+    {
+        seed = parseGenericSeed(cmdline["nodeid"].as<std::string>(), false);
+
+        if (!seed)
+            Throw<std::runtime_error>("Invalid 'nodeid' in command line");
+    }
+    else if (app.config().exists(SECTION_NODE_SEED))
     {
-        auto const seed = parseBase58<Seed>(
+        seed = parseBase58<Seed>(
             app.config().section(SECTION_NODE_SEED).lines().front());
 
         if (!seed)
-            Throw<std::runtime_error>("NodeIdentity: Bad [" SECTION_NODE_SEED
-                                      "] specified");
+            Throw<std::runtime_error>("Invalid [" SECTION_NODE_SEED
+                                      "] in configuration file");
+    }
 
+    if (seed)
+    {
         auto secretKey = generateSecretKey(KeyType::secp256k1, *seed);
         auto publicKey = derivePublicKey(KeyType::secp256k1, secretKey);
 
         return {publicKey, secretKey};
     }
 
     auto db = app.getWalletDB().checkoutDb();
+
+    if (cmdline.count("newnodeid") != 0)
+        clearNodeIdentity(*db);
+
     return getNodeIdentity(*db);
 }
 

src/ripple/app/main/NodeIdentity.h

@@ -23,13 +23,20 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/SecretKey.h>
+#include <boost/program_options.hpp>
 #include <utility>
 
 namespace ripple {
 
-/** The cryptographic credentials identifying this server instance. */
+/** The cryptographic credentials identifying this server instance.
+
+    @param app The application object
+    @param cmdline The command line parameters passed into the application.
+ */
 std::pair<PublicKey, SecretKey>
-getNodeIdentity(Application& app);
+getNodeIdentity(
+    Application& app,
+    boost::program_options::variables_map const& cmdline);
 
 }  // namespace ripple
 

src/ripple/app/rdb/Wallet.h

@@ -87,10 +87,19 @@ saveManifests(
 void
 addValidatorManifest(soci::session& session, std::string const& serialized);
 
-/**
- * @brief getNodeIdentity Returns the public and private keys of this node.
- * @param session Session with the database.
- * @return Pair of public and private keys.
+/** Delete any saved public/private key associated with this node. */
+void
+clearNodeIdentity(soci::session& session);
+
+/** Returns a stable public and private key for this node.
+
+    The node's public identity is defined by a secp256k1 keypair
+    that is (normally) randomly generated. This function will
+    return such a keypair, securely generating one if needed.
+
+    @param session Session with the database.
+
+    @return Pair of public and private secp256k1 keys.
  */
 std::pair<PublicKey, SecretKey>
 getNodeIdentity(soci::session& session);

src/ripple/app/rdb/impl/Wallet.cpp

@@ -119,6 +119,12 @@ addValidatorManifest(soci::session& session, std::string const& serialized)
     tr.commit();
 }
 
+void
+clearNodeIdentity(soci::session& session)
+{
+    session << "DELETE FROM NodeIdentity;";
+}
+
 std::pair<PublicKey, SecretKey>
 getNodeIdentity(soci::session& session)
 {

src/ripple/overlay/README.md

@@ -296,8 +296,8 @@ For more on the Peer Crawler, please visit https://xrpl.org/peer-crawler.html.
 If present, identifies the hash of the last ledger that the sending server
 considers to be closed.
 
-The value is presently encoded using **Base64** encoding, but implementations
-should support both **Base64** and **HEX** encoding for this value.
+The value is encoded as **HEX**, but implementations should support both
+**Base64** and **HEX** encoding for this value for legacy purposes.
 
 | Field Name          	|  Request          	| Response          	|
 |---------------------	|:-----------------:	|:-----------------:	|

src/ripple/overlay/impl/Handshake.cpp

@@ -204,6 +204,8 @@ buildHandshake(
         h.insert("Session-Signature", base64_encode(sig.data(), sig.size()));
     }
 
+    h.insert("Instance-Cookie", std::to_string(app.instanceID()));
+
     if (!app.config().SERVER_DOMAIN.empty())
         h.insert("Server-Domain", app.config().SERVER_DOMAIN);
 
@@ -215,14 +217,8 @@ buildHandshake(
 
     if (auto const cl = app.getLedgerMaster().getClosedLedger())
     {
-        // TODO: Use hex for these
-        h.insert(
-            "Closed-Ledger",
-            base64_encode(cl->info().hash.begin(), cl->info().hash.size()));
-        h.insert(
-            "Previous-Ledger",
-            base64_encode(
-                cl->info().parentHash.begin(), cl->info().parentHash.size()));
+        h.insert("Closed-Ledger", strHex(cl->info().hash));
+        h.insert("Previous-Ledger", strHex(cl->info().parentHash));
     }
 }
 
@@ -306,7 +302,34 @@ verifyHandshake(
     }();
 
     if (publicKey == app.nodeIdentity().first)
+    {
+        auto const peerInstanceID = [&headers]() {
+            std::uint64_t iid = 0;
+
+            if (auto const iter = headers.find("Instance-Cookie");
+                iter != headers.end())
+            {
+                if (!beast::lexicalCastChecked(iid, iter->value().to_string()))
+                    throw std::runtime_error("Invalid instance cookie");
+
+                if (iid == 0)
+                    throw std::runtime_error("Invalid instance cookie");
+            }
+
+            return iid;
+        }();
+
+        // Attempt to differentiate self-connections as opposed to accidental
+        // node identity reuse caused by accidental misconfiguration. When we
+        // detect this, we stop the process and log an error message.
+        if (peerInstanceID != app.instanceID())
+        {
+            app.signalStop("Remote server is using our node identity");
+            throw std::runtime_error("Node identity reuse detected");
+        }
+
         throw std::runtime_error("Self connection");
+    }
 
     // This check gets two birds with one stone:
     //

src/ripple/overlay/impl/ProtocolVersion.cpp

@@ -36,7 +36,6 @@ namespace ripple {
 // clang-format off
 constexpr ProtocolVersion const supportedProtocolList[]
 {
-    {2, 0},
     {2, 1},
     {2, 2}
 };

src/ripple/protocol/Seed.h

@@ -116,9 +116,13 @@ template <>
 std::optional<Seed>
 parseBase58(std::string const& s);
 
-/** Attempt to parse a string as a seed */
+/** Attempt to parse a string as a seed.
+
+    @param str the string to parse
+    @param rfc1751 true if we should attempt RFC1751 style parsing (deprecated)
+ * */
 std::optional<Seed>
-parseGenericSeed(std::string const& str);
+parseGenericSeed(std::string const& str, bool rfc1751 = true);
 
 /** Encode a Seed in RFC1751 format */
 std::string