public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH backup 3/4] tfa: add webauthn configuration API entry points
Date: Fri, 15 Jan 2021 10:20:48 +0100	[thread overview]
Message-ID: <20210115092049.16706-3-w.bumiller@proxmox.com> (raw)
In-Reply-To: <20210115092049.16706-1-w.bumiller@proxmox.com>

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 <w.bumiller@proxmox.com>
---
 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<Option<WebauthnConfig>, 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<String>,
+) -> 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<TfaConfig, Error> {
     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<Option<(WebauthnConfig, [u8; 32])>, 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<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: Option<String>,
+
+    /// Relying part ID. Must be the domain name without protocol, port or location.
+    ///
+    /// Changing this *will* break existing credentials.
+    id: Option<String>,
+}
+
+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<WebauthnConfig, Error> {
+        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
-- 
2.20.1





  parent reply	other threads:[~2021-01-15  9:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15  9:20 [pbs-devel] [PATCH backup 1/4] add tools::json for canonical json generation Wolfgang Bumiller
2021-01-15  9:20 ` [pbs-devel] [PATCH backup 2/4] bakckup::manifest: use tools::json for canonical representation Wolfgang Bumiller
2021-01-15  9:20 ` Wolfgang Bumiller [this message]
2021-01-15  9:20 ` [pbs-devel] [PATCH backup 4/4] gui: tfa configuration Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210115092049.16706-3-w.bumiller@proxmox.com \
    --to=w.bumiller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal