From ecf5e8dcf6640dacbd397e329f50da2a9b4ee533 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:49:55 +0300 Subject: [PATCH 1/7] feat(platform-wallet): secp256k1 primitives, identity-update parsing, and scoped signing keys for DashConnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The iOS DashConnect port (`dash-key:` / `dash-st:` passwordless login, already shipping on Android) needs three capabilities the FFI does not expose, and one FFI validation rule that is stricter than consensus. - `secp256k1_primitives.rs`: verify a compressed point, derive a compressed public key, and compute the raw affine ECDH X — handle-free, over the already-linked `dashcore::secp256k1`. The existing DashPay ECDH takes a derivation path and returns a finished secret, so neither the ephemeral key nor the unhashed X is reachable through it. - `identity_update.rs`: parse-only entry points for a serialized `IdentityUpdateTransition`, so the wallet can verify an app-supplied transition adds exactly the login keys it derived instead of broadcasting foreign bytes blind. No signing, no broadcast. - `KeychainSigner.withAdditionalSigningKeys`: a scoped in-memory key registry the sign trampoline consults first, zeroed on scope exit — proof of possession for keys that are derived on demand and never persisted. - `decode_contract_bounds` no longer requires contract bounds for ENCRYPTION / DECRYPTION. Consensus accepts unbounded keys for every purpose (`validate_identity_public_key_contract_bounds/v1`), and real testnet `dash-st:` transitions carry such a key, so the old guard rejected a key Platform considers valid. Co-Authored-By: Claude Opus 5 --- .../src/identity_registration_with_signer.rs | 151 +++++-- .../src/identity_update.rs | 394 ++++++++++++++++++ packages/rs-platform-wallet-ffi/src/lib.rs | 2 + .../src/secp256k1_primitives.rs | 269 ++++++++++++ .../SwiftDashSDK/FFI/KeychainSigner.swift | 244 ++++++++--- .../ManagedPlatformWallet.swift | 152 +++++++ .../Security/Secp256k1Primitives.swift | 66 +++ ...hainSignerAdditionalSigningKeysTests.swift | 192 +++++++++ 8 files changed, 1370 insertions(+), 100 deletions(-) create mode 100644 packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs create mode 100644 packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift create mode 100644 packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift diff --git a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs index 83102a1306a..f6e58733d0f 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_registration_with_signer.rs @@ -94,9 +94,12 @@ use crate::{unwrap_option_or_return, unwrap_result_or_return}; /// the caller retains ownership. Compressed secp256k1 pubkeys are /// always 33 bytes (`pubkey_len == 33`); BLS would be 48; etc. /// -/// **Contract bounds** — Encryption / Decryption keys carry a -/// reference to the contract (and optionally a document type) -/// they're allowed to operate within. Encoded inline as: +/// **Contract bounds** — keys may optionally carry a reference to +/// the contract (and optionally a document type) they're allowed to +/// operate within. Consensus accepts unbounded keys for every +/// purpose, including Encryption / Decryption; bounds only become +/// meaningful when the target contract or document type explicitly +/// requires a bounded key. Encoded inline as: /// - `contract_bounds_kind == 0` → no bounds. /// - `contract_bounds_kind == 1` → `SingleContract`. The first /// 32 bytes at `contract_bounds_id` are the contract id; the @@ -129,24 +132,24 @@ pub struct IdentityPubkeyFFI { /// Decode the optional `contract_bounds_*` payload off an /// [`IdentityPubkeyFFI`] row. /// -/// Shared by the registration and update FFI paths so -/// Encryption / Decryption keys can carry their bounds through -/// either entry point. `kind == 0` is "no bounds"; `1` is +/// Shared by the registration and update FFI paths so optional +/// key bounds can flow through either entry point. `kind == 0` is +/// "no bounds"; `1` is /// `SingleContract { id }`; `2` is /// `SingleContractDocumentType { id, document_type_name }`. /// -/// **Encryption / Decryption purposes require contract bounds.** -/// Drive scopes those purposes to a single contract (and optionally -/// a document type), so registering or updating an identity with an -/// unbounded encryption / decryption key produces a key that cannot -/// be used. We reject `kind == 0` for those purposes here so the -/// failure surfaces as a clean FFI error rather than a key Drive -/// silently can't use. +/// Consensus does not require bounds for Encryption / Decryption +/// purposes: an unbounded key is valid, and bounds are only checked +/// when the caller actually supplies them. In that bounded case, +/// consensus then constrains which purposes may carry bounds and +/// whether the referenced contract or document type opted in. /// -/// `purpose` is the parsed `Purpose` discriminant for the row, used -/// only for the encryption / decryption guard above. `row_index` and -/// `field_label` only flavour error messages (different callers want -/// different prefixes — `add_public_keys[i]` for update, +/// `purpose` is the parsed `Purpose` discriminant for the row. It is +/// not needed for `kind == 0`, but remains part of the shared helper +/// signature so callers can keep their error-message context stable if +/// future validation needs it. `row_index` and `field_label` only +/// flavour error messages (different callers want different prefixes — +/// `add_public_keys[i]` for update, /// `identity_pubkeys[i]` for registration). /// /// Returns `Err(PlatformWalletFFIResult)` carrying the FFI error the @@ -154,24 +157,12 @@ pub struct IdentityPubkeyFFI { /// caller does `unwrap_result_or_return!(decode_contract_bounds(...))`. pub(crate) unsafe fn decode_contract_bounds( row: &IdentityPubkeyFFI, - purpose: Purpose, + _purpose: Purpose, row_index: usize, field_label: &str, ) -> Result, PlatformWalletFFIResult> { match row.contract_bounds_kind { - 0 => { - if matches!(purpose, Purpose::ENCRYPTION | Purpose::DECRYPTION) { - return Err(PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorInvalidParameter, - format!( - "{field_label}[{row_index}].contract_bounds_kind = 0 (no bounds) but \ - purpose = {purpose:?} requires bounds — Drive scopes Encryption / \ - Decryption keys to a specific contract (use kind 1 or 2)" - ), - )); - } - Ok(None) - } + 0 => Ok(None), 1 => { if row.contract_bounds_id.is_null() { return Err(PlatformWalletFFIResult::err( @@ -257,8 +248,9 @@ pub(crate) unsafe fn decode_contract_bounds( /// `ErrorInvalidParameter` instead of silently coercing. /// - `pubkey_bytes` must be non-null and non-empty. /// - `contract_bounds` decoded via [`decode_contract_bounds`], which -/// enforces that Encryption / Decryption keys carry bounds -/// (Drive rejects unbounded ones). +/// mirrors consensus: unbounded keys are accepted, while supplied +/// bounds still have to decode cleanly here and satisfy chain rules +/// later. /// /// Returns `Err(PlatformWalletFFIResult)` carrying the FFI error the /// caller should bubble up directly via @@ -880,21 +872,98 @@ mod tests { assert_eq!(map.len(), 2); } - /// An ENCRYPTION key with no contract bounds is still rejected after the - /// refactor (Drive scopes those keys to a contract, so an unbounded one is - /// unusable). Confirms `decode_contract_bounds`' guard survives. + /// Consensus accepts an unbounded ENCRYPTION key, so the shared FFI decoder + /// must do the same instead of being stricter than the chain. #[test] - fn decode_identity_pubkeys_rejects_unbounded_encryption_key() { + fn decode_identity_pubkeys_accepts_unbounded_encryption_key() { let pk = [0x02u8; 33]; let mut enc = ffi_row(4, &pk); enc.purpose = 1; // Purpose::ENCRYPTION enc.security_level = 3; // SecurityLevel::MEDIUM - // contract_bounds_kind stays 0 → unbounded → rejected. + // contract_bounds_kind stays 0 → unbounded → accepted. let rows = [ffi_row(0, &pk), enc]; // SAFETY: `rows` (and the pubkey array it borrows) outlive the call. - let mut err = unsafe { decode_identity_pubkeys(rows.as_ptr(), rows.len()) } - .expect_err("unbounded encryption key must be rejected"); - assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorInvalidParameter); + let map = unsafe { decode_identity_pubkeys(rows.as_ptr(), rows.len()) } + .expect("unbounded encryption key must decode"); + assert_eq!(map.len(), 2); + } + + #[test] + fn decode_contract_bounds_accepts_none_for_authentication_encryption_and_decryption() { + let pk = [0x02u8; 33]; + + for purpose in [ + Purpose::AUTHENTICATION, + Purpose::ENCRYPTION, + Purpose::DECRYPTION, + ] { + let row = ffi_row(0, &pk); + let bounds = unsafe { decode_contract_bounds(&row, purpose, 0, "identity_pubkeys") } + .expect("kind == 0 must decode to no bounds for every supported purpose"); + assert_eq!(bounds, None); + } + } + + #[test] + fn decode_contract_bounds_kind_1_still_decodes_and_rejects_null_id() { + let pk = [0x02u8; 33]; + let contract_id = [0x11u8; 32]; + let mut row = ffi_row(0, &pk); + row.contract_bounds_kind = 1; + row.contract_bounds_id = contract_id.as_ptr(); + + let bounds = + unsafe { decode_contract_bounds(&row, Purpose::ENCRYPTION, 0, "identity_pubkeys") } + .expect("kind == 1 must still decode"); + assert_eq!( + bounds, + Some(ContractBounds::SingleContract { + id: Identifier::from(contract_id), + }) + ); + + row.contract_bounds_id = ptr::null(); + let mut err = + unsafe { decode_contract_bounds(&row, Purpose::ENCRYPTION, 0, "identity_pubkeys") } + .expect_err("kind == 1 must still reject a null contract id"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorNullPointer); + unsafe { platform_wallet_ffi_result_free(&mut err) }; + } + + #[test] + fn decode_contract_bounds_kind_2_still_decodes_and_rejects_null_payloads() { + let pk = [0x02u8; 33]; + let contract_id = [0x22u8; 32]; + let doc_type = CString::new("profile").expect("static string is valid CString"); + let mut row = ffi_row(0, &pk); + row.contract_bounds_kind = 2; + row.contract_bounds_id = contract_id.as_ptr(); + row.contract_bounds_document_type = doc_type.as_ptr(); + + let bounds = + unsafe { decode_contract_bounds(&row, Purpose::DECRYPTION, 0, "identity_pubkeys") } + .expect("kind == 2 must still decode"); + assert_eq!( + bounds, + Some(ContractBounds::SingleContractDocumentType { + id: Identifier::from(contract_id), + document_type_name: "profile".to_string(), + }) + ); + + row.contract_bounds_id = ptr::null(); + let mut err = + unsafe { decode_contract_bounds(&row, Purpose::DECRYPTION, 0, "identity_pubkeys") } + .expect_err("kind == 2 must still reject a null contract id"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorNullPointer); + unsafe { platform_wallet_ffi_result_free(&mut err) }; + + row.contract_bounds_id = contract_id.as_ptr(); + row.contract_bounds_document_type = ptr::null(); + let mut err = + unsafe { decode_contract_bounds(&row, Purpose::DECRYPTION, 0, "identity_pubkeys") } + .expect_err("kind == 2 must still reject a null document type"); + assert_eq!(err.code, PlatformWalletFFIResultCode::ErrorNullPointer); unsafe { platform_wallet_ffi_result_free(&mut err) }; } diff --git a/packages/rs-platform-wallet-ffi/src/identity_update.rs b/packages/rs-platform-wallet-ffi/src/identity_update.rs index 326062c9c85..2041e9d3b36 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_update.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_update.rs @@ -5,11 +5,19 @@ //! supplied `signer_handle` (typically the iOS-side `KeychainSigner`). use std::convert::TryFrom; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; use std::slice; +use dpp::identity::identity_public_key::contract_bounds::ContractBounds; use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; use dpp::identity::{IdentityPublicKey, KeyType, Purpose, SecurityLevel}; use dpp::platform_value::BinaryData; +use dpp::serialization::PlatformDeserializable; +use dpp::state_transition::identity_update_transition::accessors::IdentityUpdateTransitionAccessorsV0; +use dpp::state_transition::public_key_in_creation::accessors::IdentityPublicKeyInCreationV0Getters; +use dpp::state_transition::StateTransition; use rs_sdk_ffi::{SignerHandle, VTableSigner}; use crate::check_ptr; @@ -20,6 +28,238 @@ use crate::runtime::block_on_worker; use crate::types::*; use crate::{unwrap_option_or_return, unwrap_result_or_return}; +const IDENTITY_UPDATE_VARIANT_TAG: u8 = 6; + +/// Owned C representation of one public key carried by a parsed +/// `IdentityUpdateTransition`. +/// +/// The `data_ptr` buffer and optional `contract_bounds_document_type` +/// string are heap allocations owned by Rust and must be released via +/// [`platform_wallet_parse_identity_update_transition_free`]. +#[repr(C)] +pub struct ParsedIdentityUpdatePublicKeyFFI { + pub key_id: u32, + pub key_type: u8, + pub purpose: u8, + pub security_level: u8, + pub read_only: bool, + pub data_ptr: *mut u8, + pub data_len: usize, + /// 0 = none, 1 = SingleContract, 2 = SingleContractDocumentType. + pub contract_bounds_kind: u8, + pub contract_bounds_id: [u8; 32], + pub contract_bounds_document_type: *mut c_char, +} + +/// Owned C representation of the inspectable parts of a parsed +/// `IdentityUpdateTransition`. +#[repr(C)] +pub struct ParsedIdentityUpdateFFI { + pub identity_id: [u8; 32], + pub add_public_keys: *mut ParsedIdentityUpdatePublicKeyFFI, + pub add_public_keys_count: usize, + pub disable_public_key_ids: *mut u32, + pub disable_public_key_ids_count: usize, +} + +impl Default for ParsedIdentityUpdateFFI { + fn default() -> Self { + Self { + identity_id: [0u8; 32], + add_public_keys: ptr::null_mut(), + add_public_keys_count: 0, + disable_public_key_ids: ptr::null_mut(), + disable_public_key_ids_count: 0, + } + } +} + +fn deserialize_state_transition(bytes: &[u8]) -> Result { + StateTransition::deserialize_from_bytes(bytes).map_err(|error| { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorDeserialization, + format!("Failed to deserialize IdentityUpdateTransition: {error}"), + ) + }) +} + +fn parse_identity_update_transition_bytes( + bytes: &[u8], +) -> Result< + dpp::state_transition::identity_update_transition::IdentityUpdateTransition, + PlatformWalletFFIResult, +> { + let state_transition = if bytes.first().copied() == Some(IDENTITY_UPDATE_VARIANT_TAG) { + deserialize_state_transition(bytes)? + } else { + let mut prefixed = Vec::with_capacity(bytes.len() + 1); + prefixed.push(IDENTITY_UPDATE_VARIANT_TAG); + prefixed.extend_from_slice(bytes); + + match StateTransition::deserialize_from_bytes(&prefixed) { + Ok(state_transition) => state_transition, + Err(prefixed_error) => match StateTransition::deserialize_from_bytes(bytes) { + Ok(state_transition) => state_transition, + Err(tagged_error) => { + return Err(PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorDeserialization, + format!( + "Failed to deserialize IdentityUpdateTransition in either framing \ + (Yappr tagless + prefix 6 first: {prefixed_error}; tagged fallback: \ + {tagged_error})" + ), + )); + } + }, + } + }; + + match state_transition { + StateTransition::IdentityUpdate(identity_update) => Ok(identity_update), + other => Err(PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + format!("Expected IdentityUpdateTransition, got {other:?}"), + )), + } +} + +fn encode_contract_bounds(bounds: Option<&ContractBounds>) -> (u8, [u8; 32], *mut c_char) { + match bounds { + Some(ContractBounds::SingleContract { id }) => (1u8, id.to_buffer(), ptr::null_mut()), + Some(ContractBounds::SingleContractDocumentType { + id, + document_type_name, + }) => match CString::new(document_type_name.as_str()) { + Ok(value) => (2u8, id.to_buffer(), value.into_raw()), + Err(_) => (1u8, id.to_buffer(), ptr::null_mut()), + }, + None => (0u8, [0u8; 32], ptr::null_mut()), + } +} + +fn project_parsed_identity_update( + transition: &dpp::state_transition::identity_update_transition::IdentityUpdateTransition, +) -> ParsedIdentityUpdateFFI { + let identity_id = transition.identity_id().to_buffer(); + + let add_public_keys_vec: Vec = transition + .public_keys_to_add() + .iter() + .map(|public_key| { + let data = public_key.data().as_slice().to_vec().into_boxed_slice(); + let data_len = data.len(); + let data_ptr = Box::into_raw(data) as *mut u8; + let (contract_bounds_kind, contract_bounds_id, contract_bounds_document_type) = + encode_contract_bounds(public_key.contract_bounds()); + + ParsedIdentityUpdatePublicKeyFFI { + key_id: public_key.id(), + key_type: public_key.key_type() as u8, + purpose: public_key.purpose() as u8, + security_level: public_key.security_level() as u8, + read_only: public_key.read_only(), + data_ptr, + data_len, + contract_bounds_kind, + contract_bounds_id, + contract_bounds_document_type, + } + }) + .collect(); + + let add_public_keys_count = add_public_keys_vec.len(); + let add_public_keys = if add_public_keys_count == 0 { + ptr::null_mut() + } else { + Box::into_raw(add_public_keys_vec.into_boxed_slice()) + as *mut ParsedIdentityUpdatePublicKeyFFI + }; + + let disable_public_key_ids_vec = transition.public_key_ids_to_disable().to_vec(); + let disable_public_key_ids_count = disable_public_key_ids_vec.len(); + let disable_public_key_ids = if disable_public_key_ids_count == 0 { + ptr::null_mut() + } else { + Box::into_raw(disable_public_key_ids_vec.into_boxed_slice()) as *mut u32 + }; + + ParsedIdentityUpdateFFI { + identity_id, + add_public_keys, + add_public_keys_count, + disable_public_key_ids, + disable_public_key_ids_count, + } +} + +/// Deserializes a raw `IdentityUpdateTransition` (as carried by a DashConnect +/// `dash-st:` QR) into its inspectable parts. +/// +/// Accepts both normal tagged DPP state-transition bytes and Yappr's tagless +/// framing, where the positional bincode enum variant tag `6` has to be +/// prepended before deserialization. +/// +/// Does NOT sign and does NOT broadcast — the caller validates the result and +/// rebuilds the transition through the normal signing path. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_parse_identity_update_transition( + transition_bytes: *const u8, + transition_len: usize, + out: *mut ParsedIdentityUpdateFFI, +) -> PlatformWalletFFIResult { + check_ptr!(transition_bytes); + check_ptr!(out); + + *out = ParsedIdentityUpdateFFI::default(); + + let bytes = slice::from_raw_parts(transition_bytes, transition_len); + let transition = unwrap_result_or_return!(parse_identity_update_transition_bytes(bytes)); + *out = project_parsed_identity_update(&transition); + PlatformWalletFFIResult::ok() +} + +/// Frees a parsed transition previously returned by +/// [`platform_wallet_parse_identity_update_transition`]. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_parse_identity_update_transition_free( + out: *mut ParsedIdentityUpdateFFI, +) { + if out.is_null() { + return; + } + + let parsed = &mut *out; + + if !parsed.add_public_keys.is_null() && parsed.add_public_keys_count > 0 { + let keys = slice::from_raw_parts_mut(parsed.add_public_keys, parsed.add_public_keys_count); + for key in keys.iter_mut() { + if !key.data_ptr.is_null() && key.data_len > 0 { + let data_slice = slice::from_raw_parts_mut(key.data_ptr, key.data_len); + let _ = Box::from_raw(data_slice as *mut [u8]); + key.data_ptr = ptr::null_mut(); + key.data_len = 0; + } + + if !key.contract_bounds_document_type.is_null() { + let _ = CString::from_raw(key.contract_bounds_document_type); + key.contract_bounds_document_type = ptr::null_mut(); + } + } + + let _ = Box::from_raw(keys as *mut [ParsedIdentityUpdatePublicKeyFFI]); + } + + if !parsed.disable_public_key_ids.is_null() && parsed.disable_public_key_ids_count > 0 { + let disable_ids = slice::from_raw_parts_mut( + parsed.disable_public_key_ids, + parsed.disable_public_key_ids_count, + ); + let _ = Box::from_raw(disable_ids as *mut [u32]); + } + + *parsed = ParsedIdentityUpdateFFI::default(); +} + /// Update an identity by adding new public keys and/or disabling /// existing key IDs, signing the resulting `IdentityUpdateTransition` /// with the supplied `signer_handle`. @@ -102,3 +342,157 @@ pub unsafe extern "C" fn platform_wallet_update_identity_with_signer( unwrap_result_or_return!(result); PlatformWalletFFIResult::ok() } + +#[cfg(test)] +mod tests { + use super::*; + use dpp::identity::identity_public_key::contract_bounds::ContractBounds; + use dpp::platform_value::BinaryData; + use dpp::prelude::Identifier; + use dpp::serialization::PlatformSerializable; + use dpp::state_transition::identity_update_transition::v0::IdentityUpdateTransitionV0; + use dpp::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; + + fn fixture_transition_bytes() -> Vec { + let identity_id = Identifier::from([0x11; 32]); + let contract_id = Identifier::from([0x44; 32]); + + let transition = StateTransition::IdentityUpdate( + IdentityUpdateTransitionV0 { + signature: BinaryData::new(vec![0x99; 65]), + signature_public_key_id: 3, + identity_id, + revision: 7, + nonce: 9, + add_public_keys: vec![ + IdentityPublicKeyInCreationV0 { + id: 17, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::HIGH, + read_only: false, + data: BinaryData::new(vec![0x02; 33]), + signature: BinaryData::new(vec![0xaa; 65]), + contract_bounds: None, + } + .into(), + IdentityPublicKeyInCreationV0 { + id: 18, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::ENCRYPTION, + security_level: SecurityLevel::HIGH, + read_only: true, + data: BinaryData::new(vec![0x03; 33]), + signature: BinaryData::new(vec![0xbb; 65]), + contract_bounds: Some(ContractBounds::SingleContractDocumentType { + id: contract_id, + document_type_name: "profile".to_string(), + }), + } + .into(), + ], + disable_public_keys: vec![4, 8], + user_fee_increase: 2, + } + .into(), + ); + + transition + .serialize_to_bytes() + .expect("fixture transition serializes") + } + + #[test] + fn parses_tagged_identity_update_transition() { + let bytes = fixture_transition_bytes(); + let mut out = ParsedIdentityUpdateFFI::default(); + + let result = unsafe { + platform_wallet_parse_identity_update_transition(bytes.as_ptr(), bytes.len(), &mut out) + }; + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!(out.identity_id, [0x11; 32]); + assert_eq!(out.add_public_keys_count, 2); + assert_eq!(out.disable_public_key_ids_count, 2); + + let keys = unsafe { slice::from_raw_parts(out.add_public_keys, out.add_public_keys_count) }; + assert_eq!(keys[0].key_id, 17); + assert_eq!(keys[1].contract_bounds_kind, 2); + assert_eq!(keys[1].contract_bounds_id, [0x44; 32]); + let doc_type = unsafe { + std::ffi::CStr::from_ptr(keys[1].contract_bounds_document_type) + .to_str() + .expect("doc type utf8") + }; + assert_eq!(doc_type, "profile"); + + let disable_ids = unsafe { + slice::from_raw_parts(out.disable_public_key_ids, out.disable_public_key_ids_count) + }; + assert_eq!(disable_ids, &[4, 8]); + + unsafe { platform_wallet_parse_identity_update_transition_free(&mut out) }; + assert!(out.add_public_keys.is_null()); + assert!(out.disable_public_key_ids.is_null()); + } + + #[test] + fn parses_yappr_tagless_identity_update_transition() { + let tagged = fixture_transition_bytes(); + let tagless = tagged[1..].to_vec(); + let mut out = ParsedIdentityUpdateFFI::default(); + + let result = unsafe { + platform_wallet_parse_identity_update_transition( + tagless.as_ptr(), + tagless.len(), + &mut out, + ) + }; + + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!(out.identity_id, [0x11; 32]); + assert_eq!(out.add_public_keys_count, 2); + + unsafe { platform_wallet_parse_identity_update_transition_free(&mut out) }; + } + + #[test] + fn rejects_malformed_identity_update_transition_bytes() { + let bytes = [0xde, 0xad, 0xbe, 0xef]; + let mut out = ParsedIdentityUpdateFFI::default(); + + let result = unsafe { + platform_wallet_parse_identity_update_transition(bytes.as_ptr(), bytes.len(), &mut out) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorDeserialization + ); + assert!(out.add_public_keys.is_null()); + assert!(out.disable_public_key_ids.is_null()); + assert_eq!(out.add_public_keys_count, 0); + assert_eq!(out.disable_public_key_ids_count, 0); + } + + #[test] + fn rejects_truncated_identity_update_transition_bytes() { + let mut bytes = fixture_transition_bytes(); + bytes.truncate(bytes.len() - 7); + let mut out = ParsedIdentityUpdateFFI::default(); + + let result = unsafe { + platform_wallet_parse_identity_update_transition(bytes.as_ptr(), bytes.len(), &mut out) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorDeserialization + ); + assert!(out.add_public_keys.is_null()); + assert!(out.disable_public_key_ids.is_null()); + assert_eq!(out.add_public_keys_count, 0); + assert_eq!(out.disable_public_key_ids_count, 0); + } +} diff --git a/packages/rs-platform-wallet-ffi/src/lib.rs b/packages/rs-platform-wallet-ffi/src/lib.rs index 5d80c33ded5..eef2b462de9 100644 --- a/packages/rs-platform-wallet-ffi/src/lib.rs +++ b/packages/rs-platform-wallet-ffi/src/lib.rs @@ -63,6 +63,7 @@ pub mod platform_addresses; pub mod platform_wallet_info; pub mod provider_key_at_index; mod runtime; +pub mod secp256k1_primitives; #[cfg(feature = "shielded")] pub mod shielded_persistence; #[cfg(feature = "shielded")] @@ -134,6 +135,7 @@ pub use platform_address_types::*; pub use platform_addresses::*; pub use platform_wallet_info::*; pub use provider_key_at_index::*; +pub use secp256k1_primitives::*; #[cfg(feature = "shielded")] pub use shielded_send::*; #[cfg(feature = "shielded")] diff --git a/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs b/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs new file mode 100644 index 00000000000..e3e3942281d --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs @@ -0,0 +1,269 @@ +//! Standalone secp256k1 primitives used by DashConnect. + +use std::slice; + +use dashcore::secp256k1::{PublicKey, Scalar, Secp256k1, SecretKey}; +use zeroize::Zeroizing; + +use crate::error::*; +use crate::{check_ptr, unwrap_result_or_return}; + +struct WipingSecretKey(SecretKey); + +impl Drop for WipingSecretKey { + fn drop(&mut self) { + self.0.non_secure_erase(); + } +} + +struct WipingScalar(Scalar); + +impl Drop for WipingScalar { + fn drop(&mut self) { + self.0.non_secure_erase(); + } +} + +fn invalid_parameter(message: impl Into) -> PlatformWalletFFIResult { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + message.into(), + ) +} + +fn wallet_operation(message: impl Into) -> PlatformWalletFFIResult { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorWalletOperation, + message.into(), + ) +} + +fn parse_secret_key( + private_key: *const u8, + private_key_len: usize, +) -> Result { + if private_key_len != 32 { + return Err(invalid_parameter(format!( + "secp256k1 private keys must be exactly 32 bytes, got {private_key_len}" + ))); + } + + let bytes = unsafe { slice::from_raw_parts(private_key, private_key_len) }; + SecretKey::from_slice(bytes) + .map(WipingSecretKey) + .map_err(|error| invalid_parameter(format!("Invalid secp256k1 private key: {error}"))) +} + +fn parse_compressed_public_key( + public_key: *const u8, + public_key_len: usize, +) -> Result { + if public_key_len != 33 { + return Err(invalid_parameter(format!( + "Compressed secp256k1 public keys must be exactly 33 bytes, got {public_key_len}" + ))); + } + + let bytes = unsafe { slice::from_raw_parts(public_key, public_key_len) }; + if !matches!(bytes.first().copied(), Some(0x02) | Some(0x03)) { + return Err(invalid_parameter( + "Compressed secp256k1 public keys must start with 0x02 or 0x03".to_string(), + )); + } + + PublicKey::from_slice(bytes) + .map_err(|error| invalid_parameter(format!("Invalid compressed secp256k1 point: {error}"))) +} + +/// Returns `1` when `pubkey` is a valid compressed secp256k1 point, `0` otherwise. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_secp256k1_verify_compressed_point( + pubkey: *const u8, + pubkey_len: usize, +) -> i32 { + if pubkey.is_null() || pubkey_len != 33 { + return 0; + } + + match parse_compressed_public_key(pubkey, pubkey_len) { + Ok(_) => 1, + Err(_) => 0, + } +} + +/// Derive the 33-byte compressed public key for a 32-byte secp256k1 scalar. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_secp256k1_compressed_public_key( + seckey: *const u8, + seckey_len: usize, + out_pubkey: *mut u8, +) -> PlatformWalletFFIResult { + check_ptr!(seckey); + check_ptr!(out_pubkey); + + let secret_key = unwrap_result_or_return!(parse_secret_key(seckey, seckey_len)); + let compressed = PublicKey::from_secret_key(&Secp256k1::new(), &secret_key.0).serialize(); + std::ptr::copy_nonoverlapping(compressed.as_ptr(), out_pubkey, compressed.len()); + PlatformWalletFFIResult::ok() +} + +/// Derive the raw affine X coordinate of `seckey * pubkey` (unhashed ECDH output). +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_secp256k1_ecdh_shared_x( + seckey: *const u8, + seckey_len: usize, + pubkey: *const u8, + pubkey_len: usize, + out_shared_x: *mut u8, +) -> PlatformWalletFFIResult { + check_ptr!(seckey); + check_ptr!(pubkey); + check_ptr!(out_shared_x); + + let secret_key = unwrap_result_or_return!(parse_secret_key(seckey, seckey_len)); + let public_key = unwrap_result_or_return!(parse_compressed_public_key(pubkey, pubkey_len)); + + let secp = Secp256k1::new(); + let scalar_bytes = Zeroizing::new(secret_key.0.secret_bytes()); + let scalar = unwrap_result_or_return!(Scalar::from_be_bytes(*scalar_bytes) + .map(WipingScalar) + .map_err(|error| invalid_parameter(format!("Invalid secp256k1 scalar: {error}")))); + + let shared_point = unwrap_result_or_return!(public_key + .mul_tweak(&secp, &scalar.0) + .map_err(|_| wallet_operation("ECDH produced the point at infinity"))); + let uncompressed = shared_point.serialize_uncompressed(); + std::ptr::copy_nonoverlapping(uncompressed[1..33].as_ptr(), out_shared_x, 32); + PlatformWalletFFIResult::ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + const PRIVATE_KEY_A: [u8; 32] = [0x01; 32]; + const PRIVATE_KEY_B: [u8; 32] = [0x02; 32]; + const PUBLIC_KEY_A: [u8; 33] = [ + 0x03, 0x1b, 0x84, 0xc5, 0x56, 0x7b, 0x12, 0x64, 0x40, 0x99, 0x5d, 0x3e, 0xd5, 0xaa, 0xba, + 0x05, 0x65, 0xd7, 0x1e, 0x18, 0x34, 0x60, 0x48, 0x19, 0xff, 0x9c, 0x17, 0xf5, 0xe9, 0xd5, + 0xdd, 0x07, 0x8f, + ]; + const PUBLIC_KEY_B: [u8; 33] = [ + 0x02, 0x4d, 0x4b, 0x6c, 0xd1, 0x36, 0x10, 0x32, 0xca, 0x9b, 0xd2, 0xae, 0xb9, 0xd9, 0x00, + 0xaa, 0x4d, 0x45, 0xd9, 0xea, 0xd8, 0x0a, 0xc9, 0x42, 0x33, 0x74, 0xc4, 0x51, 0xa7, 0x25, + 0x4d, 0x07, 0x66, + ]; + const SHARED_X_AB: [u8; 32] = [ + 0xd0, 0x15, 0x8a, 0x38, 0xfa, 0xf6, 0x11, 0x8a, 0xf1, 0x33, 0xaf, 0x12, 0xd9, 0xbf, 0xa3, + 0x88, 0xea, 0xb4, 0xa0, 0x8d, 0x1a, 0x20, 0x88, 0xea, 0x6e, 0x6e, 0xc1, 0x26, 0x9e, 0x03, + 0x56, 0x7f, + ]; + + #[test] + fn compressed_public_key_matches_known_vector() { + let mut out = [0u8; 33]; + let result = unsafe { + platform_wallet_secp256k1_compressed_public_key( + PRIVATE_KEY_A.as_ptr(), + PRIVATE_KEY_A.len(), + out.as_mut_ptr(), + ) + }; + + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!(out, PUBLIC_KEY_A); + } + + #[test] + fn ecdh_shared_x_matches_known_vector() { + let mut out = [0u8; 32]; + let result = unsafe { + platform_wallet_secp256k1_ecdh_shared_x( + PRIVATE_KEY_A.as_ptr(), + PRIVATE_KEY_A.len(), + PUBLIC_KEY_B.as_ptr(), + PUBLIC_KEY_B.len(), + out.as_mut_ptr(), + ) + }; + + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!(out, SHARED_X_AB); + } + + #[test] + fn verify_compressed_point_rejects_invalid_point() { + let mut invalid = [0u8; 33]; + invalid[0] = 0x02; + let result = unsafe { + platform_wallet_secp256k1_verify_compressed_point(invalid.as_ptr(), invalid.len()) + }; + + assert_eq!(result, 0); + } + + #[test] + fn verify_compressed_point_accepts_known_vector() { + let result = unsafe { + platform_wallet_secp256k1_verify_compressed_point( + PUBLIC_KEY_A.as_ptr(), + PUBLIC_KEY_A.len(), + ) + }; + + assert_eq!(result, 1); + } + + #[test] + fn compressed_public_key_rejects_out_of_range_scalar() { + let invalid_scalar = [0xffu8; 32]; + let mut out = [0u8; 33]; + let result = unsafe { + platform_wallet_secp256k1_compressed_public_key( + invalid_scalar.as_ptr(), + invalid_scalar.len(), + out.as_mut_ptr(), + ) + }; + + assert_eq!( + result.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + } + + #[test] + fn ecdh_rejects_wrong_length_inputs() { + let mut out = [0u8; 32]; + let short_private = [0x01u8; 31]; + let short_public = [0x02u8; 32]; + + let private_error = unsafe { + platform_wallet_secp256k1_ecdh_shared_x( + short_private.as_ptr(), + short_private.len(), + PUBLIC_KEY_A.as_ptr(), + PUBLIC_KEY_A.len(), + out.as_mut_ptr(), + ) + }; + assert_eq!( + private_error.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + + let public_error = unsafe { + platform_wallet_secp256k1_ecdh_shared_x( + PRIVATE_KEY_B.as_ptr(), + PRIVATE_KEY_B.len(), + short_public.as_ptr(), + short_public.len(), + out.as_mut_ptr(), + ) + }; + assert_eq!( + public_error.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + } +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index 7cf64398021..fd54b01c95d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -84,6 +84,52 @@ import SwiftData /// current `passUnretained` shape removes the leak at the cost /// of the explicit keepalive contract above. public final class KeychainSigner: Signer, @unchecked Sendable { + final class AdditionalSigningKeyEntry: @unchecked Sendable { + let publicKey: Data + private var privateKeyBytes: [UInt8] + + init(publicKey: Data, privateKey: Data) { + self.publicKey = publicKey + self.privateKeyBytes = Array(privateKey) + } + + deinit { + zeroPrivateKey() + } + + func sign(data: Data, network: Network) -> Result { + do { + return .success( + try RawKeySigner.sign( + data: data, + privateKey: Data(privateKeyBytes), + network: network + ) + ) + } catch KeyManagerError.signerCreationFailed(let message) { + return .failure(.ffiSignerCreationFailed(message: message)) + } catch KeyManagerError.invalidKeyFormat(let message) { + return .failure(.ffiSignerCreationFailed(message: "invalid key format: \(message)")) + } catch KeyManagerError.signingFailed(let message) { + return .failure(.ffiSignFailed(message: message)) + } catch { + return .failure(.ffiSignFailed(message: String(describing: error))) + } + } + + func zeroPrivateKey() { + privateKeyBytes.withUnsafeMutableBufferPointer { ptr in + if let base = ptr.baseAddress { + memset_s(UnsafeMutableRawPointer(base), ptr.count, 0, ptr.count) + } + } + } + + var isZeroedForTesting: Bool { + privateKeyBytes.allSatisfy { $0 == 0 } + } + } + // MARK: Public surface /// FFI signer handle. Pass to any `*_with_signer` entry point; @@ -183,6 +229,7 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// Raw pointer to the FFI signer handle. Boxed by Rust and freed /// in `deinit`. private var handlePtr: OpaquePointer! + private var additionalSigningKeys: [Data: [AdditionalSigningKeyEntry]] = [:] // MARK: Init @@ -233,6 +280,7 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } deinit { + clearAdditionalSigningKeys() // `dash_sdk_signer_destroy` drops the Rust handle box + // vtable allocation. The destroy trampoline is a no-op // (init used `passUnretained`, so there's nothing to @@ -244,8 +292,88 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } } + public func withAdditionalSigningKeys( + _ keys: [(publicKey: Data, privateKey: Data)], + perform body: () async throws -> T + ) async rethrows -> T { + try await withAdditionalSigningKeys( + makeAdditionalSigningKeyEntries(keys), + perform: body + ) + } + + func withAdditionalSigningKeys( + _ entries: [AdditionalSigningKeyEntry], + perform body: () async throws -> T + ) async rethrows -> T { + pushAdditionalSigningKeys(entries) + defer { popAdditionalSigningKeys(entries) } + return try await body() + } + // MARK: Trampoline-callable internals + func makeAdditionalSigningKeyEntries( + _ keys: [(publicKey: Data, privateKey: Data)] + ) -> [AdditionalSigningKeyEntry] { + keys.map { AdditionalSigningKeyEntry(publicKey: $0.publicKey, privateKey: $0.privateKey) } + } + + func pushAdditionalSigningKeys(_ entries: [AdditionalSigningKeyEntry]) { + queue.sync { + for entry in entries { + additionalSigningKeys[entry.publicKey, default: []].append(entry) + } + } + } + + func popAdditionalSigningKeys(_ entries: [AdditionalSigningKeyEntry]) { + queue.sync { + for entry in entries.reversed() { + guard var stack = additionalSigningKeys[entry.publicKey] else { + entry.zeroPrivateKey() + continue + } + + if let last = stack.last, last === entry { + stack.removeLast() + } else { + stack.removeAll { $0 === entry } + } + + if stack.isEmpty { + additionalSigningKeys.removeValue(forKey: entry.publicKey) + } else { + additionalSigningKeys[entry.publicKey] = stack + } + + entry.zeroPrivateKey() + } + } + } + + func clearAdditionalSigningKeys() { + queue.sync { + for entries in additionalSigningKeys.values { + for entry in entries { + entry.zeroPrivateKey() + } + } + additionalSigningKeys.removeAll() + } + } + + func additionalSigningKey(publicKey: Data) -> AdditionalSigningKeyEntry? { + var entry: AdditionalSigningKeyEntry? + queue.sync { + // HASH160 identity keys can be requested by their 20-byte hash160 + // rather than the 33-byte compressed secp256k1 public key. The + // registry is therefore keyed by the exact bytes the FFI asks for. + entry = additionalSigningKeys[publicKey]?.last + } + return entry + } + // MARK: key_type dispatch // // The Rust signer FFI sends one of two payload shapes through the @@ -300,7 +428,8 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// private keys are NEVER persisted; they're derived per call /// from `(mnemonic, path)` inside Rust. fileprivate func lookupIdentityPrivateKey( - publicKey: Data + publicKey: Data, + keyType: UInt8 ) -> Result { var captured: Result = .failure(.publicKeyNotFound) queue.sync { @@ -328,6 +457,9 @@ public final class KeychainSigner: Signer, @unchecked Sendable { return } + print( + "⚠️ KEYCHAIN_SIGNER_PUBLIC_KEY_MISS keyType=\(keyType) requestedBytes=\(pubkeyHex)" + ) captured = .failure(.publicKeyNotFound) } return captured @@ -361,6 +493,10 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// derive-and-sign FFI requires. We do NOT actually derive a key /// here; the check is purely "are the prerequisites in place". func canSign(publicKey: Data, keyType: UInt8) -> Bool { + if additionalSigningKey(publicKey: publicKey) != nil { + return true + } + if keyType == Self.platformAddressHashKeyType { // Resolve the address row first (synchronous lookup); // mnemonic check is gated on having the wallet id. @@ -424,6 +560,53 @@ public final class KeychainSigner: Signer, @unchecked Sendable { return found } + /// Shared by the sign trampoline and tests: consult any scoped + /// in-memory registry first, then the existing platform-address / + /// breadcrumb / persisted-key paths. + func signOnDemand( + publicKey: Data, + keyType: UInt8, + data: Data + ) -> Result { + if let entry = additionalSigningKey(publicKey: publicKey) { + return entry.sign(data: data, network: network) + } + + if keyType == Self.platformAddressHashKeyType { + return signPlatformAddressOnDemand( + addressHash: publicKey, + keyType: keyType, + data: data + ) + } + + switch signIdentityKeyOnDemand( + publicKey: publicKey, + keyType: keyType, + data: data + ) { + case .success(let sig)?: + return .success(sig) + case .failure(.signWithMnemonicFailed(let tag))? + where tag == SignWithMnemonicResolverError.unsupportedKeyType.rawValue: + break + case .failure(let err)?: + print("⚠️ IDENTITY_SIGN_FALLBACK resolver-failed: \(err.localizedDescription)") + case nil: + print("⚠️ IDENTITY_SIGN_FALLBACK no-breadcrumb") + } + + let privateKey: Data + switch lookupIdentityPrivateKey(publicKey: publicKey, keyType: keyType) { + case .failure(let err): + return .failure(err) + case .success(let priv): + privateKey = priv + } + + return ffiSign(privateKey: privateKey, data: data) + } + /// Resolved context for a platform-address signing request: /// the matching `PersistentPlatformAddress`'s wallet id + /// derivation path. `nil` when no row matches the supplied @@ -790,68 +973,11 @@ private func keychainSignerSignAsyncTrampoline( } } - // Dispatch on key_type. Platform-address signing (`0xFF`) is a - // single-call derive-and-sign path — no separate key lookup, - // because the derived bytes never come back to Swift. Identity - // signing (`< 5`) prefers the same derive-sign-destroy path (from - // the key's stored breadcrumb) and falls back to the stored scalar - // when a key has no breadcrumb yet. - if keyType == KeychainSigner.platformAddressHashKeyType { - switch signer.signPlatformAddressOnDemand( - addressHash: pubkeyData, - keyType: keyType, - data: dataToSign - ) { - case .failure(let err): - reportError(err.localizedDescription) - case .success(let sig): - reportSuccess(sig) - } - return - } - - // Identity signing (`keyType < 5`): derive-sign-destroy via the resolver - // when the key carries a derivation breadcrumb; otherwise fall back to the - // stored scalar. The fallback keeps already-materialized keys — and any not - // yet backfilled — signable, so the cutover is non-lockout by construction. - // Every fallback is logged so the zero-fallback acceptance gate can catch - // un-migrated rows or resolver failures before the stored scalar is removed. - // - // Rust owns the supported-key-type decision: we attempt the resolver for - // any identity key and treat its `UNSUPPORTED_KEY_TYPE` tag as the routing - // signal (fall through to the stored scalar silently, no fallback log — - // the resolver simply doesn't handle this type). This avoids mirroring the - // Rust ECDSA-only set in Swift, so a future Rust-derivable key type is - // automatically routed through the resolver without a matching Swift edit. - switch signer.signIdentityKeyOnDemand( + switch signer.signOnDemand( publicKey: pubkeyData, keyType: keyType, data: dataToSign ) { - case .success(let sig)?: - reportSuccess(sig) - return - case .failure(.signWithMnemonicFailed(let tag))? - where tag == SignWithMnemonicResolverError.unsupportedKeyType.rawValue: - // Rust does not derive-sign this key type — route to the stored - // scalar with no spurious fallback log. - break - case .failure(let err)?: - print("⚠️ IDENTITY_SIGN_FALLBACK resolver-failed: \(err.localizedDescription)") - case nil: - print("⚠️ IDENTITY_SIGN_FALLBACK no-breadcrumb") - } - - let privateKey: Data - switch signer.lookupIdentityPrivateKey(publicKey: pubkeyData) { - case .failure(let err): - reportError(err.localizedDescription) - return - case .success(let priv): - privateKey = priv - } - - switch signer.ffiSign(privateKey: privateKey, data: dataToSign) { case .failure(let err): reportError(err.localizedDescription) case .success(let sig): diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index 4f5226ac821..21dd5f35075 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -212,6 +212,25 @@ public final class ManagedPlatformWallet: @unchecked Sendable { case singleContractDocumentType(id: Data, documentTypeName: String) } + /// Inspectable fields of a parsed raw `IdentityUpdateTransition`. + /// The keys intentionally reuse `IdentityPubkey` so callers can + /// validate and hand them back to `updateIdentity(...)` unchanged. + public struct ParsedIdentityUpdateTransition: Sendable { + public let identityId: Identifier + public let addPublicKeys: [IdentityPubkey] + public let disablePublicKeyIds: [UInt32] + + public init( + identityId: Identifier, + addPublicKeys: [IdentityPubkey], + disablePublicKeyIds: [UInt32] + ) { + self.identityId = identityId + self.addPublicKeys = addPublicKeys + self.disablePublicKeyIds = disablePublicKeyIds + } + } + /// Result of a successful identity registration. public struct CreatedIdentity: Sendable { /// 32-byte identity id. @@ -3120,6 +3139,139 @@ extension ManagedPlatformWallet { }.value } + /// Parse a raw `IdentityUpdateTransition` from DPP bytes without + /// signing or broadcasting it. Accepts both standard tagged bytes + /// and Yappr's tagless `dash-st:` framing. + public func parseIdentityUpdateTransition(_ bytes: Data) throws -> ParsedIdentityUpdateTransition { + guard !bytes.isEmpty else { + throw PlatformWalletError.deserialization( + "IdentityUpdateTransition bytes are empty" + ) + } + + var out = ParsedIdentityUpdateFFI( + identity_id: ( + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0 + ), + add_public_keys: nil, + add_public_keys_count: 0, + disable_public_key_ids: nil, + disable_public_key_ids_count: 0 + ) + + let result = bytes.withUnsafeBytes { rawBuffer -> PlatformWalletFFIResult in + let byteBuffer = rawBuffer.bindMemory(to: UInt8.self) + return platform_wallet_parse_identity_update_transition( + byteBuffer.baseAddress, + UInt(byteBuffer.count), + &out + ) + } + try result.check() + defer { platform_wallet_parse_identity_update_transition_free(&out) } + + var identityTuple = out.identity_id + let identityId = Swift.withUnsafeBytes(of: &identityTuple) { Data($0) } + + let addPublicKeys: [IdentityPubkey] + if let pointer = out.add_public_keys, out.add_public_keys_count > 0 { + let buffer = UnsafeBufferPointer(start: pointer, count: Int(out.add_public_keys_count)) + addPublicKeys = try buffer.enumerated().map { index, entry in + try Self.makeParsedIdentityPubkey(from: entry, index: index) + } + } else { + addPublicKeys = [] + } + + let disablePublicKeyIds: [UInt32] + if let pointer = out.disable_public_key_ids, out.disable_public_key_ids_count > 0 { + disablePublicKeyIds = Array( + UnsafeBufferPointer( + start: pointer, + count: Int(out.disable_public_key_ids_count) + ) + ) + } else { + disablePublicKeyIds = [] + } + + return ParsedIdentityUpdateTransition( + identityId: identityId, + addPublicKeys: addPublicKeys, + disablePublicKeyIds: disablePublicKeyIds + ) + } + + private static func makeParsedIdentityPubkey( + from entry: ParsedIdentityUpdatePublicKeyFFI, + index: Int + ) throws -> IdentityPubkey { + guard let keyType = KeyType(rawValue: entry.key_type), + let purpose = KeyPurpose(rawValue: entry.purpose), + let securityLevel = SecurityLevel(rawValue: entry.security_level) else { + throw PlatformWalletError.deserialization( + "Unknown IdentityUpdateTransition public-key enum discriminant at index \(index)" + ) + } + + let pubkeyBytes: Data + if let dataPtr = entry.data_ptr { + pubkeyBytes = Data(bytes: dataPtr, count: Int(entry.data_len)) + } else if entry.data_len == 0 { + pubkeyBytes = Data() + } else { + throw PlatformWalletError.deserialization( + "IdentityUpdateTransition public key \(index) had a null data pointer for \(entry.data_len) bytes" + ) + } + + let contractBounds = try parsedContractBounds(from: entry, index: index) + + return IdentityPubkey( + keyId: entry.key_id, + keyType: keyType, + purpose: purpose, + securityLevel: securityLevel, + pubkeyBytes: pubkeyBytes, + readOnly: entry.read_only, + contractBounds: contractBounds + ) + } + + private static func parsedContractBounds( + from entry: ParsedIdentityUpdatePublicKeyFFI, + index: Int + ) throws -> ContractBounds? { + switch entry.contract_bounds_kind { + case 0: + return nil + case 1: + var idTuple = entry.contract_bounds_id + return .singleContract( + id: Swift.withUnsafeBytes(of: &idTuple) { Data($0) } + ) + case 2: + guard let documentTypePtr = entry.contract_bounds_document_type, + let documentTypeName = String(validatingCString: documentTypePtr) else { + throw PlatformWalletError.deserialization( + "IdentityUpdateTransition contract bounds at key \(index) are missing a valid UTF-8 document type" + ) + } + var idTuple = entry.contract_bounds_id + return .singleContractDocumentType( + id: Swift.withUnsafeBytes(of: &idTuple) { Data($0) }, + documentTypeName: documentTypeName + ) + default: + throw PlatformWalletError.deserialization( + "Unknown IdentityUpdateTransition contract-bounds kind \(entry.contract_bounds_kind) at key \(index)" + ) + } + } + /// Create + broadcast a new data contract owned by /// `ownerIdentityId`. Returns the 32-byte contract id once /// Platform confirms the transition. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift new file mode 100644 index 00000000000..50ad6ed2ecc --- /dev/null +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift @@ -0,0 +1,66 @@ +import Foundation +import DashSDKFFI + +/// Standalone secp256k1 primitives exposed by the Platform FFI. +/// +/// These are handle-free on purpose: DashConnect validates scanned QR payloads +/// before a wallet is resolved, so the URI path cannot depend on any wallet instance. +public enum Secp256k1Primitives { + /// True when `pubKey` is a well-formed 33-byte compressed secp256k1 point. + public static func isValidCompressedPoint(_ pubKey: Data) -> Bool { + guard pubKey.count == 33 else { + return false + } + + return pubKey.withUnsafeBytes { rawBuffer in + let bytes = rawBuffer.bindMemory(to: UInt8.self) + return platform_wallet_secp256k1_verify_compressed_point( + bytes.baseAddress, + UInt(bytes.count) + ) == 1 + } + } + + /// The 33-byte compressed public key for a 32-byte secp256k1 private key. + public static func compressedPublicKey(privateKey: Data) throws -> Data { + var output = [UInt8](repeating: 0, count: 33) + + let result = privateKey.withUnsafeBytes { rawBuffer -> PlatformWalletFFIResult in + let bytes = rawBuffer.bindMemory(to: UInt8.self) + return output.withUnsafeMutableBufferPointer { outputBuffer in + platform_wallet_secp256k1_compressed_public_key( + bytes.baseAddress, + UInt(bytes.count), + outputBuffer.baseAddress + ) + } + } + + try result.check() + return Data(output) + } + + /// The 32-byte raw affine X coordinate of `privateKey * publicKey`. + public static func ecdhSharedX(privateKey: Data, publicKey: Data) throws -> Data { + var output = [UInt8](repeating: 0, count: 32) + + let result = privateKey.withUnsafeBytes { privateBuffer -> PlatformWalletFFIResult in + let privateBytes = privateBuffer.bindMemory(to: UInt8.self) + return publicKey.withUnsafeBytes { publicBuffer in + let publicBytes = publicBuffer.bindMemory(to: UInt8.self) + return output.withUnsafeMutableBufferPointer { outputBuffer in + platform_wallet_secp256k1_ecdh_shared_x( + privateBytes.baseAddress, + UInt(privateBytes.count), + publicBytes.baseAddress, + UInt(publicBytes.count), + outputBuffer.baseAddress + ) + } + } + } + + try result.check() + return Data(output) + } +} diff --git a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift new file mode 100644 index 00000000000..71f6754f927 --- /dev/null +++ b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift @@ -0,0 +1,192 @@ +import SwiftData +import XCTest + +@testable import SwiftDashSDK + +@MainActor +final class KeychainSignerAdditionalSigningKeysTests: XCTestCase { + private enum ScopeError: Swift.Error { + case expected + } + + func testAdditionalHash160KeyIsSignableWithinScopeAndUnavailableAfterwards() async throws { + let container = try DashModelContainer.createInMemory() + let keychain = KeychainManager(serviceName: "org.dashfoundation.tests.\(UUID().uuidString)") + let signer = KeychainSigner(modelContainer: container, network: .testnet, keychain: keychain) + + let privateKey = Data(repeating: 0x11, count: 32) + let publicKey = try Secp256k1Primitives.compressedPublicKey(privateKey: privateKey) + let publicKeyHash = try XCTUnwrap( + Data(hexString: KeychainManager.computePublicKeyHashHex(publicKey)) + ) + + try await signer.withAdditionalSigningKeys([ + (publicKey: publicKeyHash, privateKey: privateKey), + ]) { + XCTAssertTrue( + signer.canSign(publicKey: publicKeyHash, keyType: KeyType.ecdsaHash160.rawValue) + ) + + let result = signer.signOnDemand( + publicKey: publicKeyHash, + keyType: KeyType.ecdsaHash160.rawValue, + data: Data("proof-of-possession".utf8) + ) + + guard case .success(let signature) = result else { + return XCTFail("expected signature, got \(result)") + } + XCTAssertEqual(signature.count, 65) + } + + XCTAssertFalse( + signer.canSign(publicKey: publicKeyHash, keyType: KeyType.ecdsaHash160.rawValue) + ) + } + + func testAdditionalSigningKeysAreZeroedAfterThrowingScope() async throws { + let container = try DashModelContainer.createInMemory() + let keychain = KeychainManager(serviceName: "org.dashfoundation.tests.\(UUID().uuidString)") + let signer = KeychainSigner(modelContainer: container, network: .testnet, keychain: keychain) + + let privateKey = Data(repeating: 0x12, count: 32) + let publicKey = try Secp256k1Primitives.compressedPublicKey(privateKey: privateKey) + let entries = signer.makeAdditionalSigningKeyEntries([(publicKey: publicKey, privateKey: privateKey)]) + + XCTAssertFalse(entries[0].isZeroedForTesting) + + do { + _ = try await signer.withAdditionalSigningKeys(entries) { + throw ScopeError.expected + } + XCTFail("expected throwing scope") + } catch ScopeError.expected { + XCTAssertTrue(entries[0].isZeroedForTesting) + XCTAssertFalse( + signer.canSign(publicKey: publicKey, keyType: KeyType.ecdsaSecp256k1.rawValue) + ) + } + } + + func testWalletOwnedKeyStillSignsThroughPersistence() throws { + let container = try DashModelContainer.createInMemory() + let keychain = KeychainManager(serviceName: "org.dashfoundation.tests.\(UUID().uuidString)") + let signer = KeychainSigner(modelContainer: container, network: .testnet, keychain: keychain) + + let persisted = try seedPersistedIdentityKey( + container: container, + keychain: keychain, + privateKey: Data(repeating: 0x13, count: 32) + ) + defer { + _ = keychain.deleteIdentityPrivateKey( + walletId: persisted.walletId, + derivationPath: persisted.derivationPath + ) + } + + XCTAssertTrue( + signer.canSign(publicKey: persisted.publicKey, keyType: KeyType.ecdsaSecp256k1.rawValue) + ) + + let result = signer.signOnDemand( + publicKey: persisted.publicKey, + keyType: KeyType.ecdsaSecp256k1.rawValue, + data: Data("wallet-owned".utf8) + ) + + guard case .success(let signature) = result else { + return XCTFail("expected persisted key signature, got \(result)") + } + XCTAssertEqual(signature.count, 65) + } + + func testAdditionalRegistryDoesNotChangeWalletOwnedBehaviorWhenBytesMatch() async throws { + let container = try DashModelContainer.createInMemory() + let keychain = KeychainManager(serviceName: "org.dashfoundation.tests.\(UUID().uuidString)") + let signer = KeychainSigner(modelContainer: container, network: .testnet, keychain: keychain) + + let persisted = try seedPersistedIdentityKey( + container: container, + keychain: keychain, + privateKey: Data(repeating: 0x14, count: 32) + ) + defer { + _ = keychain.deleteIdentityPrivateKey( + walletId: persisted.walletId, + derivationPath: persisted.derivationPath + ) + } + + let message = Data("same-bytes".utf8) + let baseline = signer.signOnDemand( + publicKey: persisted.publicKey, + keyType: KeyType.ecdsaSecp256k1.rawValue, + data: message + ) + + guard case .success(let baselineSignature) = baseline else { + return XCTFail("expected baseline signature, got \(baseline)") + } + + try await signer.withAdditionalSigningKeys([ + (publicKey: persisted.publicKey, privateKey: persisted.privateKey), + ]) { + let scoped = signer.signOnDemand( + publicKey: persisted.publicKey, + keyType: KeyType.ecdsaSecp256k1.rawValue, + data: message + ) + + guard case .success(let scopedSignature) = scoped else { + return XCTFail("expected scoped signature, got \(scoped)") + } + XCTAssertEqual(scopedSignature, baselineSignature) + } + } + + private func seedPersistedIdentityKey( + container: ModelContainer, + keychain: KeychainManager, + privateKey: Data + ) throws -> (publicKey: Data, privateKey: Data, walletId: Data, derivationPath: String) { + let publicKey = try Secp256k1Primitives.compressedPublicKey(privateKey: privateKey) + let walletId = Data(repeating: 0xAB, count: 32) + let derivationPath = "m/9'/1'/5'/0'/0'/0'/0'" + let metadata = IdentityPrivateKeyMetadata( + identityId: "id1", + keyId: 0, + walletId: walletId.toHexString(), + identityIndex: 0, + keyIndex: 0, + derivationPath: derivationPath, + publicKey: publicKey.toHexString(), + publicKeyHash: KeychainManager.computePublicKeyHashHex(publicKey), + keyType: KeyType.ecdsaSecp256k1.rawValue, + purpose: KeyPurpose.authentication.rawValue, + securityLevel: SecurityLevel.high.rawValue + ) + let identifier = try XCTUnwrap( + keychain.storeIdentityPrivateKey( + privateKey, + derivationPath: derivationPath, + metadata: metadata + ) + ) + + let context = ModelContext(container) + let row = PersistentPublicKey( + keyId: 0, + purpose: .authentication, + securityLevel: .high, + keyType: .ecdsaSecp256k1, + publicKeyData: publicKey, + identityId: "id1" + ) + row.privateKeyKeychainIdentifier = identifier + context.insert(row) + try context.save() + + return (publicKey, privateKey, walletId, derivationPath) + } +} From fa4e859f50ef063508295e7481c4151a04b39154 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:12:54 +0300 Subject: [PATCH 2/7] fix(swift-sdk): compile the signer scope in Swift 6 mode and close the key race Two defects in the scoped signing-key registry, one found by CI and one by review. `swift test` failed to compile: the package builds in Swift 6 language mode with `-warnings-as-errors`, where a `@MainActor` caller cannot hand a non-Sendable closure to a nonisolated `async` function ("sending value of non-Sendable type '() async -> ()' risks causing data races"). The app did not catch this because it still builds in Swift 5 mode. Both `withAdditionalSigningKeys` overloads now take `isolation: isolated (any Actor)? = #isolation`, so `body` runs in the caller's context and never crosses an isolation boundary; the parameter is defaulted, so existing call sites are unchanged. `signOnDemand` also looked the entry up under `queue` but signed with it after the lock was released, while `popAdditionalSigningKeys` / `clearAdditionalSigningKeys` zero the same buffer from inside `queue`. A scope ending on another thread could therefore zero a key mid-signature and yield a garbage signature instead of a clean failure. `signWithScopedKey` now performs the lookup and the signature in one critical section. Also drops the redundant `try` from the two non-throwing test scopes, which `-warnings-as-errors` promoted to failures. Co-Authored-By: Claude Opus 5 --- .../SwiftDashSDK/FFI/KeychainSigner.swift | 25 +++++++++++++++++-- ...hainSignerAdditionalSigningKeysTests.swift | 4 +-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index fd54b01c95d..db15d36321d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -292,18 +292,24 @@ public final class KeychainSigner: Signer, @unchecked Sendable { } } + /// `isolation` keeps `body` running in the caller's actor context, so a + /// caller isolated to an actor (e.g. `@MainActor`) does not have to send a + /// non-`Sendable` closure across an isolation boundary to use the scope. public func withAdditionalSigningKeys( _ keys: [(publicKey: Data, privateKey: Data)], + isolation: isolated (any Actor)? = #isolation, perform body: () async throws -> T ) async rethrows -> T { try await withAdditionalSigningKeys( makeAdditionalSigningKeyEntries(keys), + isolation: isolation, perform: body ) } func withAdditionalSigningKeys( _ entries: [AdditionalSigningKeyEntry], + isolation: isolated (any Actor)? = #isolation, perform body: () async throws -> T ) async rethrows -> T { pushAdditionalSigningKeys(entries) @@ -374,6 +380,21 @@ public final class KeychainSigner: Signer, @unchecked Sendable { return entry } + /// Looks the scoped key up and signs with it inside one critical section. + /// `popAdditionalSigningKeys` / `clearAdditionalSigningKeys` zero an + /// entry's bytes while holding `queue`, so releasing the lock between the + /// lookup and the signature would let a scope ending on another thread + /// zero the key mid-read and yield a garbage signature instead of a clean + /// failure. Returns `nil` when no scoped key matches. + func signWithScopedKey(publicKey: Data, data: Data) -> Result? { + queue.sync { + guard let entry = additionalSigningKeys[publicKey]?.last else { + return nil + } + return entry.sign(data: data, network: network) + } + } + // MARK: key_type dispatch // // The Rust signer FFI sends one of two payload shapes through the @@ -568,8 +589,8 @@ public final class KeychainSigner: Signer, @unchecked Sendable { keyType: UInt8, data: Data ) -> Result { - if let entry = additionalSigningKey(publicKey: publicKey) { - return entry.sign(data: data, network: network) + if let result = signWithScopedKey(publicKey: publicKey, data: data) { + return result } if keyType == Self.platformAddressHashKeyType { diff --git a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift index 71f6754f927..286f16a91ed 100644 --- a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift +++ b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/KeychainSignerAdditionalSigningKeysTests.swift @@ -20,7 +20,7 @@ final class KeychainSignerAdditionalSigningKeysTests: XCTestCase { Data(hexString: KeychainManager.computePublicKeyHashHex(publicKey)) ) - try await signer.withAdditionalSigningKeys([ + await signer.withAdditionalSigningKeys([ (publicKey: publicKeyHash, privateKey: privateKey), ]) { XCTAssertTrue( @@ -129,7 +129,7 @@ final class KeychainSignerAdditionalSigningKeysTests: XCTestCase { return XCTFail("expected baseline signature, got \(baseline)") } - try await signer.withAdditionalSigningKeys([ + await signer.withAdditionalSigningKeys([ (publicKey: persisted.publicKey, privateKey: persisted.privateKey), ]) { let scoped = signer.signOnDemand( From 0ee39138916d66a10c89528fadb02f7de5ad6520 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:13:08 +0300 Subject: [PATCH 3/7] fix(platform-wallet): symmetric transition framing and honest contract bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review of the parse entry point, which reads attacker-supplied bytes from a DashConnect `dash-st:` QR. The framing choice was one-directional: a payload whose first byte is the variant tag went straight to the tagged path with no retry, while everything else got both framings. A tagless body whose first byte is 6 by coincidence therefore failed outright. Both orders now fall back to the other framing, and the combined error names which framing produced which failure. `encode_contract_bounds` downgraded `SingleContractDocumentType` to `SingleContract` when the document type name could not become a C string. That reports a broader scope than the transition declares — in the DashConnect approval flow the user would be shown contract-wide access for a key bounded to one document type. It now returns an error that propagates through `project_parsed_identity_update`, which releases the keys it had already projected on the way out; the shared `free_parsed_public_keys` helper is used by both that path and the public free entry point. Co-Authored-By: Claude Opus 5 --- .../src/identity_update.rs | 231 ++++++++++++------ 1 file changed, 154 insertions(+), 77 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/identity_update.rs b/packages/rs-platform-wallet-ffi/src/identity_update.rs index 2041e9d3b36..f148552add0 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_update.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_update.rs @@ -74,44 +74,42 @@ impl Default for ParsedIdentityUpdateFFI { } } -fn deserialize_state_transition(bytes: &[u8]) -> Result { - StateTransition::deserialize_from_bytes(bytes).map_err(|error| { - PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorDeserialization, - format!("Failed to deserialize IdentityUpdateTransition: {error}"), - ) - }) -} - fn parse_identity_update_transition_bytes( bytes: &[u8], ) -> Result< dpp::state_transition::identity_update_transition::IdentityUpdateTransition, PlatformWalletFFIResult, > { - let state_transition = if bytes.first().copied() == Some(IDENTITY_UPDATE_VARIANT_TAG) { - deserialize_state_transition(bytes)? - } else { - let mut prefixed = Vec::with_capacity(bytes.len() + 1); - prefixed.push(IDENTITY_UPDATE_VARIANT_TAG); - prefixed.extend_from_slice(bytes); + let mut prefixed = Vec::with_capacity(bytes.len() + 1); + prefixed.push(IDENTITY_UPDATE_VARIANT_TAG); + prefixed.extend_from_slice(bytes); + + // A leading variant tag usually means the payload is already framed as a + // state transition, and Yappr's tagless framing needs the tag prepended. + // Neither test is conclusive — a tagless body can start with the tag byte + // by coincidence — so the likelier framing is only tried first, and the + // other one is still tried before the payload is rejected. + let (first, first_label, second, second_label) = + if bytes.first().copied() == Some(IDENTITY_UPDATE_VARIANT_TAG) { + (bytes, "as-is", prefixed.as_slice(), "variant tag prepended") + } else { + (prefixed.as_slice(), "variant tag prepended", bytes, "as-is") + }; - match StateTransition::deserialize_from_bytes(&prefixed) { + let state_transition = match StateTransition::deserialize_from_bytes(first) { + Ok(state_transition) => state_transition, + Err(first_error) => match StateTransition::deserialize_from_bytes(second) { Ok(state_transition) => state_transition, - Err(prefixed_error) => match StateTransition::deserialize_from_bytes(bytes) { - Ok(state_transition) => state_transition, - Err(tagged_error) => { - return Err(PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorDeserialization, - format!( - "Failed to deserialize IdentityUpdateTransition in either framing \ - (Yappr tagless + prefix 6 first: {prefixed_error}; tagged fallback: \ - {tagged_error})" - ), - )); - } - }, - } + Err(second_error) => { + return Err(PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorDeserialization, + format!( + "Failed to deserialize IdentityUpdateTransition in either framing \ + ({first_label}: {first_error}; {second_label}: {second_error})" + ), + )); + } + }, }; match state_transition { @@ -123,49 +121,95 @@ fn parse_identity_update_transition_bytes( } } -fn encode_contract_bounds(bounds: Option<&ContractBounds>) -> (u8, [u8; 32], *mut c_char) { +/// Reporting a narrower bound than the transition declares would let the user +/// approve a broader scope than the one they were shown, so a document type +/// that cannot cross the FFI as a C string is an error rather than a fallback +/// to the contract-only bound. +fn encode_contract_bounds( + bounds: Option<&ContractBounds>, +) -> Result<(u8, [u8; 32], *mut c_char), PlatformWalletFFIResult> { match bounds { - Some(ContractBounds::SingleContract { id }) => (1u8, id.to_buffer(), ptr::null_mut()), + Some(ContractBounds::SingleContract { id }) => Ok((1u8, id.to_buffer(), ptr::null_mut())), Some(ContractBounds::SingleContractDocumentType { id, document_type_name, }) => match CString::new(document_type_name.as_str()) { - Ok(value) => (2u8, id.to_buffer(), value.into_raw()), - Err(_) => (1u8, id.to_buffer(), ptr::null_mut()), + Ok(value) => Ok((2u8, id.to_buffer(), value.into_raw())), + Err(error) => Err(PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + format!( + "Contract-bounds document type name cannot be represented as a C string: \ + {error}" + ), + )), }, - None => (0u8, [0u8; 32], ptr::null_mut()), + None => Ok((0u8, [0u8; 32], ptr::null_mut())), + } +} + +/// Frees the owned buffers behind already-projected keys. Shared by the free +/// entry point and the error path in [`project_parsed_identity_update`], which +/// has to release what it allocated before the caller ever sees the struct. +/// +/// # Safety +/// Every non-null `data_ptr` / `contract_bounds_document_type` must be a +/// pointer this module allocated and has not freed yet. +unsafe fn free_parsed_public_keys(keys: &mut [ParsedIdentityUpdatePublicKeyFFI]) { + for key in keys.iter_mut() { + if !key.data_ptr.is_null() && key.data_len > 0 { + let data_slice = slice::from_raw_parts_mut(key.data_ptr, key.data_len); + let _ = Box::from_raw(data_slice as *mut [u8]); + key.data_ptr = ptr::null_mut(); + key.data_len = 0; + } + + if !key.contract_bounds_document_type.is_null() { + let _ = CString::from_raw(key.contract_bounds_document_type); + key.contract_bounds_document_type = ptr::null_mut(); + } } } fn project_parsed_identity_update( transition: &dpp::state_transition::identity_update_transition::IdentityUpdateTransition, -) -> ParsedIdentityUpdateFFI { +) -> Result { let identity_id = transition.identity_id().to_buffer(); - let add_public_keys_vec: Vec = transition - .public_keys_to_add() - .iter() - .map(|public_key| { - let data = public_key.data().as_slice().to_vec().into_boxed_slice(); - let data_len = data.len(); - let data_ptr = Box::into_raw(data) as *mut u8; - let (contract_bounds_kind, contract_bounds_id, contract_bounds_document_type) = - encode_contract_bounds(public_key.contract_bounds()); - - ParsedIdentityUpdatePublicKeyFFI { - key_id: public_key.id(), - key_type: public_key.key_type() as u8, - purpose: public_key.purpose() as u8, - security_level: public_key.security_level() as u8, - read_only: public_key.read_only(), - data_ptr, - data_len, - contract_bounds_kind, - contract_bounds_id, - contract_bounds_document_type, - } - }) - .collect(); + let public_keys_to_add = transition.public_keys_to_add(); + let mut add_public_keys_vec: Vec = + Vec::with_capacity(public_keys_to_add.len()); + + for public_key in public_keys_to_add.iter() { + // Encoded before the key data is boxed, so a rejected bound leaves + // nothing of this key to release. + let (contract_bounds_kind, contract_bounds_id, contract_bounds_document_type) = + match encode_contract_bounds(public_key.contract_bounds()) { + Ok(bounds) => bounds, + Err(error) => { + // The caller never receives this struct, so nothing else + // will ever free the keys projected so far. + unsafe { free_parsed_public_keys(&mut add_public_keys_vec) }; + return Err(error); + } + }; + + let data = public_key.data().as_slice().to_vec().into_boxed_slice(); + let data_len = data.len(); + let data_ptr = Box::into_raw(data) as *mut u8; + + add_public_keys_vec.push(ParsedIdentityUpdatePublicKeyFFI { + key_id: public_key.id(), + key_type: public_key.key_type() as u8, + purpose: public_key.purpose() as u8, + security_level: public_key.security_level() as u8, + read_only: public_key.read_only(), + data_ptr, + data_len, + contract_bounds_kind, + contract_bounds_id, + contract_bounds_document_type, + }); + } let add_public_keys_count = add_public_keys_vec.len(); let add_public_keys = if add_public_keys_count == 0 { @@ -183,13 +227,13 @@ fn project_parsed_identity_update( Box::into_raw(disable_public_key_ids_vec.into_boxed_slice()) as *mut u32 }; - ParsedIdentityUpdateFFI { + Ok(ParsedIdentityUpdateFFI { identity_id, add_public_keys, add_public_keys_count, disable_public_key_ids, disable_public_key_ids_count, - } + }) } /// Deserializes a raw `IdentityUpdateTransition` (as carried by a DashConnect @@ -214,7 +258,7 @@ pub unsafe extern "C" fn platform_wallet_parse_identity_update_transition( let bytes = slice::from_raw_parts(transition_bytes, transition_len); let transition = unwrap_result_or_return!(parse_identity_update_transition_bytes(bytes)); - *out = project_parsed_identity_update(&transition); + *out = unwrap_result_or_return!(project_parsed_identity_update(&transition)); PlatformWalletFFIResult::ok() } @@ -232,20 +276,7 @@ pub unsafe extern "C" fn platform_wallet_parse_identity_update_transition_free( if !parsed.add_public_keys.is_null() && parsed.add_public_keys_count > 0 { let keys = slice::from_raw_parts_mut(parsed.add_public_keys, parsed.add_public_keys_count); - for key in keys.iter_mut() { - if !key.data_ptr.is_null() && key.data_len > 0 { - let data_slice = slice::from_raw_parts_mut(key.data_ptr, key.data_len); - let _ = Box::from_raw(data_slice as *mut [u8]); - key.data_ptr = ptr::null_mut(); - key.data_len = 0; - } - - if !key.contract_bounds_document_type.is_null() { - let _ = CString::from_raw(key.contract_bounds_document_type); - key.contract_bounds_document_type = ptr::null_mut(); - } - } - + free_parsed_public_keys(keys); let _ = Box::from_raw(keys as *mut [ParsedIdentityUpdatePublicKeyFFI]); } @@ -476,6 +507,52 @@ mod tests { assert_eq!(out.disable_public_key_ids_count, 0); } + #[test] + fn rejects_contract_bounds_document_type_that_cannot_cross_the_ffi() { + // A document type carrying an interior NUL cannot be handed over as a + // C string. Reporting the contract-only bound instead would show the + // user a broader scope than the transition declares, so this is an + // error rather than a narrowing. + let transition = + dpp::state_transition::identity_update_transition::IdentityUpdateTransition::V0( + IdentityUpdateTransitionV0 { + signature: BinaryData::new(vec![0x99; 65]), + signature_public_key_id: 3, + identity_id: Identifier::from([0x11; 32]), + revision: 7, + nonce: 9, + add_public_keys: vec![IdentityPublicKeyInCreationV0 { + id: 17, + key_type: KeyType::ECDSA_SECP256K1, + purpose: Purpose::ENCRYPTION, + security_level: SecurityLevel::MEDIUM, + read_only: false, + data: BinaryData::new(vec![0x02; 33]), + signature: BinaryData::new(vec![0xaa; 65]), + contract_bounds: Some(ContractBounds::SingleContractDocumentType { + id: Identifier::from([0x44; 32]), + document_type_name: "pro\0file".to_string(), + }), + } + .into()], + disable_public_keys: vec![], + user_fee_increase: 0, + }, + ); + + // `ParsedIdentityUpdateFFI` is a raw-pointer C struct with no `Debug`, + // so the success case is rejected by hand rather than with `expect_err`. + let error = match project_parsed_identity_update(&transition) { + Ok(_) => panic!("a document type with an interior NUL must not be narrowed"), + Err(error) => error, + }; + + assert_eq!( + error.code, + PlatformWalletFFIResultCode::ErrorInvalidParameter + ); + } + #[test] fn rejects_truncated_identity_update_transition_bytes() { let mut bytes = fixture_transition_bytes(); From fdd356b6d26a0e4c0298a0641abcafeb3ab50368 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:24:30 +0300 Subject: [PATCH 4/7] fix(swift-sdk): constrain the signing-scope result to Sendable The SwiftExampleApp target compiles the SDK sources without region-based isolation, so returning the generic result out of `body` was rejected there even though SwiftPM's Swift 6 build accepted it: "non-sendable result type 'T' cannot be sent from nonisolated context in call to parameter 'body'". Constrain `T` to `Sendable` on both overloads, as the compiler suggests. Every call site returns Void, so nothing is lost. Verified both ways this time: `swift build --build-tests` (SwiftPM, Swift 6 mode, warnings-as-errors) and `xcodebuild build -scheme SwiftExampleApp`. Co-Authored-By: Claude Opus 5 --- .../Sources/SwiftDashSDK/FFI/KeychainSigner.swift | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index db15d36321d..13d26964ad1 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -295,7 +295,10 @@ public final class KeychainSigner: Signer, @unchecked Sendable { /// `isolation` keeps `body` running in the caller's actor context, so a /// caller isolated to an actor (e.g. `@MainActor`) does not have to send a /// non-`Sendable` closure across an isolation boundary to use the scope. - public func withAdditionalSigningKeys( + /// `T` is `Sendable` because the result still leaves `body`'s context: the + /// SwiftExampleApp target compiles these sources without region-based + /// isolation, where returning a non-`Sendable` value is rejected outright. + public func withAdditionalSigningKeys( _ keys: [(publicKey: Data, privateKey: Data)], isolation: isolated (any Actor)? = #isolation, perform body: () async throws -> T @@ -307,7 +310,7 @@ public final class KeychainSigner: Signer, @unchecked Sendable { ) } - func withAdditionalSigningKeys( + func withAdditionalSigningKeys( _ entries: [AdditionalSigningKeyEntry], isolation: isolated (any Actor)? = #isolation, perform body: () async throws -> T From 7d998b8d169fe15780554cd6e4a356c4fe92742b Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 3 Aug 2026 19:34:00 +0300 Subject: [PATCH 5/7] fix(platform-wallet): constant-time ECDH, and three review follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 review of #4273. The first is a real weakness, not a style point. **ECDH ran in variable time over the secret scalar.** `PublicKey::mul_tweak` is libsecp256k1's `secp256k1_ec_pubkey_tweak_mul`, which reaches the WNAF `ecmult` whose table selection and branching depend on the scalar — correct for a *public* tweak, wrong here: the tweak is the wallet's private ephemeral key and the peer supplies the point, so the primitive leaked secret-dependent timing to the party choosing its input. Now goes through `ecdh::shared_secret_point` (`secp256k1_ecdh` / `ecmult_const`), with the 64-byte `x || y` result held in `Zeroizing`. `ecdh_shared_x_matches_known_vector` is unchanged and still passes, which is the proof the swap is equivalent. **The scoped signing key left an unzeroed copy.** `Data(privateKeyBytes)` built a buffer nothing could scrub: `RawKeySigner.sign` zeroes only its own `keyCopy`, and `popAdditionalSigningKeys` only the entry's storage. Added a `privateKeyBuffer:` overload that borrows the entry's bytes; the `Data` overload is now a wrapper, so no call site changes. **The "Yappr tagless" test used a self-serialized fixture** — the local fixture with byte 0 removed, which only proved the parser agreed with itself about framing. It now parses the payload captured from the live testnet app (the same bytes the iOS tests assert) and checks both keys' ids, purposes, levels, types, data, and that neither carries contract bounds — the real unbounded ENCRYPTION key that this PR's decoder widening exists for. The synthetic round-trip survives under an honest name. **Swift still documented encryption contract bounds as mandatory,** which contradicts the widened decoder and could lead a consumer to reject a valid transition before ever reaching the FFI. Co-Authored-By: Claude Opus 5 --- .../src/identity_update.rs | 84 +++++++++++++++++++ .../src/secp256k1_primitives.rs | 35 ++------ .../SwiftDashSDK/FFI/KeychainSigner.swift | 12 +-- .../SwiftDashSDK/KeyWallet/KeyManager.swift | 36 ++++++-- .../ManagedPlatformWallet.swift | 10 +-- 5 files changed, 133 insertions(+), 44 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/identity_update.rs b/packages/rs-platform-wallet-ffi/src/identity_update.rs index f148552add0..516ef7276d3 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_update.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_update.rs @@ -384,6 +384,15 @@ mod tests { use dpp::state_transition::identity_update_transition::v0::IdentityUpdateTransitionV0; use dpp::state_transition::public_key_in_creation::v0::IdentityPublicKeyInCreationV0; + const YAPPR_TAGLESS_IDENTITY_UPDATE_FIXTURE_HEX: &str = concat!( + "0089fd6ddba75136a4fea02dc7d89ef0ca5bcc32ccf12fb8da6a1a03740567ae72", + "01010200060200020000145e24e38a86e720f61757647996957e322686abb70000", + "07000103000021035e8cfb0785b54e8902a3dc17bdaad8a5738c6019a18ebc527f", + "79d1c64a27826a4120dc911df1d1e6cccf8c95ec0d423c928433397933de6dd9ad", + "006bc40dc0334d6d270d50c6d5e2dcdc5560e40487ddfe28bd1066d0729fad4b26", + "f92ae33f12a04b00000000" + ); + fn fixture_transition_bytes() -> Vec { let identity_id = Identifier::from([0x11; 32]); let contract_id = Identifier::from([0x44; 32]); @@ -469,6 +478,81 @@ mod tests { #[test] fn parses_yappr_tagless_identity_update_transition() { + let tagless = hex::decode(YAPPR_TAGLESS_IDENTITY_UPDATE_FIXTURE_HEX) + .expect("valid Yappr fixture hex"); + let mut out = ParsedIdentityUpdateFFI::default(); + + let result = unsafe { + platform_wallet_parse_identity_update_transition( + tagless.as_ptr(), + tagless.len(), + &mut out, + ) + }; + + assert_eq!(result.code, PlatformWalletFFIResultCode::Success); + assert_eq!( + out.identity_id.as_slice(), + hex::decode("89fd6ddba75136a4fea02dc7d89ef0ca5bcc32ccf12fb8da6a1a03740567ae72") + .expect("valid identity id hex") + .as_slice() + ); + assert_eq!(out.disable_public_key_ids_count, 0); + assert_eq!(out.add_public_keys_count, 2); + + let keys = unsafe { slice::from_raw_parts(out.add_public_keys, out.add_public_keys_count) }; + + assert_eq!(keys[0].key_id, 6); + assert_eq!( + Purpose::try_from(keys[0].purpose).expect("recognized purpose"), + Purpose::AUTHENTICATION + ); + assert_eq!( + SecurityLevel::try_from(keys[0].security_level).expect("recognized security level"), + SecurityLevel::HIGH + ); + assert_eq!( + KeyType::try_from(keys[0].key_type).expect("recognized key type"), + KeyType::ECDSA_HASH160 + ); + assert_eq!(keys[0].contract_bounds_kind, 0); + let key0_data = unsafe { slice::from_raw_parts(keys[0].data_ptr, keys[0].data_len) }; + assert_eq!( + key0_data, + hex::decode("5e24e38a86e720f61757647996957e322686abb7") + .expect("valid authentication key hex") + .as_slice() + ); + + assert_eq!(keys[1].key_id, 7); + assert_eq!( + Purpose::try_from(keys[1].purpose).expect("recognized purpose"), + Purpose::ENCRYPTION + ); + assert_eq!( + SecurityLevel::try_from(keys[1].security_level).expect("recognized security level"), + SecurityLevel::MEDIUM + ); + assert_eq!( + KeyType::try_from(keys[1].key_type).expect("recognized key type"), + KeyType::ECDSA_SECP256K1 + ); + // The real DashConnect ENCRYPTION key is intentionally unbounded, so + // this guards the decoder widening that now accepts absent bounds. + assert_eq!(keys[1].contract_bounds_kind, 0); + let key1_data = unsafe { slice::from_raw_parts(keys[1].data_ptr, keys[1].data_len) }; + assert_eq!( + key1_data, + hex::decode("035e8cfb0785b54e8902a3dc17bdaad8a5738c6019a18ebc527f79d1c64a27826a") + .expect("valid encryption key hex") + .as_slice() + ); + + unsafe { platform_wallet_parse_identity_update_transition_free(&mut out) }; + } + + #[test] + fn parses_tagless_framing_by_prepending_the_variant_tag() { let tagged = fixture_transition_bytes(); let tagless = tagged[1..].to_vec(); let mut out = ParsedIdentityUpdateFFI::default(); diff --git a/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs b/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs index e3e3942281d..476d86dc4a6 100644 --- a/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs +++ b/packages/rs-platform-wallet-ffi/src/secp256k1_primitives.rs @@ -2,7 +2,8 @@ use std::slice; -use dashcore::secp256k1::{PublicKey, Scalar, Secp256k1, SecretKey}; +use dashcore::secp256k1::ecdh::shared_secret_point; +use dashcore::secp256k1::{PublicKey, Secp256k1, SecretKey}; use zeroize::Zeroizing; use crate::error::*; @@ -16,14 +17,6 @@ impl Drop for WipingSecretKey { } } -struct WipingScalar(Scalar); - -impl Drop for WipingScalar { - fn drop(&mut self) { - self.0.non_secure_erase(); - } -} - fn invalid_parameter(message: impl Into) -> PlatformWalletFFIResult { PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorInvalidParameter, @@ -31,13 +24,6 @@ fn invalid_parameter(message: impl Into) -> PlatformWalletFFIResult { ) } -fn wallet_operation(message: impl Into) -> PlatformWalletFFIResult { - PlatformWalletFFIResult::err( - PlatformWalletFFIResultCode::ErrorWalletOperation, - message.into(), - ) -} - fn parse_secret_key( private_key: *const u8, private_key_len: usize, @@ -123,17 +109,12 @@ pub unsafe extern "C" fn platform_wallet_secp256k1_ecdh_shared_x( let secret_key = unwrap_result_or_return!(parse_secret_key(seckey, seckey_len)); let public_key = unwrap_result_or_return!(parse_compressed_public_key(pubkey, pubkey_len)); - let secp = Secp256k1::new(); - let scalar_bytes = Zeroizing::new(secret_key.0.secret_bytes()); - let scalar = unwrap_result_or_return!(Scalar::from_be_bytes(*scalar_bytes) - .map(WipingScalar) - .map_err(|error| invalid_parameter(format!("Invalid secp256k1 scalar: {error}")))); - - let shared_point = unwrap_result_or_return!(public_key - .mul_tweak(&secp, &scalar.0) - .map_err(|_| wallet_operation("ECDH produced the point at infinity"))); - let uncompressed = shared_point.serialize_uncompressed(); - std::ptr::copy_nonoverlapping(uncompressed[1..33].as_ptr(), out_shared_x, 32); + // `shared_secret_point` uses libsecp256k1's constant-time ECDH path: + // the scalar here is the wallet's private ephemeral key and the point + // comes from the peer, so the multiplication must not branch on the + // secret. Returns `x || y`; DashConnect wants the raw, unhashed X. + let shared_point = Zeroizing::new(shared_secret_point(&public_key, &secret_key.0)); + std::ptr::copy_nonoverlapping(shared_point.as_ptr(), out_shared_x, 32); PlatformWalletFFIResult::ok() } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift index 13d26964ad1..f8cda376ab2 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/FFI/KeychainSigner.swift @@ -100,11 +100,13 @@ public final class KeychainSigner: Signer, @unchecked Sendable { func sign(data: Data, network: Network) -> Result { do { return .success( - try RawKeySigner.sign( - data: data, - privateKey: Data(privateKeyBytes), - network: network - ) + try privateKeyBytes.withUnsafeBufferPointer { buffer in + try RawKeySigner.sign( + data: data, + privateKeyBuffer: buffer, + network: network + ) + } ) } catch KeyManagerError.signerCreationFailed(let message) { return .failure(.ffiSignerCreationFailed(message: message)) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swift index 28a1a098370..e4baac4e1d2 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/KeyManager.swift @@ -53,18 +53,23 @@ public enum RawKeySigner { /// - Parameters: /// - data: Raw bytes to sign. Hashed (SHA256d) inside the FFI — /// pass the full message, never a pre-computed digest. - /// - privateKey: 32-byte ECDSA scalar. A local copy is zeroed - /// before returning. + /// - privateKeyBuffer: 32-byte ECDSA scalar borrowed from the + /// caller. A local copy is still zeroed before returning so the + /// FFI signer never reads directly from caller-owned storage. /// - network: Affects WIF/address metadata inside the signer only, /// not the signature bytes. - public static func sign(data: Data, privateKey: Data, network: Network) throws -> Data { - guard privateKey.count == 32 else { + public static func sign( + data: Data, + privateKeyBuffer: UnsafeBufferPointer, + network: Network + ) throws -> Data { + guard privateKeyBuffer.count == 32 else { throw KeyManagerError.invalidKeyFormat( - "Private key must be 32 bytes, got \(privateKey.count)") + "Private key must be 32 bytes, got \(privateKeyBuffer.count)") } // Defensive copy into a mutable buffer we can zero on exit. - var keyCopy = [UInt8](privateKey) + var keyCopy = Array(privateKeyBuffer) defer { keyCopy.withUnsafeMutableBufferPointer { buf in if let base = buf.baseAddress { @@ -117,6 +122,24 @@ public enum RawKeySigner { } return Data(bytes: bytes, count: Int(sigStruct.pointee.signature_len)) } + + /// - Parameters: + /// - data: Raw bytes to sign. Hashed (SHA256d) inside the FFI — + /// pass the full message, never a pre-computed digest. + /// - privateKey: 32-byte ECDSA scalar. A local copy is zeroed + /// before returning. + /// - network: Affects WIF/address metadata inside the signer only, + /// not the signature bytes. + public static func sign(data: Data, privateKey: Data, network: Network) throws -> Data { + try privateKey.withUnsafeBytes { rawBuffer in + let keyBytes = rawBuffer.bindMemory(to: UInt8.self) + return try sign( + data: data, + privateKeyBuffer: UnsafeBufferPointer(start: keyBytes.baseAddress, count: keyBytes.count), + network: network + ) + } + } } // MARK: - Key Manager @@ -471,4 +494,3 @@ public final class KeyManager: Sendable { return privateKeyData.count == 32 } } - diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift index 21dd5f35075..f9c0f731595 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/ManagedPlatformWallet.swift @@ -173,11 +173,11 @@ public final class ManagedPlatformWallet: @unchecked Sendable { /// secp256k1; 48 for BLS; etc.). public let pubkeyBytes: Data public let readOnly: Bool - /// Optional contract-bounds restriction. Required for - /// Encryption / Decryption keys (Drive scopes those keys - /// to a specific contract / document type so a key issued - /// for App A cannot decrypt App B's payloads). `nil` for - /// every other purpose. + /// Optional contract-bounds restriction. Bounds are valid + /// for any purpose only when present and only if consensus + /// allows that purpose / contract / document-type shape. + /// `nil` is valid for every purpose, including Encryption / + /// Decryption keys. public let contractBounds: ContractBounds? public init( From 473b54c20522b4288e7ff4d4f9efaa70197a0c87 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 4 Aug 2026 02:23:08 +0300 Subject: [PATCH 6/7] fix(platform-wallet): remove avoidable secret copies and assert read_only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-3 review of #4273 — three suggestions, no blockers. **The Swift ECDH wrapper copied the shared secret twice.** Rust wrote the raw X coordinate into a scratch `[UInt8]`, then `Data(output)` copied it into the return value and the array was released unscrubbed. `ecdhSharedX` now allocates the returned `Data` up front and lets the FFI write straight into it, so only the copy the caller asked for exists. `compressedPublicKey` is left alone — its output is a public key. **`SingleKeySigner` kept unwiped copies of the scalar.** The FFI entry point already handed it bytes from a `Zeroizing` buffer, but the signer did not maintain that: `new_from_slice` copied into a plain `[u8; 32]`, the struct had no `Drop` (and `secp256k1::SecretKey` is `Copy`, so it does not erase itself), and both `sign` and `verify_key_matches` materialised another bare array per call via `secret_bytes()`. All three now go through `Zeroizing`, and the key is erased on drop — which matters here because DashConnect builds this signer per call for login keys that are deliberately never persisted. `simple-signer` is a shared crate, so its public API is untouched; `rs-sdk-ffi` and `rs-scripts` still compile against it (`cargo check` on both). **Neither identity-update fixture asserted `read_only`.** The field is projected and mapped into Swift's `IdentityPubkey`, so dropping or hardcoding it would have left the suite green. The captured Yappr keys are both asserted as `false`; the synthetic fixture asserts `false` on key 17 and `true` on key 18, proving the field is carried rather than defaulted. Co-Authored-By: Claude Opus 5 --- Cargo.lock | 1 + .../src/identity_update.rs | 4 ++ packages/simple-signer/Cargo.toml | 1 + .../simple-signer/src/single_key_signer.rs | 41 ++++++++++++------- .../Security/Secp256k1Primitives.swift | 21 ++++++---- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6fabfe1fe26..4d66a2a1281 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7192,6 +7192,7 @@ dependencies = [ "dpp", "hex", "tracing", + "zeroize", ] [[package]] diff --git a/packages/rs-platform-wallet-ffi/src/identity_update.rs b/packages/rs-platform-wallet-ffi/src/identity_update.rs index 516ef7276d3..1ff398a5c38 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_update.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_update.rs @@ -457,6 +457,8 @@ mod tests { let keys = unsafe { slice::from_raw_parts(out.add_public_keys, out.add_public_keys_count) }; assert_eq!(keys[0].key_id, 17); + assert!(!keys[0].read_only); + assert!(keys[1].read_only); assert_eq!(keys[1].contract_bounds_kind, 2); assert_eq!(keys[1].contract_bounds_id, [0x44; 32]); let doc_type = unsafe { @@ -515,6 +517,7 @@ mod tests { KeyType::try_from(keys[0].key_type).expect("recognized key type"), KeyType::ECDSA_HASH160 ); + assert!(!keys[0].read_only); assert_eq!(keys[0].contract_bounds_kind, 0); let key0_data = unsafe { slice::from_raw_parts(keys[0].data_ptr, keys[0].data_len) }; assert_eq!( @@ -537,6 +540,7 @@ mod tests { KeyType::try_from(keys[1].key_type).expect("recognized key type"), KeyType::ECDSA_SECP256K1 ); + assert!(!keys[1].read_only); // The real DashConnect ENCRYPTION key is intentionally unbounded, so // this guards the decoder widening that now accepts absent bounds. assert_eq!(keys[1].contract_bounds_kind, 0); diff --git a/packages/simple-signer/Cargo.toml b/packages/simple-signer/Cargo.toml index 4bb9d4aa765..b95f827d797 100644 --- a/packages/simple-signer/Cargo.toml +++ b/packages/simple-signer/Cargo.toml @@ -24,6 +24,7 @@ bincode = { version = "=2.0.1", features = ["serde"] } base64 = { version = "0.22.1" } hex = { version = "0.4.3" } tracing = "0.1.41" +zeroize = "1" [package.metadata.cargo-machete] ignored = ["bincode"] diff --git a/packages/simple-signer/src/single_key_signer.rs b/packages/simple-signer/src/single_key_signer.rs index c42a431f4e5..760f06e7d87 100644 --- a/packages/simple-signer/src/single_key_signer.rs +++ b/packages/simple-signer/src/single_key_signer.rs @@ -10,6 +10,7 @@ use dpp::identity::{IdentityPublicKey, KeyType}; use dpp::platform_value::BinaryData; use dpp::ProtocolError; use tracing::{debug, warn}; +use zeroize::Zeroizing; /// A simple signer that uses a single private key /// This is designed for WASM and other single-key use cases @@ -34,9 +35,9 @@ impl SingleKeySigner { if private_key_data.len() != 32 { return Err("Private key must be 32 bytes".to_string()); } - let mut arr = [0u8; 32]; + let mut arr = Zeroizing::new([0u8; 32]); arr.copy_from_slice(private_key_data); - let private_key = PrivateKey::from_byte_array(&arr, network) + let private_key = PrivateKey::from_byte_array(&*arr, network) .map_err(|e| format!("Invalid private key: {}", e))?; Ok(Self { private_key }) } @@ -83,6 +84,15 @@ impl SingleKeySigner { } } +impl Drop for SingleKeySigner { + /// `secp256k1::SecretKey` is `Copy` and does not erase itself, and this + /// signer is built per FFI call for keys that are deliberately never + /// persisted, so the scalar is cleared when the handle is destroyed. + fn drop(&mut self) { + self.private_key.inner.non_secure_erase(); + } +} + #[async_trait] impl Signer for SingleKeySigner { async fn sign( @@ -95,7 +105,8 @@ impl Signer for SingleKeySigner { KeyType::ECDSA_SECP256K1 | KeyType::ECDSA_HASH160 => { // Do not log private key material. Log data fingerprint only. debug!(data_hex = %hex::encode(data), "SingleKeySigner: signing data"); - let signature = signer::sign(data, &self.private_key.inner.secret_bytes())?; + let secret_bytes = Zeroizing::new(self.private_key.inner.secret_bytes()); + let signature = signer::sign(data, &secret_bytes[..])?; Ok(signature.to_vec().into()) } _ => { @@ -137,12 +148,12 @@ impl Signer for SingleKeySigner { KeyType::ECDSA_SECP256K1 => { // Compare full public key let secp = dashcore::secp256k1::Secp256k1::new(); - let secret_key = match dashcore::secp256k1::SecretKey::from_byte_array( - &self.private_key.inner.secret_bytes(), - ) { - Ok(sk) => sk, - Err(_) => return false, - }; + let secret_bytes = Zeroizing::new(self.private_key.inner.secret_bytes()); + let secret_key = + match dashcore::secp256k1::SecretKey::from_byte_array(&*secret_bytes) { + Ok(sk) => sk, + Err(_) => return false, + }; let public_key = dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); let public_key_bytes = public_key.serialize(); @@ -154,12 +165,12 @@ impl Signer for SingleKeySigner { use dpp::dashcore::hashes::{hash160, Hash}; let secp = dashcore::secp256k1::Secp256k1::new(); - let secret_key = match dashcore::secp256k1::SecretKey::from_byte_array( - &self.private_key.inner.secret_bytes(), - ) { - Ok(sk) => sk, - Err(_) => return false, - }; + let secret_bytes = Zeroizing::new(self.private_key.inner.secret_bytes()); + let secret_key = + match dashcore::secp256k1::SecretKey::from_byte_array(&*secret_bytes) { + Ok(sk) => sk, + Err(_) => return false, + }; let public_key = dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); let public_key_bytes = public_key.serialize(); diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift index 50ad6ed2ecc..f4463d70edc 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Security/Secp256k1Primitives.swift @@ -42,25 +42,28 @@ public enum Secp256k1Primitives { /// The 32-byte raw affine X coordinate of `privateKey * publicKey`. public static func ecdhSharedX(privateKey: Data, publicKey: Data) throws -> Data { - var output = [UInt8](repeating: 0, count: 32) + // The FFI writes the shared secret directly into the returned buffer: + // a separate scratch array would leave a second, unscrubbed copy of + // the session secret in allocator memory once it was copied into `Data`. + var output = Data(count: 32) - let result = privateKey.withUnsafeBytes { privateBuffer -> PlatformWalletFFIResult in - let privateBytes = privateBuffer.bindMemory(to: UInt8.self) - return publicKey.withUnsafeBytes { publicBuffer in - let publicBytes = publicBuffer.bindMemory(to: UInt8.self) - return output.withUnsafeMutableBufferPointer { outputBuffer in - platform_wallet_secp256k1_ecdh_shared_x( + let result = output.withUnsafeMutableBytes { outputBuffer -> PlatformWalletFFIResult in + privateKey.withUnsafeBytes { privateBuffer in + publicKey.withUnsafeBytes { publicBuffer in + let privateBytes = privateBuffer.bindMemory(to: UInt8.self) + let publicBytes = publicBuffer.bindMemory(to: UInt8.self) + return platform_wallet_secp256k1_ecdh_shared_x( privateBytes.baseAddress, UInt(privateBytes.count), publicBytes.baseAddress, UInt(publicBytes.count), - outputBuffer.baseAddress + outputBuffer.bindMemory(to: UInt8.self).baseAddress ) } } } try result.check() - return Data(output) + return output } } From 51994290b3c153e6e25055c3f9d756b8fcd9fdc5 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 4 Aug 2026 17:41:19 +0300 Subject: [PATCH 7/7] fix(simple-signer): avoid temporary secret keys Derive public keys directly from the signer-owned secret instead of parsing additional SecretKey copies in can_sign_with. Cover matching and mismatching secp256k1 and hash160 identity keys. --- .../simple-signer/src/single_key_signer.rs | 64 +++++++++++++------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/packages/simple-signer/src/single_key_signer.rs b/packages/simple-signer/src/single_key_signer.rs index 54f1cc63541..ad76b20af51 100644 --- a/packages/simple-signer/src/single_key_signer.rs +++ b/packages/simple-signer/src/single_key_signer.rs @@ -89,11 +89,11 @@ impl Drop for SingleKeySigner { /// signer is built per FFI call for keys that are deliberately never /// persisted, so the scalar is cleared when the handle is destroyed. /// - /// This covers the copies this type owns. It does not reach the ones the + /// This covers the copies this type owns. It does not reach the one the /// pinned `dashcore` makes internally: `signer::sign` parses its own - /// `SecretKey` from the bytes handed to it, as do the `can_sign_with` - /// branches below, and neither erases it before returning. Closing that - /// gap needs erasing storage inside `rust-dashcore` itself. + /// `SecretKey` from the bytes handed to it and does not erase it before + /// returning. Closing that gap needs erasing storage inside + /// `rust-dashcore` itself. /// /// TODO(dashconnect-key-hygiene): upstream zeroizing parse/sign storage to /// `rust-dashcore`, then bump the pinned revision here. @@ -157,14 +157,8 @@ impl Signer for SingleKeySigner { KeyType::ECDSA_SECP256K1 => { // Compare full public key let secp = dashcore::secp256k1::Secp256k1::new(); - let secret_bytes = Zeroizing::new(self.private_key.inner.secret_bytes()); - let secret_key = - match dashcore::secp256k1::SecretKey::from_byte_array(&secret_bytes) { - Ok(sk) => sk, - Err(_) => return false, - }; let public_key = - dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &self.private_key.inner); let public_key_bytes = public_key.serialize(); identity_public_key.data().as_slice() == public_key_bytes @@ -174,14 +168,8 @@ impl Signer for SingleKeySigner { use dpp::dashcore::hashes::{hash160, Hash}; let secp = dashcore::secp256k1::Secp256k1::new(); - let secret_bytes = Zeroizing::new(self.private_key.inner.secret_bytes()); - let secret_key = - match dashcore::secp256k1::SecretKey::from_byte_array(&secret_bytes) { - Ok(sk) => sk, - Err(_) => return false, - }; let public_key = - dashcore::secp256k1::PublicKey::from_secret_key(&secp, &secret_key); + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &self.private_key.inner); let public_key_bytes = public_key.serialize(); let public_key_hash160 = hash160::Hash::hash(&public_key_bytes) .to_byte_array() @@ -198,6 +186,22 @@ impl Signer for SingleKeySigner { mod tests { use super::*; use dashcore::Network; + use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dpp::identity::{Purpose, SecurityLevel}; + use dpp::platform_value::BinaryData; + + fn identity_key(key_type: KeyType, data: Vec) -> IdentityPublicKey { + IdentityPublicKey::V0(IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::AUTHENTICATION, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type, + read_only: false, + data: BinaryData::new(data), + disabled_at: None, + }) + } #[test] fn test_single_key_signer_from_wif() -> Result<(), String> { @@ -241,6 +245,30 @@ mod tests { assert!(SingleKeySigner::new_from_slice(&bytes, Network::Testnet).is_err()); } + #[test] + fn test_can_sign_with_ecdsa_key_types() -> Result<(), String> { + use dpp::dashcore::hashes::{hash160, Hash}; + + let signer = SingleKeySigner::new_from_slice(&[0x03; 32], Network::Testnet)?; + let secp = dashcore::secp256k1::Secp256k1::new(); + let public_key = + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &signer.private_key.inner) + .serialize(); + + let full_key = identity_key(KeyType::ECDSA_SECP256K1, public_key.to_vec()); + assert!(signer.can_sign_with(&full_key)); + let wrong_full_key = identity_key(KeyType::ECDSA_SECP256K1, vec![0; public_key.len()]); + assert!(!signer.can_sign_with(&wrong_full_key)); + + let public_key_hash = hash160::Hash::hash(&public_key).to_byte_array(); + let hash_key = identity_key(KeyType::ECDSA_HASH160, public_key_hash.to_vec()); + assert!(signer.can_sign_with(&hash_key)); + let wrong_hash_key = identity_key(KeyType::ECDSA_HASH160, vec![0; public_key_hash.len()]); + assert!(!signer.can_sign_with(&wrong_hash_key)); + + Ok(()) + } + #[test] fn test_single_key_signer_auto_detect() -> Result<(), String> { // Test hex detection