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

fix: App ignore rules are causing all grant states to be true #360

Merged
merged 7 commits into from
Jan 11, 2024
13 changes: 6 additions & 7 deletions core/main/src/firebolt/firebolt_gatekeeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -139,7 +133,11 @@ impl FireboltGatekeeper {
filtered_perm_list: Vec<FireboltPermission>,
) -> 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
{
Expand All @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions core/main/src/service/apps/delegated_launcher_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,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;
}
Expand Down
3 changes: 2 additions & 1 deletion core/main/src/state/cap/generic_cap_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ impl GenericCapState {
let not_available = self.not_available.read().unwrap();
let mut result: Vec<FireboltCap> = Vec::new();
for fb_perm in request {
if not_available.contains(&fb_perm.cap.as_str()) {
if fb_perm.role == CapabilityRole::Use && not_available.contains(&fb_perm.cap.as_str())
{
result.push(fb_perm.cap.clone())
}
}
Expand Down
5 changes: 4 additions & 1 deletion core/main/src/state/platform_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use ripple_sdk::{
manifest::{
app_library::AppLibraryState,
device_manifest::{AppLibraryEntry, DeviceManifest},
exclusory::ExclusoryImpl,
extn_manifest::ExtnManifest,
},
protocol::BridgeProtocolRequest,
Expand Down Expand Up @@ -111,6 +112,8 @@ impl PlatformState {
client: RippleClient,
app_library: Vec<AppLibraryEntry>,
) -> PlatformState {
let exclusory = ExclusoryImpl::get(&manifest);

Self {
extn_manifest,
cap_state: CapState::new(manifest.clone()),
Expand All @@ -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(),
Expand Down
95 changes: 89 additions & 6 deletions core/sdk/src/api/firebolt/fb_openrpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,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 {
Expand Down Expand Up @@ -662,3 +666,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),
"*"
);
}
}
12 changes: 10 additions & 2 deletions core/sdk/src/api/manifest/exclusory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 {
Expand Down