diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ecdec5da4..a91c6b9130 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ Added a new `INPUT_NOTE_INDEX_LOOKUP_EVENT` that lets transaction hosts provide - Added `NonFungibleFaucet::asset_status` API and `AssetStatus` enum (`NotIssued` / `Issued` / `Burned`) for querying a commitment's issuance status from account storage, mirroring the on-chain `get_asset_status` procedure ([#3222](https://github.com/0xMiden/protocol/pull/3222)). - Cleaned up `signature.masm` by removing redundant scheme-id validation and duplication, dropping the `neq.0` double-negation in `assert_supported_scheme_word`, and eliminating the unused `NUM_OF_APPROVERS_LOC` slot; also optimized `verify_signatures` to reuse the signer index and approver public key from the operand stack instead of round-tripping them through local memory ([#3230](https://github.com/0xMiden/protocol/pull/3230)). - Fixed the transaction executor host honoring `AuthRequest` events emitted outside the registered auth procedure, which let untrusted note or transaction scripts force the host to sign; signature production is now restricted to the authentication procedure ([#3233](https://github.com/0xMiden/protocol/pull/3233)). +- Fixed the `AuthRequest` gate introduced in [#3233](https://github.com/0xMiden/protocol/pull/3233) to restrict only signature production ([#3471](https://github.com/0xMiden/protocol/pull/3471)). - Changed the default `LocalTransactionProver` hash function from `BLAKE3` to `Poseidon2`, added ECDSA variants for every signature-authenticated transaction benchmark, and restructured the time counting benchmark IDs to encode the signing scheme and proving hash function (e.g. `poseidon2/falcon/single-p2id-note`) ([#3152](https://github.com/0xMiden/protocol/pull/3152)). - `ConstantFeePolicy` now aborts fee estimation for note scripts without a fee schedule entry instead of estimating them to a fee of 0; to make a note script free, schedule an explicit 0 fee for it. Fee schedule entries are stored as `[fee_amount, 0, 0, 1]`, where the last element is a set-marker distinguishing scheduled entries from unset keys ([#3326](https://github.com/0xMiden/protocol/issues/3326)). - [BREAKING] Added a fee asset ID slot to the `FeeManager` (set via the required `FeeManagerBuilder::fee_faucet_id`, read via the FPI-callable `get_fee_asset_id`); the manager asserts the fee asset returned by the active fee policy matches it, and `collect_sponsored_fees` / `create_network_note_sponsorships` now take the expected fee asset ID as a stack input ([#3347](https://github.com/0xMiden/protocol/pull/3347)). diff --git a/crates/miden-testing/src/kernel_tests/tx/test_auth.rs b/crates/miden-testing/src/kernel_tests/tx/test_auth.rs index 84619ed244..c0a3aeee82 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_auth.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_auth.rs @@ -1,7 +1,8 @@ use anyhow::Context; use assert_matches::assert_matches; use miden_protocol::account::auth::{AuthScheme, AuthSecretKey}; -use miden_protocol::account::{Account, AccountBuilder}; +use miden_protocol::account::component::AccountComponentMetadata; +use miden_protocol::account::{Account, AccountBuilder, AccountComponent}; use miden_protocol::errors::MasmError; use miden_protocol::errors::tx_kernel::ERR_EPILOGUE_AUTH_PROCEDURE_CALLED_FROM_WRONG_CONTEXT; use miden_protocol::testing::account_id::ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE; @@ -103,28 +104,95 @@ async fn test_auth_procedure_called_from_wrong_context() -> anyhow::Result<()> { Ok(()) } -/// Regression test: an untrusted transaction script must not be able to force the host to produce a -/// signature. +/// Regression test: signature production must not be forced from outside the authentication +/// procedure. /// -/// The script emits `AUTH_REQUEST` directly, supplying a precomputed message on the stack and a -/// matching signature in the advice map. This deliberately bypasses `auth::create_tx_summary` -/// (which computes `account::compute_delta_commitment` and is now gated to the account context, so -/// it cannot be called from a script) and exercises the host's context check in isolation: the -/// request must be rejected with `AuthRequestOutsideAuthProcedure` because it originates outside -/// the authentication procedure. The check runs before the signature is validated, so a throwaway -/// key is sufficient - the test is intentionally artificial and only asserts that the original -/// error path is still reachable. +/// The account exposes an `emit_auth_request` procedure that builds a real transaction summary and +/// emits `AUTH_REQUEST` for it, exactly like the standard auth procedure - but it runs as a normal +/// account procedure invoked from the transaction script, i.e. outside the epilogue authentication +/// phase. No signature is pre-supplied, so the event drives production, which must be rejected with +/// `AuthRequestOutsideAuthProcedure`. #[tokio::test] -async fn test_auth_request_from_script_is_rejected() -> anyhow::Result<()> { +async fn test_auth_request_production_outside_auth_procedure_is_rejected() -> anyhow::Result<()> { + let probe_code = CodeBuilder::default().compile_component_code( + "mock::auth_request_probe", + " + use miden::standards::auth + use {AUTH_REQUEST_EVENT} from miden::protocol::auth + + #! Inputs: [PK_COMM, scheme_id] + @account_procedure + pub proc emit_auth_request + # Prepend seven zero user params so the summary layout matches the auth procedure's. + push.0.0.0.0.0.0.0 + exec.auth::create_tx_summary + exec.auth::hash_and_insert_tx_summary + # => [MESSAGE, PK_COMM, scheme_id] + + # With no pre-supplied signature the host must produce one, which is only allowed inside + # the auth procedure; here it is not, so the transaction aborts. + emit.AUTH_REQUEST_EVENT + + dropw dropw drop + end + ", + )?; + let probe_component = AccountComponent::new( + probe_code, + vec![], + AccountComponentMetadata::new("mock::auth_request_probe"), + )?; + let mut builder = MockChain::builder(); - let account = builder.add_existing_mock_account(Auth::BasicAuth { - auth_scheme: AuthScheme::Falcon512Poseidon2, - })?; + let account = + builder.add_existing_account_from_components(Auth::IncrNonce, [probe_component.clone()])?; + let chain = builder.build()?; + + // A dummy public key commitment; the request is rejected before any signature is verified. + let pub_key_commitment = Word::from([1u32, 2, 3, 4]); + let tx_script_source = format!( + " + @transaction_script + pub proc main + push.2 + push.{pub_key_commitment} + # => [PK_COMM, scheme_id] + + call.::mock::auth_request_probe::emit_auth_request + end + " + ); + + let tx_script = CodeBuilder::new() + .with_dynamically_linked_package(probe_component.component_code())? + .compile_tx_script(&tx_script_source)?; + + let execution_result = chain + .build_transaction(account.id()) + .tx_script(tx_script) + .build()? + .execute() + .await; + + assert_matches!( + execution_result, + Err(TransactionExecutorError::AuthRequestOutsideAuthProcedure) + ); + + Ok(()) +} + +/// Complements [`test_auth_request_production_outside_auth_procedure_is_rejected`]: verifying an +/// externally supplied signature is always allowed, even outside the authentication procedure. +#[tokio::test] +async fn test_auth_request_verification_outside_auth_procedure_is_allowed() -> anyhow::Result<()> { + let mut builder = MockChain::builder(); + let account = builder.add_existing_mock_account(Auth::IncrNonce)?; let chain = builder.build()?; - // Precompute the AUTH_REQUEST inputs instead of building the summary on-chain. A throwaway key - // signs an arbitrary message; the resulting signature is placed in the advice map keyed by - // `merge(pub_key_commitment, message)`, which is exactly where the host looks it up. + // A throwaway key signs an arbitrary message; the signature is placed in the advice map keyed + // by `merge(pub_key_commitment, message)`, which is exactly where the host looks it up, so the + // event resolves to the verification path rather than production. let message = Word::from([1u32, 2, 3, 4]); let secret_key = AuthSecretKey::new_falcon512_poseidon2(); let pub_key_commitment = secret_key.public_key().to_commitment(); @@ -147,7 +215,7 @@ async fn test_auth_request_from_script_is_rejected() -> anyhow::Result<()> { emit.AUTH_REQUEST_EVENT - # unreachable once the request is rejected; keeps the script well-formed + # drop the request inputs; the pushed signature stays on the advice stack, unused dropw dropw drop end " @@ -155,18 +223,16 @@ async fn test_auth_request_from_script_is_rejected() -> anyhow::Result<()> { let tx_script = CodeBuilder::new().compile_tx_script(&tx_script_source)?; - let execution_result = chain + // The request must be honored (no `AuthRequestOutsideAuthProcedure`), so the transaction runs + // to completion under the trivial `IncrNonce` auth. + chain .build_transaction(account.id()) .tx_script(tx_script) .add_signature(pub_key_commitment, message, signature) .build()? .execute() - .await; - - assert_matches!( - execution_result, - Err(TransactionExecutorError::AuthRequestOutsideAuthProcedure) - ); + .await + .context("verifying an externally-supplied signature outside the auth procedure should be allowed")?; Ok(()) } diff --git a/crates/miden-tx/src/executor/exec_host.rs b/crates/miden-tx/src/executor/exec_host.rs index e3746d54b2..e999e4103a 100644 --- a/crates/miden-tx/src/executor/exec_host.rs +++ b/crates/miden-tx/src/executor/exec_host.rs @@ -101,9 +101,11 @@ where /// /// The epilogue wraps the auth procedure between the `EpilogueAuthProcStart` and /// `EpilogueAuthProcEnd` events, so this flag is `true` only while the registered auth - /// procedure is running. It is used to reject `AuthRequest` events emitted from any other - /// context (e.g. untrusted note or transaction scripts), which must never trigger signature - /// production. + /// procedure is running. It is used to reject signature *production* requested from any other + /// context (e.g. untrusted note or transaction scripts): an `AuthRequest` with no pre-supplied + /// signature makes the authenticator sign with the account's key and must never be honored + /// outside the auth procedure. Verifying an externally-supplied signature does not touch the + /// private key and is always allowed. in_auth_procedure: bool, /// The source manager to track source code file span information, improving any MASM related @@ -600,22 +602,17 @@ where TransactionEvent::AuthRequest { pub_key_commitment, tx_summary_or_signature, - } => { - // Signature production is only permitted while the registered auth procedure - // is executing. An `AuthRequest` emitted from any other context (e.g. an - // untrusted note or transaction script) must not force the host to sign. - if !self.in_auth_procedure { - Err(TransactionKernelError::AuthRequestOutsideAuthProcedure) - } else { - match tx_summary_or_signature { - TxSummaryOrSignature::Signature(signature) => { - Ok(self.base_host.on_auth_requested(signature)) - }, - TxSummaryOrSignature::TxSummary(tx_summary) => { - self.on_auth_requested(pub_key_commitment, tx_summary).await - }, + } => match tx_summary_or_signature { + TxSummaryOrSignature::Signature(signature) => { + Ok(self.base_host.on_auth_requested(signature)) + }, + TxSummaryOrSignature::TxSummary(tx_summary) => { + if !self.in_auth_procedure { + Err(TransactionKernelError::AuthRequestOutsideAuthProcedure) + } else { + self.on_auth_requested(pub_key_commitment, tx_summary).await } - } + }, }, // This always returns an error to abort the transaction.