From 5e5d5fdee4992b4d6969a24437541a48d9498f81 Mon Sep 17 00:00:00 2001 From: wilsonianb Date: Tue, 19 Apr 2016 11:09:06 -0700 Subject: [PATCH] Make wallet_propose seed generation consistent: Allow 'seed' or 'seed_hex' if 'key_type' is not specified. Use legacy passphrase seed generation if 'key_type' is specified. --- src/ripple/rpc/handlers/WalletPropose.cpp | 25 ++--- src/ripple/rpc/impl/KeypairForSignature.cpp | 5 +- src/ripple/rpc/tests/KeyGeneration.test.cpp | 105 +++++++++++++------- 3 files changed, 80 insertions(+), 55 deletions(-) diff --git a/src/ripple/rpc/handlers/WalletPropose.cpp b/src/ripple/rpc/handlers/WalletPropose.cpp index a4da5e7eb..d113f0729 100644 --- a/src/ripple/rpc/handlers/WalletPropose.cpp +++ b/src/ripple/rpc/handlers/WalletPropose.cpp @@ -82,31 +82,22 @@ Json::Value walletPropose (Json::Value const& params) if (keyType == KeyType::invalid) return rpcError(rpcINVALID_PARAMS); - - if (params.isMember (jss::passphrase) || - params.isMember (jss::seed) || - params.isMember (jss::seed_hex)) - { - seed = RPC::getSeedFromRPC (params); - } - else - { - seed = randomSeed (); - } } - else if (params.isMember (jss::passphrase)) + + if (params.isMember (jss::passphrase) || + params.isMember (jss::seed) || + params.isMember (jss::seed_hex)) { - seed = parseGenericSeed ( - params[jss::passphrase].asString()); + seed = RPC::getSeedFromRPC (params); + + if (!seed) + return rpcError(rpcBAD_SEED); } else { seed = randomSeed (); } - if (!seed) - return rpcError(rpcBAD_SEED); - auto const publicKey = generateKeyPair (keyType, *seed).first; Json::Value obj (Json::objectValue); diff --git a/src/ripple/rpc/impl/KeypairForSignature.cpp b/src/ripple/rpc/impl/KeypairForSignature.cpp index e057accc6..77a1916d5 100644 --- a/src/ripple/rpc/impl/KeypairForSignature.cpp +++ b/src/ripple/rpc/impl/KeypairForSignature.cpp @@ -30,9 +30,6 @@ namespace RPC { boost::optional getSeedFromRPC (Json::Value const& params) { - // This function is only called when `key_type` is present. - assert (params.isMember (jss::key_type)); - bool const hasPassphrase = params.isMember (jss::passphrase); bool const hasSeed = params.isMember (jss::seed); bool const hasHexSeed = params.isMember (jss::seed_hex); @@ -48,7 +45,7 @@ getSeedFromRPC (Json::Value const& params) return parseBase58 (params[jss::seed].asString()); if (hasPassphrase) - return generateSeed (params[jss::passphrase].asString()); + return parseGenericSeed (params[jss::passphrase].asString()); if (hasHexSeed) { diff --git a/src/ripple/rpc/tests/KeyGeneration.test.cpp b/src/ripple/rpc/tests/KeyGeneration.test.cpp index 02c7e9b1e..5a75652f5 100644 --- a/src/ripple/rpc/tests/KeyGeneration.test.cpp +++ b/src/ripple/rpc/tests/KeyGeneration.test.cpp @@ -74,9 +74,11 @@ static key_strings const ed25519_strings = class WalletPropose_test : public ripple::TestSuite { public: - void testRandomWallet() + void testRandomWallet(boost::optional const& keyType) { Json::Value params; + if (keyType) + params[jss::key_type] = *keyType; Json::Value result = walletPropose (params); expect (! contains_error (result)); @@ -86,6 +88,11 @@ public: expect (result.isMember (jss::master_seed_hex)); expect (result.isMember (jss::public_key)); expect (result.isMember (jss::public_key_hex)); + expect (result.isMember (jss::key_type)); + + expectEquals (result[jss::key_type], + params.isMember (jss::key_type) ? params[jss::key_type] + : "secp256k1"); std::string seed = result[jss::master_seed].asString(); @@ -106,55 +113,85 @@ public: expectEquals (result[jss::master_seed_hex], s.master_seed_hex); expectEquals (result[jss::public_key], s.public_key); expectEquals (result[jss::public_key_hex], s.public_key_hex); + expectEquals (result[jss::key_type], + params.isMember (jss::key_type) ? params[jss::key_type] + : "secp256k1"); } - void testLegacyPassphrase (char const* value) + void testSeed (boost::optional const& keyType, + key_strings const& strings) { - testcase (value); + testcase ("seed"); Json::Value params; - params[jss::passphrase] = value; - - testSecretWallet (params, secp256k1_strings); - } - - void testLegacyPassphrase() - { - testLegacyPassphrase (common::passphrase); - testLegacyPassphrase (secp256k1_strings.master_key); - testLegacyPassphrase (secp256k1_strings.master_seed); - testLegacyPassphrase (secp256k1_strings.master_seed_hex); - } - - void testKeyType (char const* keyType, key_strings const& strings) - { - testcase (keyType); - - Json::Value params; - params[jss::key_type] = keyType; - - expect (! contains_error (walletPropose (params))); - - params[jss::passphrase] = common::passphrase; + if (keyType) + params[jss::key_type] = *keyType; + params[jss::seed] = strings.master_seed; testSecretWallet (params, strings); + } + void testSeedHex (boost::optional const& keyType, + key_strings const& strings) + { + testcase ("seed_hex"); + + Json::Value params; + if (keyType) + params[jss::key_type] = *keyType; + params[jss::seed_hex] = strings.master_seed_hex; + + testSecretWallet (params, strings); + } + + void testLegacyPassphrase (char const* value, + boost::optional const& keyType, + key_strings const& strings) + { + Json::Value params; + if (keyType) + params[jss::key_type] = *keyType; + params[jss::passphrase] = value; + + testSecretWallet (params, strings); + } + + void testLegacyPassphrase(boost::optional const& keyType, + key_strings const& strings) + { + testcase ("passphrase"); + + testLegacyPassphrase (common::passphrase, keyType, strings); + testLegacyPassphrase (strings.master_key, keyType, strings); + testLegacyPassphrase (strings.master_seed, keyType, strings); + testLegacyPassphrase (strings.master_seed_hex, keyType, strings); + } + + void testKeyType (boost::optional const& keyType, + key_strings const& strings) + { + testcase (keyType ? *keyType : "no key_type"); + + testRandomWallet (keyType); + testSeed (keyType, strings); + testSeedHex (keyType, strings); + testLegacyPassphrase (keyType, strings); + + Json::Value params; + if (keyType) + params[jss::key_type] = *keyType; params[jss::seed] = strings.master_seed; + params[jss::seed_hex] = strings.master_seed_hex; // Secret fields are mutually exclusive. expect (contains_error (walletPropose (params))); - - params.removeMember (jss::passphrase); - - testSecretWallet (params, strings); } void run() { - testRandomWallet(); - testLegacyPassphrase(); - testKeyType ("secp256k1", secp256k1_strings); - testKeyType ("ed25519", ed25519_strings); + testKeyType (boost::none, secp256k1_strings); + testKeyType (std::string("secp256k1"), secp256k1_strings); + testKeyType (std::string("ed25519"), ed25519_strings); } };