From 6f9ca77c7cc8c244509c8f3982ffd87cc61ae1e6 Mon Sep 17 00:00:00 2001
From: katelyn martin <katelyn@penumbralabs.xyz>
Date: Tue, 9 Apr 2024 11:49:53 -0400
Subject: [PATCH] =?UTF-8?q?tests(app):=20=F0=9F=99=85=20app=20rejects=20in?=
 =?UTF-8?q?valid=20auth=20signatures?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fixes #4050. see also #3588.

this introduces a test case that demonstrates that transactions'
validator definition actions must have a valid authentication signature,
or the validator will not be added to the set of known validators.
---
 ...ator_definitions_with_invalid_auth_sigs.rs | 142 ++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs

diff --git a/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs b/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs
new file mode 100644
index 0000000000..679a4048cb
--- /dev/null
+++ b/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs
@@ -0,0 +1,142 @@
+use {
+    self::common::BuilderExt,
+    cnidarium::TempStorage,
+    decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey},
+    penumbra_app::{genesis::AppState, server::consensus::Consensus},
+    penumbra_keys::test_keys,
+    penumbra_mock_client::MockClient,
+    penumbra_mock_consensus::TestNode,
+    penumbra_proto::DomainType,
+    penumbra_stake::{
+        component::validator_handler::ValidatorDataRead as _, validator::Validator, FundingStreams,
+        GovernanceKey, IdentityKey,
+    },
+    rand_core::OsRng,
+    tap::Tap,
+    tracing::{error_span, info, Instrument},
+};
+
+mod common;
+
+/// Show that the application rejects validator definitions with an invalid auth signature.
+#[tokio::test]
+async fn app_rejects_validator_definitions_with_invalid_auth_sigs() -> anyhow::Result<()> {
+    // Install a test logger, and acquire some temporary storage.
+    let guard = common::set_tracing_subscriber();
+    let storage = TempStorage::new().await?;
+
+    // Start the test node.
+    let mut node = {
+        let consensus = Consensus::new(storage.as_ref().clone());
+        let app_state = AppState::default();
+        TestNode::builder()
+            .single_validator()
+            .with_penumbra_auto_app_state(app_state)?
+            .init_chain(consensus)
+            .await
+    }?;
+
+    // Assert that there is only a single validator definition present, before we go any further.
+    assert_eq!(
+        storage
+            .latest_snapshot()
+            .validator_definitions()
+            .await?
+            .len(),
+        1,
+        "invalid validator definition transactions should not be accepted"
+    );
+
+    // Sync the mock client, using the test wallet's spend key, to the latest snapshot.
+    let client = MockClient::new(test_keys::SPEND_KEY.clone())
+        .with_sync_to_storage(&storage)
+        .await?
+        .tap(|c| info!(client.notes = %c.notes.len(), "mock client synced to test storage"));
+
+    // To define a validator, we need to define two keypairs: an identity key
+    // for the Penumbra application and a consensus key for cometbft.
+    let new_validator_id_sk = SigningKey::<SpendAuth>::new(OsRng);
+    let new_validator_id = IdentityKey(VerificationKey::from(&new_validator_id_sk).into());
+    let new_validator_consensus_sk = ed25519_consensus::SigningKey::new(OsRng);
+    let new_validator_consensus = new_validator_consensus_sk.verification_key();
+
+    // Create a different signing key, which we will use to create a forged authentication
+    // signature in our validator definition transaction.
+    let different_signing_key = SigningKey::<SpendAuth>::new(OsRng);
+
+    // Insert the validator's consensus keypair into the keyring so it can be used to sign blocks.
+    node.keyring_mut()
+        // Keyring should just be a BTreeMap rather than creating a new API
+        .insert(new_validator_consensus, new_validator_consensus_sk);
+
+    // Now define the validator's configuration data.
+    let new_validator = Validator {
+        identity_key: new_validator_id.clone(),
+        // TODO: when https://github.com/informalsystems/tendermint-rs/pull/1401 is released,
+        // replace this with a direct `Into::into()` call. at the time of writing, v0.35.0 is the
+        // latest version. check for new releases at https://crates.io/crates/tendermint/versions.
+        consensus_key: tendermint::PublicKey::from_raw_ed25519(&new_validator_consensus.to_bytes())
+            .expect("consensus key is valid"),
+        governance_key: GovernanceKey(new_validator_id_sk.into()),
+        enabled: true,
+        sequence_number: 0,
+        name: "test validator".to_string(),
+        website: String::default(),
+        description: String::default(),
+        funding_streams: FundingStreams::default(),
+    };
+
+    // Make a transaction that defines a new validator, providing an invalid signature.
+    let plan = {
+        use {
+            penumbra_stake::validator,
+            penumbra_transaction::{ActionPlan, TransactionParameters, TransactionPlan},
+            rand_core::OsRng,
+        };
+        let bytes = new_validator.encode_to_vec();
+        // NB: we do NOT use the validator's signing key here. this transaction will contain an
+        // invalid authentication signature.
+        let auth_sig = different_signing_key.sign(OsRng, &bytes);
+        let action = ActionPlan::ValidatorDefinition(validator::Definition {
+            validator: new_validator.clone(),
+            auth_sig,
+        });
+        let mut plan = TransactionPlan {
+            actions: vec![action.into()],
+            // Now fill out the remaining parts of the transaction needed for verification:
+            memo: None,
+            detection_data: None, // We'll set this automatically below
+            transaction_parameters: TransactionParameters {
+                chain_id: TestNode::<()>::CHAIN_ID.to_string(),
+                ..Default::default()
+            },
+        };
+        plan.populate_detection_data(rand_core::OsRng, 0);
+        plan
+    };
+    let tx = client.witness_auth_build(&plan).await?;
+
+    // Execute the transaction, applying it to the chain state.
+    node.block()
+        .add_tx(tx.encode_to_vec())
+        .execute()
+        .instrument(error_span!(
+            "executing block with validator definition transaction"
+        ))
+        .await?;
+    let post_tx_snapshot = storage.latest_snapshot();
+
+    // Assert that there is still only a single validator definition present, and that this
+    // invalid transaction did not cause a new definition to appear.
+    assert_eq!(
+        post_tx_snapshot.validator_definitions().await?.len(),
+        1,
+        "invalid validator definition transactions should not be accepted"
+    );
+
+    // The test passed. Free our temporary storage and drop our tracing subscriber.
+    Ok(())
+        .tap(|_| drop(node))
+        .tap(|_| drop(storage))
+        .tap(|_| drop(guard))
+}