[pbs-devel] [PATCH v3 proxmox-backup 09/20] server/rest: add ApiAuth trait to make user auth generic

[Stefan Reiter <s.reiter@proxmox.com>]

This allows switching the base user identification/authentication method
in the rest server. Will initially be used for single file restore VMs,
where authentication is based on a ticket file, not the PBS user
backend (PAM/local).

To avoid putting generic types into the RestServer type for this, we
merge the two calls "extract_auth_data" and "check_auth" into a single
one, which can use whatever type it wants internally.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---

v3:
* merge both calls into one trait, that way it doesn't have to be generic

 src/bin/proxmox-backup-api.rs   |  13 ++-
 src/bin/proxmox-backup-proxy.rs |   7 +-
 src/server/auth.rs              | 192 +++++++++++++++++++-------------
 src/server/config.rs            |  13 ++-
 src/server/rest.rs              |  36 +++---
 5 files changed, 159 insertions(+), 102 deletions(-)

diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 7d800259..e514a801 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -6,8 +6,11 @@ use proxmox::api::RpcEnvironmentType;
 
 //use proxmox_backup::tools;
 //use proxmox_backup::api_schema::config::*;
-use proxmox_backup::server::rest::*;
-use proxmox_backup::server;
+use proxmox_backup::server::{
+    self,
+    auth::default_api_auth,
+    rest::*,
+};
 use proxmox_backup::tools::daemon;
 use proxmox_backup::auth_helpers::*;
 use proxmox_backup::config;
@@ -53,7 +56,11 @@ async fn run() -> Result<(), Error> {
     let _ = csrf_secret(); // load with lazy_static
 
     let mut config = server::ApiConfig::new(
-        buildcfg::JS_DIR, &proxmox_backup::api2::ROUTER, RpcEnvironmentType::PRIVILEGED)?;
+        buildcfg::JS_DIR,
+        &proxmox_backup::api2::ROUTER,
+        RpcEnvironmentType::PRIVILEGED,
+        default_api_auth(),
+    )?;
 
     let mut commando_sock = server::CommandoSocket::new(server::our_ctrl_sock());
 
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 541d34b5..7e026455 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -14,6 +14,7 @@ use proxmox::api::RpcEnvironmentType;
 use proxmox_backup::{
     backup::DataStore,
     server::{
+        auth::default_api_auth,
         WorkerTask,
         ApiConfig,
         rest::*,
@@ -84,7 +85,11 @@ async fn run() -> Result<(), Error> {
     let _ = csrf_secret(); // load with lazy_static
 
     let mut config = ApiConfig::new(
-        buildcfg::JS_DIR, &proxmox_backup::api2::ROUTER, RpcEnvironmentType::PUBLIC)?;
+        buildcfg::JS_DIR,
+        &proxmox_backup::api2::ROUTER,
+        RpcEnvironmentType::PUBLIC,
+        default_api_auth(),
+    )?;
 
     // Enable experimental tape UI if tape.cfg exists
     if Path::new("/etc/proxmox-backup/tape.cfg").exists() {
diff --git a/src/server/auth.rs b/src/server/auth.rs
index 24151886..0a9a740c 100644
--- a/src/server/auth.rs
+++ b/src/server/auth.rs
@@ -1,102 +1,140 @@
 //! Provides authentication primitives for the HTTP server
-use anyhow::{bail, format_err, Error};
+use anyhow::{format_err, Error};
+
+use std::sync::Arc;
 
-use crate::tools::ticket::Ticket;
-use crate::auth_helpers::*;
-use crate::tools;
-use crate::config::cached_user_info::CachedUserInfo;
 use crate::api2::types::{Authid, Userid};
+use crate::auth_helpers::*;
+use crate::config::cached_user_info::CachedUserInfo;
+use crate::tools;
+use crate::tools::ticket::Ticket;
 
 use hyper::header;
 use percent_encoding::percent_decode_str;
 
-pub struct UserAuthData {
+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,
+        user_info: &CachedUserInfo,
+    ) -> Result<Authid, AuthError>;
+}
+
+struct UserAuthData {
     ticket: String,
     csrf_token: Option<String>,
 }
 
-pub enum AuthData {
+enum AuthData {
     User(UserAuthData),
     ApiToken(String),
 }
 
-pub fn extract_auth_data(headers: &http::HeaderMap) -> Option<AuthData> {
-    if let Some(raw_cookie) = headers.get(header::COOKIE) {
-        if let Ok(cookie) = raw_cookie.to_str() {
-            if let Some(ticket) = tools::extract_cookie(cookie, "PBSAuthCookie") {
-                let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
-                    Some(Ok(v)) => Some(v.to_owned()),
-                    _ => None,
-                };
-                return Some(AuthData::User(UserAuthData {
-                    ticket,
-                    csrf_token,
-                }));
-            }
-        }
-    }
-
-    match headers.get(header::AUTHORIZATION).map(|v| v.to_str()) {
-        Some(Ok(v)) => {
-            if v.starts_with("PBSAPIToken ") || v.starts_with("PBSAPIToken=") {
-                Some(AuthData::ApiToken(v["PBSAPIToken ".len()..].to_owned()))
-            } else {
-                None
-            }
-        },
-        _ => None,
-    }
+pub struct UserApiAuth {}
+pub fn default_api_auth() -> Arc<UserApiAuth> {
+    Arc::new(UserApiAuth {})
 }
 
-pub fn check_auth(
-    method: &hyper::Method,
-    auth_data: &AuthData,
-    user_info: &CachedUserInfo,
-) -> Result<Authid, Error> {
-    match auth_data {
-        AuthData::User(user_auth_data) => {
-            let ticket = user_auth_data.ticket.clone();
-            let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
-
-            let userid: Userid = Ticket::<super::ticket::ApiTicket>::parse(&ticket)?
-                .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?
-                .require_full()?;
-
-            let auth_id = Authid::from(userid.clone());
-            if !user_info.is_active_auth_id(&auth_id) {
-                bail!("user account disabled or expired.");
-            }
-
-            if method != hyper::Method::GET {
-                if let Some(csrf_token) = &user_auth_data.csrf_token {
-                    verify_csrf_prevention_token(csrf_secret(), &userid, &csrf_token, -300, ticket_lifetime)?;
-                } else {
-                    bail!("missing CSRF prevention token");
+impl UserApiAuth {
+    fn extract_auth_data(headers: &http::HeaderMap) -> Option<AuthData> {
+        if let Some(raw_cookie) = headers.get(header::COOKIE) {
+            if let Ok(cookie) = raw_cookie.to_str() {
+                if let Some(ticket) = tools::extract_cookie(cookie, "PBSAuthCookie") {
+                    let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
+                        Some(Ok(v)) => Some(v.to_owned()),
+                        _ => None,
+                    };
+                    return Some(AuthData::User(UserAuthData { ticket, csrf_token }));
                 }
             }
+        }
 
-            Ok(auth_id)
-        },
-        AuthData::ApiToken(api_token) => {
-            let mut parts = api_token.splitn(2, ':');
-            let tokenid = parts.next()
-                .ok_or_else(|| format_err!("failed to split API token header"))?;
-            let tokenid: Authid = tokenid.parse()?;
-
-            if !user_info.is_active_auth_id(&tokenid) {
-                bail!("user account or token disabled or expired.");
+        match headers.get(header::AUTHORIZATION).map(|v| v.to_str()) {
+            Some(Ok(v)) => {
+                if v.starts_with("PBSAPIToken ") || v.starts_with("PBSAPIToken=") {
+                    Some(AuthData::ApiToken(v["PBSAPIToken ".len()..].to_owned()))
+                } else {
+                    None
+                }
             }
-
-            let tokensecret = parts.next()
-                .ok_or_else(|| format_err!("failed to split API token header"))?;
-            let tokensecret = percent_decode_str(tokensecret)
-                .decode_utf8()
-                .map_err(|_| format_err!("failed to decode API token header"))?;
-
-            crate::config::token_shadow::verify_secret(&tokenid, &tokensecret)?;
-
-            Ok(tokenid)
+            _ => None,
         }
     }
 }
 
+impl ApiAuth for UserApiAuth {
+    fn check_auth(
+        &self,
+        headers: &http::HeaderMap,
+        method: &hyper::Method,
+        user_info: &CachedUserInfo,
+    ) -> Result<Authid, AuthError> {
+        let auth_data = Self::extract_auth_data(headers);
+        match auth_data {
+            Some(AuthData::User(user_auth_data)) => {
+                let ticket = user_auth_data.ticket.clone();
+                let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
+
+                let userid: Userid = Ticket::<super::ticket::ApiTicket>::parse(&ticket)?
+                    .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?
+                    .require_full()?;
+
+                let auth_id = Authid::from(userid.clone());
+                if !user_info.is_active_auth_id(&auth_id) {
+                    return Err(format_err!("user account disabled or expired.").into());
+                }
+
+                if method != hyper::Method::GET {
+                    if let Some(csrf_token) = &user_auth_data.csrf_token {
+                        verify_csrf_prevention_token(
+                            csrf_secret(),
+                            &userid,
+                            &csrf_token,
+                            -300,
+                            ticket_lifetime,
+                        )?;
+                    } else {
+                        return Err(format_err!("missing CSRF prevention token").into());
+                    }
+                }
+
+                Ok(auth_id)
+            }
+            Some(AuthData::ApiToken(api_token)) => {
+                let mut parts = api_token.splitn(2, ':');
+                let tokenid = parts
+                    .next()
+                    .ok_or_else(|| format_err!("failed to split API token header"))?;
+                let tokenid: Authid = tokenid.parse()?;
+
+                if !user_info.is_active_auth_id(&tokenid) {
+                    return Err(format_err!("user account or token disabled or expired.").into());
+                }
+
+                let tokensecret = parts
+                    .next()
+                    .ok_or_else(|| format_err!("failed to split API token header"))?;
+                let tokensecret = percent_decode_str(tokensecret)
+                    .decode_utf8()
+                    .map_err(|_| format_err!("failed to decode API token header"))?;
+
+                crate::config::token_shadow::verify_secret(&tokenid, &tokensecret)?;
+
+                Ok(tokenid)
+            }
+            None => Err(AuthError::NoData),
+        }
+    }
+}
diff --git a/src/server/config.rs b/src/server/config.rs
index 9094fa80..ad378b0a 100644
--- a/src/server/config.rs
+++ b/src/server/config.rs
@@ -13,6 +13,7 @@ use proxmox::api::{ApiMethod, Router, RpcEnvironmentType};
 use proxmox::tools::fs::{create_path, CreateOptions};
 
 use crate::tools::{FileLogger, FileLogOptions};
+use super::auth::ApiAuth;
 
 pub struct ApiConfig {
     basedir: PathBuf,
@@ -23,11 +24,16 @@ pub struct ApiConfig {
     template_files: RwLock<HashMap<String, (SystemTime, PathBuf)>>,
     request_log: Option<Arc<Mutex<FileLogger>>>,
     pub enable_tape_ui: bool,
+    pub api_auth: Arc<dyn ApiAuth + Send + Sync>,
 }
 
 impl ApiConfig {
-
-    pub fn new<B: Into<PathBuf>>(basedir: B, router: &'static Router, env_type: RpcEnvironmentType) -> Result<Self, Error> {
+    pub fn new<B: Into<PathBuf>>(
+        basedir: B,
+        router: &'static Router,
+        env_type: RpcEnvironmentType,
+        api_auth: Arc<dyn ApiAuth + Send + Sync>,
+    ) -> Result<Self, Error> {
         Ok(Self {
             basedir: basedir.into(),
             router,
@@ -37,7 +43,8 @@ impl ApiConfig {
             template_files: RwLock::new(HashMap::new()),
             request_log: None,
             enable_tape_ui: false,
-       })
+            api_auth,
+        })
     }
 
     pub fn find_method(
diff --git a/src/server/rest.rs b/src/server/rest.rs
index 9a971890..2d033510 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -14,7 +14,6 @@ use hyper::header::{self, HeaderMap};
 use hyper::http::request::Parts;
 use hyper::{Body, Request, Response, StatusCode};
 use lazy_static::lazy_static;
-use percent_encoding::percent_decode_str;
 use regex::Regex;
 use serde_json::{json, Value};
 use tokio::fs::File;
@@ -31,16 +30,15 @@ use proxmox::api::{
 };
 use proxmox::http_err;
 
+use super::auth::AuthError;
 use super::environment::RestEnvironment;
 use super::formatter::*;
 use super::ApiConfig;
-use super::auth::{check_auth, extract_auth_data};
 
 use crate::api2::types::{Authid, Userid};
 use crate::auth_helpers::*;
 use crate::config::cached_user_info::CachedUserInfo;
 use crate::tools;
-use crate::tools::ticket::Ticket;
 use crate::tools::FileLogger;
 
 extern "C" {
@@ -614,6 +612,7 @@ 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);
     let access_forbidden_time = std::time::Instant::now() + std::time::Duration::from_millis(500);
@@ -639,13 +638,15 @@ async fn handle_request(
             }
 
             if auth_required {
-                let auth_result = match extract_auth_data(&parts.headers) {
-                    Some(auth_data) => check_auth(&method, &auth_data, &user_info),
-                    None => Err(format_err!("no authentication credentials provided.")),
-                };
-                match auth_result {
+                match auth.check_auth(&parts.headers, &method, &user_info) {
                     Ok(authid) => rpcenv.set_auth_id(Some(authid.to_string())),
-                    Err(err) => {
+                    Err(auth_err) => {
+                        let err = match auth_err {
+                            AuthError::Generic(err) => err,
+                            AuthError::NoData => {
+                                format_err!("no authentication credentials provided.")
+                            }
+                        };
                         let peer = peer.ip();
                         auth_logger()?.log(format!(
                             "authentication failure; rhost={} msg={}",
@@ -708,9 +709,9 @@ async fn handle_request(
 
         if comp_len == 0 {
             let language = extract_lang_header(&parts.headers);
-            if let Some(auth_data) = extract_auth_data(&parts.headers) {
-                match check_auth(&method, &auth_data, &user_info) {
-                    Ok(auth_id) if !auth_id.is_token() => {
+            match auth.check_auth(&parts.headers, &method, &user_info) {
+                Ok(auth_id) => {
+                    if !auth_id.is_token() {
                         let userid = auth_id.user();
                         let new_csrf_token = assemble_csrf_prevention_token(csrf_secret(), userid);
                         return Ok(get_index(
@@ -721,14 +722,13 @@ async fn handle_request(
                             parts,
                         ));
                     }
-                    _ => {
-                        tokio::time::sleep_until(Instant::from_std(delay_unauth_time)).await;
-                        return Ok(get_index(None, None, language, &api, parts));
-                    }
                 }
-            } else {
-                return Ok(get_index(None, None, language, &api, parts));
+                Err(AuthError::Generic(_)) => {
+                    tokio::time::sleep_until(Instant::from_std(delay_unauth_time)).await;
+                }
+                Err(AuthError::NoData) => {}
             }
+            return Ok(get_index(None, None, language, &api, parts));
         } else {
             let filename = api.find_alias(&components);
             return handle_static_file_download(filename).await;
-- 
2.20.1