diff --git a/prompting-client/Cargo.toml b/prompting-client/Cargo.toml index 0e7b9db..95b5bd2 100644 --- a/prompting-client/Cargo.toml +++ b/prompting-client/Cargo.toml @@ -20,6 +20,10 @@ path = "src/bin/echo.rs" name = "prompting-client-daemon" path = "src/bin/daemon.rs" +[[bin]] +name = "prompting-client-set-log-level" +path = "src/bin/set_log_level.rs" + [dependencies] chrono = "0.4.38" clap = { version = "4.5.4", features = ["derive"] } @@ -37,6 +41,7 @@ tokio-stream = "0.1.15" tokio = { version = "1.37.0", features = ["fs", "io-util", "macros", "net", "process", "signal", "rt-multi-thread"] } tonic = "0.12.0" tonic-reflection = "0.12.0" +tower = "0.4.13" tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } tracing-journald = "0.3.0" @@ -48,7 +53,6 @@ simple_test_case = "1.2.0" tokio = { version = "1.37.0", features = ["process"] } uuid = { version = "1.8.0", features = ["v4"] } protobuf = "3.5.0" -tower = "0.4.13" tokio-util = "0.7.11" [build-dependencies] diff --git a/prompting-client/build.rs b/prompting-client/build.rs index 3de1934..16fa095 100644 --- a/prompting-client/build.rs +++ b/prompting-client/build.rs @@ -12,6 +12,7 @@ fn main() -> Result<(), Box> { tonic_build::configure() .build_server(true) + .build_client(true) .out_dir("./src/protos") .compile(files, include_dirs)?; diff --git a/prompting-client/src/bin/daemon.rs b/prompting-client/src/bin/daemon.rs index 2b1b66d..c6fa23e 100644 --- a/prompting-client/src/bin/daemon.rs +++ b/prompting-client/src/bin/daemon.rs @@ -1,5 +1,8 @@ //! The daemon prompting client for apparmor prompting -use prompting_client::{daemon::run_daemon, snapd_client::SnapdSocketClient, Error, Result}; +use prompting_client::{ + daemon::run_daemon, log_filter, snapd_client::SnapdSocketClient, Error, Result, + DEFAULT_LOG_LEVEL, +}; use std::{env, io::stdout}; use tracing::subscriber::set_global_default; use tracing::warn; @@ -8,12 +11,11 @@ use tracing_subscriber::{layer::SubscriberExt, FmtSubscriber}; #[tokio::main] async fn main() -> Result<()> { let builder = FmtSubscriber::builder() - .with_env_filter("debug,hyper=error,h2=error") + .with_env_filter(log_filter(DEFAULT_LOG_LEVEL)) .with_writer(stdout) .with_filter_reloading(); - // TODO: (sminez) support modifying the logging level at runtime via dbus - // let reload_handle = builder.reload_handle(); + let reload_handle = builder.reload_handle(); let journald_layer = tracing_journald::layer().expect("unable to open journald socket"); let subscriber = builder.finish().with(journald_layer); @@ -37,5 +39,5 @@ async fn main() -> Result<()> { return Ok(()); } - run_daemon(c).await + run_daemon(c, reload_handle).await } diff --git a/prompting-client/src/bin/set_log_level.rs b/prompting-client/src/bin/set_log_level.rs new file mode 100644 index 0000000..49b7d90 --- /dev/null +++ b/prompting-client/src/bin/set_log_level.rs @@ -0,0 +1,46 @@ +//! This is a helper command for setting our logging filter within the daemon while it is running. +//! Doing this involves having to send a GRPC message to the daemon which inolves more plumbing +//! than we'd ideally like but it allows us to update the filter without having to restart the +//! daemon. +//! +//! The docs on the command itself cover how to set simple logging levels but the tracing framework +//! we are using actually supports a rich syntax for how these filters are written. As and when we +//! need to use this in anger, the docs can be found here: +//! https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html +use clap::Parser; +use prompting_client::cli_actions::set_logging_filter; +use std::process::exit; + +/// Set the logging level for a running instance of the prompting client daemon. +/// +/// Simple usage of this command involves specifying a level based filter for what logs are written +/// to the system journal by the prompting-client daemon. The supported levels are (in order of +/// least to most verbose): +/// +/// - error +/// - warn +/// - info +/// - debug +/// - trace +/// +/// The default level is "info". +#[derive(Debug, Parser)] +#[clap(about, long_about = None)] +struct Args { + /// The filter to use for determining what gets logged + #[clap(short, long, value_name = "FILTER")] + filter: String, +} + +#[tokio::main] +async fn main() { + let Args { filter } = Args::parse(); + + match set_logging_filter(filter).await { + Ok(current) => println!("logging filter set to: {current}"), + Err(e) => { + eprintln!("{e}"); + exit(1); + } + } +} diff --git a/prompting-client/src/cli_actions/log_level.rs b/prompting-client/src/cli_actions/log_level.rs new file mode 100644 index 0000000..2b6360e --- /dev/null +++ b/prompting-client/src/cli_actions/log_level.rs @@ -0,0 +1,36 @@ +use crate::{ + protos::apparmor_prompting::app_armor_prompting_client::AppArmorPromptingClient, Error, Result, + SOCKET_ENV_VAR, +}; +use hyper_util::rt::TokioIo; +use std::env; +use std::io; +use tokio::net::UnixStream; +use tonic::transport::{Channel, Endpoint, Uri}; +use tower::service_fn; + +pub async fn set_logging_filter(filter: String) -> Result { + let mut client = client_from_env().await; + + match client.set_logging_filter(filter).await { + Ok(resp) => Ok(resp.into_inner().current), + Err(e) => Err(Error::UnableToUpdateLogFilter { + reason: e.to_string(), + }), + } +} + +async fn client_from_env() -> AppArmorPromptingClient { + let path = env::var(SOCKET_ENV_VAR).expect("socket env var not set"); + + // See https://github.com/hyperium/tonic/blob/master/examples/src/uds/client.rs + let channel = Endpoint::from_static("https://not-used.com") + .connect_with_connector(service_fn(move |_: Uri| { + let path = path.clone(); + async { Ok::<_, io::Error>(TokioIo::new(UnixStream::connect(path).await?)) } + })) + .await + .unwrap(); + + AppArmorPromptingClient::new(channel) +} diff --git a/prompting-client/src/cli_actions/mod.rs b/prompting-client/src/cli_actions/mod.rs index d2fd5b6..c39a6a8 100644 --- a/prompting-client/src/cli_actions/mod.rs +++ b/prompting-client/src/cli_actions/mod.rs @@ -1,5 +1,7 @@ mod echo_loop; +mod log_level; mod scripted; pub use echo_loop::run_echo_loop; +pub use log_level::set_logging_filter; pub use scripted::run_scripted_client_loop; diff --git a/prompting-client/src/daemon/mod.rs b/prompting-client/src/daemon/mod.rs index 034e02a..3edfa73 100644 --- a/prompting-client/src/daemon/mod.rs +++ b/prompting-client/src/daemon/mod.rs @@ -1,13 +1,14 @@ use crate::{ snapd_client::{PromptId, SnapMeta, SnapdSocketClient, TypedPrompt, TypedPromptReply}, - Result, + Result, SOCKET_ENV_VAR, }; use serde::{Deserialize, Serialize}; -use std::{env, fs}; +use std::{env, fs, sync::Arc}; use tokio::sync::mpsc::unbounded_channel; use tokio_stream::wrappers::UnixListenerStream; use tonic::{async_trait, transport::Server}; use tracing::{error, info}; +use tracing_subscriber::{reload::Handle, EnvFilter}; mod poll; mod server; @@ -52,19 +53,28 @@ pub enum ActionedPrompt { /// Start our backgroud polling and processing loops before dropping into running the tonic GRPC /// server for handling incoming requestes from the Flutter UI client. -pub async fn run_daemon(c: SnapdSocketClient) -> Result<()> { +pub async fn run_daemon(c: SnapdSocketClient, reload_handle: Handle) -> Result<()> +where + L: From + Send + Sync + 'static, + S: 'static, +{ let (tx_prompts, rx_prompts) = unbounded_channel(); let (tx_actioned, rx_actioned) = unbounded_channel(); let mut worker = Worker::new(rx_prompts, rx_actioned, c.clone()); let active_prompt = worker.read_only_active_prompt(); - let path = - env::var("PROMPTING_CLIENT_SOCKET").expect("PROMPTING_CLIENT_SOCKET env var to be set"); + let path = env::var(SOCKET_ENV_VAR).expect("socket env var not set"); if let Err(e) = fs::remove_file(&path) { error!("Failed to remove old socket file: {}. Error: {}", path, e); } - let (server, listener) = new_server_and_listener(c.clone(), active_prompt, tx_actioned, path); + let (server, listener) = new_server_and_listener( + c.clone(), + Arc::new(reload_handle), + active_prompt, + tx_actioned, + path, + ); info!("spawning poll loop"); let poll_loop = PollLoop::new(c, tx_prompts); diff --git a/prompting-client/src/daemon/server.rs b/prompting-client/src/daemon/server.rs index bf87557..d228d87 100644 --- a/prompting-client/src/daemon/server.rs +++ b/prompting-client/src/daemon/server.rs @@ -1,11 +1,12 @@ //! The GRPC server that handles incoming connections from client UIs. use crate::{ daemon::{worker::ReadOnlyActivePrompt, ActionedPrompt, ReplyToPrompt}, + log_filter, protos::{ apparmor_prompting::{ self, get_current_prompt_response::Prompt, home_prompt::PatternOption, prompt_reply::PromptReply::HomePromptReply, prompt_reply_response::PromptReplyType, - HomePatternType, MetaData, PromptReply, + HomePatternType, MetaData, PromptReply, SetLoggingFilterResponse, }, AppArmorPrompting, AppArmorPromptingServer, GetCurrentPromptResponse, HomePrompt, PromptReplyResponse, ResolveHomePatternTypeResponse, @@ -20,9 +21,11 @@ use crate::{ }, Error, NO_PROMPTS_FOR_USER, PROMPT_NOT_FOUND, }; +use std::sync::Arc; use tokio::{net::UnixListener, sync::mpsc::UnboundedSender}; use tonic::{async_trait, Code, Request, Response, Status}; use tracing::{info, warn}; +use tracing_subscriber::{reload::Handle, EnvFilter}; macro_rules! map_enum { ($from:ident => $to:ident; [$($variant:ident),+]; $val:expr;) => { @@ -42,38 +45,78 @@ macro_rules! map_enum { }; } -pub fn new_server_and_listener( - client: T, +pub fn new_server_and_listener( + client: R, + reload_handle: S, active_prompt: ReadOnlyActivePrompt, tx_actioned_prompts: UnboundedSender, socket_path: String, -) -> (AppArmorPromptingServer>, UnixListener) { - let service = Service::new(client.clone(), active_prompt, tx_actioned_prompts); +) -> (AppArmorPromptingServer>, UnixListener) +where + R: ReplyToPrompt + Clone, + S: SetLogFilter, +{ + let service = Service::new( + client.clone(), + reload_handle, + active_prompt, + tx_actioned_prompts, + ); let listener = UnixListener::bind(&socket_path).expect("to be able to bind to our socket"); (AppArmorPromptingServer::new(service), listener) } -pub struct Service +pub trait SetLogFilter: Send + Sync + 'static { + fn set_filter(&self, filter: &str) -> crate::Result<()>; +} + +impl SetLogFilter for Arc> +where + L: From + Send + Sync + 'static, + S: 'static, +{ + fn set_filter(&self, filter: &str) -> crate::Result<()> { + info!(?filter, "attempting to update logging filter"); + let f = filter + .parse::() + .map_err(|_| Error::UnableToUpdateLogFilter { + reason: format!("{filter:?} is not a valid logging filter"), + })?; + + self.reload(f).map_err(|e| Error::UnableToUpdateLogFilter { + reason: format!("failed to set logging filter: {e}"), + })?; + + Ok(()) + } +} + +pub struct Service where R: ReplyToPrompt, + S: SetLogFilter, { client: R, + reload_handle: S, active_prompt: ReadOnlyActivePrompt, tx_actioned_prompts: UnboundedSender, } -impl Service +impl Service where R: ReplyToPrompt, + S: SetLogFilter, { pub fn new( client: R, + reload_handle: S, active_prompt: ReadOnlyActivePrompt, tx_actioned_prompts: UnboundedSender, ) -> Self { Self { client, + reload_handle, active_prompt, tx_actioned_prompts, } @@ -87,9 +130,10 @@ where } #[async_trait] -impl AppArmorPrompting for Service +impl AppArmorPrompting for Service where R: ReplyToPrompt, + S: SetLogFilter, { async fn get_current_prompt( &self, @@ -165,6 +209,21 @@ where "this endpoint is not yet implemented", )) } + + async fn set_logging_filter( + &self, + filter: Request, + ) -> Result, Status> { + let current = log_filter(&filter.into_inner()); + + match self.reload_handle.set_filter(¤t) { + Ok(_) => Ok(Response::new(SetLoggingFilterResponse { current })), + Err(e) => Err(Status::new( + Code::InvalidArgument, + format!("unable to set logging level: {e}"), + )), + } + } } fn map_prompt_reply(mut reply: PromptReply) -> Result { @@ -346,6 +405,13 @@ mod tests { } } + struct MockReloadHandle; + impl SetLogFilter for MockReloadHandle { + fn set_filter(&self, level: &str) -> crate::Result<()> { + panic!("attempt to set log level to {level}"); + } + } + async fn setup_server_and_client( mock_client: MockClient, active_prompt: ReadOnlyActivePrompt, @@ -357,6 +423,7 @@ mod tests { let (server, listener) = new_server_and_listener( mock_client, + MockReloadHandle, active_prompt, tx_actioned_prompts, socket_path.clone(), @@ -371,7 +438,7 @@ mod tests { }); let path = socket_path.clone(); - // No choice but to do this https://github.com/hyperium/tonic/blob/master/examples/src/uds/client.rs + // See https://github.com/hyperium/tonic/blob/master/examples/src/uds/client.rs let channel = Endpoint::from_static("https://not-used.com") .connect_with_connector(service_fn(move |_: Uri| { let path = path.clone(); diff --git a/prompting-client/src/lib.rs b/prompting-client/src/lib.rs index b9f40f5..175254f 100644 --- a/prompting-client/src/lib.rs +++ b/prompting-client/src/lib.rs @@ -1,3 +1,14 @@ +#![warn( + clippy::complexity, + clippy::correctness, + clippy::style, + future_incompatible, + missing_debug_implementations, + rust_2018_idioms, + rustdoc::all, + clippy::undocumented_unsafe_blocks +)] + use prompt_sequence::MatchError; pub mod cli_actions; @@ -11,6 +22,12 @@ mod socket_client; mod util; pub(crate) const SNAP_NAME: &str = "prompting-client"; +pub const SOCKET_ENV_VAR: &str = "PROMPTING_CLIENT_SOCKET"; +pub const DEFAULT_LOG_LEVEL: &str = "info"; + +pub fn log_filter(filter: &str) -> String { + format!("{filter},hyper=error,h2=error") +} // FIXME: having to hard code these is a problem. // We need snapd to provide structured errors we can work with programatically. @@ -63,6 +80,9 @@ pub enum Error { #[error("{interface} is not currently supported for apparmor prompting")] UnsupportedInterface { interface: String }, + + #[error("unable to update log filter: {reason}")] + UnableToUpdateLogFilter { reason: String }, } pub type Result = std::result::Result; diff --git a/prompting-client/src/protos/apparmor_prompting.rs b/prompting-client/src/protos/apparmor_prompting.rs index 8b1b3b0..fab4bb1 100644 --- a/prompting-client/src/protos/apparmor_prompting.rs +++ b/prompting-client/src/protos/apparmor_prompting.rs @@ -146,6 +146,12 @@ pub struct ResolveHomePatternTypeResponse { #[prost(enumeration = "HomePatternType", tag = "1")] pub home_pattern_type: i32, } +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct SetLoggingFilterResponse { + #[prost(string, tag = "1")] + pub current: ::prost::alloc::string::String, +} #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] #[repr(i32)] pub enum Action { @@ -417,6 +423,36 @@ pub mod app_armor_prompting_client { ); self.inner.unary(req, path, codec).await } + pub async fn set_logging_filter( + &mut self, + request: impl tonic::IntoRequest<::prost::alloc::string::String>, + ) -> std::result::Result< + tonic::Response, + tonic::Status, + > { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::new( + tonic::Code::Unknown, + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic::codec::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/apparmor_prompting.AppArmorPrompting/SetLoggingFilter", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new( + "apparmor_prompting.AppArmorPrompting", + "SetLoggingFilter", + ), + ); + self.inner.unary(req, path, codec).await + } } } /// Generated server implementations. @@ -447,6 +483,13 @@ pub mod app_armor_prompting_server { tonic::Response, tonic::Status, >; + async fn set_logging_filter( + &self, + request: tonic::Request<::prost::alloc::string::String>, + ) -> std::result::Result< + tonic::Response, + tonic::Status, + >; } #[derive(Debug)] pub struct AppArmorPromptingServer { @@ -663,6 +706,55 @@ pub mod app_armor_prompting_server { }; Box::pin(fut) } + "/apparmor_prompting.AppArmorPrompting/SetLoggingFilter" => { + #[allow(non_camel_case_types)] + struct SetLoggingFilterSvc(pub Arc); + impl< + T: AppArmorPrompting, + > tonic::server::UnaryService<::prost::alloc::string::String> + for SetLoggingFilterSvc { + type Response = super::SetLoggingFilterResponse; + type Future = BoxFuture< + tonic::Response, + tonic::Status, + >; + fn call( + &mut self, + request: tonic::Request<::prost::alloc::string::String>, + ) -> Self::Future { + let inner = Arc::clone(&self.0); + let fut = async move { + ::set_logging_filter( + &inner, + request, + ) + .await + }; + Box::pin(fut) + } + } + let accept_compression_encodings = self.accept_compression_encodings; + let send_compression_encodings = self.send_compression_encodings; + let max_decoding_message_size = self.max_decoding_message_size; + let max_encoding_message_size = self.max_encoding_message_size; + let inner = self.inner.clone(); + let fut = async move { + let method = SetLoggingFilterSvc(inner); + let codec = tonic::codec::ProstCodec::default(); + let mut grpc = tonic::server::Grpc::new(codec) + .apply_compression_config( + accept_compression_encodings, + send_compression_encodings, + ) + .apply_max_message_size_config( + max_decoding_message_size, + max_encoding_message_size, + ); + let res = grpc.unary(method, req).await; + Ok(res) + }; + Box::pin(fut) + } _ => { Box::pin(async move { Ok( diff --git a/prompting-client/tests/integration.rs b/prompting-client/tests/integration.rs index 5ffd4b6..212f7d4 100644 --- a/prompting-client/tests/integration.rs +++ b/prompting-client/tests/integration.rs @@ -266,8 +266,8 @@ async fn requesting_an_unknown_prompt_id_is_an_error() -> Result<()> { Ok(()) } -#[test_case("not a valid custom path!", "cannot decode request body into prompt reply: cannot parse path pattern"; "malformed path")] -#[test_case("/home/bob/*", "constraints in reply do not match original request"; "invalid path")] +#[test_case("not a valid custom path!", "cannot decode request body into prompt reply: invalid path pattern: pattern must start with"; "malformed path")] +#[test_case("/home/bob/*", "path pattern in reply constraints does not match originally requested path"; "invalid path")] #[tokio::test] #[serial] async fn incorrect_custom_paths_error(reply_path: &str, expected_prefix: &str) -> Result<()> { @@ -313,7 +313,7 @@ async fn invalid_timeperiod_duration_errors(timespan: &str, expected_prefix: &st match c.reply_to_prompt(&id, reply).await { Err(Error::SnapdError { message }) => assert!( - message.starts_with(expected_prefix), + message.starts_with(&format!("invalid duration: {expected_prefix}")), "message format not as expected: {message}" ), Err(e) => panic!("expected a snapd error, got: {e}"), @@ -457,7 +457,9 @@ async fn invalid_prompt_sequence_reply_errors() -> Result<()> { error: MatchError::UnexpectedError { error }, }) => { assert!( - error.starts_with("constraints in reply do not match original request"), + error.starts_with( + "path pattern in reply constraints does not match originally requested path" + ), "{error}" ); } diff --git a/protos/apparmor-prompting.proto b/protos/apparmor-prompting.proto index aa01554..f32a2fb 100644 --- a/protos/apparmor-prompting.proto +++ b/protos/apparmor-prompting.proto @@ -9,6 +9,7 @@ service AppArmorPrompting { rpc GetCurrentPrompt (google.protobuf.Empty) returns (GetCurrentPromptResponse); rpc ReplyToPrompt (PromptReply) returns (PromptReplyResponse); rpc ResolveHomePatternType (google.protobuf.StringValue) returns (ResolveHomePatternTypeResponse); + rpc SetLoggingFilter (google.protobuf.StringValue) returns (SetLoggingFilterResponse); } message PromptReply { @@ -89,3 +90,7 @@ message MetaData { message ResolveHomePatternTypeResponse { HomePatternType home_pattern_type = 1; } + +message SetLoggingFilterResponse { + string current = 1; +} diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 83bf953..45ca2ef 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -37,6 +37,12 @@ apps: restart-condition: on-success restart-delay: 2s + # Doesn't need access to home or snapd + logging-level: + command: bin/prompting-client-set-log-level + extensions: [gnome] + environment: *env + plugs: snap-interfaces-requests-control: handler-service: daemon