diff --git a/PROPOSAL.md b/PROPOSAL.md new file mode 100644 index 0000000..f954b13 --- /dev/null +++ b/PROPOSAL.md @@ -0,0 +1,397 @@ +# Smart Wallet Interface + +With the release of [Protocol 21](https://stellar.org/blog/developers/announcing-protocol-21) (and specifically the inclusion of the secp256r1 verification curve) Soroban now has tremendous first class support for passkey powered smart wallets. + +Over the past months I've been hard at work designing a solid first stab at a v1 smart wallet contract interface for mainnet use. This is the culmination of that work in proposal form. + +All the best work can reviewed in my [passkey-kit](https://github.com/kalepail/passkey-kit) repo. This repo includes the factory and wallet contracts, a demo client interface, a `passkey-kit` SDK tool to make interacting with the contract interface simple and painless and finally a [Mercury Zephyr](https://www.mercurydata.app/products/zephyr-vm) program for indexing contract events in order to make the wallet more usable client side. + +This repo also makes use of a new [Launchtube service](https://github.com/kalepail/launchtube) which makes submitting Soroban transactions simple by handling the concerns of both transaction fees and sequence numbers. + +The primary interest of this proposal is to detail the contract interface itself but many of the design decisions are informed by complexities and available solutions external to the interface. A well rounded understandings of all that's involved to make passkey powered smart wallets on Stellar actually work is necessary in order to arrive at a truly viable contract interface. + +This proposal consists of two contracts, a factory “deployer” contract and the actual smart wallet interface. + +# Contract 1: The Factory + +Stellar doesn’t allow us to both deploy and initialize a contract atomically and so the ecosystem has adopted the workaround of having a factory contract which handles the deploying and then calling of the newly deployed contract’s initialize function. This deploy and init can happen atomically within Soroban. + +The side benefit is we can ensure consistency of all contracts deployed from the same factory address. As long as the contract was deployed from a known factory address users and services have a guarantee of the initial inner form of the smart wallet. As we’ll see smart wallets have an `upgrade` method which will effectively break this guarantee but at the end of the day it’s a contract’s WASM hash we actually care about vs it’s factory address. + +## Interface + +```rust +// FUNCTIONS + +fn init(wasm_hash: bytesn<32>) -> result,error> + +fn deploy(id: bytes, pk: bytesn<65>) -> result + +// ERRORS + +#[contracterror] +enum Error { + NotInitialized = 1, + AlreadyInitialized = 2 +} +``` + +## Code + +[https://github.com/kalepail/passkey-kit/blob/main/contracts/contract-webauthn-factory/src/lib.rs](https://github.com/kalepail/passkey-kit/blob/main/contracts/contract-webauthn-factory/src/lib.rs) + + + +```rust +const WEEK_OF_LEDGERS: u32 = 60 * 60 * 24 / 5 * 7; +const STORAGE_KEY_WASM_HASH: Symbol = symbol_short!("hash"); +``` + +Only thing to note in this block is I’m opting to max extend this contract’s instance during every call with a threshold of 7 days. This will be the same for the wallet interface itself. This will cause the initial calls for any storage write function to be somewhat inflated with the beneficial tradeoff that folks won’t have to worry about their wallets or keys expiring or archiving for `max_ttl` time. In my tests this cost was minimal and the improved UX of not having to worry about restoring archived entries in my opinion was worth it. + +We could decide to make these values instance variables which could be updated or even make them configurable on a key by key basis however that would increase complexity and cost in many cases and without further real world data to support that choice I’m suggesting simplicity. + +### `init` +```rust +pub fn init(env: Env, wasm_hash: BytesN<32>) -> Result<(), Error> { + if env.storage().instance().has(&STORAGE_KEY_WASM_HASH) { + return Err(Error::AlreadyInitialized); + } + + let max_ttl = env.storage().max_ttl(); + + env.storage() + .instance() + .set(&STORAGE_KEY_WASM_HASH, &wasm_hash); + + env.storage() + .instance() + .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); + + Ok(()) +} +``` + +Nothing controversial here I don’t think. We’re storing the smart wallet’s wasm hash in order to load up the factory with the proper template to deploy in the `deploy` function. This is stored on the instance as it should be and then the instance is extended + +### `deploy` +```rust +pub fn deploy(env: Env, salt: BytesN<32>, id: Bytes, pk: BytesN<65>) -> Result { + let wasm_hash = env + .storage() + .instance() + .get::>(&STORAGE_KEY_WASM_HASH) + .ok_or(Error::NotInitialized)?; + + let address = env + .deployer() + .with_current_contract(salt) + .deploy(wasm_hash); + + let () = env.invoke_contract( + &address, + &symbol_short!("add"), + vec![&env, id.to_val(), pk.to_val(), true.into()], + ); + + let max_ttl = env.storage().max_ttl(); + + env.storage() + .instance() + .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); + + Ok(address) +} +``` + +Few things to note here: + +- Also note we’re calling the `env.invoke_contract` vs pulling in the smart wallet interface. This is a cost savings as we’re only making use of the `add` method. This requires knowing intuitively how to properly construct the invocation but let’s be honest, that’s not hard. +- Last thing is extending the interface again. If you’re gonna use the factory at least pay it forward a little to help keep the factory’s lights on. + +# Contract 2: The Smart Wallet + +The smart wallet interface while obviously more complex than the factory is still aiming to be as simple as possible and only do what’s absolutely necessary to provide a useful smart wallet interface for general purpose usage. + +I’ve intentionally left off as many bells and whistles as possible with the hope of being able to agree and progress with this interface into an audited and approved mainnet interface for general usage. Certainly there will be additional features and functions users and services will want and I hope to see a rich and diverse ecosystem of wallet interfaces arise over time but initially we just need to get something sufficiently useful live providing the basic majority needs of non crypto-native users. + +## Interface + +```rust +// FUNCTIONS + +fn add(id: bytes, pk: bytesn<65>, admin: bool) -> result,error> + +fn remove(id: bytes) -> result,error> + +fn upgrade(hash: bytesn<32>) -> result,error> + +fn __check_auth(signature_payload: bytesn<32>, signature: Signature, auth_contexts: vec) -> result,error> + +// STRUCTS + +#[contracttype] +struct Signature { + authenticator_data: bytes, + client_data_json: bytes, + id: bytes, + signature: bytesn<64> +} + +// ERRORS + +#[contracterror] +enum Error { + NotFound = 1, + NotPermitted = 2, + ClientDataJsonChallengeIncorrect = 3, + Secp256r1PublicKeyParse = 4, + Secp256r1SignatureParse = 5, + Secp256r1VerifyFailed = 6, + JsonParseError = 7, +} + +``` + +## Code + +[https://github.com/kalepail/passkey-kit/blob/main/contracts/contract-webauthn-secp256r1/src/lib.rs](https://github.com/kalepail/passkey-kit/blob/main/contracts/contract-webauthn-secp256r1/src/lib.rs) + +### `add` +```rust +pub fn add(env: Env, id: Bytes, pk: BytesN<65>, mut admin: bool) -> Result<(), Error> { + if env.storage().instance().has(&ADMIN_SIGNER_COUNT) { + env.current_contract_address().require_auth(); + } else { + admin = true; + } + + let max_ttl = env.storage().max_ttl(); + + if admin { + if env.storage().temporary().has(&id) { + env.storage().temporary().remove(&id); + } + + Self::update_admin_signer_count(&env, true); + + env.storage().persistent().set(&id, &pk); + + env.storage() + .persistent() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + } else { + if env.storage().persistent().has(&id) { + Self::update_admin_signer_count(&env, false); + + env.storage().persistent().remove(&id); + } + + env.storage().temporary().set(&id, &pk); + + env.storage() + .temporary() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + } + + env.storage() + .instance() + .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); + + env.events() + .publish((EVENT_TAG, symbol_short!("add"), id), (pk, admin)); + + Ok(()) +} +``` + +Some notable elements: + +- We use the `env.storage().instance().has(&ADMIN_SIGNER_COUNT)` to toggle between a sort of initialization call and the standard `require_auth` flow. + + ```rust + if env.storage().instance().has(&ADMIN_SIGNER_COUNT) { + env.current_contract_address().require_auth(); + } else { + admin = true; + } + ``` + + The only potential downside is `add` includes logic for storing temporary session signers which an initial call doesn't support making that logic verbose. Initially I had a separate `init` function but I think this is a better tradeoff for simplicity and efficiency even if there are some unusable if statements in the case of the initial `add` call made by the factory contract. + +- `Self::update_admin_signer_count(&env, true);` My proposal includes the concept of session and admin signers. Certain functions, well really all of the smart wallet self functions (`add`, `remove`, `upgrade`) are only callable by admin signers. Given this we need to ensure we never remove all the admin signers which necessarily requires we track the number of admin signers. This function provides that service and will be called anytime we add or remove an admin signer. +- Admin signers are persistent entries, non-admin signers are temporary. It’s also possible for signers to be toggled between admin and non however we must only ever be tracking a single `id` to a single `pk` and so we must add logic for removing any existing signers for a given `id` in the counter storage to the type we’re currently adding to. Make special note of the need to decrement the `ADMIN_SIGNER_COUNT` in case of removing an admin signer to temporary if a persistent entry for that `id` exists. + + ```rust + if admin { + if env.storage().temporary().has(&id) { + env.storage().temporary().remove(&id); + } + + Self::update_admin_signer_count(&env, true); + + env.storage().persistent().set(&id, &pk); + + env.storage() + .persistent() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + } else { + if env.storage().persistent().has(&id) { + Self::update_admin_signer_count(&env, false); + + env.storage().persistent().remove(&id); + } + + env.storage().temporary().set(&id, &pk); + + env.storage() + .temporary() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + } + ``` + +### `remove` +```rust +pub fn remove(env: Env, id: Bytes) -> Result<(), Error> { + env.current_contract_address().require_auth(); + + if env.storage().temporary().has(&id) { + env.storage().temporary().remove(&id); + } else { + Self::update_admin_signer_count(&env, false); + + env.storage().persistent().remove(&id); + } + + let max_ttl = env.storage().max_ttl(); + + env.storage() + .instance() + .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); + + env.events() + .publish((EVENT_TAG, symbol_short!("remove"), id), ()); + + Ok(()) +} +``` + +Remove is similar to `add` just in inverse with some slight simplifications. + +- Given the key could be either temporary or persistent we must include logic for checking both and removing if they exist. Again note the need to decrement the `ADMIN_SIGNER_COUNT` in case of a persistent admin `id`. +- Given each `id` can only be either a temporary or persistent entry it's safe to use `else if env.storage().persistent().has(&id)` vs a separate `if ...`. Doing so saves on some read costs if we were to try and just remove both storage type for the same `id` key. Note we do need to use the has check vs just doing an `else` check as a `storage.remove` won't error if the entry doesn't exist which would open us up to the issue of decrementing the admin key count when we didn't actually delete anything. + +### `update` +```rust +pub fn update(env: Env, hash: BytesN<32>) -> Result<(), Error> { + env.current_contract_address().require_auth(); + + env.deployer().update_current_contract_wasm(hash); + + let max_ttl = env.storage().max_ttl(); + + env.storage() + .instance() + .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); + + Ok(()) +} +``` + +An essential function for all smart wallets imo. The ability to change the interface the wallet implements. Perhaps controversial given the risk of upgrading to a bugged or malicious wallet interface but that’s an risk inherent to creating a smart wallet in the first place and given that risk I actually think part of mitigating that risk is allowing users to move their interface to alternatives should they choose to. Client interfaces should be very careful in exposing this method to wallet users but I do think it’s an essential method for the health and safety of the smart wallet ecosystem. + +- Protected such that only admin signers can perform this method. +- Allows for a wallet user to switch or update their interface should newer or different interfaces be released. + +### `__check_auth` +```rust +fn __check_auth( + env: Env, + signature_payload: Hash<32>, + signature: Signature, + auth_contexts: Vec, +) -> Result<(), Error> {...} +``` + +This is the beefy boy and most of it is only interesting to auditors ensuring the actual decoding and cryptography bits work as intended. I’ll detail the parts here which are more specific to the interface itself: + +- We need to select which `pk` to use for the provided `id` purporting to have signed for the incoming payload. + + ```rust + let pk = match env.storage().temporary().get(&id) { + Some(pk) => { + ... + + env.storage() + .temporary() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + + pk + } + None => { + env.storage() + .persistent() + .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); + + env.storage().persistent().get(&id).ok_or(Error::NotFound)? + } + }; + ``` + + We do that first by looking up the temporary entry which will be the far more common case. If we cannot find it there we look for a persistent entry. This will introduce a double look up for a temporary entry but those are cheap so this is fine. Note we also set the `admin` binary toggle for use later in blocking protected self methods. + +- If the pk is a temporary session signer we need to do an additional check to ensure the authentication request isn’t for a protected action + + ```rust + + ... + + for context in auth_contexts.iter() { + match context { + Context::Contract(c) => { + if c.contract == env.current_contract_address() + && ( + c.fn_name != symbol_short!("remove") + || ( + c.fn_name == symbol_short!("remove") + && Bytes::from_val(&env, &c.args.get(0).unwrap()) != id + ) + ) + { + return Err(Error::NotPermitted); + } + } + _ => {} + }; + } + + ... + ``` + + This is a relatively straight forward check. If the request is for the smart wallet contract ensure the only function it *might* be able to call is a `remove` of it’s own `id`. Anything else should result in an error. + + +The rest of `__check_auth` is boilerplate authentication checks of the webauthn data itself and not technically part of this interface. It needs to be audited but that won’t affect the final interface of the wallet. + +# Events + +The only other item worth mentioning are the events emitted during the `add` and `remove` methods. Events are emitted in order to allow an indexer to keep track of a wallet’s available signers and their current state as `admin` or not. + +## Add + +```rust +env.events().publish((EVENT_TAG, symbol_short!("add"), id), (pk, admin)); +``` + +- The `EVENT_TAG` is a trigger to help indexers only listen for relevant smart wallet events and while not fool proof should improve filtering out only those events which are relevant. +- The `pk` is emitted in order to allow downstream clients to queue up expired session signers to be re-added without needing to create new passkeys, you can continue to use the existing ones if you can find the `pk` for a matching `id` from a previously emitted event. + +> [!CAUTION] +> Passkey public keys are only retrievable during a passkey creation flow. They cannot be later retrieved from an authentication flow. Thus passkey public keys are special data which we should be storing inside the blockchain itself. This is normally done during an `add` event but given we’re using temporary storage these keys could be lost and unrecoverable were we not to store them inside events for indexers to keep track of and then for clients to then be able to essentially “rehydrate” at a later date without requiring the user to keep creating new passkeys every time they wanted to sign into a service after their temporary session key had expired. + + +## Remove + +```rust +env.events().publish((EVENT_TAG, symbol_short!("remove"), id), ()); +``` \ No newline at end of file diff --git a/TODO.md b/TODO.md deleted file mode 100644 index 0c8c21a..0000000 --- a/TODO.md +++ /dev/null @@ -1 +0,0 @@ -- [ ] Add timestamps to zephyr event data \ No newline at end of file diff --git a/cheatsheet b/cheatsheet index 7a53dc8..ca4c766 100644 --- a/cheatsheet +++ b/cheatsheet @@ -1,5 +1,5 @@ # Install contract sdks -npm publish --workspaces +npm publish --workspaces # Install passkey-kit pnpm publish --no-git-checks diff --git a/contracts/Makefile b/contracts/Makefile index f7c50f9..3236a60 100644 --- a/contracts/Makefile +++ b/contracts/Makefile @@ -3,8 +3,8 @@ export SOROBAN_RPC_URL=https://soroban-testnet.stellar.org export SOROBAN_NETWORK_PASSPHRASE=Test SDF Network ; September 2015 export SOROBAN_ACCOUNT=default -export WEBAUTHN_FACTORY=CAMZPCWSZYVXRTHSAIDCLVAJ4EIZGGWBQDRQLKSBQW6USC4A2Q3GDWX7 -export WEBAUTHN_WASM=e899690e8c475e0b0c55114ff31185e7331d6fe1ed11d4149b23cb431f1173bb +export WEBAUTHN_FACTORY=CB2L37OEFNW4ZUPGISREQJBEBEQRNHGBMPD2G6UUDXEXEQJXS5JBSBO2 +export WEBAUTHN_WASM=bd9894429318e74c773fa613caed390e130b44d93796b63ecec022ed3419fd3c build: rm -rf out/ diff --git a/contracts/contract-webauthn-factory/src/lib.rs b/contracts/contract-webauthn-factory/src/lib.rs index ec60b75..de83f06 100644 --- a/contracts/contract-webauthn-factory/src/lib.rs +++ b/contracts/contract-webauthn-factory/src/lib.rs @@ -13,8 +13,7 @@ pub enum Error { AlreadyInitialized = 2, } -const DAY_OF_LEDGERS: u32 = 60 * 60 * 24 / 5; -const WEEK_OF_LEDGERS: u32 = DAY_OF_LEDGERS * 7; +const WEEK_OF_LEDGERS: u32 = 60 * 60 * 24 / 5 * 7; const STORAGE_KEY_WASM_HASH: Symbol = symbol_short!("hash"); /* NOTE @@ -50,22 +49,19 @@ impl Contract { Ok(()) } - pub fn deploy(env: Env, id: Bytes, pk: BytesN<65>) -> Result { + pub fn deploy(env: Env, salt: BytesN<32>, id: Bytes, pk: BytesN<65>) -> Result { let wasm_hash = env .storage() .instance() .get::>(&STORAGE_KEY_WASM_HASH) .ok_or(Error::NotInitialized)?; - let address = env - .deployer() - .with_current_contract(env.crypto().sha256(&id)) - .deploy(wasm_hash); + let address = env.deployer().with_current_contract(salt).deploy(wasm_hash); let () = env.invoke_contract( &address, - &symbol_short!("init"), - vec![&env, id.to_val(), pk.to_val()], + &symbol_short!("add"), + vec![&env, id.to_val(), pk.to_val(), true.into()], ); let max_ttl = env.storage().max_ttl(); diff --git a/contracts/contract-webauthn-secp256r1/src/base64_url.rs b/contracts/contract-webauthn-secp256r1/src/base64_url.rs index b162733..7b5515c 100644 --- a/contracts/contract-webauthn-secp256r1/src/base64_url.rs +++ b/contracts/contract-webauthn-secp256r1/src/base64_url.rs @@ -40,7 +40,7 @@ const ALPHABET: &[u8] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz01 pub fn encode(dst: &mut [u8], src: &[u8]) { let mut di: usize = 0; let mut si: usize = 0; - let n = (src.len() / 3) * 3; + let n = (src.len() / 3) * 3; // TODO divide by 3 and multiply by 3? Why? Seems like useless arithmetic while si < n { let val = (src[si] as usize) << 16 | (src[si + 1] as usize) << 8 | (src[si + 2] as usize); diff --git a/contracts/contract-webauthn-secp256r1/src/lib.rs b/contracts/contract-webauthn-secp256r1/src/lib.rs index f9e9c76..53e3d9d 100644 --- a/contracts/contract-webauthn-secp256r1/src/lib.rs +++ b/contracts/contract-webauthn-secp256r1/src/lib.rs @@ -1,7 +1,10 @@ #![no_std] use soroban_sdk::{ - auth::{Context, CustomAccountInterface}, contract, contracterror, contractimpl, contracttype, crypto::Hash, panic_with_error, symbol_short, Bytes, BytesN, Env, Symbol, Vec + auth::{Context, CustomAccountInterface}, + contract, contracterror, contractimpl, contracttype, + crypto::Hash, + panic_with_error, symbol_short, Bytes, BytesN, Env, FromVal, Symbol, Vec, }; mod base64_url; @@ -16,67 +19,26 @@ pub struct Contract; pub enum Error { NotFound = 1, NotPermitted = 2, - AlreadyInitialized = 3, - ClientDataJsonChallengeIncorrect = 4, - Secp256r1PublicKeyParse = 5, - Secp256r1SignatureParse = 6, - Secp256r1VerifyFailed = 7, - JsonParseError = 8, + ClientDataJsonChallengeIncorrect = 3, + Secp256r1PublicKeyParse = 4, + Secp256r1SignatureParse = 5, + Secp256r1VerifyFailed = 6, + JsonParseError = 7, } -const DAY_OF_LEDGERS: u32 = 60 * 60 * 24 / 5; -const WEEK_OF_LEDGERS: u32 = DAY_OF_LEDGERS * 7; +const WEEK_OF_LEDGERS: u32 = 60 * 60 * 24 / 5 * 7; const EVENT_TAG: Symbol = symbol_short!("sw_v1"); const ADMIN_SIGNER_COUNT: Symbol = symbol_short!("admins"); #[contractimpl] impl Contract { - pub fn init(env: Env, id: Bytes, pk: BytesN<65>) -> Result<(), Error> { - /* NOTE - - You can't call `add` without some admin key so there has to be a method to add the first admin key - however once that has been called it must not be able to be called again - currently just storing the EVENT_TAG on the instance to mark that the wallet has been initialized - if some day we get something better we can use that - */ - + pub fn add(env: Env, id: Bytes, pk: BytesN<65>, mut admin: bool) -> Result<(), Error> { if env.storage().instance().has(&ADMIN_SIGNER_COUNT) { - return Err(Error::AlreadyInitialized); + env.current_contract_address().require_auth(); + } else { + admin = true; // Ensure if this is the first signer they are an admin } - Self::update_admin_signer_count(&env, true); - - env.storage().persistent().set(&id, &pk); - - let max_ttl = env.storage().max_ttl(); - - env.storage() - .persistent() - .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); - - env.storage() - .instance() - .extend_ttl(max_ttl - WEEK_OF_LEDGERS, max_ttl); - - // TEMP until Zephyr fixes their event processing system to allow for bytesn arrays in the data field - // env.events().publish( - // (EVENT_TAG, symbol_short!("add"), id, symbol_short!("init")), - // (pk, true), - // ); - - env.events().publish( - (EVENT_TAG, symbol_short!("add"), id, pk), - true, - ); - - Ok(()) - } - pub fn add(env: Env, id: Bytes, pk: BytesN<65>, admin: bool) -> Result<(), Error> { - /* NOTE - - We're not doing any existence checks so it's possible to overwrite a key or "dupe" a key (which could cause issues for indexers if they aren't handling dupe events) - */ - - env.current_contract_address().require_auth(); - let max_ttl = env.storage().max_ttl(); if admin { @@ -122,9 +84,7 @@ impl Contract { if env.storage().temporary().has(&id) { env.storage().temporary().remove(&id); - } - - if env.storage().persistent().has(&id) { + } else if env.storage().persistent().has(&id) { Self::update_admin_signer_count(&env, false); env.storage().persistent().remove(&id); @@ -141,7 +101,7 @@ impl Contract { Ok(()) } - pub fn upgrade(env: Env, hash: BytesN<32>) -> Result<(), Error> { + pub fn update(env: Env, hash: BytesN<32>) -> Result<(), Error> { env.current_contract_address().require_auth(); env.deployer().update_current_contract_wasm(hash); @@ -166,10 +126,9 @@ impl Contract { panic_with_error!(env, Error::NotPermitted) } - env.storage().instance().set::( - &ADMIN_SIGNER_COUNT, - &count, - ); + env.storage() + .instance() + .set::(&ADMIN_SIGNER_COUNT, &count); } } @@ -206,12 +165,29 @@ impl CustomAccountInterface for Contract { signature, } = signature; - let admin; let max_ttl = env.storage().max_ttl(); let pk = match env.storage().temporary().get(&id) { Some(pk) => { - admin = false; + // Error if a session signer is trying to perform protected actions + for context in auth_contexts.iter() { + match context { + Context::Contract(c) => { + if c.contract == env.current_contract_address() // if we're calling self + && ( // and + c.fn_name != symbol_short!("remove") // the method isn't the only potentially available self command + || ( // we're not removing ourself + c.fn_name == symbol_short!("remove") + && Bytes::from_val(&env, &c.args.get(0).unwrap()) != id + ) + ) + { + return Err(Error::NotPermitted); + } + } + _ => {} // Don't block for example the deploying of new contracts from this contract + }; + } env.storage() .temporary() @@ -220,8 +196,6 @@ impl CustomAccountInterface for Contract { pk } None => { - admin = true; - env.storage() .persistent() .extend_ttl(&id, max_ttl - WEEK_OF_LEDGERS, max_ttl); @@ -230,36 +204,15 @@ impl CustomAccountInterface for Contract { } }; - // Only admin signers can `upgrade`, `add` and `remove` - for context in auth_contexts.iter() { - match context { - Context::Contract(c) => { - if c.contract == env.current_contract_address() // calling the smart wallet - && (c.fn_name == symbol_short!("upgrade") // calling a protected function - || c.fn_name == symbol_short!("add") - || c.fn_name == symbol_short!("remove")) - && !admin - // signature key is not an admin key - { - return Err(Error::NotPermitted); - } - } - _ => {} // Don't block for example the deploying of new contracts from this contract - }; - } - - let client_data_json_hash = env.crypto().sha256(&client_data_json).to_array(); - - authenticator_data.extend_from_array(&client_data_json_hash); - - let digest = env.crypto().sha256(&authenticator_data); + authenticator_data.extend_from_array(&env.crypto().sha256(&client_data_json).to_array()); - env.crypto().secp256r1_verify(&pk, &digest, &signature); + env.crypto() + .secp256r1_verify(&pk, &env.crypto().sha256(&authenticator_data), &signature); // Parse the client data JSON, extracting the base64 url encoded challenge. let client_data_json = client_data_json.to_buffer::<1024>(); // <- TODO why 1024? let client_data_json = client_data_json.as_slice(); - let (client_data, _): (ClientDataJson, _) = + let (client_data_json, _): (ClientDataJson, _) = serde_json_core::de::from_slice(client_data_json).map_err(|_| Error::JsonParseError)?; // Build what the base64 url challenge is expecting. @@ -268,7 +221,8 @@ impl CustomAccountInterface for Contract { base64_url::encode(&mut expected_challenge, &signature_payload.to_array()); // Check that the challenge inside the client data JSON that was signed is identical to the expected challenge. - if client_data.challenge.as_bytes() != expected_challenge { + // TODO is this check actually necessary or is the secp256r1_verify enough? I think it's necessary + if client_data_json.challenge.as_bytes() != expected_challenge { return Err(Error::ClientDataJsonChallengeIncorrect); } diff --git a/contracts/contract-webauthn-secp256r1/src/test.rs b/contracts/contract-webauthn-secp256r1/src/test.rs index 416ea49..2b8a7d4 100644 --- a/contracts/contract-webauthn-secp256r1/src/test.rs +++ b/contracts/contract-webauthn-secp256r1/src/test.rs @@ -55,9 +55,10 @@ fn test() { 133, 215, 200, 208, 230, 51, 210, 94, 214, ], ); + // let salt = env.crypto().sha256(&id); // factory_client.init(&passkkey_hash); - deployee_client.init(&id, &pk); + deployee_client.add(&id, &pk, &true); let signature_payload = BytesN::from_array( &env, diff --git a/demo/src/App.svelte b/demo/src/App.svelte index a659314..f60d335 100644 --- a/demo/src/App.svelte +++ b/demo/src/App.svelte @@ -8,6 +8,7 @@ getBalance, getSigners, transferSAC, + type Signer, } from "./lib/account"; import { fundKeypair, fundPubkey } from "./lib/common"; @@ -16,7 +17,7 @@ let admins: number; let adminKeyId: string | undefined; let balance: string; - let signers: { id: string; pk: string; admin: boolean }[] = []; + let signers: Signer[] = []; let keyName: string = ""; let keyAdmin: boolean = false; @@ -40,7 +41,7 @@ if (!user) return; const { - keyId, + keyId: kid, contractId: cid, xdr, } = await account.createWallet("Super Peach", user); @@ -48,7 +49,8 @@ console.log(res); - localStorage.setItem("sp:keyId", base64url(keyId)); + keyId = base64url(kid); + localStorage.setItem("sp:keyId", keyId); contractId = cid; console.log("register", cid); @@ -56,13 +58,14 @@ await getWalletSigners(); await fundWallet(); } - async function connect(keyId?: string) { + async function connect(keyId_?: string) { const { keyId: kid, contractId: cid } = await account.connectWallet({ - keyId, + keyId: keyId_, getContractId, // only strictly needed when the passed keyId will not derive to any or the correct contractId }); - localStorage.setItem("sp:keyId", base64url(kid)); + keyId = base64url(kid); + localStorage.setItem("sp:keyId", keyId); contractId = cid; console.log("connect", cid); @@ -131,8 +134,11 @@ async function getWalletSigners() { signers = await getSigners(contractId); console.log(signers); - adminKeyId = signers.find(({ admin }) => admin)?.id; - admins = signers.filter(({ admin }) => admin).length; + + const adminKeys = signers.filter(({ admin }) => admin); + adminKeyId = (adminKeys.find(({ id }) => keyId === id) || adminKeys[0]) + .id; + admins = adminKeys.length; } async function fundWallet() { @@ -220,17 +226,15 @@ {/if}
    - {#each signers as { id, pk, admin }} + {#each signers as { id, pk, admin, expired }}
  • - {#if admin} - - {:else} - - {/if} + {id} @@ -238,16 +242,16 @@ >Transfer 1 XLM - {#if !admin || admins > 1} + {#if (!admin || admins > 1) && id !== keyId} {/if} - {#if account.keyExpired && id === account.keyId} - - {:else if admin && id !== adminKeyId} + {#if admin && id !== adminKeyId} + {:else if expired && id === account.keyId} + {/if}
    • {/each} diff --git a/demo/src/lib/account.ts b/demo/src/lib/account.ts index a722d6d..638a32e 100644 --- a/demo/src/lib/account.ts +++ b/demo/src/lib/account.ts @@ -2,8 +2,15 @@ import { xdr, Operation, SorobanRpc, TransactionBuilder, nativeToScVal, scValToN import { mockSource, rpc } from './common' import base64url from 'base64url' +export type Signer = { + id: string; + pk: string; + admin: boolean; + expired: boolean; +} + export async function getSigners(contractId: string) { - const res = await fetch(`${import.meta.env.VITE_mercuryUrl}/zephyr/execute`, { + const signers: Signer[] = await fetch(`${import.meta.env.VITE_mercuryUrl}/zephyr/execute`, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -27,12 +34,20 @@ export async function getSigners(contractId: string) { throw await res.json() }) - return res - .map(({ id, pk, admin }: { id: number[], pk: number[], admin: boolean }) => ({ - id: base64url(id), - pk: base64url(pk), - admin - })) + for (const signer of signers) { + if (!signer.admin) { + try { + await rpc.getContractData(contractId, xdr.ScVal.scvBytes(signer.id), SorobanRpc.Durability.Temporary) + } catch { + signer.expired = true + } + } + + signer.id = base64url(signer.id) + signer.pk = base64url(signer.pk) + } + + return signers } export async function getContractId(signer: string) { diff --git a/packages/passkey-factory-sdk/src/index.ts b/packages/passkey-factory-sdk/src/index.ts index a51ee7d..91ebfb0 100644 --- a/packages/passkey-factory-sdk/src/index.ts +++ b/packages/passkey-factory-sdk/src/index.ts @@ -15,7 +15,7 @@ if (typeof window !== 'undefined') { export const networks = { testnet: { networkPassphrase: "Test SDF Network ; September 2015", - contractId: "CBCPNJNIR7I6ZI5AIXTMYI3MCDCSTNUZ57XL75HQWI6Y4ESWUP24HRBG", + contractId: "CCNG4UOYRSULIMD2AVF5LWAQRM3XY4L3HQSARBLQ5A7HQBRLDVFUTZ7X", } } as const @@ -48,7 +48,7 @@ export interface Client { /** * Construct and simulate a deploy transaction. Returns an `AssembledTransaction` object which will have a `result` field containing the result of the simulation. If this transaction changes contract state, you will need to call `signAndSend()` on the returned object. */ - deploy: ({ id, pk }: { id: Buffer, pk: Buffer }, options?: { + deploy: ({ salt, id, pk }: { salt: Buffer, id: Buffer, pk: Buffer }, options?: { /** * The fee to pay for the transaction. Default: BASE_FEE */ @@ -70,7 +70,7 @@ export class Client extends ContractClient { super( new ContractSpec(["AAAABAAAAAAAAAAAAAAABUVycm9yAAAAAAAAAgAAAAAAAAAOTm90SW5pdGlhbGl6ZWQAAAAAAAEAAAAAAAAAEkFscmVhZHlJbml0aWFsaXplZAAAAAAAAg==", "AAAAAAAAAAAAAAAEaW5pdAAAAAEAAAAAAAAACXdhc21faGFzaAAAAAAAA+4AAAAgAAAAAQAAA+kAAAPtAAAAAAAAAAM=", - "AAAAAAAAAAAAAAAGZGVwbG95AAAAAAACAAAAAAAAAAJpZAAAAAAADgAAAAAAAAACcGsAAAAAA+4AAABBAAAAAQAAA+kAAAATAAAAAw=="]), + "AAAAAAAAAAAAAAAGZGVwbG95AAAAAAADAAAAAAAAAARzYWx0AAAD7gAAACAAAAAAAAAAAmlkAAAAAAAOAAAAAAAAAAJwawAAAAAD7gAAAEEAAAABAAAD6QAAABMAAAAD"]), options ) } diff --git a/packages/passkey-kit-sdk/src/index.ts b/packages/passkey-kit-sdk/src/index.ts index 98f8223..c64c16a 100644 --- a/packages/passkey-kit-sdk/src/index.ts +++ b/packages/passkey-kit-sdk/src/index.ts @@ -15,7 +15,7 @@ if (typeof window !== 'undefined') { export const networks = { testnet: { networkPassphrase: "Test SDF Network ; September 2015", - contractId: "CBCPNJNIR7I6ZI5AIXTMYI3MCDCSTNUZ57XL75HQWI6Y4ESWUP24HRBG", + contractId: "CCNG4UOYRSULIMD2AVF5LWAQRM3XY4L3HQSARBLQ5A7HQBRLDVFUTZ7X", } } as const @@ -38,26 +38,6 @@ export interface Signature { } export interface Client { - /** - * Construct and simulate a init transaction. Returns an `AssembledTransaction` object which will have a `result` field containing the result of the simulation. If this transaction changes contract state, you will need to call `signAndSend()` on the returned object. - */ - init: ({ id, pk }: { id: Buffer, pk: Buffer }, options?: { - /** - * The fee to pay for the transaction. Default: BASE_FEE - */ - fee?: number; - - /** - * The maximum amount of time to wait for the transaction to complete. Default: DEFAULT_TIMEOUT - */ - timeoutInSeconds?: number; - - /** - * Whether to automatically simulate the transaction when constructing the AssembledTransaction. Default: true - */ - simulate?: boolean; - }) => Promise>> - /** * Construct and simulate a add transaction. Returns an `AssembledTransaction` object which will have a `result` field containing the result of the simulation. If this transaction changes contract state, you will need to call `signAndSend()` on the returned object. */ @@ -122,7 +102,6 @@ export class Client extends ContractClient { constructor(public readonly options: ContractClientOptions) { super( new ContractSpec(["AAAABAAAAAAAAAAAAAAABUVycm9yAAAAAAAACAAAAAAAAAAITm90Rm91bmQAAAABAAAAAAAAAAxOb3RQZXJtaXR0ZWQAAAACAAAAAAAAABJBbHJlYWR5SW5pdGlhbGl6ZWQAAAAAAAMAAAAAAAAAIENsaWVudERhdGFKc29uQ2hhbGxlbmdlSW5jb3JyZWN0AAAABAAAAAAAAAAXU2VjcDI1NnIxUHVibGljS2V5UGFyc2UAAAAABQAAAAAAAAAXU2VjcDI1NnIxU2lnbmF0dXJlUGFyc2UAAAAABgAAAAAAAAAVU2VjcDI1NnIxVmVyaWZ5RmFpbGVkAAAAAAAABwAAAAAAAAAOSnNvblBhcnNlRXJyb3IAAAAAAAg=", - "AAAAAAAAAAAAAAAEaW5pdAAAAAIAAAAAAAAAAmlkAAAAAAAOAAAAAAAAAAJwawAAAAAD7gAAAEEAAAABAAAD6QAAA+0AAAAAAAAAAw==", "AAAAAAAAAAAAAAADYWRkAAAAAAMAAAAAAAAAAmlkAAAAAAAOAAAAAAAAAAJwawAAAAAD7gAAAEEAAAAAAAAABWFkbWluAAAAAAAAAQAAAAEAAAPpAAAD7QAAAAAAAAAD", "AAAAAAAAAAAAAAAGcmVtb3ZlAAAAAAABAAAAAAAAAAJpZAAAAAAADgAAAAEAAAPpAAAD7QAAAAAAAAAD", "AAAAAAAAAAAAAAAHdXBncmFkZQAAAAABAAAAAAAAAARoYXNoAAAD7gAAACAAAAABAAAD6QAAA+0AAAAAAAAAAw==", @@ -132,7 +111,6 @@ export class Client extends ContractClient { ) } public readonly fromJSON = { - init: this.txFromJSON>, add: this.txFromJSON>, remove: this.txFromJSON>, upgrade: this.txFromJSON> diff --git a/src/kit.ts b/src/kit.ts index b10476e..5af96a9 100644 --- a/src/kit.ts +++ b/src/kit.ts @@ -12,7 +12,6 @@ type GetContractIdFunction = (keyId: string) => Promise; export class PasskeyKit extends PasskeyBase { public keyId: string | undefined - public keyExpired: boolean | undefined public wallet: PasskeyClient | undefined public factory: FactoryClient public networkPassphrase: Networks @@ -53,8 +52,9 @@ export class PasskeyKit extends PasskeyBase { const { keyId, publicKey } = await this.createKey(app, user) const { result, built } = await this.factory.deploy({ + salt: hash(keyId), id: keyId, - pk: publicKey! + pk: publicKey }) const contractId = result.unwrap() as string @@ -81,7 +81,7 @@ export class PasskeyKit extends PasskeyBase { // id: undefined, name: app, }, - user: { // TODO there's a real danger here of overwriting a user's key if they use the same `user` name + user: { id: base64url(`${user}:${now.getTime()}:${Math.random()}`), name: displayName, displayName @@ -98,11 +98,9 @@ export class PasskeyKit extends PasskeyBase { if (!this.keyId) this.keyId = id; - const publicKey = this.getPublicKey(response); - return { keyId: base64url.toBuffer(id), - publicKey + publicKey: this.getPublicKey(response) } } @@ -138,7 +136,7 @@ export class PasskeyKit extends PasskeyBase { // Check for the contractId on-chain as a derivation from the keyId. This is the easiest and "cheapest" check however it will only work for the initially deployed passkey if it was used as derivation let contractId: string | undefined = StrKey.encodeContract(hash(xdr.HashIdPreimage.envelopeTypeContractId( new xdr.HashIdPreimageContractId({ - networkId: hash(Buffer.from(this.networkPassphrase, 'utf-8')), + networkId: hash(Buffer.from(this.networkPassphrase)), contractIdPreimage: xdr.ContractIdPreimage.contractIdPreimageFromAddress( new xdr.ContractIdPreimageFromAddress({ address: Address.fromString(this.factory.options.contractId).toScAddress(), @@ -150,23 +148,15 @@ export class PasskeyKit extends PasskeyBase { // attempt passkey id derivation try { + // TODO what is the error if the entry exists but is archived? await this.rpc.getContractData(contractId, xdr.ScVal.scvLedgerKeyContractInstance()) } // if that fails look up from the factory mapper catch { contractId = undefined - if (getContractId) { + if (getContractId) contractId = await getContractId(keyId) - - // Handle case where temporary session signer is missing on-chain (so we can queue up a re-add) - try { - await this.rpc.getContractData(contractId, xdr.ScVal.scvBytes(keyIdBuffer), SorobanRpc.Durability.Temporary) - // throw true - } catch { - this.keyExpired = true - } - } } if (!contractId) @@ -255,6 +245,7 @@ export class PasskeyKit extends PasskeyBase { return entry } + public async signAuthEntries( entries: xdr.SorobanAuthorizationEntry[], options?: { @@ -275,6 +266,7 @@ export class PasskeyKit extends PasskeyBase { return entries } + public async sign( txn: Transaction | string, options?: { @@ -355,6 +347,7 @@ export class PasskeyKit extends PasskeyBase { return publicKey } + private compactSignature(signature: Buffer) { // Decode the DER signature let offset = 2;