From 947f4cbf750919b3d80c4a6c8d5505e33d69ffd6 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Fri, 20 Aug 2021 00:42:16 +1200 Subject: [PATCH] Move register/login signer creation from ctor/pre-ctor to call() Delaying creation until its is needed means we capture/handle any errors correctly in the auth_handlers call() context instead of failing the API call. Simplify some session interfaces as a result. --- src/ga_auth_handlers.cpp | 49 ++++++++++++++++++++++++---------------- src/ga_auth_handlers.hpp | 3 ++- src/ga_rust.cpp | 20 ++++++---------- src/ga_rust.hpp | 4 ++-- src/ga_session.cpp | 19 ++++++---------- src/ga_session.hpp | 3 +-- src/session.cpp | 8 ------- src/session.hpp | 2 -- src/session_impl.cpp | 6 +++++ src/session_impl.hpp | 4 ++-- 10 files changed, 56 insertions(+), 62 deletions(-) diff --git a/src/ga_auth_handlers.cpp b/src/ga_auth_handlers.cpp index ee082ae9b..27c3cc892 100644 --- a/src/ga_auth_handlers.cpp +++ b/src/ga_auth_handlers.cpp @@ -123,8 +123,8 @@ namespace sdk { // Register // register_call::register_call(session& session, const nlohmann::json& hw_device, const std::string& mnemonic) - : auth_handler_impl( - session, "register_user", make_login_signer(session.get_network_parameters(), hw_device, mnemonic)) + : auth_handler_impl(session, "register_user", std::shared_ptr()) + , m_hw_device(hw_device) , m_mnemonic(mnemonic) , m_initialized(false) { @@ -136,6 +136,9 @@ namespace sdk { // Register is a no-op for electrum sessions m_state = state_type::done; } else { + // Create our signer to handle xpub deriving + m_signer = make_login_signer(m_net_params, m_hw_device, m_mnemonic); + // To register, we need the master xpub to identify the wallet, // and the registration xpub to compute the gait_path. signal_hw_request(hw_request::get_xpubs); @@ -173,13 +176,14 @@ namespace sdk { // class login_call : public auth_handler_impl { public: - login_call(session& session, const nlohmann::json& hw_device, const std::string& mnemonic, - const std::string& password); + login_call(session& session, const nlohmann::json& hw_device, const nlohmann::json& credential_data); private: state_type call_impl() override; void initialize(); + const nlohmann::json m_hw_device; + const nlohmann::json m_credential_data; std::string m_challenge; std::string m_master_xpub_bip32; @@ -188,19 +192,33 @@ namespace sdk { bool m_initialized; }; - login_call::login_call( - session& session, const nlohmann::json& hw_device, const std::string& mnemonic, const std::string& password) - : auth_handler_impl(session, "login_user", - make_login_signer(session.get_network_parameters(), hw_device, decrypt_mnemonic(mnemonic, password))) + login_call::login_call(session& session, const nlohmann::json& hw_device, const nlohmann::json& m_credential_data) + : auth_handler_impl(session, "login_user", std::shared_ptr()) + , m_hw_device(hw_device) + , m_credential_data(m_credential_data) , m_initialized(false) { } void login_call::initialize() { + std::string mnemonic; + std::string password; + + if (!m_hw_device.empty() || m_credential_data.contains("mnemonic")) { + mnemonic = json_get_value(m_credential_data, "mnemonic"); + password = json_get_value(m_credential_data, "password"); + // FIXME: Allow a "bip39_passphrase" element to enable bip39 logins + } else if (m_credential_data.contains("pin")) { + mnemonic = m_session->mnemonic_from_pin_data(m_credential_data); + } else { + GDK_RUNTIME_ASSERT_MSG(false, "Invalid credentials"); + } + m_signer = make_login_signer(m_net_params, m_hw_device, decrypt_mnemonic(mnemonic, password)); + if (m_net_params.is_electrum()) { // FIXME: Implement rust login via authenticate() - m_result = m_session->login(m_signer->get_mnemonic(std::string())); + m_result = m_session->login(m_signer); m_state = state_type::done; } else { // We first need the challenge, so ask the caller for the master pubkey. @@ -360,17 +378,8 @@ namespace sdk { auth_handler* get_login_call( session& session, const nlohmann::json& hw_device, const nlohmann::json& credential_data) { - if (!hw_device.empty() || credential_data.contains("mnemonic")) { - const auto mnemonic = json_get_value(credential_data, "mnemonic"); - const auto password = json_get_value(credential_data, "password"); - // FIXME: Allow a "bip39_passphrase" element to enable bip39 logins - return new login_call(session, hw_device, mnemonic, password); - } else if (credential_data.contains("pin")) { - const auto pin = credential_data.at("pin"); - const auto& pin_data = credential_data.at("pin_data"); - const auto mnemonic = session.mnemonic_from_pin_data(pin, pin_data); - const auto password = std::string(); - return new login_call(session, hw_device, mnemonic, password); + if (!hw_device.empty() || credential_data.contains("mnemonic") || credential_data.contains("pin")) { + return new login_call(session, hw_device, credential_data); } else { // Assume watch-only return new watch_only_login_call(session, credential_data); diff --git a/src/ga_auth_handlers.hpp b/src/ga_auth_handlers.hpp index 9abcee561..c4350f038 100644 --- a/src/ga_auth_handlers.hpp +++ b/src/ga_auth_handlers.hpp @@ -14,7 +14,8 @@ namespace sdk { state_type call_impl() override; void initialize(); - std::string m_mnemonic; + const nlohmann::json m_hw_device; + const std::string m_mnemonic; bool m_initialized; }; diff --git a/src/ga_rust.cpp b/src/ga_rust.cpp index 9f0ed9d09..0613c4184 100644 --- a/src/ga_rust.cpp +++ b/src/ga_rust.cpp @@ -180,22 +180,16 @@ namespace sdk { { throw std::runtime_error("register_subaccount_xpubs not implemented"); } - nlohmann::json ga_rust::login(const std::string& mnemonic) + nlohmann::json ga_rust::login(std::shared_ptr signer) { - auto details = nlohmann::json({ { "mnemonic", mnemonic }, { "password", std::string() } }); - - auto ret = call_session("login", details); - m_signer = signer::make_software_signer(m_net_params, mnemonic); - return ret; + m_signer = signer; + auto details + = nlohmann::json({ { "mnemonic", signer->get_mnemonic(std::string()) }, { "password", std::string() } }); + return call_session("login", details); } - std::string ga_rust::mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data) + std::string ga_rust::mnemonic_from_pin_data(const nlohmann::json& pin_data) { - auto details = nlohmann::json({ - { "pin", pin }, - { "pin_data", pin_data }, - }); - - return call_session("mnemonic_from_pin_data", details); + return call_session("mnemonic_from_pin_data", pin_data); } nlohmann::json ga_rust::login_watch_only(const std::string& username, const std::string& password) { diff --git a/src/ga_rust.hpp b/src/ga_rust.hpp index 47f551968..3692bce7f 100644 --- a/src/ga_rust.hpp +++ b/src/ga_rust.hpp @@ -33,8 +33,8 @@ namespace sdk { nlohmann::json authenticate(const std::string& sig_der_hex, const std::string& path_hex, const std::string& root_xpub_bip32, std::shared_ptr signer); void register_subaccount_xpubs(const std::vector& bip32_xpubs); - nlohmann::json login(const std::string& mnemonic); - std::string mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data); + nlohmann::json login(std::shared_ptr signer); + std::string mnemonic_from_pin_data(const nlohmann::json& pin_data); nlohmann::json login_watch_only(const std::string& username, const std::string& password); bool set_watch_only(const std::string& username, const std::string& password); std::string get_watch_only_username(); diff --git a/src/ga_session.cpp b/src/ga_session.cpp index 1c24090a1..3f3b4dd02 100644 --- a/src/ga_session.cpp +++ b/src/ga_session.cpp @@ -1514,12 +1514,6 @@ namespace sdk { } } - nlohmann::json ga_session::login(const std::string& /*mnemonic*/) - { - GDK_RUNTIME_ASSERT(false); - return nlohmann::json(); - } - void ga_session::push_appearance_to_server(session_impl::locker_t& locker) const { GDK_RUNTIME_ASSERT(locker.owns_lock()); @@ -1882,18 +1876,19 @@ namespace sdk { remap_appearance_setting("required_num_blocks", "required_num_blocks"); } - std::string ga_session::mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data) + std::string ga_session::mnemonic_from_pin_data(const nlohmann::json& pin_data) { try { // FIXME: clear password after use - const auto password = get_pin_password(pin, pin_data.at("pin_identifier")); - const std::string salt = pin_data.at("salt"); + const auto& pin = pin_data.at("pin"); + const auto& data = pin_data.at("pin_data"); + const auto password = get_pin_password(pin, data.at("pin_identifier")); + const std::string salt = data.at("salt"); const auto key = pbkdf2_hmac_sha512_256(password, ustring_span(salt)); // FIXME: clear data after use - const auto data = nlohmann::json::parse(aes_cbc_decrypt(key, pin_data.at("encrypted_data"))); - - return data.at("mnemonic"); + const auto decrypted = nlohmann::json::parse(aes_cbc_decrypt(key, data.at("encrypted_data"))); + return decrypted.at("mnemonic"); } catch (const autobahn::call_error& e) { GDK_LOG_SEV(log_level::warning) << "pin login failed:" << e.what(); reset_all_session_data(); diff --git a/src/ga_session.hpp b/src/ga_session.hpp index 11bc6967d..3c98552f2 100644 --- a/src/ga_session.hpp +++ b/src/ga_session.hpp @@ -63,8 +63,7 @@ namespace sdk { void register_subaccount_xpubs(const std::vector& bip32_xpubs); - nlohmann::json login(const std::string& mnemonic); - std::string mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data); + std::string mnemonic_from_pin_data(const nlohmann::json& pin_data); nlohmann::json login_watch_only(const std::string& username, const std::string& password); bool set_watch_only(const std::string& username, const std::string& password); diff --git a/src/session.cpp b/src/session.cpp index 4978d82f3..6143c532f 100644 --- a/src/session.cpp +++ b/src/session.cpp @@ -249,14 +249,6 @@ namespace sdk { }); } - std::string session::mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data) - { - return exception_wrapper([&] { - auto p = get_nonnull_impl(); - return p->mnemonic_from_pin_data(pin, pin_data); - }); - } - bool session::set_watch_only(const std::string& username, const std::string& password) { return exception_wrapper([&] { diff --git a/src/session.hpp b/src/session.hpp index 42f6e4d06..d0a1037e1 100644 --- a/src/session.hpp +++ b/src/session.hpp @@ -40,8 +40,6 @@ namespace sdk { nlohmann::json refresh_assets(const nlohmann::json& params); nlohmann::json validate_asset_domain_name(const nlohmann::json& params); - std::string mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data); - bool set_watch_only(const std::string& username, const std::string& password); std::string get_watch_only_username(); diff --git a/src/session_impl.cpp b/src/session_impl.cpp index f88013e87..b558a1749 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -96,6 +96,12 @@ namespace sdk { // Default impl is a no-op; registration is only meaningful in multisig } + nlohmann::json session_impl::login(std::shared_ptr /*signer*/) + { + GDK_RUNTIME_ASSERT(false); // Only used by rust until it supports HWW + return nlohmann::json(); + } + bool session_impl::set_blinding_nonce( const std::string& /*pubkey_hex*/, const std::string& /*script_hex*/, const std::string& /*nonce_hex*/) { diff --git a/src/session_impl.hpp b/src/session_impl.hpp index bfbadd6fd..b16195351 100644 --- a/src/session_impl.hpp +++ b/src/session_impl.hpp @@ -82,8 +82,8 @@ namespace sdk { const std::string& root_xpub_bip32, std::shared_ptr signer) = 0; virtual void register_subaccount_xpubs(const std::vector& bip32_xpubs) = 0; - virtual nlohmann::json login(const std::string& mnemonic) = 0; - virtual std::string mnemonic_from_pin_data(const std::string& pin, const nlohmann::json& pin_data) = 0; + virtual nlohmann::json login(std::shared_ptr signer); + virtual std::string mnemonic_from_pin_data(const nlohmann::json& pin_data) = 0; virtual nlohmann::json login_watch_only(const std::string& username, const std::string& password) = 0; virtual bool set_watch_only(const std::string& username, const std::string& password) = 0; virtual std::string get_watch_only_username() = 0;