From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 21FD069D96
 for <pbs-devel@lists.proxmox.com>; Thu, 25 Feb 2021 10:01:56 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 1CE7F31F16
 for <pbs-devel@lists.proxmox.com>; Thu, 25 Feb 2021 10:01:26 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [212.186.127.180])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 835E431EF5
 for <pbs-devel@lists.proxmox.com>; Thu, 25 Feb 2021 10:01:24 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 50D0343738
 for <pbs-devel@lists.proxmox.com>; Thu, 25 Feb 2021 10:01:24 +0100 (CET)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Thu, 25 Feb 2021 10:01:21 +0100
Message-Id: <20210225090122.1094-4-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.20.1
In-Reply-To: <20210225090122.1094-1-d.csapak@proxmox.com>
References: <20210225090122.1094-1-d.csapak@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.206 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_DNSWL_MED        -2.3 Sender listed at https://www.dnswl.org/,
 medium trust
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH proxmox-backup v2 3/4] config/tfa: webauthn:
 disallow registering a token twice
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Thu, 25 Feb 2021 09:01:56 -0000

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 = {
-- 
2.20.1