From 0ca834f97af356db431e15a637b4b8789d020569 Mon Sep 17 00:00:00 2001 From: Vinod Sathyaseelan Date: Thu, 14 Dec 2023 12:29:53 -0800 Subject: [PATCH] fix: App ignore rules are causing all grant states for be true --- core/main/src/firebolt/firebolt_gatekeeper.rs | 13 ++- .../apps/delegated_launcher_handler.rs | 2 - core/main/src/state/platform_state.rs | 5 +- core/sdk/src/api/firebolt/fb_openrpc.rs | 95 +++++++++++++++++-- core/sdk/src/api/manifest/exclusory.rs | 12 ++- 5 files changed, 109 insertions(+), 18 deletions(-) diff --git a/core/main/src/firebolt/firebolt_gatekeeper.rs b/core/main/src/firebolt/firebolt_gatekeeper.rs index 2ff8d757a..8664fe232 100644 --- a/core/main/src/firebolt/firebolt_gatekeeper.rs +++ b/core/main/src/firebolt/firebolt_gatekeeper.rs @@ -78,12 +78,6 @@ impl FireboltGatekeeper { } // TODO return Deny Reason into ripple error pub async fn gate(state: PlatformState, request: RpcRequest) -> Result<(), DenyReasonWithCap> { - let open_rpc_state = state.clone().open_rpc_state; - if open_rpc_state.is_excluded(request.clone().method, request.clone().ctx.app_id) { - trace!("Method is exluded from gating {}", request.method); - return Ok(()); - } - // if let Some(caps) = open_rpc_state.get_caps_for_method(&request.method) { let caps_opt = Self::get_resolved_caps_for_method(&state, &request.method, request.ctx.gateway_secure); if caps_opt.is_none() { @@ -139,7 +133,11 @@ impl FireboltGatekeeper { filtered_perm_list: Vec, ) -> Result<(), DenyReasonWithCap> { // permission checks - if let Err(e) = + let open_rpc_state = state.clone().open_rpc_state; + // check if the app or method is in permission exclusion list + if open_rpc_state.is_excluded(request.clone().method, request.clone().ctx.app_id) { + trace!("Method is exluded from permission check {}", request.method); + } else if let Err(e) = PermissionHandler::check_permitted(&state, &request.ctx.app_id, &filtered_perm_list) .await { @@ -150,6 +148,7 @@ impl FireboltGatekeeper { ); return Err(e); } + trace!("check_permitted for method ({}) succeded", request.method); //usergrants check if let Err(e) = GrantState::check_with_roles( diff --git a/core/main/src/service/apps/delegated_launcher_handler.rs b/core/main/src/service/apps/delegated_launcher_handler.rs index f3bc1e296..377bc8275 100644 --- a/core/main/src/service/apps/delegated_launcher_handler.rs +++ b/core/main/src/service/apps/delegated_launcher_handler.rs @@ -866,8 +866,6 @@ impl DelegatedLauncherHandler { .into_iter() .filter(|perm| { let filtered_policy_opt = grant_polices_map.get(&perm.cap.as_str()); - debug!("permission: {:?}", perm); - debug!("filtered_policy_opt: {:?}", filtered_policy_opt); if filtered_policy_opt.is_none() { return false; } diff --git a/core/main/src/state/platform_state.rs b/core/main/src/state/platform_state.rs index 5155a14c8..a8d7dadc5 100644 --- a/core/main/src/state/platform_state.rs +++ b/core/main/src/state/platform_state.rs @@ -21,6 +21,7 @@ use ripple_sdk::{ manifest::{ app_library::AppLibraryState, device_manifest::{AppLibraryEntry, DeviceManifest}, + exclusory::ExclusoryImpl, extn_manifest::ExtnManifest, }, protocol::BridgeProtocolRequest, @@ -111,6 +112,8 @@ impl PlatformState { client: RippleClient, app_library: Vec, ) -> PlatformState { + let exclusory = ExclusoryImpl::get(&manifest); + Self { extn_manifest, cap_state: CapState::new(manifest.clone()), @@ -121,7 +124,7 @@ impl PlatformState { app_events_state: AppEventsState::default(), provider_broker_state: ProviderBrokerState::default(), app_manager_state: AppManagerState::new(&manifest.configuration.saved_dir), - open_rpc_state: OpenRpcState::new(manifest.configuration.exclusory), + open_rpc_state: OpenRpcState::new(Some(exclusory)), router_state: RouterState::new(), data_governance: DataGovernanceState::default(), metrics: MetricsState::default(), diff --git a/core/sdk/src/api/firebolt/fb_openrpc.rs b/core/sdk/src/api/firebolt/fb_openrpc.rs index b6a364984..2b73a3914 100644 --- a/core/sdk/src/api/firebolt/fb_openrpc.rs +++ b/core/sdk/src/api/firebolt/fb_openrpc.rs @@ -355,13 +355,17 @@ impl FireboltOpenRpcMethod { } pub fn name_with_lowercase_module(method: &str) -> String { - let mut parts: Vec<&str> = method.split('.').collect(); - if parts.len() < 2 { - return String::from(method); + // Check if delimter ('.') is present in method name. + // If found, idx will be the index of the delimiter + if let Some((idx, _)) = method.chars().enumerate().find(|&(_, c)| c == '.') { + // Split method to module and method name at the delimiter + let (module, method_name) = method.split_at(idx); + // convert module to lowercase and concatenate with method name + format!("{}.{}", module.to_lowercase(), &method_name[1..]) + } else { + // No delimiter found, return method as is + method.to_string() } - let module = parts.remove(0); - let method_name = parts.join("."); - format!("{}.{}", module.to_lowercase(), method_name) } pub fn is_named(&self, method_name: &str) -> bool { @@ -664,3 +668,82 @@ impl CapabilitySet { } } } + +#[cfg(test)] +mod tests { + use crate::api::firebolt::fb_openrpc::FireboltOpenRpcMethod; + + #[test] + pub fn test_firebolt_method_casing() { + let m1 = FireboltOpenRpcMethod { + name: String::from("SecureStorage.get"), + tags: None, + }; + let m2 = FireboltOpenRpcMethod { + name: String::from("SecureStorage.getItem"), + tags: None, + }; + let m3 = FireboltOpenRpcMethod { + name: String::from("SecureStorage.get.item"), + tags: None, + }; + let m4 = FireboltOpenRpcMethod { + name: String::from("get"), + tags: None, + }; + + let m5 = FireboltOpenRpcMethod { + name: String::from("*"), + tags: None, + }; + + let m6 = FireboltOpenRpcMethod { + name: String::from("secureStorage.get"), + tags: None, + }; + let m7 = FireboltOpenRpcMethod { + name: String::from("secureStorage.getItem"), + tags: None, + }; + let m8 = FireboltOpenRpcMethod { + name: String::from("secureStorage.get.item"), + tags: None, + }; + + assert!(m1.is_named("securestorage.get")); + assert!(m2.is_named("secureStorage.getItem")); + assert!(!m2.is_named("secureStorage.getitem")); + assert!(m3.is_named("Securestorage.get.item")); + assert!(m4.is_named("get")); + assert!(!m4.is_named("Get")); + assert!(!m4.is_named("SecureStorage.get")); + assert!(m5.is_named("*")); + assert!(m6.is_named("securestorage.get")); + assert!(!m6.is_named("securestorage.Get")); + assert!(!m7.is_named("securestorage.getitem")); + assert!(m7.is_named("securestorage.getItem")); + assert!(m8.is_named("securestorage.get.item")); + assert!(!m8.is_named("securestorage.get.Item")); + + assert_eq!( + FireboltOpenRpcMethod::name_with_lowercase_module(&m1.name), + "securestorage.get" + ); + assert_eq!( + FireboltOpenRpcMethod::name_with_lowercase_module(&m2.name), + "securestorage.getItem" + ); + assert_eq!( + FireboltOpenRpcMethod::name_with_lowercase_module(&m3.name), + "securestorage.get.item" + ); + assert_eq!( + FireboltOpenRpcMethod::name_with_lowercase_module(&m4.name), + "get" + ); + assert_eq!( + FireboltOpenRpcMethod::name_with_lowercase_module(&m5.name), + "*" + ); + } +} diff --git a/core/sdk/src/api/manifest/exclusory.rs b/core/sdk/src/api/manifest/exclusory.rs index 348e4df17..624302624 100644 --- a/core/sdk/src/api/manifest/exclusory.rs +++ b/core/sdk/src/api/manifest/exclusory.rs @@ -19,6 +19,8 @@ use std::collections::HashMap; use serde::Deserialize; +use crate::api::firebolt::fb_openrpc::FireboltOpenRpcMethod; + use super::device_manifest::DeviceManifest; #[derive(Debug, Clone, Deserialize)] @@ -37,8 +39,14 @@ pub struct ExclusoryImpl { } impl ExclusoryImpl { - pub fn get(dm: DeviceManifest) -> ExclusoryImpl { - if let Some(e) = dm.configuration.exclusory { + pub fn get(dm: &DeviceManifest) -> ExclusoryImpl { + if let Some(mut e) = dm.configuration.exclusory.clone() { + // convert module part of method in method_ignore_rules to lowercase + e.method_ignore_rules = e + .method_ignore_rules + .iter() + .map(|m| FireboltOpenRpcMethod::name_with_lowercase_module(m)) + .collect(); return e; } ExclusoryImpl {