move ApiConfig, FileLogger and CommandoSocket to proxmox-rest-server workspace

ApiConfig: avoid using  pbs_config::backup_user()
CommandoSocket: avoid using  pbs_config::backup_user()
FileLogger: avoid using  pbs_config::backup_user()
- use atomic_open_or_create_file()

Auth Trait: moved definitions to proxmox-rest-server/src/lib.rs
- removed CachedUserInfo patrameter
- return user as String (not Authid)

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

proxmox-rest-server/Cargo.toml

@@ -7,3 +7,18 @@ description = "REST server implementation"
 
 [dependencies]
 anyhow = "1.0"
+futures = "0.3"
+handlebars = "3.0"
+http = "0.2"
+hyper = { version = "0.14", features = [ "full" ] }
+lazy_static = "1.4"
+libc = "0.2"
+nix = "0.19.1"
+serde = { version = "1.0", features = [] }
+serde_json = "1.0"
+tokio = { version = "1.6", features = ["signal", "process"] }
+
+proxmox = { version = "0.13.3", features = [ "router"] }
+
+# fixme: remove this dependency (pbs_tools::broadcast_future)
+pbs-tools = { path = "../pbs-tools" }

proxmox-rest-server/src/api_config.rs

@@ -12,8 +12,7 @@ use serde::Serialize;
 use proxmox::api::{ApiMethod, Router, RpcEnvironmentType};
 use proxmox::tools::fs::{create_path, CreateOptions};
 
-use crate::tools::{FileLogger, FileLogOptions};
-use super::auth::ApiAuth;
+use crate::{ApiAuth, FileLogger, FileLogOptions, CommandoSocket};
 
 pub struct ApiConfig {
     basedir: PathBuf,
@@ -134,7 +133,9 @@ impl ApiConfig {
     pub fn enable_file_log<P>(
         &mut self,
         path: P,
-        commando_sock: &mut super::CommandoSocket,
+        dir_opts: Option<CreateOptions>,
+        file_opts: Option<CreateOptions>,
+        commando_sock: &mut CommandoSocket,
     ) -> Result<(), Error>
     where
         P: Into<PathBuf>
@@ -142,15 +143,13 @@ impl ApiConfig {
         let path: PathBuf = path.into();
         if let Some(base) = path.parent() {
             if !base.exists() {
-                let backup_user = pbs_config::backup_user()?;
-                let opts = CreateOptions::new().owner(backup_user.uid).group(backup_user.gid);
-                create_path(base, None, Some(opts)).map_err(|err| format_err!("{}", err))?;
+                create_path(base, None, dir_opts).map_err(|err| format_err!("{}", err))?;
             }
         }
 
         let logger_options = FileLogOptions {
             append: true,
-            owned_by_backup: true,
+            file_opts: file_opts.unwrap_or(CreateOptions::default()),
             ..Default::default()
         };
         let request_log = Arc::new(Mutex::new(FileLogger::new(&path, logger_options)?));

proxmox-rest-server/src/command_socket.rs

@@ -10,17 +10,17 @@ use tokio::net::UnixListener;
 use serde::Serialize;
 use serde_json::Value;
 use nix::sys::socket;
+use nix::unistd::Gid;
 
-/// Listens on a Unix Socket to handle simple command asynchronously
-fn create_control_socket<P, F>(path: P, func: F) -> Result<impl Future<Output = ()>, Error>
+// Listens on a Unix Socket to handle simple command asynchronously
+fn create_control_socket<P, F>(path: P, gid: Gid, func: F) -> Result<impl Future<Output = ()>, Error>
 where
     P: Into<PathBuf>,
     F: Fn(Value) -> Result<Value, Error> + Send + Sync + 'static,
 {
     let path: PathBuf = path.into();
 
-    let backup_user = pbs_config::backup_user()?;
-    let backup_gid = backup_user.gid.as_raw();
+    let gid = gid.as_raw();
 
     let socket = UnixListener::bind(&path)?;
 
@@ -47,7 +47,7 @@ where
 
             // check permissions (same gid, root user, or backup group)
             let mygid = unsafe { libc::getgid() };
-            if !(cred.uid() == 0 || cred.gid() == mygid || cred.gid() == backup_gid) {
+            if !(cred.uid() == 0 || cred.gid() == mygid || cred.gid() == gid) {
                 eprintln!("no permissions for {:?}", cred);
                 continue;
             }
@@ -93,7 +93,7 @@ where
         }
     }.boxed();
 
-    let abort_future = super::last_worker_future().map_err(|_| {});
+    let abort_future = crate::last_worker_future().map_err(|_| {});
     let task = futures::future::select(
         control_future,
         abort_future,
@@ -154,15 +154,17 @@ pub type CommandoSocketFn = Box<(dyn Fn(Option<&Value>) -> Result<Value, Error>
 /// You need to call `spawn()` to make the socket active.
 pub struct CommandoSocket {
     socket: PathBuf,
+    gid: Gid,
     commands: HashMap<String, CommandoSocketFn>,
 }
 
 impl CommandoSocket {
-    pub fn new<P>(path: P) -> Self
+    pub fn new<P>(path: P, gid: Gid) -> Self
         where P: Into<PathBuf>,
     {
         CommandoSocket {
             socket: path.into(),
+            gid,
             commands: HashMap::new(),
         }
     }
@@ -170,7 +172,7 @@ impl CommandoSocket {
     /// Spawn the socket and consume self, meaning you cannot register commands anymore after
     /// calling this.
     pub fn spawn(self) -> Result<(), Error> {
-        let control_future = create_control_socket(self.socket.to_owned(), move |param| {
+        let control_future = create_control_socket(self.socket.to_owned(), self.gid, move |param| {
             let param = param
                 .as_object()
                 .ok_or_else(|| format_err!("unable to parse parameters (expected json object)"))?;

proxmox-rest-server/src/file_logger.rs

@@ -1,6 +1,10 @@
-use anyhow::Error;
 use std::io::Write;
 
+use anyhow::Error;
+use nix::fcntl::OFlag;
+
+use proxmox::tools::fs::{CreateOptions, atomic_open_or_create_file};
+
 /// Log messages with optional automatically added timestamps into files
 ///
 /// Logs messages to file, and optionally to standard output.
@@ -9,8 +13,7 @@ use std::io::Write;
 /// #### Example:
 /// ```
 /// # use anyhow::{bail, format_err, Error};
-/// use proxmox_backup::flog;
-/// use proxmox_backup::tools::{FileLogger, FileLogOptions};
+/// use proxmox_rest_server::{flog, FileLogger, FileLogOptions};
 ///
 /// # std::fs::remove_file("test.log");
 /// let options = FileLogOptions {
@@ -23,7 +26,7 @@ use std::io::Write;
 /// # std::fs::remove_file("test.log");
 /// ```
 
-#[derive(Debug, Default)]
+#[derive(Default)]
 /// Options to control the behavior of a ['FileLogger'] instance
 pub struct FileLogOptions {
     /// Open underlying log file in append mode, useful when multiple concurrent processes
@@ -39,13 +42,11 @@ pub struct FileLogOptions {
     pub to_stdout: bool,
     /// Prefix messages logged to the file with the current local time as RFC 3339
     pub prefix_time: bool,
-    /// if set, the file is tried to be chowned by the backup:backup user/group
-    /// Note, this is not designed race free as anybody could set it to another user afterwards
-    /// anyway. It must thus be used by all processes which doe not run as backup uid/gid.
-    pub owned_by_backup: bool,
+    /// File owner/group and mode
+    pub file_opts: CreateOptions,
+
 }
 
-#[derive(Debug)]
 pub struct FileLogger {
     file: std::fs::File,
     file_name: std::path::PathBuf,
@@ -82,19 +83,24 @@ impl FileLogger {
         file_name: P,
         options: &FileLogOptions,
     ) -> Result<std::fs::File, Error> {
-        let file = std::fs::OpenOptions::new()
-            .read(options.read)
-            .write(true)
-            .append(options.append)
-            .create_new(options.exclusive)
-            .create(!options.exclusive)
-            .open(&file_name)?;
 
-        if options.owned_by_backup {
-            let backup_user = pbs_config::backup_user()?;
-            nix::unistd::chown(file_name.as_ref(), Some(backup_user.uid), Some(backup_user.gid))?;
+        let mut flags = OFlag::O_CLOEXEC;
+
+        if options.read  {
+            flags |=  OFlag::O_RDWR;
+        } else {
+            flags |=  OFlag::O_WRONLY;
         }
 
+        if options.append {
+            flags |=  OFlag::O_APPEND;
+        }
+        if options.exclusive {
+            flags |=  OFlag::O_EXCL;
+        }
+
+        let file = atomic_open_or_create_file(&file_name, flags, &[], options.file_opts.clone())?;
+
         Ok(file)
     }
 

proxmox-rest-server/src/lib.rs

@@ -0,0 +1,54 @@
+use anyhow::{bail, Error};
+
+mod state;
+pub use state::*;
+
+mod command_socket;
+pub use command_socket::*;
+
+mod file_logger;
+pub use file_logger::{FileLogger, FileLogOptions};
+
+mod api_config;
+pub use api_config::ApiConfig;
+
+pub enum AuthError {
+    Generic(Error),
+    NoData,
+}
+
+impl From<Error> for AuthError {
+    fn from(err: Error) -> Self {
+        AuthError::Generic(err)
+    }
+}
+
+pub trait ApiAuth {
+    fn check_auth(
+        &self,
+        headers: &http::HeaderMap,
+        method: &hyper::Method,
+    ) -> Result<String, AuthError>;
+}
+
+static mut SHUTDOWN_REQUESTED: bool = false;
+
+pub fn request_shutdown() {
+    unsafe {
+        SHUTDOWN_REQUESTED = true;
+    }
+    crate::server_shutdown();
+}
+
+#[inline(always)]
+pub fn shutdown_requested() -> bool {
+    unsafe { SHUTDOWN_REQUESTED }
+}
+
+pub fn fail_on_shutdown() -> Result<(), Error> {
+    if shutdown_requested() {
+        bail!("Server shutdown requested - aborting task");
+    }
+    Ok(())
+}
+

proxmox-rest-server/src/state.rs

@@ -42,7 +42,7 @@ pub fn server_state_init() -> Result<(), Error> {
         while stream.recv().await.is_some() {
             println!("got shutdown request (SIGINT)");
             SERVER_STATE.lock().unwrap().reload_request = false;
-            crate::tools::request_shutdown();
+            crate::request_shutdown();
         }
     }.boxed();
 
@@ -57,7 +57,7 @@ pub fn server_state_init() -> Result<(), Error> {
         while stream.recv().await.is_some() {
             println!("got reload request (SIGHUP)");
             SERVER_STATE.lock().unwrap().reload_request = true;
-            crate::tools::request_shutdown();
+            crate::request_shutdown();
         }
     }.boxed();
 

src/server/rest.rs

@@ -29,21 +29,20 @@ use proxmox::api::{
     RpcEnvironmentType,
 };
 use proxmox::http_err;
+use proxmox::tools::fs::CreateOptions;
 
 use pbs_tools::compression::{DeflateEncoder, Level};
 use pbs_tools::stream::AsyncReaderStream;
 use pbs_api_types::{Authid, Userid};
+use proxmox_rest_server::{ApiConfig, FileLogger, FileLogOptions, AuthError};
 
-use super::auth::AuthError;
 use super::environment::RestEnvironment;
 use super::formatter::*;
-use super::ApiConfig;
 
 use crate::auth_helpers::*;
 use pbs_config::CachedUserInfo;
 use crate::tools;
 use crate::tools::compression::CompressionMethod;
-use crate::tools::FileLogger;
 
 extern "C" {
     fn tzset();
@@ -196,10 +195,16 @@ fn log_response(
     }
 }
 pub fn auth_logger() -> Result<FileLogger, Error> {
-    let logger_options = tools::FileLogOptions {
+    let backup_user = pbs_config::backup_user()?;
+
+    let file_opts = CreateOptions::new()
+        .owner(backup_user.uid)
+        .group(backup_user.gid);
+
+    let logger_options = FileLogOptions {
         append: true,
         prefix_time: true,
-        owned_by_backup: true,
+        file_opts,
         ..Default::default()
     };
     FileLogger::new(pbs_buildcfg::API_AUTH_LOG_FN, logger_options)
@@ -681,7 +686,6 @@ async fn handle_request(
 
     rpcenv.set_client_ip(Some(*peer));
 
-    let user_info = CachedUserInfo::new()?;
     let auth = &api.api_auth;
 
     let delay_unauth_time = std::time::Instant::now() + std::time::Duration::from_millis(3000);
@@ -708,8 +712,8 @@ async fn handle_request(
             }
 
             if auth_required {
-                match auth.check_auth(&parts.headers, &method, &user_info) {
-                    Ok(authid) => rpcenv.set_auth_id(Some(authid.to_string())),
+                match auth.check_auth(&parts.headers, &method) {
+                    Ok(authid) => rpcenv.set_auth_id(Some(authid)),
                     Err(auth_err) => {
                         let err = match auth_err {
                             AuthError::Generic(err) => err,
@@ -738,6 +742,8 @@ async fn handle_request(
                 }
                 Some(api_method) => {
                     let auth_id = rpcenv.get_auth_id();
+                    let user_info = CachedUserInfo::new()?;
+
                     if !check_api_permission(
                         api_method.access.permission,
                         auth_id.as_deref(),
@@ -779,8 +785,9 @@ async fn handle_request(
 
         if comp_len == 0 {
             let language = extract_lang_header(&parts.headers);
-            match auth.check_auth(&parts.headers, &method, &user_info) {
+            match auth.check_auth(&parts.headers, &method) {
                 Ok(auth_id) => {
+                    let auth_id: Authid = auth_id.parse()?;
                     if !auth_id.is_token() {
                         let userid = auth_id.user();
                         let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), userid);

src/tools/daemon.rs

@@ -16,7 +16,6 @@ use futures::future::{self, Either};
 
 use proxmox::tools::io::{ReadExt, WriteExt};
 
-use crate::server;
 use crate::tools::{fd_change_cloexec, self};
 
 #[link(name = "systemd")]
@@ -274,11 +273,11 @@ where
     ).await?;
 
     let server_future = create_service(listener, NotifyReady)?;
-    let shutdown_future = server::shutdown_future();
+    let shutdown_future = proxmox_rest_server::shutdown_future();
 
     let finish_future = match future::select(server_future, shutdown_future).await {
         Either::Left((_, _)) => {
-            crate::tools::request_shutdown(); // make sure we are in shutdown mode
+            proxmox_rest_server::request_shutdown(); // make sure we are in shutdown mode
             None
         }
         Either::Right((_, server_future)) => Some(server_future),
@@ -286,7 +285,7 @@ where
 
     let mut reloader = Some(reloader);
 
-    if server::is_reload_request() {
+    if proxmox_rest_server::is_reload_request() {
         log::info!("daemon reload...");
         if let Err(e) = systemd_notify(SystemdNotify::Reloading) {
             log::error!("failed to notify systemd about the state change: {}", e);
@@ -305,7 +304,7 @@ where
     }
 
     // FIXME: this is a hack, replace with sd_notify_barrier when available
-    if server::is_reload_request() {
+    if proxmox_rest_server::is_reload_request() {
         wait_service_is_not_state(service_name, "reloading").await?;
     }