[pbs-devel] [PATCH v2 backup 1/4] add tools::json for canonical json generation

[pbs-devel] [PATCH v2 backup 1/4] add tools::json for canonical json generation

[Wolfgang Bumiller]

moving this from backup::manifest, no functional changes

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
no changs since v2

 src/tools.rs      |  1 +
 src/tools/json.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 src/tools/json.rs

diff --git a/src/tools.rs b/src/tools.rs
index 599bbd04..d96cd647 100644
--- a/src/tools.rs
+++ b/src/tools.rs
@@ -28,6 +28,7 @@ pub mod format;
 pub mod fs;
 pub mod fuse_loop;
 pub mod http;
+pub mod json;
 pub mod logrotate;
 pub mod loopdev;
 pub mod lru_cache;
diff --git a/src/tools/json.rs b/src/tools/json.rs
new file mode 100644
index 00000000..c81afccb
--- /dev/null
+++ b/src/tools/json.rs
@@ -0,0 +1,49 @@
+use anyhow::{bail, Error};
+use serde_json::Value;
+
+// Generate canonical json
+pub fn to_canonical_json(value: &Value) -> Result<Vec<u8>, Error> {
+    let mut data = Vec::new();
+    write_canonical_json(value, &mut data)?;
+    Ok(data)
+}
+
+pub fn write_canonical_json(value: &Value, output: &mut Vec<u8>) -> Result<(), Error> {
+    match value {
+        Value::Null => bail!("got unexpected null value"),
+        Value::String(_) | Value::Number(_) | Value::Bool(_) => {
+            serde_json::to_writer(output, &value)?;
+        }
+        Value::Array(list) => {
+            output.push(b'[');
+            let mut iter = list.iter();
+            if let Some(item) = iter.next() {
+                write_canonical_json(item, output)?;
+                for item in iter {
+                    output.push(b',');
+                    write_canonical_json(item, output)?;
+                }
+            }
+            output.push(b']');
+        }
+        Value::Object(map) => {
+            output.push(b'{');
+            let mut keys: Vec<&str> = map.keys().map(String::as_str).collect();
+            keys.sort();
+            let mut iter = keys.into_iter();
+            if let Some(key) = iter.next() {
+                serde_json::to_writer(&mut *output, &key)?;
+                output.push(b':');
+                write_canonical_json(&map[key], output)?;
+                for key in iter {
+                    output.push(b',');
+                    serde_json::to_writer(&mut *output, &key)?;
+                    output.push(b':');
+                    write_canonical_json(&map[key], output)?;
+                }
+            }
+            output.push(b'}');
+        }
+    }
+    Ok(())
+}
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 2/4] bakckup::manifest: use tools::json for canonical representation

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
no changs since v2

 src/backup/manifest.rs | 44 +-----------------------------------------
 1 file changed, 1 insertion(+), 43 deletions(-)

diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index 8742cb0a..d9a55655 100644
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -149,49 +149,7 @@ impl BackupManifest {
 
     // Generate canonical json
     fn to_canonical_json(value: &Value) -> Result<Vec<u8>, Error> {
-        let mut data = Vec::new();
-        Self::write_canonical_json(value, &mut data)?;
-        Ok(data)
-    }
-
-    fn write_canonical_json(value: &Value, output: &mut Vec<u8>) -> Result<(), Error> {
-        match value {
-            Value::Null => bail!("got unexpected null value"),
-            Value::String(_) | Value::Number(_) | Value::Bool(_) => {
-                serde_json::to_writer(output, &value)?;
-            }
-            Value::Array(list) => {
-                output.push(b'[');
-                let mut iter = list.iter();
-                if let Some(item) = iter.next() {
-                    Self::write_canonical_json(item, output)?;
-                    for item in iter {
-                        output.push(b',');
-                        Self::write_canonical_json(item, output)?;
-                    }
-                }
-                output.push(b']');
-              }
-            Value::Object(map) => {
-                output.push(b'{');
-                let mut keys: Vec<&str> = map.keys().map(String::as_str).collect();
-                keys.sort();
-                let mut iter = keys.into_iter();
-                if let Some(key) = iter.next() {
-                    serde_json::to_writer(&mut *output, &key)?;
-                    output.push(b':');
-                    Self::write_canonical_json(&map[key], output)?;
-                    for key in iter {
-                        output.push(b',');
-                        serde_json::to_writer(&mut *output, &key)?;
-                        output.push(b':');
-                        Self::write_canonical_json(&map[key], output)?;
-                    }
-                }
-                output.push(b'}');
-            }
-        }
-        Ok(())
+        crate::tools::json::to_canonical_json(value)
     }
 
     /// Compute manifest signature
-- 
2.20.1

[pbs-devel] [PATCH v2 backup 3/4] tfa: add webauthn configuration API entry points

[Wolfgang Bumiller]

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>
---
no changs since v2

 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

[pbs-devel] [PATCH v2 backup 4/4] gui: tfa configuration

[Wolfgang Bumiller]

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
Changes to v1:
* removed initComponent functions in favor of inlined data
* fixed a wrong label copy-pasta

 www/Makefile               |  1 +
 www/SystemConfiguration.js | 26 ++++++++++++
 www/config/WebauthnView.js | 83 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 www/config/WebauthnView.js

diff --git a/www/Makefile b/www/Makefile
index 3d8d4fa1..c2d80c74 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -30,6 +30,7 @@ JSSRC=							\
 	config/ACLView.js				\
 	config/SyncView.js				\
 	config/VerifyView.js				\
+	config/WebauthnView.js				\
 	window/ACLEdit.js				\
 	window/AddTfaRecovery.js			\
 	window/AddTotp.js				\
diff --git a/www/SystemConfiguration.js b/www/SystemConfiguration.js
index d82396af..025e0273 100644
--- a/www/SystemConfiguration.js
+++ b/www/SystemConfiguration.js
@@ -44,6 +44,27 @@ Ext.define('PBS.SystemConfiguration', {
 		},
 	    ],
 	},
+	{
+	    title: gettext('Authentication'),
+	    itemId: 'authentication',
+	    xtype: 'panel',
+	    layout: {
+		type: 'vbox',
+		align: 'stretch',
+		multi: true,
+	    },
+	    defaults: {
+		collapsible: true,
+		animCollapse: false,
+		margin: '10 10 0 10',
+	    },
+	    items: [
+		{
+		    title: gettext('Webauthn'),
+		    xtype: 'pbsWebauthnConfigView',
+		},
+	    ],
+	},
     ],
 
     initComponent: function() {
@@ -55,6 +76,11 @@ Ext.define('PBS.SystemConfiguration', {
 	Ext.Array.forEach(networktime.query(), function(item) {
 	    item.relayEvents(networktime, ['activate', 'deactivate', 'destroy']);
 	});
+
+	let authentication = me.getComponent('authentication');
+	Ext.Array.forEach(authentication.query(), function(item) {
+	    item.relayEvents(authentication, ['activate', 'deactivate', 'destroy']);
+	});
     },
 });
 
diff --git a/www/config/WebauthnView.js b/www/config/WebauthnView.js
new file mode 100644
index 00000000..914abcdc
--- /dev/null
+++ b/www/config/WebauthnView.js
@@ -0,0 +1,83 @@
+Ext.define('PBS.WebauthnConfigView', {
+    extend: 'Proxmox.grid.ObjectGrid',
+    alias: ['widget.pbsWebauthnConfigView'],
+
+    url: "/api2/json/config/access/tfa/webauthn",
+    cwidth1: 150,
+    interval: 1000,
+
+    rows: {
+	rp: {
+	    header: gettext('Relying Party'),
+	    required: true,
+	},
+	origin: {
+	    header: gettext('Origin'),
+	    required: true,
+	},
+	id: {
+	    header: gettext('Id'),
+	    required: true,
+	},
+    },
+
+    tbar: [
+	{
+	    text: gettext("Edit"),
+	    handler: 'runEditor',
+	},
+    ],
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	runEditor: function() {
+	    let win = Ext.create('PBS.WebauthnConfigEdit');
+	    win.show();
+	},
+
+	startStore: function() { this.getView().getStore().rstore.startUpdate(); },
+	stopStore: function() { this.getView().getStore().rstore.stopUpdate(); },
+    },
+
+
+    listeners: {
+	itemdblclick: 'runEditor',
+	activate: 'startStore',
+	deactivate: 'stopStore',
+	destroy: 'stopStore',
+    },
+});
+
+Ext.define('PBS.WebauthnConfigEdit', {
+    extend: 'Proxmox.window.Edit',
+    alias: ['widget.pbsWebauthnConfigEdit'],
+
+    subject: gettext('Webauthn'),
+    url: "/api2/extjs/config/access/tfa/webauthn",
+    autoLoad: true,
+
+    fieldDefaults: {
+	labelWidth: 120,
+    },
+
+    items: [
+	{
+	    xtype: 'textfield',
+	    fieldLabel: gettext('Relying Party'),
+	    name: 'rp',
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'textfield',
+	    fieldLabel: gettext('Origin'),
+	    name: 'origin',
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'textfield',
+	    fieldLabel: gettext('id'),
+	    name: 'id',
+	    allowBlank: false,
+	},
+    ],
+});
-- 
2.20.1

[pbs-devel] applied-series: [PATCH v2 backup 1/4] add tools::json for canonical json generation

[Thomas Lamprecht]

On 15.01.21 11:06, Wolfgang Bumiller wrote:
> moving this from backup::manifest, no functional changes
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> no changs since v2
> 
>  src/tools.rs      |  1 +
>  src/tools/json.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>  create mode 100644 src/tools/json.rs
> 
>

applied series, thanks!