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 CFAC56C257
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Feb 2021 10:43:04 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id CE0A92D314
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Feb 2021 10:43:04 +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) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 0777D2D2EF
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Feb 2021 10:43:03 +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 CE62D41ADC
 for <pbs-devel@lists.proxmox.com>; Mon, 22 Feb 2021 10:43:02 +0100 (CET)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Mon, 22 Feb 2021 10:42:59 +0100
Message-Id: <20210222094301.13858-2-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.20.1
In-Reply-To: <20210222094301.13858-1-d.csapak@proxmox.com>
References: <20210222094301.13858-1-d.csapak@proxmox.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.212 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 1/3] config/tfa: set
 UserVerificationPolicy to Discouraged
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: Mon, 22 Feb 2021 09:43:04 -0000

the current default is 'Preferred', which is not really useful, as the
(web) client can simply change this to discouraged, since the
webauthn_rs crate does not verify the 'user_verified' bit of the
response in that case

setting this to 'Required' is not really useful either at the moment,
since a user can have a mix of different authenticators that may or
may not support user verification

there is ongoing discussion in the crate how to handle user verification[0]

we could probably expose this setting(discouraged/required) to the user/admin
and save it to the credential and allow only registering credentials
of the same type or filter them out on login (i.e. if there is an
authenticator that can handle userVerification, require it)

in any case, the current default is not helpful for security, but
makes logging in harder, since the key will by default want to verify
the user (if it can)

0: https://github.com/kanidm/webauthn-rs/pull/49

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/config/tfa.rs | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index 5afb5827..29e0fb48 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -13,7 +13,7 @@ use openssl::pkey::PKey;
 use openssl::sign::Signer;
 use serde::{de::Deserializer, Deserialize, Serialize};
 use serde_json::Value;
-use webauthn_rs::Webauthn;
+use webauthn_rs::{proto::UserVerificationPolicy, Webauthn};
 
 use webauthn_rs::proto::Credential as WebauthnCredential;
 
@@ -804,7 +804,8 @@ impl TfaUserData {
         description: String,
     ) -> Result<String, Error> {
         let userid_str = userid.to_string();
-        let (challenge, state) = webauthn.generate_challenge_register(&userid_str, None)?;
+        let (challenge, state) = webauthn
+            .generate_challenge_register(&userid_str, Some(UserVerificationPolicy::Discouraged))?;
         let challenge_string = challenge.public_key.challenge.to_string();
         let challenge = serde_json::to_string(&challenge)?;
 
@@ -923,7 +924,8 @@ impl TfaUserData {
             return Ok(None);
         }
 
-        let (challenge, state) = webauthn.generate_challenge_authenticate(creds, None)?;
+        let (challenge, state) = webauthn
+            .generate_challenge_authenticate(creds, Some(UserVerificationPolicy::Discouraged))?;
         let challenge_string = challenge.public_key.challenge.to_string();
         let mut data = TfaUserChallengeData::open(userid)?;
         data.inner
-- 
2.20.1