From a43ca7e43d52a9aa54bd08da035b493f98bde34b Mon Sep 17 00:00:00 2001 From: Razvan Cojocaru Date: Sun, 15 Dec 2024 20:20:19 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20zb,zm:=20Support=20Option>=20parameter=20in=20property=20getters?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows for fine-grained connecting-UID checking, in order to be able to control who is able to access property values. Signed-off-by: Razvan Cojocaru --- zbus/src/fdo/properties.rs | 16 +- zbus/src/object_server/interface/mod.rs | 28 +++- zbus/src/object_server/node.rs | 2 +- zbus/tests/e2e.rs | 8 + zbus_macros/src/iface.rs | 187 ++++++++++++++---------- 5 files changed, 154 insertions(+), 87 deletions(-) diff --git a/zbus/src/fdo/properties.rs b/zbus/src/fdo/properties.rs index 9be71ad1d..9fba485e2 100644 --- a/zbus/src/fdo/properties.rs +++ b/zbus/src/fdo/properties.rs @@ -9,7 +9,7 @@ use zbus_names::InterfaceName; use zvariant::{OwnedValue, Value}; use super::{Error, Result}; -use crate::{interface, message::Header, object_server::SignalEmitter, ObjectServer}; +use crate::{interface, message::Header, object_server::SignalEmitter, Connection, ObjectServer}; /// Service-side implementation for the `org.freedesktop.DBus.Properties` interface. /// This interface is implemented automatically for any object registered to the @@ -29,6 +29,7 @@ impl Properties { &self, interface_name: InterfaceName<'_>, property_name: &str, + #[zbus(connection)] conn: &Connection, #[zbus(object_server)] server: &ObjectServer, #[zbus(header)] header: Header<'_>, ) -> Result { @@ -41,7 +42,12 @@ impl Properties { Error::UnknownInterface(format!("Unknown interface '{interface_name}'")) })?; - let res = iface.instance.read().await.get(property_name).await; + let res = iface + .instance + .read() + .await + .get(property_name, server, conn, &Some(header)) + .await; res.unwrap_or_else(|| { Err(Error::UnknownProperty(format!( "Unknown property '{property_name}'" @@ -72,7 +78,7 @@ impl Properties { .instance .read() .await - .set(property_name, &value, &emitter) + .set(property_name, &value, &emitter, &header) { zbus::object_server::DispatchResult::RequiresMut => {} zbus::object_server::DispatchResult::NotFound => { @@ -88,7 +94,7 @@ impl Properties { .instance .write() .await - .set_mut(property_name, &value, &emitter) + .set_mut(property_name, &value, &emitter, &header) .await; res.unwrap_or_else(|| { Err(Error::UnknownProperty(format!( @@ -113,7 +119,7 @@ impl Properties { Error::UnknownInterface(format!("Unknown interface '{interface_name}'")) })?; - let res = iface.instance.read().await.get_all().await?; + let res = iface.instance.read().await.get_all(&Some(header)).await?; Ok(res) } diff --git a/zbus/src/object_server/interface/mod.rs b/zbus/src/object_server/interface/mod.rs index c3a771033..82d0a532e 100644 --- a/zbus/src/object_server/interface/mod.rs +++ b/zbus/src/object_server/interface/mod.rs @@ -17,8 +17,11 @@ use zbus_names::{InterfaceName, MemberName}; use zvariant::{OwnedValue, Value}; use crate::{ - async_lock::RwLock, fdo, message::Message, object_server::SignalEmitter, Connection, - ObjectServer, + async_lock::RwLock, + fdo, + message::{self, Header, Message}, + object_server::SignalEmitter, + Connection, ObjectServer, }; /// This trait is used to dispatch messages to an interface instance. @@ -45,10 +48,23 @@ pub trait Interface: Any + Send + Sync { } /// Get a property value. Returns `None` if the property doesn't exist. - async fn get(&self, property_name: &str) -> Option>; + /// + /// Note: The header parameter will be None when the getter is used by the current process, + /// where we just query the value for administrative purposes. Since in that case there are no + /// "real" callers, it doesn't make sense to think of this parameter being relevant. + async fn get( + &self, + property_name: &str, + server: &ObjectServer, + connection: &Connection, + header: &Option>, + ) -> Option>; /// Return all the properties. - async fn get_all(&self) -> fdo::Result>; + async fn get_all( + &self, + header: &Option>, + ) -> fdo::Result>; /// Set a property value. /// @@ -60,8 +76,9 @@ pub trait Interface: Any + Send + Sync { property_name: &'call str, value: &'call Value<'_>, emitter: &'call SignalEmitter<'_>, + header: &'call message::Header<'_>, ) -> DispatchResult<'call> { - let _ = (property_name, value, emitter); + let _ = (property_name, value, emitter, header); DispatchResult::RequiresMut } @@ -75,6 +92,7 @@ pub trait Interface: Any + Send + Sync { property_name: &str, value: &Value<'_>, emitter: &SignalEmitter<'_>, + header: &Header<'_>, ) -> Option>; /// Call a method. diff --git a/zbus/src/object_server/node.rs b/zbus/src/object_server/node.rs index c0423fc00..190e5b9e8 100644 --- a/zbus/src/object_server/node.rs +++ b/zbus/src/object_server/node.rs @@ -241,7 +241,7 @@ impl Node { .instance .read() .await - .get_all() + .get_all(&None) .await } } diff --git a/zbus/tests/e2e.rs b/zbus/tests/e2e.rs index 12107f369..43a1c4cdb 100644 --- a/zbus/tests/e2e.rs +++ b/zbus/tests/e2e.rs @@ -262,6 +262,13 @@ impl MyIface { self.count } + #[instrument] + #[zbus(property)] + fn test_header_prop(&self, #[zbus(header)] header: Option>) -> bool { + debug!("`TestHeaderProp` getter called: {:?}.", header); + header.is_some() + } + #[instrument] #[zbus(property)] async fn hash_map(&self) -> HashMap { @@ -570,6 +577,7 @@ async fn my_iface_test(conn: Connection, event: Event) -> zbus::Result { drop(props_changed_stream); proxy.ping().await?; + assert_eq!(proxy.test_header_prop().await?, true); assert_eq!(proxy.count().await?, 1); assert_eq!(proxy.cached_count()?, None); diff --git a/zbus_macros/src/iface.rs b/zbus_macros/src/iface.rs index 7f10602b8..4ae37977d 100644 --- a/zbus_macros/src/iface.rs +++ b/zbus_macros/src/iface.rs @@ -103,8 +103,8 @@ enum MethodType { #[derive(Debug, PartialEq, Copy, Clone)] enum PropertyType { - Inputs, - NoInputs, + Setter, + Getter, } #[derive(Debug, Clone)] @@ -190,7 +190,25 @@ impl MethodInfo { let is_signal = attrs.signal; assert!(!is_property || !is_signal); - let has_inputs = inputs.len() > 1; + let mut typed_inputs = inputs + .iter() + .filter_map(typed_arg) + .cloned() + .collect::>(); + + let has_inputs = count_regular_args(&typed_inputs) > 0; + + let method_type = if is_signal { + MethodType::Signal + } else if is_property { + if has_inputs { + MethodType::Property(PropertyType::Setter) + } else { + MethodType::Property(PropertyType::Getter) + } + } else { + MethodType::Other + }; let is_mut = if let FnArg::Receiver(r) = inputs .first() @@ -211,11 +229,6 @@ impl MethodInfo { quote! {} }; - let mut typed_inputs = inputs - .iter() - .filter_map(typed_arg) - .cloned() - .collect::>(); let signal_emitter_arg: Option = if is_signal { if typed_inputs.is_empty() { return Err(Error::new_spanned( @@ -237,7 +250,7 @@ impl MethodInfo { cfg_attrs, )?; - let (args_from_msg, args_names) = get_args_from_inputs(&typed_inputs, zbus)?; + let (args_from_msg, args_names) = get_args_from_inputs(&typed_inputs, method_type, zbus)?; let reply = if is_result_output { let ret = quote!(r); @@ -259,18 +272,6 @@ impl MethodInfo { pascal_case(&name) }); - let method_type = if is_signal { - MethodType::Signal - } else if is_property { - if has_inputs { - MethodType::Property(PropertyType::Inputs) - } else { - MethodType::Property(PropertyType::NoInputs) - } - } else { - MethodType::Other - }; - Ok(MethodInfo { ident: ident.clone(), method_type, @@ -404,7 +405,7 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re )?; let attr_property = method_attrs.property; if let Some(prop_attrs) = &attr_property { - if method_info.method_type == MethodType::Property(PropertyType::NoInputs) { + if method_info.method_type == MethodType::Property(PropertyType::Getter) { let emits_changed_signal = if let Some(s) = &prop_attrs.emits_changed_signal { PropertyEmitsChangedSignal::parse(s, method.span())? } else { @@ -659,10 +660,10 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re .map_err(|e| #zbus::fdo::Error::Failed(e.to_string())) ); let inner = if is_fallible_property { - quote!(self.#ident() #method_await .and_then(|value| #value_convert)) + quote!(self.#ident(#args_names) #method_await .and_then(|value| #value_convert)) } else { quote!({ - let value = self.#ident()#method_await; + let value = self.#ident(#args_names)#method_await; #value_convert }) }; @@ -670,13 +671,16 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re let q = quote!( #(#cfg_attrs)* #member_name => { + #args_from_msg ::std::option::Option::Some(#inner) }, ); get_dispatch.extend(q); let q = if is_fallible_property { - quote!(if let Ok(prop) = self.#ident()#method_await { + quote!( + #args_from_msg + if let Ok(prop) = self.#ident(#args_names)#method_await { props.insert( ::std::string::ToString::to_string(#member_name), <#zbus::zvariant::OwnedValue as ::std::convert::TryFrom<_>>::try_from( @@ -688,11 +692,13 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re ); }) } else { - quote!(props.insert( + quote!( + #args_from_msg + props.insert( ::std::string::ToString::to_string(#member_name), <#zbus::zvariant::OwnedValue as ::std::convert::TryFrom<_>>::try_from( <#zbus::zvariant::Value as ::std::convert::From<_>>::from( - self.#ident()#method_await, + self.#ident(#args_names)#method_await, ), ) .map_err(|e| #zbus::fdo::Error::Failed(e.to_string()))?, @@ -701,10 +707,17 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re get_all.extend(q); + // The only argument we're expecting is possibly a Header<'_> argument. + let arg_none_if_header_present = if args_names.is_empty() { + args_names + } else { + quote! { None } + }; + let prop_value_handled = if is_fallible_property { - quote!(self.#ident()#method_await?) + quote!(self.#ident(#arg_none_if_header_present)#method_await?) } else { - quote!(self.#ident()#method_await) + quote!(self.#ident(#arg_none_if_header_present)#method_await) }; if p.emits_changed_signal == PropertyEmitsChangedSignal::True { @@ -852,6 +865,9 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re async fn get( &self, property_name: &str, + s: &#zbus::ObjectServer, + c: &#zbus::Connection, + h: &Option<#zbus::message::Header<'_>>, ) -> ::std::option::Option<#zbus::fdo::Result<#zbus::zvariant::OwnedValue>> { match property_name { #get_dispatch @@ -861,6 +877,7 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re async fn get_all( &self, + h: &Option<#zbus::message::Header<'_>>, ) -> #zbus::fdo::Result<::std::collections::HashMap< ::std::string::String, #zbus::zvariant::OwnedValue, @@ -878,6 +895,7 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re property_name: &'call str, value: &'call #zbus::zvariant::Value<'_>, signal_emitter: &'call #zbus::object_server::SignalEmitter<'_>, + h: &'call #zbus::message::Header<'_>, ) -> #zbus::object_server::DispatchResult<'call> { match property_name { #set_dispatch @@ -890,6 +908,7 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re property_name: &str, value: &#zbus::zvariant::Value<'_>, signal_emitter: &#zbus::object_server::SignalEmitter<'_>, + h: &#zbus::message::Header<'_>, ) -> ::std::option::Option<#zbus::fdo::Result<()>> { match property_name { #set_mut_dispatch @@ -946,6 +965,7 @@ pub fn expand(args: Punctuated, mut input: ItemImpl) -> syn::Re fn get_args_from_inputs( inputs: &[PatType], + method_type: MethodType, zbus: &TokenStream, ) -> syn::Result<(TokenStream, TokenStream)> { if inputs.is_empty() { @@ -997,9 +1017,10 @@ fn get_args_from_inputs( let header_arg = &input.pat; - header_arg_decl = Some(quote! { - let #header_arg = m.header(); - }); + header_arg_decl = match method_type { + MethodType::Property(_) => Some(quote! { let #header_arg = h.clone(); }), + _ => Some(quote! { let #header_arg = m.header(); }), + }; } else if signal_context || signal_emitter { if signal_emitter_arg_decl.is_some() { return Err(Error::new_spanned( @@ -1027,26 +1048,31 @@ fn get_args_from_inputs( } } - let args_from_msg = quote! { - let hdr = m.header(); - let msg_body = m.body(); + let args_from_msg = match method_type { + MethodType::Property(_) => quote! { + #header_arg_decl + }, + _ => quote! { + let hdr = m.header(); + let msg_body = m.body(); - #server_arg_decl + #server_arg_decl - #conn_arg_decl + #conn_arg_decl - #header_arg_decl + #header_arg_decl - #signal_emitter_arg_decl + #signal_emitter_arg_decl - let (#(#args_names),*): (#(#tys),*) = - match msg_body.deserialize() { - ::std::result::Result::Ok(r) => r, - ::std::result::Result::Err(e) => { - let err = <#zbus::fdo::Error as ::std::convert::From<_>>::from(e); - return c.reply_dbus_error(&hdr, err).await; - } - }; + let (#(#args_names),*): (#(#tys),*) = + match msg_body.deserialize() { + ::std::result::Result::Ok(r) => r, + ::std::result::Result::Err(e) => { + let err = <#zbus::fdo::Error as ::std::convert::From<_>>::from(e); + return c.reply_dbus_error(&hdr, err).await; + } + }; + }, }; let all_args_names = inputs.iter().filter_map(pat_ident); @@ -1097,35 +1123,7 @@ fn introspect_input_args<'i>( inputs .iter() .filter_map(move |pat_type @ PatType { ty, attrs, .. }| { - let is_special_arg = attrs.iter().any(|attr| { - if !attr.path().is_ident("zbus") { - return false; - } - - let Ok(list) = &attr.meta.require_list() else { - return false; - }; - let Ok(nested) = - list.parse_args_with(Punctuated::::parse_terminated) - else { - return false; - }; - - let res = nested.iter().any(|nested_meta| { - matches!( - nested_meta, - Meta::Path(path) - if path.is_ident("object_server") || - path.is_ident("connection") || - path.is_ident("header") || - path.is_ident("signal_context") || - path.is_ident("signal_emitter") - ) - }); - - res - }); - if is_special_arg { + if is_special_arg(attrs) { return None; } @@ -1143,6 +1141,43 @@ fn introspect_input_args<'i>( }) } +fn count_regular_args(inputs: &[PatType]) -> usize { + inputs + .iter() + .filter(|PatType { attrs, .. }| !is_special_arg(attrs)) + .count() +} + +fn is_special_arg(attrs: &[Attribute]) -> bool { + attrs.iter().any(|attr| { + if !attr.path().is_ident("zbus") { + return false; + } + + let Ok(list) = &attr.meta.require_list() else { + return false; + }; + let Ok(nested) = list.parse_args_with(Punctuated::::parse_terminated) + else { + return false; + }; + + let res = nested.iter().any(|nested_meta| { + matches!( + nested_meta, + Meta::Path(path) + if path.is_ident("object_server") || + path.is_ident("connection") || + path.is_ident("header") || + path.is_ident("signal_context") || + path.is_ident("signal_emitter") + ) + }); + + res + }) +} + fn introspect_output_arg( ty: &Type, arg_name: Option<&String>,