From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup v2 3/4] config/tfa: webauthn: disallow registering a token twice
Date: Thu, 25 Feb 2021 10:01:21 +0100 [thread overview]
Message-ID: <20210225090122.1094-4-d.csapak@proxmox.com> (raw)
In-Reply-To: <20210225090122.1094-1-d.csapak@proxmox.com>
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
next prev parent reply other threads:[~2021-02-25 9:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-25 9:01 [pbs-devel] [PATCH proxmox-backup v2 0/4] improving webauthn handling Dominik Csapak
2021-02-25 9:01 ` [pbs-devel] [PATCH proxmox-backup v2 1/4] config/tfa: set UserVerificationPolicy to Discouraged Dominik Csapak
2021-02-25 9:01 ` [pbs-devel] [PATCH proxmox-backup v2 2/4] Revert "ui: window/Settings / WebAuthn: add browser setting for userVerificationo" Dominik Csapak
2021-02-25 9:01 ` Dominik Csapak [this message]
2021-02-25 9:01 ` [pbs-devel] [PATCH proxmox-backup v2 4/4] ui: LoginView: show webauthn errors in window Dominik Csapak
2021-03-03 13:05 ` [pbs-devel] applied-series: [PATCH proxmox-backup v2 0/4] improving webauthn handling Thomas Lamprecht
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=20210225090122.1094-4-d.csapak@proxmox.com \
--to=d.csapak@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal