From 462152f4a7b3e2d5d012adc79a9c021424dee453 Mon Sep 17 00:00:00 2001 From: isavaima8-alt Date: Sun, 27 Sep 2026 15:44:31 +0000 Subject: [PATCH] fix: harden sealed seeds and trustline validation --- bin/migrate-keys/src/main.rs | 24 ++++-- crates/api/src/routes/sponsor.rs | 15 +++- crates/api/src/state.rs | 11 ++- crates/crypto/src/error.rs | 8 +- crates/crypto/src/lib.rs | 118 +++++++++++++++++++++++++++- crates/wallet-core/src/provision.rs | 16 +++- crates/wallet-core/src/signer.rs | 45 ++++++++++- 7 files changed, 212 insertions(+), 25 deletions(-) diff --git a/bin/migrate-keys/src/main.rs b/bin/migrate-keys/src/main.rs index 4631a3c..dd054d8 100644 --- a/bin/migrate-keys/src/main.rs +++ b/bin/migrate-keys/src/main.rs @@ -53,7 +53,7 @@ use anyhow::{Context, Result}; use base64::Engine; -use octo_crypto::{master_key_from_slice, reseal, MASTER_KEY_LEN, SCHEME_V1}; +use octo_crypto::{master_key_from_slice, reseal_with_account_id, MASTER_KEY_LEN, SCHEME_V2}; use octo_store::Store; use uuid::Uuid; @@ -84,7 +84,7 @@ async fn main() -> Result<()> { loop { let batch = store - .list_wallets_needing_reseal(SCHEME_V1 as i16, cfg.batch_size, after_id) + .list_wallets_needing_reseal(SCHEME_V2 as i16, cfg.batch_size, after_id) .await .context("list_wallets_needing_reseal")?; @@ -117,15 +117,25 @@ async fn main() -> Result<()> { ciphertext.clone(), nonce, salt, - scheme as u8, + u8::try_from(scheme).context("sealed scheme must fit in an unsigned byte")?, ) .with_context(|| format!("from_parts wallet {}", wallet.id))?; // Context is the network string bound into the AEAD AAD (e.g. "octo:mainnet"). let context = format!("octo:{}", wallet.network); - - // reseal: open under old key → re-seal under new key (Zeroizing throughout). - let new_sealed = reseal(&cfg.old_key, &cfg.new_key, &sealed, context.as_bytes()) + let account_id = wallet + .gas_tank_account_g + .as_deref() + .unwrap_or(&wallet.stellar_account_g); + + // Reseal into the account-bound scheme (Zeroizing throughout). + let new_sealed = reseal_with_account_id( + &cfg.old_key, + &cfg.new_key, + &sealed, + context.as_bytes(), + account_id, + ) .with_context(|| format!("reseal wallet {}", wallet.id))?; // Atomically swap the DB record. The idempotency guard (expected_old_scheme) @@ -136,7 +146,7 @@ async fn main() -> Result<()> { &new_sealed.ciphertext, &new_sealed.nonce, &new_sealed.salt, - SCHEME_V1 as i16, + SCHEME_V2 as i16, scheme, ) .await diff --git a/crates/api/src/routes/sponsor.rs b/crates/api/src/routes/sponsor.rs index bfbd912..9d5b62f 100644 --- a/crates/api/src/routes/sponsor.rs +++ b/crates/api/src/routes/sponsor.rs @@ -10,7 +10,9 @@ use axum::extract::{Path, Query, State}; use axum::http::{HeaderMap, StatusCode}; use axum::Json; use octo_crypto::SealedSeed; -use octo_wallet_core::{compute_inner_tx_hash, sign_fee_bump, FeeBumpRequest}; +use octo_wallet_core::{ + compute_inner_tx_hash, sign_fee_bump_with_account_id, FeeBumpRequest, +}; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -117,17 +119,22 @@ pub async fn sponsor( let scheme = wallet .sealed_scheme .unwrap_or(octo_crypto::SCHEME_V1 as i16); - let sealed = SealedSeed::from_parts_with_scheme(ciphertext.clone(), nonce, salt, scheme as u8) + let scheme_byte = u8::try_from(scheme).map_err(|_| ApiError::Internal)?; + let sealed = SealedSeed::from_parts_with_scheme(ciphertext.clone(), nonce, salt, scheme_byte) .map_err(|_| ApiError::Internal)?; let fb = FeeBumpRequest { inner_xdr: &inner_xdr, max_base_fee_stroops: max_fee, }; - let signed = match sign_fee_bump( - state.master_key_for_scheme(scheme), + let Some(account_id) = wallet.gas_tank_account_g.as_deref() else { + return Err(ApiError::Internal); + }; + let signed = match sign_fee_bump_with_account_id( + state.master_key_for_scheme(i16::from(scheme_byte)), &sealed, state.network(), 0, + account_id, &fb, ) { Ok(s) => s, diff --git a/crates/api/src/state.rs b/crates/api/src/state.rs index 05c9f67..bfc724a 100644 --- a/crates/api/src/state.rs +++ b/crates/api/src/state.rs @@ -188,8 +188,7 @@ impl AppState { let raw = base64::engine::general_purpose::STANDARD .decode(b64.trim()) .map_err(|_| ApiError::BadRequest("invalid MASTER_KEY (base64)".into()))?; - master_key_from_slice(&raw) - .map_err(|_| ApiError::BadRequest("MASTER_KEY must be 32 bytes".into())) + master_key_from_slice(&raw).map_err(|error| ApiError::BadRequest(error.to_string())) } pub fn store(&self) -> &Store { @@ -222,9 +221,13 @@ impl AppState { /// /// When no next key is configured, always returns `master_key`. pub fn master_key_for_scheme(&self, sealed_scheme: i16) -> &[u8; MASTER_KEY_LEN] { - use octo_crypto::SCHEME_V1; + use octo_crypto::{SCHEME_V1, SCHEME_V2}; match &self.inner.master_key_next { - Some(next_key) if sealed_scheme == SCHEME_V1 as i16 => next_key, + Some(next_key) + if sealed_scheme == SCHEME_V1 as i16 || sealed_scheme == SCHEME_V2 as i16 => + { + next_key + } _ => &self.inner.master_key, } } diff --git a/crates/crypto/src/error.rs b/crates/crypto/src/error.rs index e9e5bca..fdcb384 100644 --- a/crates/crypto/src/error.rs +++ b/crates/crypto/src/error.rs @@ -10,8 +10,8 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum CryptoError { /// The master key was not exactly 32 bytes (AES-256 requires a 256-bit key). - #[error("invalid master key length: expected 32 bytes")] - InvalidKeyLength, + #[error("invalid master key length: expected {expected} bytes, got {actual}")] + InvalidMasterKeyLength { expected: usize, actual: usize }, /// A stored nonce was not the expected 12 bytes (corrupt record). #[error("invalid nonce length: expected 12 bytes")] @@ -26,6 +26,10 @@ pub enum CryptoError { #[error("encryption failed")] EncryptionFailed, + /// A V2 record was opened without the account identity bound into its AAD. + #[error("account identity is required to open this sealed seed")] + AccountIdentityRequired, + /// The `scheme` tag stored in a [`crate::SealedSeed`] is not a value this version of the /// code knows how to handle. The record must be migrated (re-sealed under the current scheme) /// before it can be opened. diff --git a/crates/crypto/src/lib.rs b/crates/crypto/src/lib.rs index bad41d2..af12430 100644 --- a/crates/crypto/src/lib.rs +++ b/crates/crypto/src/lib.rs @@ -57,6 +57,8 @@ pub const SALT_LEN: usize = 32; /// The current sealing scheme: AES-256-GCM with per-record HKDF-SHA256 subkey derivation and /// context-bound AAD. All new seals are produced with this scheme tag. pub const SCHEME_V1: u8 = 1; +/// AES-256-GCM with the network context and owning account id bound into the AAD. +pub const SCHEME_V2: u8 = 2; /// A sealed secret: the AES-256-GCM ciphertext (including the authentication tag) plus the /// public, non-secret `nonce` and `salt` needed to open it, and an explicit `scheme` version tag @@ -104,6 +106,10 @@ impl SealedSeed { salt: &[u8], scheme: u8, ) -> Result { + match scheme { + SCHEME_V1 | SCHEME_V2 => {} + _ => return Err(CryptoError::UnknownScheme(scheme)), + } let nonce: [u8; NONCE_LEN] = nonce .try_into() .map_err(|_| CryptoError::InvalidNonceLength)?; @@ -175,6 +181,24 @@ pub fn seal( }) } +/// Seal a secret while binding its owning Stellar account id into the authenticated context. +pub fn seal_with_account_id( + master_key: &[u8; MASTER_KEY_LEN], + plaintext: &[u8], + context: &[u8], + account_id: &str, +) -> Result { + if account_id.is_empty() { + return Err(CryptoError::AccountIdentityRequired); + } + let mut bound_context = context.to_vec(); + bound_context.push(0); + bound_context.extend_from_slice(account_id.as_bytes()); + let mut sealed = seal(master_key, plaintext, &bound_context)?; + sealed.scheme = SCHEME_V2; + Ok(sealed) +} + /// Authenticated-decrypt a [`SealedSeed`] produced by [`seal`]. /// /// Returns the plaintext wrapped in [`Zeroizing`] so it is wiped when dropped. Fails with @@ -190,6 +214,7 @@ pub fn open( // Validate the scheme tag before attempting any cryptographic operation. match sealed.scheme { SCHEME_V1 => {} // the only supported scheme + SCHEME_V2 => return Err(CryptoError::AccountIdentityRequired), _ => return Err(CryptoError::UnknownScheme(sealed.scheme)), } @@ -214,6 +239,27 @@ pub fn open( Ok(Zeroizing::new(plaintext)) } +/// Open a V2 secret using the expected owning Stellar account id. +pub fn open_with_account_id( + master_key: &[u8; MASTER_KEY_LEN], + sealed: &SealedSeed, + context: &[u8], + account_id: &str, +) -> Result>, CryptoError> { + if sealed.scheme != SCHEME_V2 { + return open(master_key, sealed, context); + } + if account_id.is_empty() { + return Err(CryptoError::AccountIdentityRequired); + } + let mut bound_context = context.to_vec(); + bound_context.push(0); + bound_context.extend_from_slice(account_id.as_bytes()); + let mut v1 = sealed.clone(); + v1.scheme = SCHEME_V1; + open(master_key, &v1, &bound_context) +} + /// Rotate the master key protecting an already-sealed secret. /// /// Opens `sealed` under `old_key`/`context`, then seals the recovered plaintext under `new_key` @@ -230,9 +276,24 @@ pub fn reseal( seal(new_key, plaintext.as_ref(), context) } +/// Rotate a sealed secret into the account-bound V2 scheme. +pub fn reseal_with_account_id( + old_key: &[u8; MASTER_KEY_LEN], + new_key: &[u8; MASTER_KEY_LEN], + sealed: &SealedSeed, + context: &[u8], + account_id: &str, +) -> Result { + let plaintext = open_with_account_id(old_key, sealed, context, account_id)?; + seal_with_account_id(new_key, plaintext.as_ref(), context, account_id) +} + /// Convenience: parse a 32-byte master key from a byte slice (e.g. decoded from a KMS/env value). pub fn master_key_from_slice(bytes: &[u8]) -> Result<[u8; MASTER_KEY_LEN], CryptoError> { - bytes.try_into().map_err(|_| CryptoError::InvalidKeyLength) + bytes.try_into().map_err(|_| CryptoError::InvalidMasterKeyLength { + expected: MASTER_KEY_LEN, + actual: bytes.len(), + }) } #[cfg(test)] @@ -266,6 +327,45 @@ mod tests { ); } + #[test] + fn account_bound_v2_rejects_the_wrong_account_id() { + let mk = key(); + let sealed = seal_with_account_id(&mk, b"seed", CTX, "GGOOD").unwrap(); + assert!(matches!( + open_with_account_id(&mk, &sealed, CTX, "GBAD"), + Err(CryptoError::DecryptionFailed) + )); + } + + #[test] + fn v1_rows_still_open_with_the_original_context() { + let mk = key(); + let sealed = seal(&mk, b"seed", CTX).unwrap(); + assert_eq!(open(&mk, &sealed, CTX).unwrap().as_slice(), b"seed"); + } + + #[test] + fn reseal_migrates_v1_to_account_bound_v2() { + let old_mk = key(); + let new_mk = key(); + let sealed = seal(&old_mk, b"seed", CTX).unwrap(); + let migrated = reseal_with_account_id( + &old_mk, + &new_mk, + &sealed, + CTX, + "GACCOUNT", + ) + .unwrap(); + assert_eq!(migrated.scheme, SCHEME_V2); + assert_eq!( + open_with_account_id(&new_mk, &migrated, CTX, "GACCOUNT") + .unwrap() + .as_slice(), + b"seed" + ); + } + #[test] fn ciphertext_is_not_plaintext() { let mk = key(); @@ -405,14 +505,26 @@ mod tests { assert!(master_key_from_slice(&[0u8; 32]).is_ok()); assert!(matches!( master_key_from_slice(&[0u8; 31]), - Err(CryptoError::InvalidKeyLength) + Err(CryptoError::InvalidMasterKeyLength { .. }) )); assert!(matches!( master_key_from_slice(&[0u8; 33]), - Err(CryptoError::InvalidKeyLength) + Err(CryptoError::InvalidMasterKeyLength { .. }) )); } + #[test] + fn from_parts_with_scheme_rejects_unknown_scheme() { + let error = SealedSeed::from_parts_with_scheme( + vec![0u8; 16], + &[0u8; NONCE_LEN], + &[0u8; SALT_LEN], + 255, + ) + .unwrap_err(); + assert!(matches!(error, CryptoError::UnknownScheme(255))); + } + // --------------------------------------------------------------------------- // Size-boundary tests // diff --git a/crates/wallet-core/src/provision.rs b/crates/wallet-core/src/provision.rs index 16e79d0..d349e9e 100644 --- a/crates/wallet-core/src/provision.rs +++ b/crates/wallet-core/src/provision.rs @@ -4,7 +4,7 @@ use crate::derive::WalletSeed; use crate::error::WalletError; use crate::signer::StellarNetwork; -use octo_crypto::{seal, SealedSeed, MASTER_KEY_LEN}; +use octo_crypto::{seal_with_account_id, SealedSeed, MASTER_KEY_LEN}; use stellar_base::crypto::DalekKeyPair; use zeroize::Zeroizing; @@ -29,7 +29,12 @@ pub fn provision_wallet( ) -> Result { let (mnemonic, seed) = WalletSeed::generate(); let account_g = master_account_id(&seed)?; - let sealed = seal(master_key, seed.as_bytes(), network.crypto_context())?; + let sealed = seal_with_account_id( + master_key, + seed.as_bytes(), + network.crypto_context(), + &account_g, + )?; Ok(ProvisionedWallet { account_g, sealed, @@ -45,7 +50,12 @@ pub fn import_wallet( ) -> Result { let seed = WalletSeed::from_phrase(mnemonic)?; let account_g = master_account_id(&seed)?; - let sealed = seal(master_key, seed.as_bytes(), network.crypto_context())?; + let sealed = seal_with_account_id( + master_key, + seed.as_bytes(), + network.crypto_context(), + &account_g, + )?; Ok(ProvisionedWallet { account_g, sealed, diff --git a/crates/wallet-core/src/signer.rs b/crates/wallet-core/src/signer.rs index 25b07a2..cf6b992 100644 --- a/crates/wallet-core/src/signer.rs +++ b/crates/wallet-core/src/signer.rs @@ -12,7 +12,7 @@ use crate::derive::WalletSeed; use crate::error::WalletError; -use octo_crypto::{open, SealedSeed, MASTER_KEY_LEN}; +use octo_crypto::{open, open_with_account_id, SealedSeed, MASTER_KEY_LEN}; use stellar_base::crypto::DalekKeyPair; use stellar_base::network::Network; // sign_fee_bump (production, not test-gated) rejects sub-minimum fees against this constant. @@ -260,6 +260,9 @@ pub fn sign_change_trust( account_index: u32, req: &ChangeTrustRequest<'_>, ) -> Result { + if !is_valid_asset_code(req.asset_code) { + return Err(WalletError::InvalidAssetCode); + } if let Some(limit) = req.limit_stroops { if limit < 0 { return Err(WalletError::InvalidAmount); @@ -337,6 +340,36 @@ pub fn sign_fee_bump( network: StellarNetwork, account_index: u32, req: &FeeBumpRequest<'_>, +) -> Result { + sign_fee_bump_impl(master_key, sealed, network, account_index, req, None) +} + +/// Sign a fee bump while binding a V2 sealed seed to its owning account. +pub fn sign_fee_bump_with_account_id( + master_key: &[u8; MASTER_KEY_LEN], + sealed: &SealedSeed, + network: StellarNetwork, + account_index: u32, + account_id: &str, + req: &FeeBumpRequest<'_>, +) -> Result { + sign_fee_bump_impl( + master_key, + sealed, + network, + account_index, + req, + Some(account_id), + ) +} + +fn sign_fee_bump_impl( + master_key: &[u8; MASTER_KEY_LEN], + sealed: &SealedSeed, + network: StellarNetwork, + account_index: u32, + req: &FeeBumpRequest<'_>, + account_id: Option<&str>, ) -> Result { use sha2::{Digest, Sha256}; use stellar_base::xdr::{ @@ -357,7 +390,15 @@ pub fn sign_fee_bump( let inner_v1 = parse_inner_v1(req.inner_xdr)?; // Derive the signing key for the fee source (decrypt → derive → zeroize on drop). - let seed_bytes = open(master_key, sealed, network.crypto_context())?; + let seed_bytes = match account_id { + Some(account_id) => open_with_account_id( + master_key, + sealed, + network.crypto_context(), + account_id, + )?, + None => open(master_key, sealed, network.crypto_context())?, + }; let seed = WalletSeed::from_bytes(seed_bytes.to_vec()); let secret = seed.derive_ed25519_secret(account_index); let signing_key = ed25519_dalek::SigningKey::from_bytes(&secret);