From a670b99db1512b6d4b24455f2c987d6e5599c462 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 15 Jan 2021 11:06:17 +0100 Subject: [PATCH] tfa: add webauthn configuration API entry points Currently there's not yet a node config and the WA config is somewhat "tightly coupled" to the user entries in that changing it can lock them all out, so for now I opted for fewer reorganization and just use a digest of the canonicalized config here, and keep it all in the tfa.json file. Experimentally using the flatten feature on the methods with an`Updater` struct similar to what the api macro is supposed to be able to derive on its own in the future. Signed-off-by: Wolfgang Bumiller --- src/api2/config.rs | 4 +- src/api2/config/access/mod.rs | 10 ++++ src/api2/config/access/tfa/mod.rs | 84 +++++++++++++++++++++++++++++++ src/config/tfa.rs | 72 ++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 src/api2/config/access/mod.rs create mode 100644 src/api2/config/access/tfa/mod.rs diff --git a/src/api2/config.rs b/src/api2/config.rs index 0e269b77..7ad43f9f 100644 --- a/src/api2/config.rs +++ b/src/api2/config.rs @@ -1,6 +1,7 @@ use proxmox::api::router::{Router, SubdirMap}; use proxmox::list_subdirs_api_method; +pub mod access; pub mod datastore; pub mod remote; pub mod sync; @@ -10,13 +11,14 @@ pub mod changer; pub mod media_pool; const SUBDIRS: SubdirMap = &[ + ("access", &access::ROUTER), ("changer", &changer::ROUTER), ("datastore", &datastore::ROUTER), ("drive", &drive::ROUTER), ("media-pool", &media_pool::ROUTER), ("remote", &remote::ROUTER), ("sync", &sync::ROUTER), - ("verify", &verify::ROUTER) + ("verify", &verify::ROUTER), ]; pub const ROUTER: Router = Router::new() diff --git a/src/api2/config/access/mod.rs b/src/api2/config/access/mod.rs new file mode 100644 index 00000000..659815e0 --- /dev/null +++ b/src/api2/config/access/mod.rs @@ -0,0 +1,10 @@ +use proxmox::api::{Router, SubdirMap}; +use proxmox::list_subdirs_api_method; + +pub mod tfa; + +const SUBDIRS: SubdirMap = &[("tfa", &tfa::ROUTER)]; + +pub const ROUTER: Router = Router::new() + .get(&list_subdirs_api_method!(SUBDIRS)) + .subdirs(SUBDIRS); diff --git a/src/api2/config/access/tfa/mod.rs b/src/api2/config/access/tfa/mod.rs new file mode 100644 index 00000000..63c34815 --- /dev/null +++ b/src/api2/config/access/tfa/mod.rs @@ -0,0 +1,84 @@ +//! For now this only has the TFA subdir, which is in this file. +//! If we add more, it should be moved into a sub module. + +use anyhow::Error; + +use crate::api2::types::PROXMOX_CONFIG_DIGEST_SCHEMA; +use proxmox::api::{api, Permission, Router, RpcEnvironment, SubdirMap}; +use proxmox::list_subdirs_api_method; + +use crate::config::tfa::{self, WebauthnConfig, WebauthnConfigUpdater}; + +pub const ROUTER: Router = Router::new() + .get(&list_subdirs_api_method!(SUBDIRS)) + .subdirs(SUBDIRS); + +const SUBDIRS: SubdirMap = &[("webauthn", &WEBAUTHN_ROUTER)]; + +const WEBAUTHN_ROUTER: Router = Router::new() + .get(&API_METHOD_GET_WEBAUTHN_CONFIG) + .put(&API_METHOD_UPDATE_WEBAUTHN_CONFIG); + +#[api( + protected: true, + input: { + properties: {}, + }, + returns: { + type: WebauthnConfig, + optional: true, + }, + access: { + permission: &Permission::Anybody, + }, +)] +/// Get the TFA configuration. +pub fn get_webauthn_config( + mut rpcenv: &mut dyn RpcEnvironment, +) -> Result, Error> { + let (config, digest) = match tfa::webauthn_config()? { + Some(c) => c, + None => return Ok(None), + }; + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); + Ok(Some(config)) +} + +#[api( + protected: true, + input: { + properties: { + webauthn: { + flatten: true, + type: WebauthnConfigUpdater, + }, + digest: { + optional: true, + schema: PROXMOX_CONFIG_DIGEST_SCHEMA, + }, + }, + }, +)] +/// Update the TFA configuration. +pub fn update_webauthn_config( + webauthn: WebauthnConfigUpdater, + digest: Option, +) -> Result<(), Error> { + let _lock = tfa::write_lock(); + + let mut tfa = tfa::read()?; + + if let Some(wa) = &mut tfa.webauthn { + if let Some(ref digest) = digest { + let digest = proxmox::tools::hex_to_digest(digest)?; + crate::tools::detect_modified_configuration_file(&digest, &wa.digest()?)?; + } + webauthn.apply_to(wa); + } else { + tfa.webauthn = Some(webauthn.build()?); + } + + tfa::write(&tfa)?; + + Ok(()) +} diff --git a/src/config/tfa.rs b/src/config/tfa.rs index 9e9e9516..e0f2fcfe 100644 --- a/src/config/tfa.rs +++ b/src/config/tfa.rs @@ -58,6 +58,21 @@ pub fn read() -> Result { Ok(serde_json::from_reader(file)?) } +/// Get the webauthn config with a digest. +/// +/// This is meant only for configuration updates, which currently only means webauthn updates. +/// Since this is meant to be done only once (since changes will lock out users), this should be +/// used rarely, since the digest calculation is currently a bit more involved. +pub fn webauthn_config() -> Result, Error>{ + Ok(match read()?.webauthn { + Some(wa) => { + let digest = wa.digest()?; + Some((wa, digest)) + } + None => None, + }) +} + /// Requires the write lock to be held. pub fn write(data: &TfaConfig) -> Result<(), Error> { let options = CreateOptions::new().perm(Mode::from_bits_truncate(0o0600)); @@ -71,7 +86,10 @@ pub struct U2fConfig { appid: String, } +#[api] #[derive(Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +/// Server side webauthn server configuration. pub struct WebauthnConfig { /// Relying party name. Any text identifier. /// @@ -90,6 +108,60 @@ pub struct WebauthnConfig { id: String, } +impl WebauthnConfig { + pub fn digest(&self) -> Result<[u8; 32], Error> { + let digest_data = crate::tools::json::to_canonical_json(&serde_json::to_value(self)?)?; + Ok(openssl::sha::sha256(&digest_data)) + } +} + +// TODO: api macro should be able to generate this struct & impl automatically: +#[api] +#[derive(Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +/// Server side webauthn server configuration. +pub struct WebauthnConfigUpdater { + /// Relying party name. Any text identifier. + /// + /// Changing this *may* break existing credentials. + rp: Option, + + /// 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: Option, + + /// Relying part ID. Must be the domain name without protocol, port or location. + /// + /// Changing this *will* break existing credentials. + id: Option, +} + +impl WebauthnConfigUpdater { + pub fn apply_to(self, target: &mut WebauthnConfig) { + if let Some(val) = self.rp { + target.rp = val; + } + + if let Some(val) = self.origin { + target.origin = val; + } + + if let Some(val) = self.id { + target.id = val; + } + } + + pub fn build(self) -> Result { + Ok(WebauthnConfig { + rp: self.rp.ok_or_else(|| format_err!("missing required field: `rp`"))?, + origin: self.origin.ok_or_else(|| format_err!("missing required field: `origin`"))?, + id: self.id.ok_or_else(|| format_err!("missing required field: `origin`"))?, + }) + } +} + /// 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