Reject durable nonce transactions not signed by authority (backport #…

…25831) (#25849)

* Reject durable nonce transactions not signed by authority (#25831)

(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/bank.rs
#	runtime/src/nonce_keyed_account.rs
#	sdk/program/src/message/sanitized.rs
#	sdk/src/feature_set.rs
#	send-transaction-service/src/send_transaction_service.rs

* resolve conflicts

Co-authored-by: Justin Starry <justin@solana.com>

runtime/src/accounts.rs

@@ -3452,7 +3452,8 @@ mod tests {
         assert!(nonce_account::verify_nonce_account(
             &collected_nonce_account,
             durable_nonce.as_hash(),
-        ));
+        )
+        .is_some());
     }
 
     #[test]
@@ -3558,7 +3559,8 @@ mod tests {
         assert!(nonce_account::verify_nonce_account(
             &collected_nonce_account,
             durable_nonce.as_hash(),
-        ));
+        )
+        .is_some());
     }
 
     #[test]

runtime/src/bank.rs

@@ -124,7 +124,7 @@ use {
         },
         native_loader,
         native_token::sol_to_lamports,
-        nonce::{self, state::DurableNonce},
+        nonce::{self, state::DurableNonce, NONCED_TX_MARKER_IX_INDEX},
         nonce_account,
         packet::PACKET_DATA_SIZE,
         precompiles::get_precompiles,
@@ -3748,15 +3748,25 @@ impl Bank {
         &self,
         message: &SanitizedMessage,
     ) -> Option<(Pubkey, AccountSharedData)> {
-        message
-            .get_durable_nonce(self.feature_set.is_active(&nonce_must_be_writable::id()))
-            .and_then(|nonce_address| {
-                self.get_account_with_fixed_root(nonce_address)
-                    .map(|nonce_account| (*nonce_address, nonce_account))
-            })
-            .filter(|(_, nonce_account)| {
-                nonce_account::verify_nonce_account(nonce_account, message.recent_blockhash())
-            })
+        let nonce_address =
+            message.get_durable_nonce(self.feature_set.is_active(&nonce_must_be_writable::id()))?;
+        let nonce_account = self.get_account_with_fixed_root(nonce_address)?;
+        let nonce_data =
+            nonce_account::verify_nonce_account(&nonce_account, message.recent_blockhash())?;
+
+        if self
+            .feature_set
+            .is_active(&feature_set::nonce_must_be_authorized::ID)
+        {
+            let nonce_is_authorized = message
+                .get_ix_signers(NONCED_TX_MARKER_IX_INDEX as usize)
+                .any(|signer| signer == &nonce_data.authority);
+            if !nonce_is_authorized {
+                return None;
+            }
+        }
+
+        Some((*nonce_address, nonce_account))
     }
 
     fn check_transaction_for_nonce(
@@ -12162,22 +12172,16 @@ pub(crate) mod tests {
         let initial_custodian_balance = custodian_account.lamports();
         assert_eq!(
             bank.process_transaction(&nonce_tx),
-            Err(TransactionError::InstructionError(
-                0,
-                InstructionError::MissingRequiredSignature,
-            ))
+            Err(TransactionError::BlockhashNotFound),
         );
-        /* Check fee charged and nonce has *not* advanced */
+        /* Check fee was *not* charged and nonce has *not* advanced */
         let mut recent_message = nonce_tx.message;
         recent_message.recent_blockhash = bank.last_blockhash();
         assert_eq!(
             bank.get_balance(&custodian_pubkey),
             initial_custodian_balance
-                - bank
-                    .get_fee_for_message(&recent_message.try_into().unwrap())
-                    .unwrap()
         );
-        assert_ne!(
+        assert_eq!(
             nonce_hash,
             get_nonce_blockhash(&bank, &nonce_pubkey).unwrap()
         );

runtime/src/nonce_keyed_account.rs

@@ -1098,17 +1098,17 @@ mod test {
             assert!(verify_nonce_account(
                 &nonce_account.account.borrow(),
                 get_durable_nonce(&invoke_context).as_hash(),
-            ));
+            )
+            .is_some());
         });
     }
 
     #[test]
     fn verify_nonce_bad_acc_state_fail() {
         with_test_keyed_account(42, true, |nonce_account| {
-            assert!(!verify_nonce_account(
-                &nonce_account.account.borrow(),
-                &Hash::default()
-            ));
+            assert!(
+                verify_nonce_account(&nonce_account.account.borrow(), &Hash::default()).is_none()
+            );
         });
     }
 
@@ -1126,10 +1126,11 @@ mod test {
                 .initialize_nonce_account(authorized, &Rent::free(), &invoke_context)
                 .unwrap();
             let invoke_context = create_invoke_context_with_blockhash(1);
-            assert!(!verify_nonce_account(
+            assert!(verify_nonce_account(
                 &nonce_account.account.borrow(),
                 get_durable_nonce(&invoke_context).as_hash(),
-            ));
+            )
+            .is_none());
         });
     }
 }

sdk/program/src/message/sanitized.rs

@@ -261,6 +261,21 @@ impl SanitizedMessage {
         }
     }
 
+    /// Get a list of signers for the instruction at the given index
+    pub fn get_ix_signers(&self, ix_index: usize) -> impl Iterator<Item = &Pubkey> {
+        self.instructions()
+            .get(ix_index)
+            .into_iter()
+            .flat_map(|ix| {
+                ix.accounts
+                    .iter()
+                    .copied()
+                    .map(usize::from)
+                    .filter(|index| self.is_signer(*index))
+                    .filter_map(|signer_index| self.get_account_key(signer_index))
+            })
+    }
+
     /// If the message uses a durable nonce, return the pubkey of the nonce account
     pub fn get_durable_nonce(&self, nonce_must_be_writable: bool) -> Option<&Pubkey> {
         self.instructions()
@@ -298,6 +313,7 @@ mod tests {
             instruction::{AccountMeta, Instruction},
             message::v0,
         },
+        std::collections::HashSet,
     };
 
     #[test]
@@ -461,4 +477,44 @@ mod tests {
                 .is_none());
         }
     }
+
+    #[test]
+    fn test_get_ix_signers() {
+        let signer0 = Pubkey::new_unique();
+        let signer1 = Pubkey::new_unique();
+        let non_signer = Pubkey::new_unique();
+        let loader_key = Pubkey::new_unique();
+        let instructions = vec![
+            CompiledInstruction::new(3, &(), vec![2, 0]),
+            CompiledInstruction::new(3, &(), vec![0, 1]),
+            CompiledInstruction::new(3, &(), vec![0, 0]),
+        ];
+
+        let message = SanitizedMessage::try_from(LegacyMessage::new_with_compiled_instructions(
+            2,
+            1,
+            2,
+            vec![signer0, signer1, non_signer, loader_key],
+            Hash::default(),
+            instructions,
+        ))
+        .unwrap();
+
+        assert_eq!(
+            message.get_ix_signers(0).collect::<HashSet<_>>(),
+            HashSet::from_iter([&signer0])
+        );
+        assert_eq!(
+            message.get_ix_signers(1).collect::<HashSet<_>>(),
+            HashSet::from_iter([&signer0, &signer1])
+        );
+        assert_eq!(
+            message.get_ix_signers(2).collect::<HashSet<_>>(),
+            HashSet::from_iter([&signer0])
+        );
+        assert_eq!(
+            message.get_ix_signers(3).collect::<HashSet<_>>(),
+            HashSet::default()
+        );
+    }
 }

sdk/src/feature_set.rs

@@ -331,6 +331,10 @@ pub mod enable_durable_nonce {
     solana_sdk::declare_id!("4EJQtF2pkRyawwcTVfQutzq4Sa5hRhibF6QAK1QXhtEX");
 }
 
+pub mod nonce_must_be_authorized {
+    solana_sdk::declare_id!("HxrEu1gXuH7iD3Puua1ohd5n4iUKJyFNtNxk9DVJkvgr");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -408,6 +412,7 @@ lazy_static! {
         (warp_timestamp_with_a_vengeance::id(), "warp timestamp again, adjust bounding to 150% slow #25666"),
         (separate_nonce_from_blockhash::id(), "separate durable nonce and blockhash domains #25744"),
         (enable_durable_nonce::id(), "enable durable nonce #25744"),
+        (nonce_must_be_authorized::id(), "nonce must be authorized"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()

sdk/src/nonce_account.rs

@@ -3,7 +3,10 @@ use {
         account::{AccountSharedData, ReadableAccount},
         account_utils::StateMut,
         hash::Hash,
-        nonce::{state::Versions, State},
+        nonce::{
+            state::{Data, Versions},
+            State,
+        },
     },
     std::cell::RefCell,
 };
@@ -21,13 +24,13 @@ pub fn create_account(lamports: u64) -> RefCell<AccountSharedData> {
 }
 
 // TODO: Consider changing argument from Hash to DurableNonce.
-pub fn verify_nonce_account(acc: &AccountSharedData, hash: &Hash) -> bool {
+pub fn verify_nonce_account(acc: &AccountSharedData, hash: &Hash) -> Option<Data> {
     if acc.owner() != &crate::system_program::id() {
-        return false;
+        return None;
     }
     match StateMut::<Versions>::state(acc).map(|v| v.convert_to_current()) {
-        Ok(State::Initialized(ref data)) => hash == &data.blockhash(),
-        _ => false,
+        Ok(State::Initialized(data)) => (hash == &data.blockhash()).then(|| data),
+        _ => None,
     }
 }
 
@@ -56,6 +59,6 @@ mod tests {
             &program_id,
         )
         .expect("nonce_account");
-        assert!(!verify_nonce_account(&account, &Hash::default()));
+        assert!(verify_nonce_account(&account, &Hash::default()).is_none());
     }
 }

send-transaction-service/src/send_transaction_service.rs

@@ -237,7 +237,7 @@ impl SendTransactionService {
             }
             if let Some((nonce_pubkey, durable_nonce)) = transaction_info.durable_nonce_info {
                 let nonce_account = working_bank.get_account(&nonce_pubkey).unwrap_or_default();
-                if !nonce_account::verify_nonce_account(&nonce_account, &durable_nonce)
+                if nonce_account::verify_nonce_account(&nonce_account, &durable_nonce).is_none()
                     && working_bank.get_signature_status_slot(signature).is_none()
                 {
                     info!("Dropping expired durable-nonce transaction: {}", signature);