From 831c43c91b99451dc4b64d7a08d78f6b10fb4de8 Mon Sep 17 00:00:00 2001
From: Dominik Csapak <d.csapak@proxmox.com>
Date: Thu, 25 Feb 2021 10:01:21 +0100
Subject: [PATCH] config/tfa: webauthn: disallow registering a token twice

by adding the existing credential id to the 'excludeCredentials' list

this prevents the browser from registering a token twice, which
lets authentication fail on some browser/token combinations
(e.g. onlykey/solokey+chromium)
while is seems this is currently a bug in chromium, in a future spec
update the underlying behaviour should be better defined, making this
an authenticator bug

also explicitly catch registering errors and show appropriate error messages

0: https://bugs.chromium.org/p/chromium/issues/detail?id=1087642

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/config/tfa.rs         | 15 +++++++++++++--
 www/window/AddWebauthn.js | 27 ++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index 29e0fb48..7c656d20 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -803,9 +803,20 @@ impl TfaUserData {
         userid: &Userid,
         description: String,
     ) -> Result<String, Error> {
+        let cred_ids: Vec<_> = self
+            .enabled_webauthn_entries()
+            .map(|cred| cred.cred_id.clone())
+            .collect();
+
         let userid_str = userid.to_string();
-        let (challenge, state) = webauthn
-            .generate_challenge_register(&userid_str, Some(UserVerificationPolicy::Discouraged))?;
+        let (challenge, state) = webauthn.generate_challenge_register_options(
+            userid_str.as_bytes().to_vec(),
+            userid_str.clone(),
+            userid_str.clone(),
+            Some(cred_ids),
+            Some(UserVerificationPolicy::Discouraged),
+        )?;
+
         let challenge_string = challenge.public_key.challenge.to_string();
         let challenge = serde_json::to_string(&challenge)?;
 
diff --git a/www/window/AddWebauthn.js b/www/window/AddWebauthn.js
index 16731a63..f6d1df61 100644
--- a/www/window/AddWebauthn.js
+++ b/www/window/AddWebauthn.js
@@ -82,13 +82,38 @@ Ext.define('PBS.window.AddWebauthn', {
 		challenge_obj.publicKey.user.id =
 		    PBS.Utils.base64url_to_bytes(challenge_obj.publicKey.user.id);
 
+		// convert existing authenticators structure
+		challenge_obj.publicKey.excludeCredentials =
+		    (challenge_obj.publicKey.excludeCredentials || []).map((cred) => ({
+			id: PBS.Utils.base64url_to_bytes(cred.id),
+			type: cred.type,
+		    }));
+
 		let msg = Ext.Msg.show({
 		    title: `Webauthn: ${gettext('Setup')}`,
 		    message: gettext('Please press the button on your Webauthn Device'),
 		    buttons: [],
 		});
 
-		let token_response = await navigator.credentials.create(challenge_obj);
+		let token_response;
+		try {
+		    token_response = await navigator.credentials.create(challenge_obj);
+		} catch (error) {
+		    let errmsg = `<i class="fa fa-warning warning"></i>
+			${error.name}: ${error.message}`;
+		    if (error.name === 'InvalidStateError') {
+			// probably a duplicate token
+			throw `${gettext('There was an error during authenticator registration.')}
+			    <br>
+			    ${gettext('This probably means that this authenticator is already registered.')}
+			    <br><br>
+			    ${errmsg}`;
+		    } else {
+			throw `${gettext('There was an error during token registration.')}
+			    <br><br>
+			    ${errmsg}`;
+		    }
+		}
 
 		// We cannot pass ArrayBuffers to the API, so extract & convert the data.
 		let response = {