ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2025-11-19 10:05:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Disallow using the master key as the regular key:

Browse Source 
The XRP Ledger allows an account to authorize a secondary key pair,
called a regular key pair, to sign future transactions, while keeping
the master key pair offline.

The regular key pair can be changed as often as desired, without
requiring other changes on the account.

If merged, this commit corrects a minor technical flaw which would
allow an account holder to specify the master key as the account's
new regular key.

The change is controlled by the `fixMasterKeyAsRegularKey` amendment
which, if enabled, will:

1. Prevent specifying an account's master key as the account's
   regular key.
2. Prevent the "Disable Master Key" flag from incorrectly affecting
   regular keys.
This commit is contained in:
John Freeman
2019-03-05 12:14:37 -06:00
committed by seelabs
parent 9372a587e4
commit c5a938de55
8 changed files with 168 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/ripple/app/tx/impl/SetRegularKey.cpp
View File

@@ -19,6 +19,7 @@
#include <ripple/app/tx/impl/SetRegularKey.h>
#include <ripple/basics/Log.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/TxFlags.h>
namespace ripple {
@@ -65,6 +66,14 @@ SetRegularKey::preflight (PreflightContext const& ctx)
return temINVALID_FLAG;
}
if (ctx.rules.enabled(fixMasterKeyAsRegularKey)
&& ctx.tx.isFieldPresent(sfRegularKey)
&& (ctx.tx.getAccountID(sfRegularKey) == ctx.tx.getAccountID(sfAccount)))
{
return temBAD_REGKEY;
}
return preflight2(ctx);
}
@@ -84,9 +93,9 @@ SetRegularKey::doApply ()
}
else
{
// Account has disabled master key and no multi-signer signer list.
if (sle->isFlag (lsfDisableMaster) &&
!view().peek (keylet::signers (account_)))
// Account has disabled master key and no multi-signer signer list.
return tecNO_ALTERNATIVE_KEY;
sle->makeFieldAbsent (sfRegularKey);

63
src/ripple/app/tx/impl/Transactor.cpp
View File

@@ -31,6 +31,7 @@
#include <ripple/protocol/Indexes.h>
#include <ripple/protocol/UintTypes.h>
#include <ripple/protocol/Protocol.h>
#include <ripple/protocol/STAccount.h>
namespace ripple {
@@ -340,44 +341,68 @@ Transactor::checkSign (PreclaimContext const& ctx)
NotTEC
Transactor::checkSingleSign (PreclaimContext const& ctx)
{
auto const id = ctx.tx.getAccountID(sfAccount);
auto const sle = ctx.view.read(
keylet::account(id));
auto const hasAuthKey = sle->isFieldPresent (sfRegularKey);
// Consistency: Check signature & verify the transaction's signing
// public key is authorized for signing.
auto const spk = ctx.tx.getSigningPubKey();
if (!publicKeyType (makeSlice (spk)))
// Check that the value in the signing key slot is a public key.
auto const pkSigner = ctx.tx.getSigningPubKey();
if (!publicKeyType(makeSlice(pkSigner)))
{
JLOG(ctx.j.trace()) <<
"checkSingleSign: signing public key type is unknown";
return tefBAD_AUTH; // FIXME: should be better error!
}
auto const pkAccount = calcAccountID (
PublicKey (makeSlice (spk)));
// Look up the account.
auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner)));
auto const idAccount = ctx.tx.getAccountID(sfAccount);
auto const sleAccount = ctx.view.read(keylet::account(idAccount));
bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster);
if (pkAccount == id)
if (ctx.view.rules().enabled(fixMasterKeyAsRegularKey))
{
// Authorized to continue.
if (sle->isFlag(lsfDisableMaster))
// Signed with regular key.
if ((*sleAccount)[~sfRegularKey] == idSigner)
{
return tesSUCCESS;
}
// Signed with enabled mater key.
if (!isMasterDisabled && idAccount == idSigner)
{
return tesSUCCESS;
}
// Signed with disabled master key.
if (isMasterDisabled && idAccount == idSigner)
{
return tefMASTER_DISABLED;
}
// Signed with any other key.
return tefBAD_AUTH;
}
if (idSigner == idAccount)
{
// Signing with the master key. Continue if it is not disabled.
if (isMasterDisabled)
return tefMASTER_DISABLED;
}
else if (hasAuthKey &&
(pkAccount == sle->getAccountID (sfRegularKey)))
else if ((*sleAccount)[~sfRegularKey] == idSigner)
{
// Authorized to continue.
// Signing with the regular key. Continue.
}
else if (hasAuthKey)
else if (sleAccount->isFieldPresent(sfRegularKey))
{
// Signing key does not match master or regular key.
JLOG(ctx.j.trace()) <<
"checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH;
}
else
{
// No regular key on account and signing key does not match master key.
// FIXME: Why differentiate this case from tefBAD_AUTH?
JLOG(ctx.j.trace()) <<
"checkSingleSign: Not authorized to use account.";
return tefBAD_AUTH_MASTER;

4
src/ripple/protocol/Feature.h
View File

@@ -83,7 +83,8 @@ class FeatureCollections
"fix1515",
"fix1578",
"MultiSignReserve",
"fixTakerDryOfferRemoval"
"fixTakerDryOfferRemoval",
"fixMasterKeyAsRegularKey"
};
std::vector<uint256> features;
@@ -372,6 +373,7 @@ extern uint256 const fix1515;
extern uint256 const fix1578;
extern uint256 const featureMultiSignReserve;
extern uint256 const fixTakerDryOfferRemoval;
extern uint256 const fixMasterKeyAsRegularKey;
} // ripple

1
src/ripple/protocol/TER.h
View File

@@ -87,6 +87,7 @@ enum TEMcodes : TERUnderlyingType
temBAD_OFFER,
temBAD_PATH,
temBAD_PATH_LOOP,
temBAD_REGKEY,
temBAD_SEND_XRP_LIMIT,
temBAD_SEND_XRP_MAX,
temBAD_SEND_XRP_NO_DIRECT,

2
src/ripple/protocol/impl/Feature.cpp
View File

@@ -117,6 +117,7 @@ detail::supportedAmendments ()
"fix1578",
"MultiSignReserve",
"fixTakerDryOfferRemoval",
"fixMasterKeyAsRegularKey",
};
return supported;
}
@@ -173,5 +174,6 @@ uint256 const fix1515 = *getRegisteredFeature("fix1515");
uint256 const fix1578 = *getRegisteredFeature("fix1578");
uint256 const featureMultiSignReserve = *getRegisteredFeature("MultiSignReserve");
uint256 const fixTakerDryOfferRemoval = *getRegisteredFeature("fixTakerDryOfferRemoval");
uint256 const fixMasterKeyAsRegularKey = *getRegisteredFeature("fixMasterKeyAsRegularKey");
} // ripple

1
src/ripple/protocol/impl/TER.cpp
View File

@@ -119,6 +119,7 @@ transResults()
{ temBAD_PATH, { "temBAD_PATH", "Malformed: Bad path." } },
{ temBAD_PATH_LOOP, { "temBAD_PATH_LOOP", "Malformed: Loop in path." } },
{ temBAD_QUORUM, { "temBAD_QUORUM", "Malformed: Quorum is unreachable." } },
{ temBAD_REGKEY, { "temBAD_REGKEY", "Malformed: Regular key cannot be same as master key." } },
{ temBAD_SEND_XRP_LIMIT, { "temBAD_SEND_XRP_LIMIT", "Malformed: Limit quality is not allowed for XRP to XRP." } },
{ temBAD_SEND_XRP_MAX, { "temBAD_SEND_XRP_MAX", "Malformed: Send max is not allowed for XRP to XRP." } },
{ temBAD_SEND_XRP_NO_DIRECT, { "temBAD_SEND_XRP_NO_DIRECT","Malformed: No Ripple direct is not allowed for XRP to XRP." } },

107
src/test/app/SetRegularKey_test.cpp
View File

@@ -30,33 +30,121 @@ public:
void testDisableMasterKey()
{
using namespace test::jtx;
Env env(*this);
testcase("Set regular key");
Env env {*this, supported_amendments() - fixMasterKeyAsRegularKey};
Account const alice("alice");
Account const bob("bob");
env.fund(XRP(10000), alice, bob);
// Master and Regular key
env(regkey(alice, bob));
auto const ar = env.le(alice);
BEAST_EXPECT(ar->isFieldPresent(sfRegularKey) && (ar->getAccountID(sfRegularKey) == bob.id()));
env(noop(alice));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice));
// Regular key only
testcase("Disable master key");
env(fset(alice, asfDisableMaster), sig(alice));
env(noop(alice));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice), ter(tefMASTER_DISABLED));
testcase("Re-enable master key");
env(fclear(alice, asfDisableMaster), sig(alice), ter(tefMASTER_DISABLED));
env(fclear(alice, asfDisableMaster), sig(bob));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice));
testcase("Revoke regular key");
env(regkey(alice, disabled));
env(noop(alice), sig(bob), ter(tefBAD_AUTH_MASTER));
env(noop(alice), sig(alice));
}
void testDisableMasterKeyAfterFix()
{
using namespace test::jtx;
testcase("Set regular key");
Env env{*this, supported_amendments() | fixMasterKeyAsRegularKey};
Account const alice("alice");
Account const bob("bob");
env.fund(XRP(10000), alice, bob);
env(regkey(alice, bob));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice));
testcase("Disable master key");
env(fset(alice, asfDisableMaster), sig(alice));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice), ter(tefMASTER_DISABLED));
testcase("Re-enable master key");
env(
fclear(alice, asfDisableMaster),
sig(alice),
ter(tefMASTER_DISABLED)
);
env(fclear(alice, asfDisableMaster), sig(bob));
env(noop(alice), sig(bob));
env(noop(alice), sig(alice));
testcase("Revoke regular key");
env(regkey(alice, disabled));
env(noop(alice), sig(bob), ter(tefBAD_AUTH));
env(noop(alice), sig(alice));
}
void testDisabledRegularKey()
{
using namespace test::jtx;
// See https://ripplelabs.atlassian.net/browse/RIPD-1721.
testcase("Set regular key to master key (before fixMasterKeyAsRegularKey)");
Env env {*this, supported_amendments() - fixMasterKeyAsRegularKey};
Account const alice("alice");
env.fund(XRP(10000), alice);
// Must be possible unless amendment `fixMasterKeyAsRegularKey` enabled.
env(regkey(alice, alice), sig(alice));
env(fset(alice, asfDisableMaster), sig(alice));
// No way to sign...
env(noop(alice), ter(tefMASTER_DISABLED));
env(noop(alice), sig(alice), ter(tefMASTER_DISABLED));
// ... until now.
env.enableFeature(fixMasterKeyAsRegularKey);
env(noop(alice));
env(noop(alice), sig(alice));
env(regkey(alice, disabled), ter(tecNO_ALTERNATIVE_KEY));
env(fclear(alice, asfDisableMaster));
env(regkey(alice, disabled));
env(fset(alice, asfDisableMaster), ter(tecNO_ALTERNATIVE_KEY));
}
void testDisableRegularKeyAfterFix()
{
using namespace test::jtx;
testcase("Set regular key to master key (after fixMasterKeyAsRegularKey)");
Env env {*this, supported_amendments() | fixMasterKeyAsRegularKey};
Account const alice("alice");
env.fund(XRP(10000), alice);
// Must be possible unless amendment `fixMasterKeyAsRegularKey` enabled.
env(regkey(alice, alice), ter(temBAD_REGKEY));
}
void testPasswordSpent()
{
using namespace test::jtx;
testcase("Password spent");
Env env(*this);
Account const alice("alice");
Account const bob("bob");
@@ -79,9 +167,11 @@ public:
BEAST_EXPECT(ar->isFieldPresent(sfFlags) && ((ar->getFieldU32(sfFlags) & lsfPasswordSpent) == 0));
}
void testUniversalMaskError()
void testUniversalMask()
{
using namespace test::jtx;
testcase("Universal mask");
Env env(*this);
Account const alice("alice");
Account const bob("bob");
@@ -95,8 +185,11 @@ public:
void run() override
{
testDisableMasterKey();
testDisableMasterKeyAfterFix();
testDisabledRegularKey();
testDisableRegularKeyAfterFix();
testPasswordSpent();
testUniversalMaskError();
testUniversalMask();
}
};

14
src/test/jtx/Env_test.cpp
View File

@@ -250,7 +250,7 @@ public:
{
using namespace jtx;
Env env(*this);
Env env{*this, supported_amendments() | fixMasterKeyAsRegularKey};
Account const alice("alice", KeyType::ed25519);
Account const bob("bob", KeyType::secp256k1);
Account const carol("carol");
@@ -259,12 +259,12 @@ public:
// Master key only
env(noop(alice));
env(noop(bob));
env(noop(alice), sig("alice"), ter(tefBAD_AUTH_MASTER));
env(noop(alice), sig("alice"), ter(tefBAD_AUTH));
env(noop(alice), sig(Account("alice",
KeyType::secp256k1)), ter(tefBAD_AUTH_MASTER));
KeyType::secp256k1)), ter(tefBAD_AUTH));
env(noop(bob), sig(Account("bob",
KeyType::ed25519)), ter(tefBAD_AUTH_MASTER));
env(noop(alice), sig(carol), ter(tefBAD_AUTH_MASTER));
KeyType::ed25519)), ter(tefBAD_AUTH));
env(noop(alice), sig(carol), ter(tefBAD_AUTH));
// Master and Regular key
env(regkey(alice, bob));
@@ -302,7 +302,7 @@ public:
env(pay(env.master, "alice", XRP(1000)), seq(none), ter(temMALFORMED));
env(pay(env.master, "alice", XRP(1000)), seq(20), ter(terPRE_SEQ));
env(pay(env.master, "alice", XRP(1000)), sig(none), ter(temMALFORMED));
env(pay(env.master, "alice", XRP(1000)), sig("bob"), ter(tefBAD_AUTH_MASTER));
env(pay(env.master, "alice", XRP(1000)), sig("bob"), ter(tefBAD_AUTH));
env(pay(env.master, "dilbert", XRP(1000)), sig(env.master));
@@ -344,7 +344,7 @@ public:
env(fclear("alice", asfDisableMaster));
env.require(nflags("alice", asfDisableMaster));
env(regkey("alice", disabled));
env(noop("alice"), sig("eric"), ter(tefBAD_AUTH_MASTER));
env(noop("alice"), sig("eric"), ter(tefBAD_AUTH));
env(noop("alice"));
}