Add an option to return an error when encountering non-root auth in r…

…ecording mode. (#991)

This is a safer default for most of the contracts that don't need to do non-atomic contract call bundling.

Also added some passing-by auth error improvements.

soroban-env-host/src/auth.rs

@@ -150,6 +150,9 @@ struct RecordingAuthInfo {
     // value, but are specified as two different objects (e.g. as two different
     // contract function inputs).
     tracker_by_address_handle: RefCell<HashMap<u32, usize>>,
+    // Whether to allow root authorized invocation to not match the root
+    // contract invocation.
+    disable_non_root_auth: bool,
 }
 
 impl RecordingAuthInfo {
@@ -604,10 +607,11 @@ impl AuthorizationManager {
     // All the authorization requirements will be recorded and can then be
     // retrieved using `get_recorded_auth_payloads`.
     // metering: free
-    pub(crate) fn new_recording() -> Self {
+    pub(crate) fn new_recording(allow_non_root_auth: bool) -> Self {
         Self {
             mode: AuthorizationMode::Recording(RecordingAuthInfo {
                 tracker_by_address_handle: Default::default(),
+                disable_non_root_auth: allow_non_root_auth,
             }),
             call_stack: RefCell::new(vec![]),
             account_trackers: RefCell::new(vec![]),
@@ -824,6 +828,18 @@ impl AuthorizationManager {
                         ));
                     }
                 }
+                if recording_info.disable_non_root_auth
+                    && self.try_borrow_call_stack(host)?.len() != 1
+                {
+                    return Err(host.err(
+                        ScErrorType::Auth,
+                        ScErrorCode::InvalidAction,
+                        "[recording authorization only] encountered authorization not tied \
+                        to the root contract invocation for an address. Use `require_auth()` \
+                        in the top invocation or enable non-root authorization.",
+                        &[address.into()],
+                    ));
+                }
                 // If a tracker for the new tree doesn't exist yet, create
                 // it and initialize with the current invocation.
                 self.try_borrow_account_trackers_mut(host)?
@@ -1113,11 +1129,13 @@ impl AuthorizationManager {
     // metering: free, testutils
     #[cfg(any(test, feature = "testutils"))]
     pub(crate) fn reset(&mut self) {
-        *self = match self.mode {
+        *self = match &self.mode {
             AuthorizationMode::Enforcing => {
                 AuthorizationManager::new_enforcing_without_authorizations()
             }
-            AuthorizationMode::Recording(_) => AuthorizationManager::new_recording(),
+            AuthorizationMode::Recording(rec_info) => {
+                AuthorizationManager::new_recording(rec_info.disable_non_root_auth)
+            }
         }
     }
 
@@ -1354,7 +1372,7 @@ impl AccountAuthorizationTracker {
                 host.source_account_address()?.ok_or_else(|| {
                     host.err(
                         ScErrorType::Auth,
-                        ScErrorCode::InvalidInput,
+                        ScErrorCode::InternalError,
                         "source account is missing when setting auth entries",
                         &[],
                     )
@@ -1473,7 +1491,7 @@ impl AccountAuthorizationTracker {
                             ScErrorType::Auth,
                             ScErrorCode::InvalidAction,
                             "failed account authentication",
-                            &[err.error.to_val()],
+                            &[self.address.into(), err.error.to_val()],
                         )
                     } else {
                         err
@@ -1561,6 +1579,7 @@ impl AccountAuthorizationTracker {
                     ScErrorCode::InvalidInput,
                     "signature has expired",
                     &[
+                        self.address.into(),
                         ledger_seq.try_into_val(host)?,
                         expiration_ledger.try_into_val(host)?,
                     ],
@@ -1573,6 +1592,7 @@ impl AccountAuthorizationTracker {
                     ScErrorCode::InvalidInput,
                     "signature expiration is too late",
                     &[
+                        self.address.into(),
                         max_expiration_ledger.try_into_val(host)?,
                         expiration_ledger.try_into_val(host)?,
                     ],
@@ -1764,8 +1784,8 @@ impl Host {
                 return Err(self.err(
                     ScErrorType::Auth,
                     ScErrorCode::ExistingValue,
-                    "nonce already exists",
-                    &[],
+                    "nonce already exists for address",
+                    &[address.into()],
                 ));
             }
             let body = ContractDataEntryBody::DataEntry(ContractDataEntryData {

soroban-env-host/src/host.rs

@@ -299,8 +299,9 @@ impl Host {
         }
     }
 
-    pub fn switch_to_recording_auth(&self) -> Result<(), HostError> {
-        *self.try_borrow_authorization_manager_mut()? = AuthorizationManager::new_recording();
+    pub fn switch_to_recording_auth(&self, disable_non_root_auth: bool) -> Result<(), HostError> {
+        *self.try_borrow_authorization_manager_mut()? =
+            AuthorizationManager::new_recording(disable_non_root_auth);
         Ok(())
     }
 

soroban-env-host/src/test/auth.rs

@@ -255,7 +255,7 @@ impl AuthTest {
         fn_name: Symbol,
         args: HostVec,
     ) -> Vec<RecordedAuthPayload> {
-        self.host.switch_to_recording_auth().unwrap();
+        self.host.switch_to_recording_auth(false).unwrap();
         self.host
             .call(contract_address.clone().into(), fn_name, args.into())
             .unwrap();
@@ -841,6 +841,44 @@ fn test_two_authorized_trees() {
     );
 }
 
+#[test]
+fn test_disable_non_root_recording_auth() {
+    let test = AuthTest::setup(1, 3);
+    test.host.switch_to_recording_auth(true).unwrap();
+    let setup = SetupNode::new(
+        &test.contracts[0],
+        vec![false],
+        vec![
+            SetupNode::new(&test.contracts[1], vec![true], vec![]),
+            SetupNode::new(&test.contracts[2], vec![true], vec![]),
+        ],
+    );
+    let addresses = test.get_addresses();
+    let tree = test.convert_setup_tree(&setup);
+    let err = test
+        .host
+        .call(
+            setup.contract_address.clone().into(),
+            Symbol::try_from_small_str("tree_fn").unwrap(),
+            host_vec![&test.host, addresses, tree].into(),
+        )
+        .err()
+        .unwrap();
+    assert!(err.error.is_type(ScErrorType::Auth));
+    assert!(err.error.is_code(ScErrorCode::InvalidAction));
+
+    // Now enable non root auth - the call should succeed.
+    test.host.switch_to_recording_auth(false).unwrap();
+    assert!(test
+        .host
+        .call(
+            setup.contract_address.into(),
+            Symbol::try_from_small_str("tree_fn").unwrap(),
+            host_vec![&test.host, addresses, tree].into(),
+        )
+        .is_ok());
+}
+
 #[test]
 fn test_three_authorized_trees() {
     let mut test = AuthTest::setup(1, 5);

soroban-env-host/src/test/lifecycle.rs

@@ -364,7 +364,7 @@ fn test_create_contract_from_source_account_recording_auth() {
     let source_account = generate_account_id();
     let salt = generate_bytes_array();
     host.set_source_account(source_account.clone()).unwrap();
-    host.switch_to_recording_auth().unwrap();
+    host.switch_to_recording_auth(true).unwrap();
     let contract_id_preimage = ContractIdPreimage::Address(ContractIdPreimageFromAddress {
         address: ScAddress::Account(source_account.clone()),
         salt: Uint256(salt.to_vec().try_into().unwrap()),

soroban-env-host/src/test/token.rs

@@ -2827,7 +2827,7 @@ fn test_recording_auth_for_token() {
     let user = TestSigner::account(&test.user_key);
     test.create_default_account(&user);
     test.create_default_trustline(&user);
-    test.host.switch_to_recording_auth().unwrap();
+    test.host.switch_to_recording_auth(true).unwrap();
 
     let args = host_vec![&test.host, user.address(&test.host), 100_i128];
     test.host

soroban-env-host/src/test/util.rs

@@ -181,7 +181,7 @@ impl Host {
         let prev_source_account = self.source_account_id().unwrap();
         // Use recording auth to skip specifying the auth payload.
         let prev_auth_manager = self.snapshot_auth_manager().unwrap();
-        self.switch_to_recording_auth().unwrap();
+        self.switch_to_recording_auth(true).unwrap();
 
         let wasm_hash = self
             .upload_wasm(self.bytes_new_from_slice(contract_wasm).unwrap())