[pbs-devel] [RFC backup 3/6] api: tfa management and login

Wolfgang Bumiller w.bumiller at proxmox.com
Thu Nov 19 15:56:05 CET 2020


Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 Cargo.toml             |   1 +
 src/api2/access.rs     | 171 ++++++++-----
 src/api2/access/tfa.rs | 567 +++++++++++++++++++++++++++++++++++++++++
 src/config/tfa.rs      | 474 ++++++++++++++++++++++++++++++----
 src/server.rs          |   2 +
 src/server/rest.rs     |   5 +-
 src/server/ticket.rs   |  77 ++++++
 7 files changed, 1185 insertions(+), 112 deletions(-)
 create mode 100644 src/api2/access/tfa.rs
 create mode 100644 src/server/ticket.rs

diff --git a/Cargo.toml b/Cargo.toml
index 87aa9cba..1a26bbd0 100644
--- a/Cargo.toml
+++ b/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"
diff --git a/src/api2/access.rs b/src/api2/access.rs
index 3b59b3d3..a44d21a2 100644
--- a/src/api2/access.rs
+++ b/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
-            .ok_or_else(|| format_err!("missing privilege name for termproxy ticket"))?;
+        let privilege_name =
+            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)
-            .and_then(|ticket| ticket.verify(
+        if let Ok(Empty) = Ticket::parse(password).and_then(|ticket| {
+            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)?;
-    Ok(true)
+    let _: () = crate::auth::authenticate_user(userid, password)?;
+
+    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) {
-        Ok(true) => {
-            let ticket = Ticket::new("PBS", &username)?.sign(private_auth_key(), None)?;
-
+    match authenticate_user(&username, &password, path, privs, port, tfa_challenge) {
+        Ok(AuthResult::Success) => Ok(json!({ "username": username })),
+        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!({
-            "username": username,
-        })),
+        Ok(AuthResult::Partial(challenge)) => {
+            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),
@@ -206,7 +255,6 @@ fn change_password(
     password: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
     let current_user: Userid = rpcenv
         .get_auth_id()
         .ok_or_else(|| format_err!("unknown user"))?
@@ -215,12 +263,16 @@ fn change_password(
 
     let mut allowed = userid == current_user;
 
-    if userid == "root at pam" { allowed = true; }
+    if userid == "root at 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 {
@@ -277,12 +329,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 {
@@ -292,11 +345,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);
@@ -311,7 +363,7 @@ pub fn list_permissions(
             let mut paths = HashSet::new();
             paths.insert(path);
             paths
-        },
+        }
         None => {
             let mut paths = HashSet::new();
 
@@ -326,31 +378,35 @@ pub fn list_permissions(
             paths.insert("/system".to_string());
 
             paths
-        },
+        }
     };
 
-    let map = paths
-        .into_iter()
-        .fold(HashMap::new(), |mut map: HashMap<String, HashMap<String, bool>>, path: String| {
+    let map = paths.into_iter().fold(
+        HashMap::new(),
+        |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
-                        .iter()
-                        .fold(HashMap::new(), |mut priv_map, (name, value)| {
-                            if value & privs != 0 {
-                                priv_map.insert(name.to_string(), value & propagated_privs != 0);
-                            }
-                            priv_map
-                        });
+                    let priv_map =
+                        PRIVILEGES
+                            .iter()
+                            .fold(HashMap::new(), |mut priv_map, (name, value)| {
+                                if value & privs != 0 {
+                                    priv_map
+                                        .insert(name.to_string(), value & propagated_privs != 0);
+                                }
+                                priv_map
+                            });
 
                     map.insert(path, priv_map);
                     map
-                },
-            }});
+                }
+            }
+        },
+    );
 
     Ok(map)
 }
@@ -358,21 +414,16 @@ pub fn list_permissions(
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     ("acl", &acl::ROUTER),
+    ("password", &Router::new().put(&API_METHOD_CHANGE_PASSWORD)),
     (
-        "password", &Router::new()
-            .put(&API_METHOD_CHANGE_PASSWORD)
-    ),
-    (
-        "permissions", &Router::new()
-            .get(&API_METHOD_LIST_PERMISSIONS)
-    ),
-    (
-        "ticket", &Router::new()
-            .post(&API_METHOD_CREATE_TICKET)
+        "permissions",
+        &Router::new().get(&API_METHOD_LIST_PERMISSIONS)
     ),
+    ("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()
diff --git a/src/api2/access/tfa.rs b/src/api2/access/tfa.rs
new file mode 100644
index 00000000..68ab810a
--- /dev/null
+++ b/src/api2/access/tfa.rs
@@ -0,0 +1,567 @@
+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. Make
+/// sure a user can only update its own account!
+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() {
+        return Ok(());
+    }
+
+    let password = password.ok_or_else(|| format_err!("missing password"))?;
+    let _: () = crate::auth::authenticate_user(&userid, &password)?;
+
+    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.recovery.is_empty() { 1 } else { 0 },
+    );
+    if !data.recovery.is_empty() {
+        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.recovery.is_empty() {
+                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.recovery.is_empty();
+        user_data.recovery.clear();
+        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 type of TFA entry this is referring to.
+    userid: Userid,
+
+    /// User chosen description for this entry.
+    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": {
+                description: "The TFA type to add.",
+                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);
diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index da3a4f9a..ca39b3c0 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -5,6 +5,7 @@ use std::time::Duration;
 use anyhow::{bail, format_err, Error};
 use serde::{de::Deserializer, Deserialize, Serialize};
 use serde_json::Value;
+use webauthn_rs::Webauthn;
 
 use proxmox::api::api;
 use proxmox::sys::error::SysError;
@@ -29,38 +30,98 @@ pub struct U2fConfig {
     appid: String,
 }
 
+#[derive(Clone, Deserialize, Serialize)]
+pub struct WebauthnConfig {
+    /// Relying party name. Any text identifier.
+    ///
+    /// Changing this *may* break existing credentials.
+    rp: String,
+
+    /// Site origin. Must be a `https://` URL (or `http://localhost`). Should contain the address
+    /// users type in their browsers to access the web interface.
+    ///
+    /// Changing this *may* break existing credentials.
+    origin: String,
+
+    /// Relying part ID. Must be the domain name without protocol, port or location.
+    ///
+    /// Changing this *will* break existing credentials.
+    id: String,
+}
+
+/// For now we just implement this on the configuration this way.
+///
+/// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by
+/// the connecting client.
+impl webauthn_rs::WebauthnConfig for WebauthnConfig {
+    fn get_relying_party_name(&self) -> String {
+        self.rp.clone()
+    }
+
+    fn get_origin(&self) -> &String {
+        &self.origin
+    }
+
+    fn get_relying_party_id(&self) -> String {
+        self.id.clone()
+    }
+}
+
 #[derive(Default, Deserialize, Serialize)]
 pub struct TfaConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub u2f: Option<U2fConfig>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub webauthn: Option<WebauthnConfig>,
+
     #[serde(skip_serializing_if = "TfaUsers::is_empty", default)]
     pub users: TfaUsers,
 }
 
 /// Heper to get a u2f instance from a u2f config, or `None` if there isn't one configured.
 fn get_u2f(u2f: &Option<U2fConfig>) -> Option<u2f::U2f> {
-    u2f.as_ref().map(|cfg| u2f::U2f::new(cfg.appid.clone(), cfg.appid.clone()))
+    u2f.as_ref()
+        .map(|cfg| u2f::U2f::new(cfg.appid.clone(), cfg.appid.clone()))
 }
 
 /// Heper to get a u2f instance from a u2f config.
-// deduplicate error message while working around self-borrow issue
+///
+/// This is outside of `TfaConfig` to not borrow its `&self`.
 fn need_u2f(u2f: &Option<U2fConfig>) -> Result<u2f::U2f, Error> {
     get_u2f(u2f).ok_or_else(|| format_err!("no u2f configuration available"))
 }
 
-impl TfaConfig {
-    fn u2f(&self) -> Option<u2f::U2f> {
-        get_u2f(&self.u2f)
-    }
+/// Heper to get a u2f instance from a u2f config, or `None` if there isn't one configured.
+fn get_webauthn(waconfig: &Option<WebauthnConfig>) -> Option<Webauthn<WebauthnConfig>> {
+    waconfig.clone().map(Webauthn::new)
+}
 
+/// Heper to get a u2f instance from a u2f config.
+///
+/// This is outside of `TfaConfig` to not borrow its `&self`.
+fn need_webauthn(waconfig: &Option<WebauthnConfig>) -> Result<Webauthn<WebauthnConfig>, Error> {
+    get_webauthn(waconfig).ok_or_else(|| format_err!("no webauthn configuration available"))
+}
+
+impl TfaConfig {
+    /// Helper to get a u2f instance. Note that there's a non-&self borrowing version if needed.
     fn need_u2f(&self) -> Result<u2f::U2f, Error> {
         need_u2f(&self.u2f)
     }
 
+    /// Helper to get a Webauthn instance. Note that there's a non-&self borrowing version if
+    /// needed.
+    fn need_webauthn(&self) -> Result<Webauthn<WebauthnConfig>, Error> {
+        need_webauthn(&self.webauthn)
+    }
+
     /// Get a two factor authentication challenge for a user, if the user has TFA set up.
-    pub fn login_challenge(&self, userid: &Userid) -> Result<Option<TfaChallenge>, Error> {
-        match self.users.get(userid) {
-            Some(udata) => udata.challenge(self.u2f().as_ref()),
+    pub fn login_challenge(&mut self, userid: &Userid) -> Result<Option<TfaChallenge>, Error> {
+        match self.users.get_mut(userid) {
+            Some(udata) => {
+                udata.challenge(get_webauthn(&self.webauthn), get_u2f(&self.u2f).as_ref())
+            }
             None => Ok(None),
         }
     }
@@ -94,6 +155,39 @@ impl TfaConfig {
         }
     }
 
+    /// Get a webauthn registration challenge.
+    fn webauthn_registration_challenge(
+        &mut self,
+        user: &Userid,
+        description: String,
+    ) -> Result<String, Error> {
+        let webauthn = self.need_webauthn()?;
+
+        self.users
+            .entry(user.clone())
+            .or_default()
+            .webauthn_registration_challenge(webauthn, user, description)
+    }
+
+    /// Finish a webauthn registration challenge.
+    fn webauthn_registration_finish(
+        &mut self,
+        userid: &Userid,
+        challenge: &str,
+        response: &str,
+    ) -> Result<String, Error> {
+        let webauthn = self.need_webauthn()?;
+
+        let response: webauthn_rs::proto::RegisterPublicKeyCredential =
+            serde_json::from_str(response)
+                .map_err(|err| format_err!("error parsing challenge response: {}", err))?;
+
+        match self.users.get_mut(userid) {
+            Some(user) => user.webauthn_registration_finish(webauthn, challenge, response),
+            None => bail!("no such challenge"),
+        }
+    }
+
     /// Verify a TFA response.
     fn verify(
         &mut self,
@@ -102,19 +196,21 @@ impl TfaConfig {
         response: TfaResponse,
     ) -> Result<(), Error> {
         match self.users.get_mut(userid) {
-            Some(user) => {
-                match response {
-                    TfaResponse::Totp(value) => user.verify_totp(&value),
-                    TfaResponse::U2f(value) => match &challenge.u2f {
-                        Some(challenge) => {
-                            let u2f = need_u2f(&self.u2f)?;
-                            user.verify_u2f(u2f, &challenge.challenge, value)
-                        }
-                        None => bail!("no u2f factor available for user '{}'", userid),
+            Some(user) => match response {
+                TfaResponse::Totp(value) => user.verify_totp(&value),
+                TfaResponse::U2f(value) => match &challenge.u2f {
+                    Some(challenge) => {
+                        let u2f = need_u2f(&self.u2f)?;
+                        user.verify_u2f(u2f, &challenge.challenge, value)
                     }
-                    TfaResponse::Recovery(value) => user.verify_recovery(&value),
+                    None => bail!("no u2f factor available for user '{}'", userid),
+                },
+                TfaResponse::Webauthn(value) => {
+                    let webauthn = need_webauthn(&self.webauthn)?;
+                    user.verify_webauthn(webauthn, value)
                 }
-            }
+                TfaResponse::Recovery(value) => user.verify_recovery(&value),
+            },
             None => bail!("no 2nd factor available for user '{}'", userid),
         }
     }
@@ -175,6 +271,10 @@ impl<T> TfaEntry<T> {
     }
 }
 
+trait IsExpired {
+    fn is_expired(&self, at_epoch: i64) -> bool;
+}
+
 /// A u2f registration challenge.
 #[derive(Deserialize, Serialize)]
 #[serde(deny_unknown_fields)]
@@ -197,7 +297,79 @@ impl U2fRegistrationChallenge {
             created: proxmox::tools::time::epoch_i64(),
         }
     }
+}
 
+impl IsExpired for U2fRegistrationChallenge {
+    fn is_expired(&self, at_epoch: i64) -> bool {
+        self.created < at_epoch
+    }
+}
+
+/// A webauthn registration challenge.
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct WebauthnRegistrationChallenge {
+    /// Server side registration state data.
+    state: webauthn_rs::RegistrationState,
+
+    /// While this is basically the content of a `RegistrationState`, the webauthn-rs crate doesn't
+    /// make this public.
+    challenge: String,
+
+    /// The description chosen by the user for this registration.
+    description: String,
+
+    /// When the challenge was created as unix epoch. They are supposed to be short-lived.
+    created: i64,
+}
+
+impl WebauthnRegistrationChallenge {
+    pub fn new(
+        state: webauthn_rs::RegistrationState,
+        challenge: String,
+        description: String,
+    ) -> Self {
+        Self {
+            state,
+            challenge,
+            description,
+            created: proxmox::tools::time::epoch_i64(),
+        }
+    }
+}
+
+impl IsExpired for WebauthnRegistrationChallenge {
+    fn is_expired(&self, at_epoch: i64) -> bool {
+        self.created < at_epoch
+    }
+}
+
+/// A webauthn authentication challenge.
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct WebauthnAuthChallenge {
+    /// Server side authentication state.
+    state: webauthn_rs::AuthenticationState,
+
+    /// While this is basically the content of a `AuthenticationState`, the webauthn-rs crate
+    /// doesn't make this public.
+    challenge: String,
+
+    /// When the challenge was created as unix epoch. They are supposed to be short-lived.
+    created: i64,
+}
+
+impl WebauthnAuthChallenge {
+    pub fn new(state: webauthn_rs::AuthenticationState, challenge: String) -> Self {
+        Self {
+            state,
+            challenge,
+            created: proxmox::tools::time::epoch_i64(),
+        }
+    }
+}
+
+impl IsExpired for WebauthnAuthChallenge {
     fn is_expired(&self, at_epoch: i64) -> bool {
         self.created < at_epoch
     }
@@ -216,6 +388,10 @@ pub struct TfaUserData {
     #[serde(skip_serializing_if = "Vec::is_empty", default)]
     pub(crate) u2f: Vec<TfaEntry<u2f::Registration>>,
 
+    /// Registered webauthn tokens for a user.
+    #[serde(skip_serializing_if = "Vec::is_empty", default)]
+    pub(crate) webauthn: Vec<TfaEntry<webauthn_rs::proto::Credential>>,
+
     /// Recovery keys. (Unordered OTP values).
     #[serde(skip_serializing_if = "Vec::is_empty", default)]
     pub(crate) recovery: Vec<String>,
@@ -224,23 +400,36 @@ pub struct TfaUserData {
     ///
     /// Expired values are automatically filtered out while parsing the tfa configuration file.
     #[serde(skip_serializing_if = "Vec::is_empty", default)]
-    #[serde(deserialize_with = "filter_expired_registrations")]
+    #[serde(deserialize_with = "filter_expired_challenge")]
     u2f_registrations: Vec<U2fRegistrationChallenge>,
+
+    /// Active webauthn registration challenges for a user.
+    ///
+    /// Expired values are automatically filtered out while parsing the tfa configuration file.
+    #[serde(skip_serializing_if = "Vec::is_empty", default)]
+    #[serde(deserialize_with = "filter_expired_challenge")]
+    webauthn_registrations: Vec<WebauthnRegistrationChallenge>,
+
+    /// Active webauthn registration challenges for a user.
+    ///
+    /// Expired values are automatically filtered out while parsing the tfa configuration file.
+    #[serde(skip_serializing_if = "Vec::is_empty", default)]
+    #[serde(deserialize_with = "filter_expired_challenge")]
+    webauthn_auths: Vec<WebauthnAuthChallenge>,
 }
 
 /// Serde helper using our `FilteredVecVisitor` to filter out expired entries directly at load
 /// time.
-fn filter_expired_registrations<'de, D>(
-    deserializer: D,
-) -> Result<Vec<U2fRegistrationChallenge>, D::Error>
+fn filter_expired_challenge<'de, D, T>(deserializer: D) -> Result<Vec<T>, D::Error>
 where
     D: Deserializer<'de>,
+    T: Deserialize<'de> + IsExpired,
 {
     let expire_before = proxmox::tools::time::epoch_i64() - CHALLENGE_TIMEOUT;
     Ok(
         deserializer.deserialize_seq(crate::tools::serde_filter::FilteredVecVisitor::new(
-            "a u2f registration challenge entry",
-            move |reg: &U2fRegistrationChallenge| !reg.is_expired(expire_before),
+            "a challenge entry",
+            move |reg: &T| !reg.is_expired(expire_before),
         ))?,
     )
 }
@@ -248,7 +437,10 @@ where
 impl TfaUserData {
     /// `true` if no second factors exist
     pub fn is_empty(&self) -> bool {
-        self.totp.is_empty() && self.u2f.is_empty() && self.recovery.is_empty()
+        self.totp.is_empty()
+            && self.u2f.is_empty()
+            && self.webauthn.is_empty()
+            && self.recovery.is_empty()
     }
 
     /// Find an entry by id, except for the "recovery" entry which we're currently treating
@@ -260,6 +452,12 @@ impl TfaUserData {
             }
         }
 
+        for entry in &mut self.webauthn {
+            if entry.info.id == id {
+                return Some(&mut entry.info);
+            }
+        }
+
         for entry in &mut self.u2f {
             if entry.info.id == id {
                 return Some(&mut entry.info);
@@ -337,8 +535,71 @@ impl TfaUserData {
         Ok(id)
     }
 
+    /// Create a webauthn registration challenge.
+    ///
+    /// The description is required at this point already mostly to better be able to identify such
+    /// challenges in the tfa config file if necessary. The user otherwise has no access to this
+    /// information at this point, as the challenge is identified by its actual challenge data
+    /// instead.
+    fn webauthn_registration_challenge(
+        &mut self,
+        mut webauthn: Webauthn<WebauthnConfig>,
+        userid: &Userid,
+        description: String,
+    ) -> Result<String, Error> {
+        let userid_str = userid.to_string();
+        let (challenge, state) = webauthn.generate_challenge_register(&userid_str, None)?;
+        let challenge_string = challenge.public_key.challenge.to_string();
+        let challenge = serde_json::to_string(&challenge)?;
+
+        self.webauthn_registrations
+            .push(WebauthnRegistrationChallenge::new(
+                state,
+                challenge_string,
+                description,
+            ));
+
+        Ok(challenge)
+    }
+
+    /// Finish a webauthn registration. The challenge should correspond to an output of
+    /// `webauthn_registration_challenge`. The response should come directly from the client.
+    fn webauthn_registration_finish(
+        &mut self,
+        webauthn: Webauthn<WebauthnConfig>,
+        challenge: &str,
+        response: webauthn_rs::proto::RegisterPublicKeyCredential,
+    ) -> Result<String, Error> {
+        let expire_before = proxmox::tools::time::epoch_i64() - CHALLENGE_TIMEOUT;
+
+        let index = self
+            .webauthn_registrations
+            .iter()
+            .position(|r| r.challenge == challenge)
+            .ok_or_else(|| format_err!("no such challenge"))?;
+
+        let reg = self.webauthn_registrations.remove(index);
+        if reg.is_expired(expire_before) {
+            bail!("no such challenge");
+        }
+
+        let credential =
+            webauthn.register_credential(response, reg.state, |id| -> Result<bool, ()> {
+                Ok(self.webauthn.iter().any(|cred| cred.entry.cred_id == *id))
+            })?;
+
+        let entry = TfaEntry::new(reg.description, credential);
+        let id = entry.info.id.clone();
+        self.webauthn.push(entry);
+        Ok(id)
+    }
+
     /// Generate a generic TFA challenge. See the [`TfaChallenge`] description for details.
-    pub fn challenge(&self, u2f: Option<&u2f::U2f>) -> Result<Option<TfaChallenge>, Error> {
+    pub fn challenge(
+        &mut self,
+        webauthn: Option<Webauthn<WebauthnConfig>>,
+        u2f: Option<&u2f::U2f>,
+    ) -> Result<Option<TfaChallenge>, Error> {
         if self.is_empty() {
             return Ok(None);
         }
@@ -346,6 +607,10 @@ impl TfaUserData {
         Ok(Some(TfaChallenge {
             totp: self.totp.iter().any(|e| e.info.enable),
             recovery: RecoveryState::from_count(self.recovery.len()),
+            webauthn: match webauthn {
+                Some(webauthn) => self.webauthn_challenge(webauthn)?,
+                None => None,
+            },
             u2f: match u2f {
                 Some(u2f) => self.u2f_challenge(u2f)?,
                 None => None,
@@ -357,26 +622,21 @@ impl TfaUserData {
     fn enabled_totp_entries(&self) -> impl Iterator<Item = &Totp> {
         self.totp
             .iter()
-            .filter_map(|e| {
-                if e.info.enable {
-                    Some(&e.entry)
-                } else {
-                    None
-                }
-            })
+            .filter_map(|e| if e.info.enable { Some(&e.entry) } else { None })
     }
 
     /// Helper to iterate over enabled u2f entries.
     fn enabled_u2f_entries(&self) -> impl Iterator<Item = &u2f::Registration> {
         self.u2f
             .iter()
-            .filter_map(|e| {
-                if e.info.enable {
-                    Some(&e.entry)
-                } else {
-                    None
-                }
-            })
+            .filter_map(|e| if e.info.enable { Some(&e.entry) } else { None })
+    }
+
+    /// Helper to iterate over enabled u2f entries.
+    fn enabled_webauthn_entries(&self) -> impl Iterator<Item = &webauthn_rs::proto::Credential> {
+        self.webauthn
+            .iter()
+            .filter_map(|e| if e.info.enable { Some(&e.entry) } else { None })
     }
 
     /// Generate an optional u2f challenge.
@@ -400,6 +660,29 @@ impl TfaUserData {
         }))
     }
 
+    /// Generate an optional webauthn challenge.
+    fn webauthn_challenge(
+        &mut self,
+        mut webauthn: Webauthn<WebauthnConfig>,
+    ) -> Result<Option<webauthn_rs::proto::RequestChallengeResponse>, Error> {
+        if self.webauthn.is_empty() {
+            return Ok(None);
+        }
+
+        let creds: Vec<_> = self.enabled_webauthn_entries().map(Clone::clone).collect();
+
+        if creds.is_empty() {
+            return Ok(None);
+        }
+
+        let (challenge, state) = webauthn.generate_challenge_authenticate(creds, None)?;
+        let challenge_string = challenge.public_key.challenge.to_string();
+        self.webauthn_auths
+            .push(WebauthnAuthChallenge::new(state, challenge_string));
+
+        Ok(Some(challenge))
+    }
+
     /// Verify a totp challenge. The `value` should be the totp digits as plain text.
     fn verify_totp(&self, value: &str) -> Result<(), Error> {
         let now = std::time::SystemTime::now();
@@ -425,9 +708,12 @@ impl TfaUserData {
 
         if let Some(entry) = self
             .enabled_u2f_entries()
-            .find(|e| e.key.key_handle == response.key_handle)
+            .find(|e| e.key.key_handle == response.key_handle())
         {
-            if u2f.auth_verify_obj(&entry.public_key, &challenge.challenge, response)?.is_some() {
+            if u2f
+                .auth_verify_obj(&entry.public_key, &challenge.challenge, response)?
+                .is_some()
+            {
                 return Ok(());
             }
         }
@@ -435,6 +721,44 @@ impl TfaUserData {
         bail!("u2f verification failed");
     }
 
+    /// Verify a webauthn response.
+    fn verify_webauthn(
+        &mut self,
+        mut webauthn: Webauthn<WebauthnConfig>,
+        mut response: Value,
+    ) -> Result<(), Error> {
+        let expire_before = proxmox::tools::time::epoch_i64() - CHALLENGE_TIMEOUT;
+
+        let challenge = match response
+            .as_object_mut()
+            .ok_or_else(|| format_err!("invalid response, must be a json object"))?
+            .remove("challenge")
+            .ok_or_else(|| format_err!("missing challenge data in response"))?
+        {
+            Value::String(s) => s,
+            _ => bail!("invalid challenge data in response"),
+        };
+
+        let response: webauthn_rs::proto::PublicKeyCredential = serde_json::from_value(response)
+            .map_err(|err| format_err!("invalid webauthn response: {}", err))?;
+
+        let index = self
+            .webauthn_auths
+            .iter()
+            .position(|r| r.challenge == challenge)
+            .ok_or_else(|| format_err!("no such challenge"))?;
+
+        let challenge = self.webauthn_auths.remove(index);
+        if challenge.is_expired(expire_before) {
+            bail!("no such challenge");
+        }
+
+        match webauthn.authenticate_credential(response, challenge.state)? {
+            Some((_cred, _counter)) => Ok(()),
+            None => bail!("webauthn authentication failed"),
+        }
+    }
+
     /// Verify a recovery key.
     ///
     /// NOTE: If successful, the key will automatically be removed from the list of available
@@ -458,7 +782,8 @@ impl TfaUserData {
         let mut key_data = [0u8; 40]; // 10 keys of 32 bits
         proxmox::sys::linux::fill_with_random_data(&mut key_data)?;
         for b in key_data.chunks(4) {
-            self.recovery.push(format!("{:02x}{:02x}{:02x}{:02x}", b[0], b[1], b[2], b[3]));
+            self.recovery
+                .push(format!("{:02x}{:02x}{:02x}{:02x}", b[0], b[1], b[2], b[3]));
         }
 
         Ok(self.recovery.clone())
@@ -493,9 +818,23 @@ pub fn write_lock() -> Result<File, Error> {
     proxmox::tools::fs::open_file_locked(LOCK_FILE, LOCK_TIMEOUT, true)
 }
 
+/// Get an optional TFA challenge for a user.
+pub fn login_challenge(userid: &Userid) -> Result<Option<TfaChallenge>, Error> {
+    let _lock = write_lock()?;
+
+    let mut data = read()?;
+    Ok(match data.login_challenge(userid)? {
+        Some(challenge) => {
+            write(&data)?;
+            Some(challenge)
+        }
+        None => None,
+    })
+}
+
 /// Add a TOTP entry for a user. Returns the ID.
 pub fn add_totp(userid: &Userid, description: String, value: Totp) -> Result<String, Error> {
-    let _lock = crate::config::tfa::write_lock();
+    let _lock = write_lock();
     let mut data = read()?;
     let entry = TfaEntry::new(description, value);
     let id = entry.info.id.clone();
@@ -510,10 +849,14 @@ pub fn add_totp(userid: &Userid, description: String, value: Totp) -> Result<Str
 
 /// Add recovery tokens for the user. Returns the token list.
 pub fn add_recovery(userid: &Userid) -> Result<Vec<String>, Error> {
-    let _lock = crate::config::tfa::write_lock();
+    let _lock = write_lock();
 
     let mut data = read()?;
-    let out = data.users.entry(userid.clone()).or_default().add_recovery()?;
+    let out = data
+        .users
+        .entry(userid.clone())
+        .or_default()
+        .add_recovery()?;
     write(&data)?;
     Ok(out)
 }
@@ -535,11 +878,33 @@ pub fn finish_u2f_registration(
 ) -> Result<String, Error> {
     let _lock = crate::config::tfa::write_lock();
     let mut data = read()?;
-    let challenge = data.u2f_registration_finish(userid, challenge, response)?;
+    let id = data.u2f_registration_finish(userid, challenge, response)?;
+    write(&data)?;
+    Ok(id)
+}
+
+/// Add a webauthn registration challenge for a user.
+pub fn add_webauthn_registration(userid: &Userid, description: String) -> Result<String, Error> {
+    let _lock = crate::config::tfa::write_lock();
+    let mut data = read()?;
+    let challenge = data.webauthn_registration_challenge(userid, description)?;
     write(&data)?;
     Ok(challenge)
 }
 
+/// Finish a webauthn registration challenge for a user.
+pub fn finish_webauthn_registration(
+    userid: &Userid,
+    challenge: &str,
+    response: &str,
+) -> Result<String, Error> {
+    let _lock = crate::config::tfa::write_lock();
+    let mut data = read()?;
+    let id = data.webauthn_registration_finish(userid, challenge, response)?;
+    write(&data)?;
+    Ok(id)
+}
+
 /// Verify a TFA challenge.
 pub fn verify_challenge(
     userid: &Userid,
@@ -585,7 +950,8 @@ impl Default for RecoveryState {
 }
 
 /// When sending a TFA challenge to the user, we include information about what kind of challenge
-/// the user may perform. If u2f devices are available, a u2f challenge will be included.
+/// the user may perform. If webauthn credentials are available, a webauthn challenge will be
+/// included.
 #[derive(Deserialize, Serialize)]
 #[serde(rename_all = "kebab-case")]
 pub struct TfaChallenge {
@@ -599,6 +965,11 @@ pub struct TfaChallenge {
     /// If the user has any u2f tokens registered, this will contain the U2F challenge data.
     #[serde(skip_serializing_if = "Option::is_none")]
     u2f: Option<U2fChallenge>,
+
+    /// If the user has any webauthn credentials registered, this will contain the corresponding
+    /// challenge data.
+    #[serde(skip_serializing_if = "Option::is_none", skip_deserializing)]
+    webauthn: Option<webauthn_rs::proto::RequestChallengeResponse>,
 }
 
 /// Data used for u2f challenges.
@@ -615,6 +986,7 @@ pub struct U2fChallenge {
 pub enum TfaResponse {
     Totp(String),
     U2f(Value),
+    Webauthn(Value),
     Recovery(String),
 }
 
@@ -626,6 +998,8 @@ impl std::str::FromStr for TfaResponse {
             TfaResponse::Totp(s[5..].to_string())
         } else if s.starts_with("u2f:") {
             TfaResponse::U2f(serde_json::from_str(&s[4..])?)
+        } else if s.starts_with("webauthn:") {
+            TfaResponse::Webauthn(serde_json::from_str(&s[9..])?)
         } else if s.starts_with("recovery:") {
             TfaResponse::Recovery(s[9..].to_string())
         } else {
diff --git a/src/server.rs b/src/server.rs
index 983a300d..7c159c23 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -87,3 +87,5 @@ pub use email_notifications::*;
 
 mod report;
 pub use report::*;
+
+pub mod ticket;
diff --git a/src/server/rest.rs b/src/server/rest.rs
index da110507..7a5bf3ea 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -599,8 +599,9 @@ fn check_auth(
             let ticket = user_auth_data.ticket.clone();
             let ticket_lifetime = tools::ticket::TICKET_LIFETIME;
 
-            let userid: Userid = Ticket::parse(&ticket)?
-                .verify_with_time_frame(public_auth_key(), "PBS", None, -300..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) {
diff --git a/src/server/ticket.rs b/src/server/ticket.rs
new file mode 100644
index 00000000..0142a03a
--- /dev/null
+++ b/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()?))
+        }
+    }
+}
-- 
2.20.1






More information about the pbs-devel mailing list