Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to return an error when encountering non-root auth in recording mode. #991

Merged
merged 3 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions soroban-env-host/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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![]),
Expand Down Expand Up @@ -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)?
Expand Down Expand Up @@ -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)
}
}
}

Expand Down Expand Up @@ -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",
&[],
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)?,
],
Expand All @@ -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)?,
],
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
40 changes: 39 additions & 1 deletion soroban-env-host/src/test/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down