ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-27 15:28:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: refuse symlinked key file temporaries and keep secrets out of errors

Browse Source
This commit is contained in:
Denis Angell
2026-09-14 14:35:34 -04:00
parent b51d8c1140
commit 5e7da075fa
3 changed files with 32 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/tests/tools/validator-keys/SigningKeys.cpp
View File

@@ -69,10 +69,9 @@ protected:
}
[[nodiscard]] std::string
invalidField(json::Value const& jv, std::string const& field) const
invalidField(std::string const& field) const
{
return "Key file '" + keyFile_.string() + "' contains invalid \"" + field +
"\" field: " + jv[field].toStyledString();
return "Key file '" + keyFile_.string() + "' contains invalid \"" + field + "\" field";
}
static json::Value
@@ -171,6 +170,13 @@ TEST_F(SigningKeysTest, write_to_file_errors)
errorOf([&] { keys.writeToFile(inSealed); }),
"Cannot write key file: " + inSealed.string());
std::filesystem::permissions(sealed, std::filesystem::perms::owner_all);
// The temporary beside the key file is a symlink
auto const linked = std::filesystem::path(dir_.file("linked.json"));
std::filesystem::create_symlink(dir_.file("elsewhere.json"), linked.string() + ".tmp");
EXPECT_EQ(
errorOf([&] { keys.writeToFile(linked); }),
"Refusing to write through a symlink: " + linked.string() + ".tmp");
}
TEST_F(SigningKeysTest, key_file_fields)
@@ -193,19 +199,19 @@ TEST_F(SigningKeysTest, key_file_fields)
"Key file '" + keyFile_.string() + "' is missing \"" + field + "\" field");
jv[field] = "dummy";
}
EXPECT_EQ(loadError(jv), invalidField(jv, "key_type"));
EXPECT_EQ(loadError(jv), invalidField("key_type"));
auto const kp = generateKeyPair(KeyType::Ed25519, randomSeed());
jv["key_type"] = "ed25519";
EXPECT_EQ(loadError(jv), invalidField(jv, "token_sequence"));
EXPECT_EQ(loadError(jv), invalidField("token_sequence"));
jv["token_sequence"] = -1;
EXPECT_EQ(loadError(jv), invalidField(jv, "token_sequence"));
EXPECT_EQ(loadError(jv), invalidField("token_sequence"));
jv["token_sequence"] = true;
EXPECT_EQ(loadError(jv), invalidField(jv, "token_sequence"));
EXPECT_EQ(loadError(jv), invalidField("token_sequence"));
jv["token_sequence"] = json::UInt(kMaxSequence);
EXPECT_EQ(loadError(jv), invalidField(jv, "revoked"));
EXPECT_EQ(loadError(jv), invalidField("revoked"));
jv["revoked"] = false;
EXPECT_EQ(loadError(jv), invalidField(jv, "secret_key"));
EXPECT_EQ(loadError(jv), invalidField("secret_key"));
jv["secret_key"] = toBase58(TokenType::NodePrivate, kp.second);
EXPECT_EQ(loadError(jv), "");
@@ -223,19 +229,19 @@ TEST_F(SigningKeysTest, key_file_fields)
{
bad["pending_key_type"] = "ed25519";
}
EXPECT_EQ(loadError(bad), invalidField(bad, field)) << field;
EXPECT_EQ(loadError(bad), invalidField(field)) << field;
}
for (auto const* field : {"manifest", "pending_token_secret", "pending_signing_key"})
{
auto bad = baseKeyFile(kp.second);
bad[field] = "not valid";
bad["pending_key_type"] = "ed25519";
EXPECT_EQ(loadError(bad), invalidField(bad, field)) << field;
EXPECT_EQ(loadError(bad), invalidField(field)) << field;
}
{
auto bad = baseKeyFile(kp.second);
bad["manifest"] = "";
EXPECT_EQ(loadError(bad), invalidField(bad, "manifest"));
EXPECT_EQ(loadError(bad), invalidField("manifest"));
}
{
auto bad = baseKeyFile(kp.second);
@@ -251,7 +257,7 @@ TEST_F(SigningKeysTest, key_file_fields)
loadError(bad),
"Key file '" + keyFile_.string() + "' is missing \"pending_key_type\" field");
bad["pending_key_type"] = "dummy";
EXPECT_EQ(loadError(bad), invalidField(bad, "pending_key_type"));
EXPECT_EQ(loadError(bad), invalidField("pending_key_type"));
bad["pending_key_type"] = "ed25519";
bad["pending_signing_key"] = toBase58(TokenType::NodePublic, kp.first);
EXPECT_EQ(
@@ -272,7 +278,7 @@ TEST_F(SigningKeysTest, external_key_file_fields)
EXPECT_EQ(
loadError(jv), "Key file '" + keyFile_.string() + "' is missing \"public_key\" field");
jv["public_key"] = "dummy public";
EXPECT_EQ(loadError(jv), invalidField(jv, "public_key"));
EXPECT_EQ(loadError(jv), invalidField("public_key"));
jv["public_key"] = toBase58(TokenType::NodePublic, kp.first);
jv["key_type"] = "secp256k1";
EXPECT_EQ(

7
src/tools/validator-keys/Commands.cpp
View File

@@ -101,6 +101,13 @@ public:
file_,
std::filesystem::perms::owner_read | std::filesystem::perms::owner_write,
ec);
if (ec)
{
stream_.close(); // LCOV_EXCL_LINE
std::filesystem::remove(file_, ec); // LCOV_EXCL_LINE
throw std::runtime_error( // LCOV_EXCL_LINE
"Cannot restrict output file: " + file_.string()); // LCOV_EXCL_LINE
}
}
}

8
src/tools/validator-keys/SigningKeys.cpp
View File

@@ -159,10 +159,10 @@ SigningKeys::makeSigningKeys(std::filesystem::path const& keyFile)
}
}
auto const invalidField = [&keyFile, &jKeys](std::string const& field) {
// The value is not repeated: it may be a secret.
auto const invalidField = [&keyFile](std::string const& field) {
return std::runtime_error(
"Key file '" + keyFile.string() + "' contains invalid \"" + field +
"\" field: " + jKeys[field].toStyledString());
"Key file '" + keyFile.string() + "' contains invalid \"" + field + "\" field");
};
auto const keyType = keyTypeFromString(jKeys["key_type"].asString());
@@ -308,6 +308,8 @@ SigningKeys::writeToFile(std::filesystem::path const& keyFile) const
// Write beside the key file, restrict it to the owner, then replace the
// key file in one step.
auto const temp = fs::path(keyFile.string() + ".tmp");
if (fs::is_symlink(temp))
throw std::runtime_error("Refusing to write through a symlink: " + temp.string());
{
std::ofstream o(temp, std::ios_base::trunc);
if (!o.fail())