test: cover the tool's sources; appended blobs verify under their own manifest

src/tools/validator-keys/ListSigning.cpp

@@ -276,6 +276,15 @@ makeSignedList(
             throw std::runtime_error(
                 "The list to append to already holds " + std::to_string(kMaxBlobs) + " blobs");
         jv = existing;
+        // Blobs signed under an earlier manifest keep it, so they still verify
+        // once the top-level manifest names the current signing key.
+        auto const previous = existing.isMember(jss::manifest) && existing[jss::manifest].isString()
+            ? existing[jss::manifest].asString()
+            : std::string();
+        if (!previous.empty() && previous != manifestBase64)
+            for (auto& entry : jv[jss::blobs_v2])
+                if (!entry.isMember(jss::manifest))
+                    entry[jss::manifest] = previous;
     }
     else
     {
@@ -361,8 +370,14 @@ verifyList(
     if (expectedKey && *expectedKey != manifest->masterKey)
         fail("the master key is not the expected key");
 
-    // The blobs of either version as (blob, signature) pairs.
-    std::vector<std::pair<std::string, std::string>> blobs;
+    // The blobs of either version, each with the signing key it was signed under.
+    struct BlobEntry
+    {
+        std::string blob;
+        std::string signature;
+        PublicKey signingKey;
+    };
+    std::vector<BlobEntry> blobs;
     if (*version == 1)
     {
         if (!list.isMember(jss::blob) || !list[jss::blob].isString() ||
@@ -372,7 +387,8 @@ verifyList(
             fail("a version 1 list needs \"blob\" and \"signature\" and no \"blobs_v2\"");
             return result;
         }
-        blobs.emplace_back(list[jss::blob].asString(), list[jss::signature].asString());
+        blobs.push_back(
+            {list[jss::blob].asString(), list[jss::signature].asString(), *manifest->signingKey});
     }
     else
     {
@@ -393,6 +409,7 @@ verifyList(
                 fail("every \"blobs_v2\" entry needs \"blob\" and \"signature\"");
                 return result;
             }
+            auto signingKey = *manifest->signingKey;
             if (entry.isMember(jss::manifest))
             {
                 if (!entry[jss::manifest].isString())
@@ -401,23 +418,26 @@ verifyList(
                     return result;
                 }
                 auto const m = parseManifest(entry[jss::manifest].asString());
-                if (!m || m->masterKey != manifest->masterKey)
+                if (!m || m->masterKey != manifest->masterKey || !m->signingKey)
                     fail("a \"blobs_v2\" entry's \"manifest\" is not this publisher's");
+                else
+                    signingKey = *m->signingKey;
             }
-            blobs.emplace_back(entry[jss::blob].asString(), entry[jss::signature].asString());
+            blobs.push_back(
+                {entry[jss::blob].asString(), entry[jss::signature].asString(), signingKey});
         }
     }
 
     result.report["blobs"] = json::Value(json::ValueType::Array);
     std::size_t index = 0;
-    for (auto const& [blob, signature] : blobs)
+    for (auto const& [blob, signature, signingKey] : blobs)
     {
         auto const where = "blob " + std::to_string(index++);
         json::Value entry(json::ValueType::Object);
 
         auto const sig = strUnHex(signature);
         auto const data = base64Decode(blob);
-        if (!sig || !verify(*manifest->signingKey, makeSlice(data), makeSlice(*sig)))
+        if (!sig || !verify(signingKey, makeSlice(data), makeSlice(*sig)))
             fail(where + ": the signature does not verify under the signing key");
 
         std::optional<UnsignedList> parsed;

src/tools/validator-keys/ListSigning.h

@@ -95,7 +95,8 @@ signList(UnsignedList const& list, PublicKey const& signingKey, SecretKey const&
  * Version 1 is `{blob, manifest, public_key, signature, version}`. Version 2
  * carries the blob and signature inside `blobs_v2`; when @p append is given it
  * must be a version 2 document for the same master key and the new blob is
- * added to it.
+ * added to it. Blobs signed under an earlier manifest keep that manifest in
+ * their entry.
  *
  * @throws std::runtime_error if @p append is not a version 2 document for
  *         @p masterKey or already holds the maximum number of blobs

src/tools/validator-keys/SigningKeys.cpp

@@ -255,7 +255,7 @@ SigningKeys::verifyManifest() const
         fail();
 
     auto const pk = get<PublicKey>(st, sfPublicKey);
-    if (!pk || !verify(st, HashPrefix::Manifest, *pk, sfMasterSignature))
+    if (!pk || *pk != keys_.publicKey || !verify(st, HashPrefix::Manifest, *pk, sfMasterSignature))
         fail();
 }
 
@@ -355,11 +355,11 @@ SigningKeys::startValidatorToken(
         generatePartialManifest(tokenSequence_ + 1, keys_.publicKey, tokenPublic, domain_));
 }
 
-std::optional<ValidatorToken>
+ValidatorToken
 SigningKeys::finishToken(Blob const& masterSig)
 {
     if (revoked())
-        return std::nullopt;
+        throw std::runtime_error("Validator keys have been revoked.");
 
     if (!pendingTokenSecret_ || !pendingKeyType_)
         throw std::runtime_error("No pending token to finish");
@@ -379,11 +379,11 @@ SigningKeys::finishToken(Blob const& masterSig)
     return ValidatorToken{xrpl::base64Encode(manifest_.data(), manifest_.size()), tokenSecret};
 }
 
-std::optional<std::string>
+std::string
 SigningKeys::finishExternalToken(Blob const& masterSig, Blob const& signingSig)
 {
     if (revoked())
-        return std::nullopt;
+        throw std::runtime_error("Validator keys have been revoked.");
 
     if (!pendingSigningKey_)
         throw std::runtime_error("No pending token with an external signing key to finish");

src/tools/validator-keys/SigningKeys.h

@@ -166,12 +166,12 @@ public:
      * @param masterSig Master signature over the bytes `startValidatorToken`
      *                  returned
      *
-     * @return The token, or nullopt if the keys are revoked
+     * @return The token
      *
-     * @throws std::runtime_error if no such token is pending or the
-     *         signature does not verify
+     * @throws std::runtime_error if the keys are revoked, no such token is
+     *         pending, or the signature does not verify
      */
-    std::optional<ValidatorToken>
+    ValidatorToken
     finishToken(Blob const& masterSig);
 
     /**
@@ -181,12 +181,12 @@ public:
      *                  returned
      * @param signingSig Signing-key signature over the same bytes
      *
-     * @return The base64 manifest, or nullopt if the keys are revoked
+     * @return The base64 manifest
      *
-     * @throws std::runtime_error if no such token is pending or a signature
-     *         does not verify
+     * @throws std::runtime_error if the keys are revoked, no such token is
+     *         pending, or a signature does not verify
      */
-    std::optional<std::string>
+    std::string
     finishExternalToken(Blob const& masterSig, Blob const& signingSig);
 
     /**

src/tools/validator-keys/ValidatorKeysTool.cpp

@@ -265,42 +265,27 @@ finishToken(std::vector<std::string> const& signatures, ToolOptions const& optio
 
     auto keys = SigningKeys::make_SigningKeys(options.keyFile);
 
-    if (keys.revoked())
-        throw std::runtime_error("Validator keys have been revoked.");
-
     auto const masterSig = decodeSignature(signatures.at(0));
 
     if (signatures.size() == 2)
     {
-        auto const signingSig = decodeSignature(signatures.at(1));
-        auto const manifest = keys.finishExternalToken(masterSig, signingSig);
-        if (!manifest)
-            throw std::runtime_error("Validator keys have been revoked.");
-
+        auto const manifest =
+            keys.finishExternalToken(masterSig, decodeSignature(signatures.at(1)));
         keys.writeToFile(options.keyFile);
-
         emitBlock(
             "validator_manifest",
             toBase58(TokenType::NodePublic, keys.publicKey()),
-            *manifest,
+            manifest,
             options.outFile);
         return;
     }
 
     auto const token = keys.finishToken(masterSig);
-
-    if (!token)
-        throw std::runtime_error(
-            "Maximum number of tokens have already been generated.\n"
-            "Revoke validator keys if previous token has been compromised.");
-
-    // Update key file with new token sequence
     keys.writeToFile(options.keyFile);
-
     emitBlock(
         "validator_token",
         toBase58(TokenType::NodePublic, keys.publicKey()),
-        tokenToBase64(*token),
+        tokenToBase64(token),
         options.outFile);
 }
 
@@ -908,5 +893,5 @@ main(int argc, char** argv)
     }
 
     return EXIT_SUCCESS;
-    // LCOV_EXCL_STOP
 }
+// LCOV_EXCL_STOP

src/tools/validator-keys/test/KeyFileGuard.h

@@ -18,15 +18,16 @@ private:
     path subDir_;
     beast::unit_test::Suite& test_;
 
-    auto
+    void
     rmDir(path const& toRm)
     {
-        if (is_directory(toRm))
-            remove_all(toRm);
+        boost::system::error_code ec;
+        if (is_directory(toRm, ec))
+            remove_all(toRm, ec);
         else
             test_.log << "Expected " << toRm.string() << " to be an existing directory."
                       << std::endl;
-    };
+    }
 
 public:
     KeyFileGuard(beast::unit_test::Suite& test, std::string const& subDir)
@@ -43,17 +44,7 @@ public:
     }
     ~KeyFileGuard()
     {
-        try
-        {
-            using namespace boost::filesystem;
-
-            rmDir(subDir_);
-        }
-        catch (std::exception& e)
-        {
-            // if we throw here, just let it die.
-            test_.log << "Error in ~KeyFileGuard: " << e.what() << std::endl;
-        };
+        rmDir(subDir_);
     }
 };
 

src/tools/validator-keys/test/ListSigning_test.cpp

@@ -158,9 +158,16 @@ private:
         expectError(
             "{\"sequence\": 1, \"effective\": 1000, \"expiration\": 1000, \"validators\": []}",
             "\"effective\" must be earlier than \"expiration\"");
+        expectError(
+            "{\"sequence\": 1, \"effective\": \"x\", \"expiration\": 1000, \"validators\": []}",
+            "\"effective\" must be an unsigned integer");
         expectError(
             "{\"sequence\": 1, \"expiration\": 1000, \"validators\": []}",
             "\"validators\" must be a non-empty array");
+        expectError(
+            "{\"sequence\": 1, \"expiration\": 1000, \"validators\": [{\"validation_public_key\": "
+            "\"zz\"}]}",
+            "\"validation_public_key\" is not a hex public key: zz");
         expectError(
             "{\"sequence\": 1, \"expiration\": 1000, \"validators\": [{}]}",
             "every validator needs a \"validation_public_key\" string");
@@ -186,6 +193,11 @@ private:
                 "[{\"validation_public_key\": \"" +
                 strHex(key) + "\", \"manifest\": \"AAAA\"}]}";
             expectError(text, "\"manifest\" does not verify for " + strHex(key));
+            auto const notString =
+                "{\"sequence\": 1, \"expiration\": 1000, \"validators\": "
+                "[{\"validation_public_key\": \"" +
+                strHex(key) + "\", \"manifest\": 5}]}";
+            expectError(notString, "\"manifest\" must be a base64 string for " + strHex(key));
         }
     }
 
@@ -249,6 +261,40 @@ private:
             BEAST_EXPECT(result.report["blobs"][1u][jss::effective].asUInt() == now + 200);
         }
 
+        try
+        {
+            makeSignedList(
+                publisher.token.manifest,
+                publisher.manifest.masterKey,
+                list,
+                signature,
+                3,
+                std::nullopt);
+            fail();
+        }
+        catch (std::runtime_error const& e)
+        {
+            BEAST_EXPECT(e.what() == std::string("Unsupported list version"));
+        }
+
+        // Append after the signing key rotated: the earlier blobs keep their manifest
+        {
+            SigningKeys rotated = publisher.keys;
+            auto const token2 = *rotated.createValidatorToken(KeyType::Ed25519);
+            auto const manifest2 = *deserializeManifest(base64Decode(token2.manifest));
+            auto const sig2 = signList(later, *manifest2.signingKey, token2.validationSecret);
+            auto const v2c =
+                makeSignedList(token2.manifest, manifest2.masterKey, later, sig2, 2, v2b);
+            BEAST_EXPECT(v2c[jss::blobs_v2].size() == 3);
+            BEAST_EXPECT(v2c[jss::manifest].asString() == token2.manifest);
+            BEAST_EXPECT(
+                v2c[jss::blobs_v2][0u][jss::manifest].asString() == publisher.token.manifest);
+            BEAST_EXPECT(!v2c[jss::blobs_v2][2u].isMember(jss::manifest));
+            auto const result = verifyList(v2c, std::nullopt, std::nullopt, now);
+            BEAST_EXPECTS(result.ok, to_string(result.report));
+            BEAST_EXPECT(result.report["manifest_sequence"].asUInt() == 2);
+        }
+
         // Append refuses the wrong shape, another publisher, and a full list
         try
         {
@@ -415,6 +461,99 @@ private:
                 std::nullopt,
                 now,
                 "\"manifest\" does not deserialize and verify");
+
+            expectError(
+                json::Value(json::ValueType::Array),
+                std::nullopt,
+                std::nullopt,
+                now,
+                "the list is not a JSON object");
+            {
+                auto bad = good;
+                bad[jss::public_key] = 1;
+                expectError(
+                    bad,
+                    std::nullopt,
+                    std::nullopt,
+                    now,
+                    "\"public_key\" and \"manifest\" must be strings");
+            }
+            {
+                // A revoked publisher
+                SigningKeys revoked(KeyType::Ed25519);
+                auto bad = good;
+                bad[jss::manifest] = revoked.revoke();
+                bad[jss::public_key] = strHex(revoked.publicKey());
+                expectError(
+                    bad, std::nullopt, std::nullopt, now, "the publisher's master key is revoked");
+            }
+            {
+                // A blob that parses as JSON but is not a list; the signature fails too
+                auto bad = good;
+                bad[jss::blob] = base64Encode("{}");
+                expectError(
+                    bad,
+                    std::nullopt,
+                    std::nullopt,
+                    now,
+                    "blob 0: \"sequence\" must be a positive integer");
+            }
+
+            // Version 2 structure
+            auto const v2 = makeSignedList(
+                publisher.token.manifest,
+                publisher.manifest.masterKey,
+                list,
+                signature,
+                2,
+                std::nullopt);
+            std::string const v2Shape =
+                "a version 2 list needs 1 to 5 \"blobs_v2\" entries and no top-level \"blob\"";
+            {
+                auto bad = v2;
+                bad[jss::blobs_v2] = json::Value(json::ValueType::Array);
+                expectError(bad, std::nullopt, std::nullopt, now, v2Shape);
+            }
+            {
+                auto bad = v2;
+                bad[jss::blob] = "x";
+                expectError(bad, std::nullopt, std::nullopt, now, v2Shape);
+            }
+            {
+                auto bad = v2;
+                bad[jss::blobs_v2][0u].removeMember(jss::signature);
+                expectError(
+                    bad,
+                    std::nullopt,
+                    std::nullopt,
+                    now,
+                    "every \"blobs_v2\" entry needs \"blob\" and \"signature\"");
+            }
+            {
+                auto bad = v2;
+                bad[jss::blobs_v2][0u][jss::manifest] = 5;
+                expectError(
+                    bad,
+                    std::nullopt,
+                    std::nullopt,
+                    now,
+                    "a \"blobs_v2\" entry's \"manifest\" must be a string");
+            }
+            {
+                Publisher const other;
+                auto bad = v2;
+                bad[jss::blobs_v2][0u][jss::manifest] = other.token.manifest;
+                expectError(
+                    bad,
+                    std::nullopt,
+                    std::nullopt,
+                    now,
+                    "a \"blobs_v2\" entry's \"manifest\" is not this publisher's");
+                // The publisher's own manifest in an entry is accepted
+                auto fine = v2;
+                fine[jss::blobs_v2][0u][jss::manifest] = publisher.token.manifest;
+                BEAST_EXPECT(verifyList(fine, std::nullopt, std::nullopt, now).ok);
+            }
         }
     }
 
@@ -471,6 +610,21 @@ private:
         {
             BEAST_EXPECT(e.what() == "Not a validator token: " + manifestFile.string());
         }
+        {
+            path const bad = subdir / "bad-manifest.txt";
+            std::ofstream o(bad.string());
+            o << "AAAA\n";
+            o.close();
+            try
+            {
+                loadManifestFile(bad);
+                fail();
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == "Not a valid manifest: " + bad.string());
+            }
+        }
         try
         {
             loadManifestFile(subdir / "missing.txt");

src/tools/validator-keys/test/SigningKeys_test.cpp

@@ -1,11 +1,14 @@
 #include <xrpl/basics/StringUtilities.h>
 #include <xrpl/basics/base64.h>
+#include <xrpl/json/json_reader.h>
 #include <xrpl/protocol/HashPrefix.h>
 #include <xrpl/protocol/Sign.h>
 
 #include <tools/validator-keys/SigningKeys.h>
 #include <tools/validator-keys/test/KeyFileGuard.h>
 
+#include <fstream>
+
 namespace xrpl {
 
 namespace tests {
@@ -565,13 +568,10 @@ private:
                     continue;
                 auto const token = keys.finishToken(*sigBlob);
 
-                if (!BEAST_EXPECT(token))
-                    continue;
-
-                auto const tokenPublicKey = derivePublicKey(tokenKeyType, token->validationSecret);
+                auto const tokenPublicKey = derivePublicKey(tokenKeyType, token.validationSecret);
 
                 STObject st(sfGeneric);
-                auto const manifest = xrpl::base64Decode(token->manifest);
+                auto const manifest = xrpl::base64Decode(token.manifest);
                 SerialIter sit(manifest.data(), manifest.size());
                 st.set(sit);
 
@@ -717,9 +717,7 @@ private:
                     return;
 
                 // Overwrite file with new sequence
-                auto const token = keys.finishToken(*sigBlob);
-                if (!BEAST_EXPECT(token))
-                    return;
+                keys.finishToken(*sigBlob);
                 BEAST_EXPECT(keys != fileKeys);
                 keys.writeToFile(keyFile);
             }
@@ -731,6 +729,258 @@ private:
         }
     }
 
+    void
+    testKeyFileFields()
+    {
+        testcase("Key File Fields");
+
+        using namespace boost::filesystem;
+
+        path const subdir = "test_key_file";
+        KeyFileGuard const g(*this, subdir.string());
+        path const keyFile = subdir / "validator_keys.json";
+
+        auto const kp = generateKeyPair(KeyType::Ed25519, randomSeed());
+        auto const base = [&kp]() {
+            json::Value jv;
+            jv["key_type"] = "ed25519";
+            jv["secret_key"] = toBase58(TokenType::NodePrivate, kp.second);
+            jv["token_sequence"] = 1;
+            jv["revoked"] = false;
+            return jv;
+        };
+        auto const invalid = [&keyFile](json::Value const& jv, std::string const& field) {
+            return "Key file '" + keyFile.string() + "' contains invalid \"" + field +
+                "\" field: " + jv[field].toStyledString();
+        };
+
+        for (auto const field :
+             {"domain",
+              "manifest",
+              "pending_token_secret",
+              "pending_signing_key",
+              "pending_key_type"})
+        {
+            auto jv = base();
+            jv[field] = 1;
+            testKeyFile(keyFile, jv, invalid(jv, field));
+        }
+        for (auto const field :
+             {"manifest", "pending_token_secret", "pending_signing_key", "pending_key_type"})
+        {
+            auto jv = base();
+            jv[field] = "not valid";
+            testKeyFile(keyFile, jv, invalid(jv, field));
+        }
+        {
+            auto jv = base();
+            jv["manifest"] = "";
+            testKeyFile(keyFile, jv, invalid(jv, "manifest"));
+        }
+        {
+            // An external key whose key_type disagrees with its public key
+            auto jv = base();
+            jv["secret_key"] = "external";
+            jv["public_key"] = toBase58(TokenType::NodePublic, kp.first);
+            jv["key_type"] = "secp256k1";
+            testKeyFile(
+                keyFile,
+                jv,
+                "Key file '" + keyFile.string() +
+                    "' has a \"key_type\" that does not match \"public_key\"");
+        }
+    }
+
+    void
+    testVerifyManifest()
+    {
+        testcase("Verify Manifest");
+
+        using namespace boost::filesystem;
+
+        path const subdir = "test_key_file";
+        KeyFileGuard const g(*this, subdir.string());
+        path const keyFile = subdir / "validator_keys.json";
+
+        auto expectBadManifest = [this, &keyFile](json::Value const& jv) {
+            std::ofstream o(keyFile.string(), std::ios_base::trunc);
+            o << jv.toStyledString();
+            o.close();
+            try
+            {
+                SigningKeys::make_SigningKeys(keyFile).manifest();
+                fail();
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == std::string("Manifest is not properly signed"));
+            }
+        };
+
+        SigningKeys keys(KeyType::Ed25519);
+        keys.createValidatorToken(KeyType::Ed25519);
+        auto const tokenManifest = strHex(makeSlice(keys.manifest()));
+        keys.writeToFile(keyFile);
+        json::Value jv;
+        {
+            std::ifstream in(keyFile.string());
+            json::Reader reader;
+            reader.parse(in, jv);
+        }
+
+        // A token manifest on revoked keys
+        jv["revoked"] = true;
+        expectBadManifest(jv);
+
+        // A revocation manifest on keys that are not revoked
+        SigningKeys revoked(KeyType::Ed25519);
+        revoked.revoke();
+        jv["revoked"] = false;
+        jv["manifest"] = strHex(makeSlice(revoked.manifest()));
+        expectBadManifest(jv);
+
+        // A token manifest of another key
+        jv["manifest"] = tokenManifest;
+        jv["secret_key"] = toBase58(
+            TokenType::NodePrivate, generateKeyPair(KeyType::Ed25519, randomSeed()).second);
+        expectBadManifest(jv);
+    }
+
+    void
+    testExternalSigningKey()
+    {
+        testcase("External Signing Key");
+
+        using namespace boost::filesystem;
+
+        path const subdir = "test_key_file";
+        KeyFileGuard const g(*this, subdir.string());
+        path const keyFile = subdir / "validator_keys.json";
+
+        // The master key is in software here; the signing key is held elsewhere.
+        SigningKeys const signer(KeyType::Ed25519);
+        SigningKeys keys(KeyType::Ed25519);
+
+        auto const start = keys.startValidatorToken(KeyType::Ed25519, signer.publicKey());
+        BEAST_EXPECT(start);
+        // The pending signing key survives a round trip through the key file
+        keys.writeToFile(keyFile);
+        auto fileKeys = SigningKeys::make_SigningKeys(keyFile);
+        BEAST_EXPECT(keys == fileKeys);
+
+        auto const masterSig = *strUnHex(keys.signHex(*start));
+        auto const signingSig = *strUnHex(signer.signHex(*start));
+        auto const manifest =
+            deserializeManifest(base64Decode(fileKeys.finishExternalToken(masterSig, signingSig)));
+        BEAST_EXPECT(manifest && manifest->verify());
+        BEAST_EXPECT(manifest && manifest->signingKey == signer.publicKey());
+        BEAST_EXPECT(manifest && manifest->sequence == 1);
+        BEAST_EXPECT(fileKeys.sequence() == 1);
+
+        // Both signatures must be right
+        try
+        {
+            keys.finishExternalToken(masterSig, masterSig);
+            fail();
+        }
+        catch (std::runtime_error const& e)
+        {
+            BEAST_EXPECT(e.what() == std::string("Manifest is not properly signed"));
+        }
+        // Nothing pending
+        try
+        {
+            SigningKeys(KeyType::Ed25519).finishExternalToken(masterSig, signingSig);
+            fail();
+        }
+        catch (std::runtime_error const& e)
+        {
+            BEAST_EXPECT(
+                e.what() == std::string("No pending token with an external signing key to finish"));
+        }
+        // Revoked keys finish nothing
+        keys.revoke();
+        BEAST_EXPECT(!keys.startValidatorToken(KeyType::Ed25519, signer.publicKey()));
+        for (auto const external : {false, true})
+        {
+            try
+            {
+                if (external)
+                    keys.finishExternalToken(masterSig, signingSig);
+                else
+                    keys.finishToken(masterSig);
+                fail();
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == std::string("Validator keys have been revoked."));
+            }
+        }
+    }
+
+    void
+    testDomainAndHex()
+    {
+        testcase("Domain and Hex");
+
+        SigningKeys keys(KeyType::Ed25519);
+        auto expectError = [this, &keys](std::string const& domain, std::string const& expected) {
+            try
+            {
+                keys.domain(domain);
+                fail(expected);
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == expected);
+            }
+        };
+        expectError("a.b", "The domain must be between 4 and 128 characters long.");
+        expectError(
+            std::string(126, 'a') + ".com",
+            "The domain must be between 4 and 128 characters long.");
+        expectError(
+            "-bad.example", "The domain field must use the '[host.][subdomain.]domain.tld' format");
+        keys.domain("good.example");
+        BEAST_EXPECT(keys.domain() == "good.example");
+
+        try
+        {
+            keys.signHex("zz");
+            fail();
+        }
+        catch (std::runtime_error const& e)
+        {
+            BEAST_EXPECT(e.what() == std::string("Could not decode hex string: zz"));
+        }
+    }
+
+    void
+    testKeyFileGuard()
+    {
+        testcase("Key File Guard");
+
+        using namespace boost::filesystem;
+
+        path const subdir = "test_key_file";
+        {
+            KeyFileGuard const g(*this, subdir.string());
+            // A second guard for the same directory cannot set up
+            try
+            {
+                KeyFileGuard const again(*this, subdir.string());
+                fail();
+            }
+            catch (std::runtime_error const& e)
+            {
+                BEAST_EXPECT(e.what() == "Cannot create directory: " + subdir.string());
+            }
+            // The directory disappearing early is only logged at teardown
+            remove_all(subdir);
+        }
+        BEAST_EXPECT(!exists(subdir));
+    }
+
 public:
     void
     run() override
@@ -746,6 +996,11 @@ public:
         testExternalCreateValidatorToken();
         testExternalRevoke();
         testExternalWriteToFile();
+        testKeyFileFields();
+        testVerifyManifest();
+        testExternalSigningKey();
+        testDomainAndHex();
+        testKeyFileGuard();
     }
 };
 

src/tools/validator-keys/test/ValidatorKeysTool_test.cpp

@@ -868,7 +868,6 @@ private:
         BEAST_EXPECT(toSign);
         auto const manifest = master.finishExternalToken(
             *strUnHex(master.signHex(*toSign)), *strUnHex(external.signHex(*toSign)));
-        BEAST_EXPECT(manifest);
         master.writeToFile(publisher.keyFile);
 
         ToolOptions hardware;
@@ -876,7 +875,7 @@ private:
         hardware.manifestFile = subdir / "manifest.txt";
         {
             std::ofstream o(hardware.manifestFile->string());
-            o << *manifest << "\n";
+            o << manifest << "\n";
         }
         run("start_sign_list",
             {unsignedList.string()},
@@ -895,6 +894,174 @@ private:
             "The signature does not verify under the manifest's signing key");
         run("finish_sign_list", {external.signHex(bytes), unsignedList.string()}, hardware, "");
         BEAST_EXPECT(run("verify_list", {hardware.outFile->string()}, verifier, "") == 0);
+
+        // finish_sign_list appends to a version 2 list
+        {
+            ToolOptions append = hardware;
+            append.listVersion = 2;
+            append.appendFile = v2.outFile;
+            append.outFile = subdir / "vl2c.json";
+            run("finish_sign_list", {external.signHex(bytes), unsignedList.string()}, append, "");
+            BEAST_EXPECT(run("verify_list", {append.outFile->string()}, verifier, "") == 0);
+        }
+        run("finish_sign_list",
+            {external.signHex(bytes), unsignedList.string()},
+            publisher,
+            "finish_sign_list needs --manifest-file");
+
+        // The signed list goes to stdout without --out
+        {
+            coutCapture.str("");
+            ToolOptions stdoutSigner = signer;
+            stdoutSigner.outFile.reset();
+            run("sign_list", {unsignedList.string()}, stdoutSigner, "");
+            BEAST_EXPECT(coutCapture.str().find("\"public_key\"") != std::string::npos);
+        }
+        // Output paths that cannot be opened
+        {
+            ToolOptions bad = signer;
+            bad.outFile = subdir / "missing" / "vl.json";
+            run("sign_list",
+                {unsignedList.string()},
+                bad,
+                "Cannot open output file: " + bad.outFile->string());
+            ToolOptions badToken = publisher;
+            badToken.outFile = subdir / "missing" / "token.txt";
+            run("create_token",
+                {},
+                badToken,
+                "Cannot open output file: " + badToken.outFile->string());
+        }
+        // verify_list input problems
+        run("verify_list",
+            {(subdir / "missing.json").string()},
+            verifier,
+            "Failed to open file: " + (subdir / "missing.json").string());
+        {
+            path const notJson = subdir / "not.json";
+            std::ofstream o(notJson.string());
+            o << "nope\n";
+            o.close();
+            run("verify_list",
+                {notJson.string()},
+                verifier,
+                "Not a JSON document: " + notJson.string());
+        }
+        // Tokens that cannot sign a list: an invalid manifest, a secret of another key
+        auto writeToken = [](path const& file, ValidatorToken const& token) {
+            std::ofstream o(file.string());
+            o << "[validator_token]\n" << tokenToBase64(token) << "\n";
+        };
+        {
+            ToolOptions badManifest = signer;
+            badManifest.tokenFile = subdir / "bad-manifest-token.txt";
+            writeToken(
+                *badManifest.tokenFile,
+                ValidatorToken{"AAAA", generateSecretKey(KeyType::Ed25519, randomSeed())});
+            run("sign_list",
+                {unsignedList.string()},
+                badManifest,
+                "The token's manifest is not valid");
+
+            ToolOptions wrongSecret = signer;
+            wrongSecret.tokenFile = subdir / "wrong-secret-token.txt";
+            writeToken(
+                *wrongSecret.tokenFile,
+                ValidatorToken{manifest, generateSecretKey(KeyType::Ed25519, randomSeed())});
+            run("sign_list",
+                {unsignedList.string()},
+                wrongSecret,
+                "The token's secret does not match its manifest");
+        }
+        // A revoked manifest signs nothing
+        {
+            SigningKeys revokedKeys(KeyType::Ed25519);
+            ToolOptions revoked = hardware;
+            revoked.manifestFile = subdir / "revoked-manifest.txt";
+            std::ofstream o(revoked.manifestFile->string());
+            o << revokedKeys.revoke() << "\n";
+            o.close();
+            run("start_sign_list", {unsignedList.string()}, revoked, "The manifest is revoked");
+            run("finish_sign_list",
+                {external.signHex(bytes), unsignedList.string()},
+                revoked,
+                "The manifest is revoked");
+        }
+    }
+
+    void
+    testDomainCommands()
+    {
+        testcase("Domain Commands");
+
+        std::stringstream coutCapture;
+        CoutRedirect coutRedirect{coutCapture};
+
+        using namespace boost::filesystem;
+
+        path const subdir = "test_key_file";
+        KeyFileGuard const g(*this, subdir.string());
+        ToolOptions options = toolOptions(subdir / "validator_keys.json");
+
+        auto run = [this](
+                       std::string const& command,
+                       std::vector<std::string> const& args,
+                       ToolOptions const& options,
+                       std::string const& expectedError) {
+            try
+            {
+                runCommand(command, args, options);
+                BEAST_EXPECTS(expectedError.empty(), "expected: " + expectedError);
+            }
+            catch (std::exception const& e)
+            {
+                BEAST_EXPECTS(e.what() == expectedError, e.what());
+            }
+        };
+
+        run("create_keys", {}, options, "");
+        // No manifest yet
+        coutCapture.str("");
+        run("show_manifest", {"hex"}, options, "");
+        BEAST_EXPECT(coutCapture.str().find("unavailable") != std::string::npos);
+
+        run("clear_domain", {}, options, "");  // already clear
+        run("set_domain", {"validator.example.com"}, options, "");
+        run("set_domain", {"validator.example.com"}, options, "");  // already set
+        run("attest_domain", {}, options, "");
+        coutCapture.str("");
+        run("show_manifest", {"base64"}, options, "");
+        BEAST_EXPECT(coutCapture.str().find("(Base64)") != std::string::npos);
+        coutCapture.str("");
+        run("show_manifest", {"hex"}, options, "");
+        BEAST_EXPECT(coutCapture.str().find("(Hex)") != std::string::npos);
+
+        // The token sequence is exhausted
+        {
+            auto const kp = generateKeyPair(KeyType::Ed25519, randomSeed());
+            SigningKeys(KeyType::Ed25519, kp.second, std::numeric_limits<std::uint32_t>::max() - 1)
+                .writeToFile(options.keyFile);
+            run("set_domain",
+                {"other.example.com"},
+                options,
+                "Maximum number of tokens have already been generated.\n"
+                "Revoke validator keys if previous token has been compromised.");
+        }
+
+        // Revoked keys refuse domain work and tokens
+        ToolOptions revoked = toolOptions(subdir / "revoked.json");
+        run("create_keys", {}, revoked, "");
+        run("revoke_keys", {}, revoked, "");
+        std::string const revokedError =
+            "Operation error: The specified master key has been revoked!";
+        run("set_domain", {"validator.example.com"}, revoked, revokedError);
+        run("attest_domain", {}, revoked, revokedError);
+        run("finish_token", {"00"}, revoked, "Validator keys have been revoked.");
+        run("finish_token", {"00", "00"}, revoked, "Validator keys have been revoked.");
+        run("finish_token",
+            {"00", "00"},
+            options,
+            "No pending token with an external signing key to finish");
     }
 
 public:
@@ -913,6 +1080,7 @@ public:
         testHexSign();
         testRunCommand();
         testListCommands();
+        testDomainCommands();
     }
 };