From 80605e4f3ac0b7895a6a73e756e4e5c346881287 Mon Sep 17 00:00:00 2001 From: Trey Del Bonis Date: Mon, 3 Mar 2025 16:49:10 -0500 Subject: [PATCH] bridge-client: improved master xpriv resolution --- .../src/modes/operator/bootstrap.rs | 16 ++- bin/bridge-client/src/xpriv.rs | 99 +++++++++---------- 2 files changed, 60 insertions(+), 55 deletions(-) diff --git a/bin/bridge-client/src/modes/operator/bootstrap.rs b/bin/bridge-client/src/modes/operator/bootstrap.rs index 5a0823fd1..214466686 100644 --- a/bin/bridge-client/src/modes/operator/bootstrap.rs +++ b/bin/bridge-client/src/modes/operator/bootstrap.rs @@ -1,6 +1,6 @@ //! Module to bootstrap the operator node by hooking up all the required services. -use std::{path::PathBuf, sync::Arc, time::Duration}; +use std::{env, path::PathBuf, sync::Arc, time::Duration}; use bitcoin::{ key::{Keypair, Parity}, @@ -36,7 +36,7 @@ use crate::{ }, db::open_rocksdb_database, rpc_server::{self, BridgeRpc}, - xpriv::resolve_xpriv, + xpriv::{resolve_xpriv, OPXPRIV_ENVVAR}, }; // TODO: move this to some common util and make this usable outside tokio @@ -112,13 +112,23 @@ pub(crate) async fn bootstrap(args: Cli) -> anyhow::Result<()> { .expect("cannot get RPC client from pool"); // Get the keypair after deriving the wallet xpriv. - let operator_keys = resolve_xpriv(args.master_xpriv, args.master_xpriv_path)?; + let env_key = match env::var(OPXPRIV_ENVVAR) { + Ok(k) => Some(k), + Err(env::VarError::NotPresent) => None, + Err(env::VarError::NotUnicode(_)) => { + error!("operator master xpriv envvar not unicode, ignoring"); + None + } + }; + + let operator_keys = resolve_xpriv(args.master_xpriv, args.master_xpriv_path, env_key)?; let wallet_xpriv = operator_keys.wallet_xpriv(); let mut keypair = wallet_xpriv.to_keypair(SECP256K1); let mut sk = SecretKey::from_keypair(&keypair); // adjust for parity, which should always be even + // FIXME bake this into the key derivation fn! let (_, parity) = XOnlyPublicKey::from_keypair(&keypair); if matches!(parity, Parity::Odd) { sk = sk.negate(); diff --git a/bin/bridge-client/src/xpriv.rs b/bin/bridge-client/src/xpriv.rs index 1847f8739..30e5f28d2 100644 --- a/bin/bridge-client/src/xpriv.rs +++ b/bin/bridge-client/src/xpriv.rs @@ -1,73 +1,68 @@ //! Parses the operator's master xpriv from a file. -use std::{ - env, - fs::read_to_string, - path::{Path, PathBuf}, -}; +use std::fs::read_to_string; use bitcoin::bip32::Xpriv; use strata_key_derivation::operator::OperatorKeys; -use strata_primitives::keys::ZeroizableXpriv; +use tracing::*; use zeroize::Zeroize; /// The environment variable that contains the operator's master [`Xpriv`]. -const OPXPRIV_ENVVAR: &str = "STRATA_OP_MASTER_XPRIV"; +pub const OPXPRIV_ENVVAR: &str = "STRATA_OP_MASTER_XPRIV"; -/// Parses the master [`Xpriv`] from a file. -pub(crate) fn parse_master_xpriv(path: &Path) -> anyhow::Result { - let mut xpriv_str = read_to_string(path)?; - match xpriv_str.parse::() { - Ok(mut xpriv) => { - // Zeroize the xpriv string after parsing it. - xpriv_str.zeroize(); - - // Parse into ZeroizableXpriv - let zeroizable_xpriv: ZeroizableXpriv = xpriv.into(); - - // Zeroize the xpriv after parsing it. - xpriv.private_key.non_secure_erase(); - - // Finally return the operator keys - // - // NOTE: `zeroizable_xpriv` is zeroized on drop. - Ok(OperatorKeys::new(&zeroizable_xpriv) - .map_err(|_| anyhow::anyhow!("invalid master xpriv"))?) - } - Err(e) => anyhow::bail!("invalid master xpriv: {}", e), - } -} - -/// Resolves the master [`Xpriv`] from CLI arguments or environment variables. +/// Resolves the master [`Xpriv`] from the various sources. /// -/// Precedence order for resolving the master xpriv: +/// Rules: /// -/// 1. If a key is supplied via the `--master-xpriv` CLI argument, it is used. -/// 2. Otherwise, if a file path is supplied via CLI, the key is read from that file. -/// 3. Otherwise, if the `STRATA_OP_MASTER_XPRIV` environment variable is set, its value is used. -/// 4. Otherwise, returns an error. +/// 1. If none are set, error out. +/// 2. If multiple are set, error out. +/// 3. If we have the verbatim key provided, parse it. +/// 4. If we have a path provided, load it and parse that instead. /// /// # Errors /// -/// Returns an error if the master xpriv is invalid or not found. +/// Returns an error if the master xpriv is invalid or not found, or if +/// conflicting options are set. pub(crate) fn resolve_xpriv( cli_arg: Option, cli_path: Option, + env_val: Option, ) -> anyhow::Result { - match (cli_arg, cli_path) { - (Some(xpriv), _) => OperatorKeys::new(&xpriv.parse::()?) - .map_err(|_| anyhow::anyhow!("invalid master xpriv from CLI")), + if cli_arg.is_some() { + error!("FOUND CLI ARG KEY, THIS IS INSECURE!"); + } - (_, Some(path)) => parse_master_xpriv(&PathBuf::from(path)), + let mut xpriv_str: String = match (cli_arg, cli_path, env_val) { + // If there's none set then we error out. + (None, None, None) => { + anyhow::bail!( + "must provide root xpriv with either `--master-xpriv-path` or {OPXPRIV_ENVVAR}" + ) + } - (None, None) => match env::var(OPXPRIV_ENVVAR) { - Ok(xpriv_env_str) => OperatorKeys::new(&xpriv_env_str.parse::()?) - .map_err(|_| anyhow::anyhow!("invalid master xpriv from envvar")), - Err(_) => { - anyhow::bail!( - "must either set {OPXPRIV_ENVVAR} envvar or pass with `--master-xpriv`" - ) - } - }, - } + // If multiple are set then we error out. + (_, Some(_), Some(_)) | (Some(_), Some(_), _) | (Some(_), _, Some(_)) => { + anyhow::bail!("multiple root xpriv options specified, don't know what to do, aborting"); + } + + // In these cases we have the string explicitly. + (Some(xpriv_str), _, _) | (_, _, Some(xpriv_str)) => xpriv_str.to_owned(), + + // In this case we fetch it from file. + (_, Some(path), _) => read_to_string(path)?, + }; + + // Some fancy dance to securely erase things. + let Ok(raw) = xpriv_str.parse::() else { + xpriv_str.zeroize(); + anyhow::bail!("invalid master xpriv"); + }; + + let Ok(keys) = OperatorKeys::new(&raw) else { + xpriv_str.zeroize(); + // TODO how to secure erase raw? + anyhow::bail!("unable to generate leaf keys"); + }; + + Ok(keys) }