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

…25831) (#25850)

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

(cherry picked from commit b2b426d)

# Conflicts:
#	runtime/src/nonce_keyed_account.rs

* resolve conflicts

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

runtime/src/accounts.rs

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

runtime/src/bank.rs

@@ -119,7 +119,7 @@ use {
         message::{AccountKeys, SanitizedMessage},
         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,
@@ -3824,15 +3824,25 @@ impl Bank {
     }
 
     fn check_message_for_nonce(&self, message: &SanitizedMessage) -> Option<TransactionAccount> {
-        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(
@@ -12287,22 +12297,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

@@ -1313,7 +1313,8 @@ mod test {
                 .unwrap()
                 .borrow(),
             get_durable_nonce(&invoke_context).as_hash(),
-        ));
+        )
+        .is_some());
     }
 
     #[test]
@@ -1325,13 +1326,14 @@ mod test {
             _instruction_context,
             instruction_accounts
         );
-        assert!(!verify_nonce_account(
+        assert!(verify_nonce_account(
             &transaction_context
                 .get_account_at_index(NONCE_ACCOUNT_INDEX)
                 .unwrap()
                 .borrow(),
             &Hash::default()
-        ));
+        )
+        .is_none());
     }
 
     #[test]
@@ -1358,12 +1360,13 @@ mod test {
             .initialize_nonce_account(&authorized, &Rent::free(), &invoke_context)
             .unwrap();
         set_invoke_context_blockhash!(invoke_context, 1);
-        assert!(!verify_nonce_account(
+        assert!(verify_nonce_account(
             &transaction_context
                 .get_account_at_index(NONCE_ACCOUNT_INDEX)
                 .unwrap()
                 .borrow(),
             get_durable_nonce(&invoke_context).as_hash(),
-        ));
+        )
+        .is_none());
     }
 }

sdk/program/src/message/sanitized.rs

@@ -225,6 +225,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.account_keys().get(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()
@@ -256,7 +271,7 @@ impl SanitizedMessage {
 
 #[cfg(test)]
 mod tests {
-    use {super::*, crate::message::v0};
+    use {super::*, crate::message::v0, std::collections::HashSet};
 
     #[test]
     fn test_try_from_message() {
@@ -336,4 +351,44 @@ mod tests {
 
         assert_eq!(v0_message.num_readonly_accounts(), 3);
     }
+
+    #[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

@@ -383,6 +383,10 @@ pub mod quick_bail_on_panic {
     solana_sdk::declare_id!("DpJREPyuMZ5nDfU6H3WTqSqUFSXAfw8u7xqmWtEwJDcP");
 }
 
+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> = [
@@ -472,6 +476,7 @@ lazy_static! {
         (enable_durable_nonce::id(), "enable durable nonce #25744"),
         (executables_incur_cpi_data_cost::id(), "Executables incure CPI data costs"),
         (quick_bail_on_panic::id(), "quick bail on panic"),
+        (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

@@ -597,7 +597,7 @@ impl SendTransactionService {
                     .last_sent_time
                     .map(|last| now.duration_since(last) >= retry_rate)
                     .unwrap_or(false);
-                if !nonce_account::verify_nonce_account(&nonce_account, &durable_nonce)
+                if nonce_account::verify_nonce_account(&nonce_account, &durable_nonce).is_none()
                     && signature_status.is_none()
                     && expired
                 {