api: tfa management and login

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

Cargo.toml

@@ -68,6 +68,7 @@ udev = ">= 0.3, <0.5"
 url = "2.1"
 #valgrind_request = { git = "https://github.com/edef1c/libvalgrind_request", version = "1.1.0", optional = true }
 walkdir = "2"
+webauthn-rs = "0.2.5"
 xdg = "2.2"
 zstd = { version = "0.4", features = [ "bindgen" ] }
 nom = "5.1"

src/api2/access.rs

@@ -4,33 +4,46 @@ use serde_json::{json, Value};
 use std::collections::HashMap;
 use std::collections::HashSet;
 
-use proxmox::api::{api, RpcEnvironment, Permission};
 use proxmox::api::router::{Router, SubdirMap};
-use proxmox::{sortable, identity};
+use proxmox::api::{api, Permission, RpcEnvironment};
 use proxmox::{http_err, list_subdirs_api_method};
+use proxmox::{identity, sortable};
 
-use crate::tools::ticket::{self, Empty, Ticket};
-use crate::auth_helpers::*;
 use crate::api2::types::*;
+use crate::auth_helpers::*;
+use crate::server::ticket::ApiTicket;
+use crate::tools::ticket::{self, Empty, Ticket};
 
 use crate::config::acl as acl_config;
-use crate::config::acl::{PRIVILEGES, PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
+use crate::config::acl::{PRIVILEGES, PRIV_PERMISSIONS_MODIFY, PRIV_SYS_AUDIT};
 use crate::config::cached_user_info::CachedUserInfo;
+use crate::config::tfa::TfaChallenge;
 
-pub mod user;
-pub mod domain;
 pub mod acl;
+pub mod domain;
 pub mod role;
+pub mod tfa;
+pub mod user;
+
+enum AuthResult {
+    /// Successful authentication which does not require a new ticket.
+    Success,
+
+    /// Successful authentication which requires a ticket to be created.
+    CreateTicket,
+
+    /// A partial ticket which requires a 2nd factor will be created.
+    Partial(TfaChallenge),
+}
 
-/// returns Ok(true) if a ticket has to be created
-/// and Ok(false) if not
 fn authenticate_user(
     userid: &Userid,
     password: &str,
     path: Option<String>,
     privs: Option<String>,
     port: Option<u16>,
-) -> Result<bool, Error> {
+    tfa_challenge: Option<String>,
+) -> Result<AuthResult, Error> {
     let user_info = CachedUserInfo::new()?;
 
     let auth_id = Authid::from(userid.clone());
@@ -38,12 +51,16 @@ fn authenticate_user(
         bail!("user account disabled or expired.");
     }
 
+    if let Some(tfa_challenge) = tfa_challenge {
+        return authenticate_2nd(userid, &tfa_challenge, password);
+    }
+
     if password.starts_with("PBS:") {
         if let Ok(ticket_userid) = Ticket::<Userid>::parse(password)
             .and_then(|ticket| ticket.verify(public_auth_key(), "PBS", None))
         {
             if *userid == ticket_userid {
-                return Ok(true);
+                return Ok(AuthResult::CreateTicket);
             }
             bail!("ticket login failed - wrong userid");
         }
@@ -53,17 +70,17 @@ fn authenticate_user(
         }
 
         let path = path.ok_or_else(|| format_err!("missing path for termproxy ticket"))?;
-        let privilege_name = privs
+        let privilege_name =
-            .ok_or_else(|| format_err!("missing privilege name for termproxy ticket"))?;
+            privs.ok_or_else(|| format_err!("missing privilege name for termproxy ticket"))?;
         let port = port.ok_or_else(|| format_err!("missing port for termproxy ticket"))?;
 
-        if let Ok(Empty) = Ticket::parse(password)
+        if let Ok(Empty) = Ticket::parse(password).and_then(|ticket| {
-            .and_then(|ticket| ticket.verify(
+            ticket.verify(
                 public_auth_key(),
                 ticket::TERM_PREFIX,
                 Some(&ticket::term_aad(userid, &path, port)),
-            ))
+            )
-        {
+        }) {
             for (name, privilege) in PRIVILEGES {
                 if *name == privilege_name {
                     let mut path_vec = Vec::new();
@@ -73,7 +90,7 @@ fn authenticate_user(
                         }
                     }
                     user_info.check_privs(&auth_id, &path_vec, *privilege, false)?;
-                    return Ok(false);
+                    return Ok(AuthResult::Success);
                 }
             }
 
@@ -81,8 +98,26 @@ fn authenticate_user(
         }
     }
 
-    let _ = crate::auth::authenticate_user(userid, password)?;
+    let _: () = crate::auth::authenticate_user(userid, password)?;
-    Ok(true)
+
+    Ok(match crate::config::tfa::login_challenge(userid)? {
+        None => AuthResult::CreateTicket,
+        Some(challenge) => AuthResult::Partial(challenge),
+    })
+}
+
+fn authenticate_2nd(
+    userid: &Userid,
+    challenge_ticket: &str,
+    response: &str,
+) -> Result<AuthResult, Error> {
+    let challenge: TfaChallenge = Ticket::<ApiTicket>::parse(&challenge_ticket)?
+        .verify_with_time_frame(public_auth_key(), "PBS", Some(userid.as_str()), -120..240)?
+        .require_partial()?;
+
+    let _: () = crate::config::tfa::verify_challenge(userid, &challenge, response.parse()?)?;
+
+    Ok(AuthResult::CreateTicket)
 }
 
 #[api(
@@ -109,6 +144,11 @@ fn authenticate_user(
                 description: "Port for verifying terminal tickets.",
                 optional: true,
             },
+            "tfa-challenge": {
+                type: String,
+                description: "The signed TFA challenge string the user wants to respond to.",
+                optional: true,
+            },
         },
     },
     returns: {
@@ -141,15 +181,18 @@ fn create_ticket(
     path: Option<String>,
     privs: Option<String>,
     port: Option<u16>,
+    tfa_challenge: Option<String>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-    match authenticate_user(&username, &password, path, privs, port) {
+    match authenticate_user(&username, &password, path, privs, port, tfa_challenge) {
-        Ok(true) => {
+        Ok(AuthResult::Success) => Ok(json!({ "username": username })),
-            let ticket = Ticket::new("PBS", &username)?.sign(private_auth_key(), None)?;
+        Ok(AuthResult::CreateTicket) => {
-
+            let api_ticket = ApiTicket::full(username.clone());
+            let ticket = Ticket::new("PBS", &api_ticket)?.sign(private_auth_key(), None)?;
             let token = assemble_csrf_prevention_token(csrf_secret(), &username);
 
-            crate::server::rest::auth_logger()?.log(format!("successful auth for user '{}'", username));
+            crate::server::rest::auth_logger()?
+                .log(format!("successful auth for user '{}'", username));
 
             Ok(json!({
                 "username": username,
@@ -157,9 +200,15 @@ fn create_ticket(
                 "CSRFPreventionToken": token,
             }))
         }
-        Ok(false) => Ok(json!({
+        Ok(AuthResult::Partial(challenge)) => {
-            "username": username,
+            let api_ticket = ApiTicket::partial(challenge);
-        })),
+            let ticket = Ticket::new("PBS", &api_ticket)?
+                .sign(private_auth_key(), Some(username.as_str()))?;
+            Ok(json!({
+                "username": username,
+                "ticket": ticket,
+            }))
+        }
         Err(err) => {
             let client_ip = match rpcenv.get_client_ip().map(|addr| addr.ip()) {
                 Some(ip) => format!("{}", ip),
@@ -219,12 +268,16 @@ fn change_password(
 
     let mut allowed = userid == *current_user;
 
-    if current_user == "root@pam" { allowed = true; }
+    if current_user == "root@pam" {
+        allowed = true;
+    }
 
     if !allowed {
         let user_info = CachedUserInfo::new()?;
         let privs = user_info.lookup_privs(&current_auth, &[]);
-        if (privs & PRIV_PERMISSIONS_MODIFY) != 0 { allowed = true; }
+        if (privs & PRIV_PERMISSIONS_MODIFY) != 0 {
+            allowed = true;
+        }
     }
 
     if !allowed {
@@ -281,12 +334,13 @@ pub fn list_permissions(
                     auth_id
                 } else if auth_id.is_token()
                     && !current_auth_id.is_token()
-                    && auth_id.user() == current_auth_id.user() {
+                    && auth_id.user() == current_auth_id.user()
+                {
                     auth_id
                 } else {
                     bail!("not allowed to list permissions of {}", auth_id);
                 }
-            },
+            }
             None => current_auth_id,
         }
     } else {
@@ -296,11 +350,10 @@ pub fn list_permissions(
         }
     };
 
-
     fn populate_acl_paths(
         mut paths: HashSet<String>,
         node: acl_config::AclTreeNode,
-        path: &str
+        path: &str,
     ) -> HashSet<String> {
         for (sub_path, child_node) in node.children {
             let sub_path = format!("{}/{}", path, &sub_path);
@@ -315,7 +368,7 @@ pub fn list_permissions(
             let mut paths = HashSet::new();
             paths.insert(path);
             paths
-        },
+        }
         None => {
             let mut paths = HashSet::new();
 
@@ -330,31 +383,35 @@ pub fn list_permissions(
             paths.insert("/system".to_string());
 
             paths
-        },
+        }
     };
 
-    let map = paths
+    let map = paths.into_iter().fold(
-        .into_iter()
+        HashMap::new(),
-        .fold(HashMap::new(), |mut map: HashMap<String, HashMap<String, bool>>, path: String| {
+        |mut map: HashMap<String, HashMap<String, bool>>, path: String| {
             let split_path = acl_config::split_acl_path(path.as_str());
             let (privs, propagated_privs) = user_info.lookup_privs_details(&auth_id, &split_path);
 
             match privs {
                 0 => map, // Don't leak ACL paths where we don't have any privileges
                 _ => {
-                    let priv_map = PRIVILEGES
+                    let priv_map =
-                        .iter()
+                        PRIVILEGES
-                        .fold(HashMap::new(), |mut priv_map, (name, value)| {
+                            .iter()
-                            if value & privs != 0 {
+                            .fold(HashMap::new(), |mut priv_map, (name, value)| {
-                                priv_map.insert(name.to_string(), value & propagated_privs != 0);
+                                if value & privs != 0 {
-                            }
+                                    priv_map
-                            priv_map
+                                        .insert(name.to_string(), value & propagated_privs != 0);
-                        });
+                                }
+                                priv_map
+                            });
 
                     map.insert(path, priv_map);
                     map
-                },
+                }
-            }});
+            }
+        },
+    );
 
     Ok(map)
 }
@@ -362,21 +419,16 @@ pub fn list_permissions(
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     ("acl", &acl::ROUTER),
+    ("password", &Router::new().put(&API_METHOD_CHANGE_PASSWORD)),
     (
-        "password", &Router::new()
+        "permissions",
-            .put(&API_METHOD_CHANGE_PASSWORD)
+        &Router::new().get(&API_METHOD_LIST_PERMISSIONS)
-    ),
-    (
-        "permissions", &Router::new()
-            .get(&API_METHOD_LIST_PERMISSIONS)
-    ),
-    (
-        "ticket", &Router::new()
-            .post(&API_METHOD_CREATE_TICKET)
     ),
+    ("ticket", &Router::new().post(&API_METHOD_CREATE_TICKET)),
     ("domains", &domain::ROUTER),
     ("roles", &role::ROUTER),
     ("users", &user::ROUTER),
+    ("tfa", &tfa::ROUTER),
 ]);
 
 pub const ROUTER: Router = Router::new()

src/api2/access/tfa.rs

@@ -0,0 +1,575 @@
+use anyhow::{bail, format_err, Error};
+use serde::{Deserialize, Serialize};
+use serde_json::Value;
+
+use proxmox::api::{api, Permission, Router, RpcEnvironment};
+use proxmox::tools::tfa::totp::Totp;
+use proxmox::{http_bail, http_err};
+
+use crate::api2::types::{Authid, Userid, PASSWORD_SCHEMA};
+use crate::config::acl::{PRIV_PERMISSIONS_MODIFY, PRIV_SYS_AUDIT};
+use crate::config::cached_user_info::CachedUserInfo;
+use crate::config::tfa::{TfaInfo, TfaUserData};
+
+/// Perform first-factor (password) authentication only. Ignore password for the root user.
+/// Otherwise check the current user's password.
+///
+/// This means that user admins need to type in their own password while editing a user, and
+/// regular users, which can only change their own TFA settings (checked at the API level), can
+/// change their own settings using their own password.
+fn tfa_update_auth(
+    rpcenv: &mut dyn RpcEnvironment,
+    userid: &Userid,
+    password: Option<String>,
+) -> Result<(), Error> {
+    let authid: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
+    if authid.user() != Userid::root_userid() {
+        let password = password.ok_or_else(|| format_err!("missing password"))?;
+        let _: () = crate::auth::authenticate_user(authid.user(), &password)?;
+    }
+
+    // After authentication, verify that the to-be-modified user actually exists:
+    if authid.user() != userid {
+        let (config, _digest) = crate::config::user::config()?;
+
+        if config.sections.get(userid.as_str()).is_none() {
+            bail!("user '{}' does not exists.", userid);
+        }
+    }
+
+    Ok(())
+}
+
+#[api]
+/// A TFA entry type.
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "lowercase")]
+pub enum TfaType {
+    /// A TOTP entry type.
+    Totp,
+    /// A U2F token entry.
+    U2f,
+    /// A Webauthn token entry.
+    Webauthn,
+    /// Recovery tokens.
+    Recovery,
+}
+
+#[api(
+    properties: {
+        type: { type: TfaType },
+        info: { type: TfaInfo },
+    },
+)]
+/// A TFA entry for a user.
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct TypedTfaInfo {
+    #[serde(rename = "type")]
+    pub ty: TfaType,
+
+    #[serde(flatten)]
+    pub info: TfaInfo,
+}
+
+fn to_data(data: TfaUserData) -> Vec<TypedTfaInfo> {
+    let mut out = Vec::with_capacity(
+        data.totp.len()
+            + data.u2f.len()
+            + data.webauthn.len()
+            + if data.has_recovery() { 1 } else { 0 },
+    );
+    if data.has_recovery() {
+        out.push(TypedTfaInfo {
+            ty: TfaType::Recovery,
+            info: TfaInfo::recovery(),
+        })
+    }
+    for entry in data.totp {
+        out.push(TypedTfaInfo {
+            ty: TfaType::Totp,
+            info: entry.info,
+        });
+    }
+    for entry in data.webauthn {
+        out.push(TypedTfaInfo {
+            ty: TfaType::Webauthn,
+            info: entry.info,
+        });
+    }
+    for entry in data.u2f {
+        out.push(TypedTfaInfo {
+            ty: TfaType::U2f,
+            info: entry.info,
+        });
+    }
+    out
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: { userid: { type: Userid } },
+    },
+    access: {
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
+    },
+)]
+/// Add a TOTP secret to the user.
+pub fn list_user_tfa(userid: Userid) -> Result<Vec<TypedTfaInfo>, Error> {
+    let _lock = crate::config::tfa::read_lock()?;
+
+    Ok(match crate::config::tfa::read()?.users.remove(&userid) {
+        Some(data) => to_data(data),
+        None => Vec::new(),
+    })
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: { type: Userid },
+            id: { description: "the tfa entry id" }
+        },
+    },
+    access: {
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
+    },
+)]
+/// Get a single TFA entry.
+pub fn get_tfa(userid: Userid, id: String) -> Result<TypedTfaInfo, Error> {
+    let _lock = crate::config::tfa::read_lock()?;
+
+    if let Some(user_data) = crate::config::tfa::read()?.users.remove(&userid) {
+        if id == "recovery" {
+            if user_data.has_recovery() {
+                return Ok(TypedTfaInfo {
+                    ty: TfaType::Recovery,
+                    info: TfaInfo::recovery(),
+                });
+            }
+        } else {
+            for tfa in user_data.totp {
+                if tfa.info.id == id {
+                    return Ok(TypedTfaInfo {
+                        ty: TfaType::Totp,
+                        info: tfa.info,
+                    });
+                }
+            }
+
+            for tfa in user_data.webauthn {
+                if tfa.info.id == id {
+                    return Ok(TypedTfaInfo {
+                        ty: TfaType::Webauthn,
+                        info: tfa.info,
+                    });
+                }
+            }
+
+            for tfa in user_data.u2f {
+                if tfa.info.id == id {
+                    return Ok(TypedTfaInfo {
+                        ty: TfaType::U2f,
+                        info: tfa.info,
+                    });
+                }
+            }
+        }
+    }
+
+    http_bail!(NOT_FOUND, "no such tfa entry: {}/{}", userid, id);
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: { type: Userid },
+            id: {
+                description: "the tfa entry id",
+            },
+            password: {
+                schema: PASSWORD_SCHEMA,
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
+    },
+)]
+/// Get a single TFA entry.
+pub fn delete_tfa(
+    userid: Userid,
+    id: String,
+    password: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    tfa_update_auth(rpcenv, &userid, password)?;
+
+    let _lock = crate::config::tfa::write_lock()?;
+
+    let mut data = crate::config::tfa::read()?;
+
+    let user_data = data
+        .users
+        .get_mut(&userid)
+        .ok_or_else(|| http_err!(NOT_FOUND, "no such entry: {}/{}", userid, id))?;
+
+    let found = if id == "recovery" {
+        let found = user_data.has_recovery();
+        user_data.recovery = None;
+        found
+    } else if let Some(i) = user_data.totp.iter().position(|entry| entry.info.id == id) {
+        user_data.totp.remove(i);
+        true
+    } else if let Some(i) = user_data
+        .webauthn
+        .iter()
+        .position(|entry| entry.info.id == id)
+    {
+        user_data.webauthn.remove(i);
+        true
+    } else if let Some(i) = user_data.u2f.iter().position(|entry| entry.info.id == id) {
+        user_data.u2f.remove(i);
+        true
+    } else {
+        false
+    };
+
+    if !found {
+        http_bail!(NOT_FOUND, "no such tfa entry: {}/{}", userid, id);
+    }
+
+    if user_data.is_empty() {
+        data.users.remove(&userid);
+    }
+
+    crate::config::tfa::write(&data)?;
+
+    Ok(())
+}
+
+#[api(
+    properties: {
+        "userid": { type: Userid },
+        "entries": {
+            type: Array,
+            items: { type: TypedTfaInfo },
+        },
+    },
+)]
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+/// Over the API we only provide the descriptions for TFA data.
+pub struct TfaUser {
+    /// The user this entry belongs to.
+    userid: Userid,
+
+    /// TFA entries.
+    entries: Vec<TypedTfaInfo>,
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {},
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Returns all or just the logged-in user, depending on privileges.",
+    },
+)]
+/// List user TFA configuration.
+pub fn list_tfa(rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let authid: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let top_level_privs = user_info.lookup_privs(&authid, &["access", "users"]);
+    let top_level_allowed = (top_level_privs & PRIV_SYS_AUDIT) != 0;
+
+    let _lock = crate::config::tfa::read_lock()?;
+    let tfa_data = crate::config::tfa::read()?.users;
+
+    let mut out = Vec::<TfaUser>::new();
+    if top_level_allowed {
+        for (user, data) in tfa_data {
+            out.push(TfaUser {
+                userid: user,
+                entries: to_data(data),
+            });
+        }
+    } else {
+        if let Some(data) = { tfa_data }.remove(authid.user()) {
+            out.push(TfaUser {
+                userid: authid.into(),
+                entries: to_data(data),
+            });
+        }
+    }
+
+    Ok(serde_json::to_value(out)?)
+}
+
+#[api(
+    properties: {
+        recovery: {
+            description: "A list of recovery codes as integers.",
+            type: Array,
+            items: {
+                type: Integer,
+                description: "A one-time usable recovery code entry.",
+            },
+        },
+    },
+)]
+/// The result returned when adding TFA entries to a user.
+#[derive(Default, Serialize)]
+struct TfaUpdateInfo {
+    /// The id if a newly added TFA entry.
+    id: Option<String>,
+
+    /// When adding u2f entries, this contains a challenge the user must respond to in order to
+    /// finish the registration.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    challenge: Option<String>,
+
+    /// When adding recovery codes, this contains the list of codes to be displayed to the user
+    /// this one time.
+    #[serde(skip_serializing_if = "Vec::is_empty", default)]
+    recovery: Vec<String>,
+}
+
+impl TfaUpdateInfo {
+    fn id(id: String) -> Self {
+        Self {
+            id: Some(id),
+            ..Default::default()
+        }
+    }
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: { type: Userid },
+            description: {
+                description: "A description to distinguish multiple entries from one another",
+                type: String,
+                max_length: 255,
+                optional: true,
+            },
+            "type": { type: TfaType },
+            totp: {
+                description: "A totp URI.",
+                optional: true,
+            },
+            value: {
+                description:
+            "The current value for the provided totp URI, or a Webauthn/U2F challenge response",
+                optional: true,
+            },
+            challenge: {
+                description: "When responding to a u2f challenge: the original challenge string",
+                optional: true,
+            },
+            password: {
+                schema: PASSWORD_SCHEMA,
+                optional: true,
+            },
+        },
+    },
+    returns: { type: TfaUpdateInfo },
+    access: {
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
+    },
+)]
+/// Add a TFA entry to the user.
+fn add_tfa_entry(
+    userid: Userid,
+    description: Option<String>,
+    totp: Option<String>,
+    value: Option<String>,
+    challenge: Option<String>,
+    password: Option<String>,
+    mut params: Value, // FIXME: once api macro supports raw parameters names, use `r#type`
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<TfaUpdateInfo, Error> {
+    tfa_update_auth(rpcenv, &userid, password)?;
+
+    let tfa_type: TfaType = serde_json::from_value(params["type"].take())?;
+
+    let need_description =
+        move || description.ok_or_else(|| format_err!("'description' is required for new entries"));
+
+    match tfa_type {
+        TfaType::Totp => match (totp, value) {
+            (Some(totp), Some(value)) => {
+                if challenge.is_some() {
+                    bail!("'challenge' parameter is invalid for 'totp' entries");
+                }
+                let description = need_description()?;
+
+                let totp: Totp = totp.parse()?;
+                if totp
+                    .verify(&value, std::time::SystemTime::now(), -1..=1)?
+                    .is_none()
+                {
+                    bail!("failed to verify TOTP challenge");
+                }
+                crate::config::tfa::add_totp(&userid, description, totp).map(TfaUpdateInfo::id)
+            }
+            _ => bail!("'totp' type requires both 'totp' and 'value' parameters"),
+        },
+        TfaType::Webauthn => {
+            if totp.is_some() {
+                bail!("'totp' parameter is invalid for 'totp' entries");
+            }
+
+            match challenge {
+                None => crate::config::tfa::add_webauthn_registration(&userid, need_description()?)
+                    .map(|c| TfaUpdateInfo {
+                        challenge: Some(c),
+                        ..Default::default()
+                    }),
+                Some(challenge) => {
+                    let value = value.ok_or_else(|| {
+                        format_err!(
+                            "missing 'value' parameter (webauthn challenge response missing)"
+                        )
+                    })?;
+                    crate::config::tfa::finish_webauthn_registration(&userid, &challenge, &value)
+                        .map(TfaUpdateInfo::id)
+                }
+            }
+        }
+        TfaType::U2f => {
+            if totp.is_some() {
+                bail!("'totp' parameter is invalid for 'totp' entries");
+            }
+
+            match challenge {
+                None => crate::config::tfa::add_u2f_registration(&userid, need_description()?).map(
+                    |c| TfaUpdateInfo {
+                        challenge: Some(c),
+                        ..Default::default()
+                    },
+                ),
+                Some(challenge) => {
+                    let value = value.ok_or_else(|| {
+                        format_err!("missing 'value' parameter (u2f challenge response missing)")
+                    })?;
+                    crate::config::tfa::finish_u2f_registration(&userid, &challenge, &value)
+                        .map(TfaUpdateInfo::id)
+                }
+            }
+        }
+        TfaType::Recovery => {
+            if totp.or(value).or(challenge).is_some() {
+                bail!("generating recovery tokens does not allow additional parameters");
+            }
+
+            let recovery = crate::config::tfa::add_recovery(&userid)?;
+
+            Ok(TfaUpdateInfo {
+                id: Some("recovery".to_string()),
+                recovery,
+                ..Default::default()
+            })
+        }
+    }
+}
+
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            userid: { type: Userid },
+            id: {
+                description: "the tfa entry id",
+            },
+            description: {
+                description: "A description to distinguish multiple entries from one another",
+                type: String,
+                max_length: 255,
+                optional: true,
+            },
+            enable: {
+                description: "Whether this entry should currently be enabled or disabled",
+                optional: true,
+            },
+            password: {
+                schema: PASSWORD_SCHEMA,
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
+    },
+)]
+/// Update user's TFA entry description.
+pub fn update_tfa_entry(
+    userid: Userid,
+    id: String,
+    description: Option<String>,
+    enable: Option<bool>,
+    password: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    tfa_update_auth(rpcenv, &userid, password)?;
+
+    let _lock = crate::config::tfa::write_lock()?;
+
+    let mut data = crate::config::tfa::read()?;
+
+    let mut entry = data
+        .users
+        .get_mut(&userid)
+        .and_then(|user| user.find_entry_mut(&id))
+        .ok_or_else(|| http_err!(NOT_FOUND, "no such entry: {}/{}", userid, id))?;
+
+    if let Some(description) = description {
+        entry.description = description;
+    }
+
+    if let Some(enable) = enable {
+        entry.enable = enable;
+    }
+
+    crate::config::tfa::write(&data)?;
+    Ok(())
+}
+
+pub const ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_TFA)
+    .match_all("userid", &USER_ROUTER);
+
+const USER_ROUTER: Router = Router::new()
+    .get(&API_METHOD_LIST_USER_TFA)
+    .post(&API_METHOD_ADD_TFA_ENTRY)
+    .match_all("id", &ITEM_ROUTER);
+
+const ITEM_ROUTER: Router = Router::new()
+    .get(&API_METHOD_GET_TFA)
+    .put(&API_METHOD_UPDATE_TFA_ENTRY)
+    .delete(&API_METHOD_DELETE_TFA);

src/server.rs

@@ -87,3 +87,5 @@ pub use email_notifications::*;
 
 mod report;
 pub use report::*;
+
+pub mod ticket;

src/server/rest.rs

@@ -600,8 +600,9 @@ fn check_auth(
             let ticket = user_auth_data.ticket.clone();
             let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
 
-            let userid: Userid = Ticket::parse(&ticket)?
+            let userid: Userid = Ticket::<super::ticket::ApiTicket>::parse(&ticket)?
-                .verify_with_time_frame(public_auth_key(), "PBS", None, -300..ticket_lifetime)?;
+                .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) {

src/server/ticket.rs

@@ -0,0 +1,77 @@
+use std::fmt;
+
+use anyhow::{bail, Error};
+use serde::{Deserialize, Serialize};
+
+use crate::api2::types::Userid;
+use crate::config::tfa;
+
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct PartialTicket {
+    #[serde(rename = "u")]
+    userid: Userid,
+
+    #[serde(rename = "c")]
+    challenge: tfa::TfaChallenge,
+}
+
+/// A new ticket struct used in rest.rs's `check_auth` - mostly for better errors than failing to
+/// parse the userid ticket content.
+pub enum ApiTicket {
+    Full(Userid),
+    Partial(tfa::TfaChallenge),
+}
+
+impl ApiTicket {
+    /// Require the ticket to be a full ticket, otherwise error with a meaningful error message.
+    pub fn require_full(self) -> Result<Userid, Error> {
+        match self {
+            ApiTicket::Full(userid) => Ok(userid),
+            ApiTicket::Partial(_) => bail!("access denied - second login factor required"),
+        }
+    }
+
+    /// Expect the ticket to contain a tfa challenge, otherwise error with a meaningful error
+    /// message.
+    pub fn require_partial(self) -> Result<tfa::TfaChallenge, Error> {
+        match self {
+            ApiTicket::Full(_) => bail!("invalid tfa challenge"),
+            ApiTicket::Partial(challenge) => Ok(challenge),
+        }
+    }
+
+    /// Create a new full ticket.
+    pub fn full(userid: Userid) -> Self {
+        ApiTicket::Full(userid)
+    }
+
+    /// Create a new partial ticket.
+    pub fn partial(challenge: tfa::TfaChallenge) -> Self {
+        ApiTicket::Partial(challenge)
+    }
+}
+
+impl fmt::Display for ApiTicket {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ApiTicket::Full(userid) => fmt::Display::fmt(userid, f),
+            ApiTicket::Partial(partial) => {
+                let data = serde_json::to_string(partial).map_err(|_| fmt::Error)?;
+                write!(f, "!tfa!{}", data)
+            }
+        }
+    }
+}
+
+impl std::str::FromStr for ApiTicket {
+    type Err = Error;
+
+    fn from_str(s: &str) -> Result<Self, Error> {
+        if s.starts_with("!tfa!") {
+            Ok(ApiTicket::Partial(serde_json::from_str(&s[5..])?))
+        } else {
+            Ok(ApiTicket::Full(s.parse()?))
+        }
+    }
+}