tfa: bump webauthn-rs to 0.3

switch WebauthnConfig to use Url for the origin field, via a wrapper
type to make Updater and ApiType happy.

the two new Credential fields `verified` and `registration_policy` are
always set to `false` and `Discouraged`, to get the same behaviour as
before.

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler 2021-11-18 13:04:33 +01:00
parent 148950fd17
commit 91932da15c
3 changed files with 44 additions and 23 deletions

View File

@ -19,11 +19,12 @@ serde = "1.0"
serde_plain = "1.0" serde_plain = "1.0"
serde_json = { version = "1.0", optional = true } serde_json = { version = "1.0", optional = true }
libc = { version = "0.2", optional = true } libc = { version = "0.2", optional = true }
url = "2.2"
proxmox-schema = { version = "1", path = "../proxmox-schema", features = [ "api-macro" ], optional = true } proxmox-schema = { version = "1", path = "../proxmox-schema", features = [ "api-macro" ], optional = true }
proxmox-time = { version = "1", path = "../proxmox-time", optional = true } proxmox-time = { version = "1", path = "../proxmox-time", optional = true }
proxmox-uuid = { version = "1", path = "../proxmox-uuid", optional = true } proxmox-uuid = { version = "1", path = "../proxmox-uuid", optional = true }
webauthn-rs = { version = "0.2.5", optional = true } webauthn-rs = { version = "0.3", optional = true }
[features] [features]
default = [] default = []

View File

@ -421,7 +421,7 @@ impl TfaUserData {
fn webauthn_registration_challenge<A: OpenUserChallengeData>( fn webauthn_registration_challenge<A: OpenUserChallengeData>(
&mut self, &mut self,
access: A, access: A,
mut webauthn: Webauthn<WebauthnConfig>, webauthn: Webauthn<WebauthnConfig>,
userid: &str, userid: &str,
description: String, description: String,
) -> Result<String, Error> { ) -> Result<String, Error> {
@ -436,6 +436,7 @@ impl TfaUserData {
userid.to_owned(), userid.to_owned(),
Some(cred_ids), Some(cred_ids),
Some(UserVerificationPolicy::Discouraged), Some(UserVerificationPolicy::Discouraged),
None,
)?; )?;
let challenge_string = challenge.public_key.challenge.to_string(); let challenge_string = challenge.public_key.challenge.to_string();
@ -587,7 +588,7 @@ impl TfaUserData {
&mut self, &mut self,
access: A, access: A,
userid: &str, userid: &str,
mut webauthn: Webauthn<WebauthnConfig>, webauthn: Webauthn<WebauthnConfig>,
) -> Result<Option<webauthn_rs::proto::RequestChallengeResponse>, Error> { ) -> Result<Option<webauthn_rs::proto::RequestChallengeResponse>, Error> {
if self.webauthn.is_empty() { if self.webauthn.is_empty() {
return Ok(None); return Ok(None);
@ -602,8 +603,8 @@ impl TfaUserData {
return Ok(None); return Ok(None);
} }
let (challenge, state) = webauthn let (challenge, state) = webauthn.generate_challenge_authenticate(creds)?;
.generate_challenge_authenticate(creds, Some(UserVerificationPolicy::Discouraged))?;
let challenge_string = challenge.public_key.challenge.to_string(); let challenge_string = challenge.public_key.challenge.to_string();
let mut data = access.open(userid)?; let mut data = access.open(userid)?;
data.get_mut() data.get_mut()
@ -699,7 +700,7 @@ impl TfaUserData {
&mut self, &mut self,
access: A, access: A,
userid: &str, userid: &str,
mut webauthn: Webauthn<WebauthnConfig>, webauthn: Webauthn<WebauthnConfig>,
mut response: Value, mut response: Value,
) -> Result<(), Error> { ) -> Result<(), Error> {
let expire_before = proxmox_time::epoch_i64() - CHALLENGE_TIMEOUT_SECS; let expire_before = proxmox_time::epoch_i64() - CHALLENGE_TIMEOUT_SECS;
@ -738,10 +739,9 @@ impl TfaUserData {
data.save() data.save()
.map_err(|err| format_err!("failed to save challenge file: {}", err))?; .map_err(|err| format_err!("failed to save challenge file: {}", err))?;
match webauthn.authenticate_credential(response, challenge.state)? { webauthn.authenticate_credential(&response, &challenge.state)?;
Some((_cred, _counter)) => Ok(()),
None => bail!("webauthn authentication failed"), Ok(())
}
} }
/// Verify a recovery key. /// Verify a recovery key.
@ -1010,8 +1010,8 @@ impl TfaUserChallenges {
bail!("no such challenge"); bail!("no such challenge");
} }
let credential = let (credential, _authenticator) =
webauthn.register_credential(response, reg.state, |id| -> Result<bool, ()> { webauthn.register_credential(&response, &reg.state, |id| -> Result<bool, ()> {
Ok(existing_registrations Ok(existing_registrations
.iter() .iter()
.any(|cred| cred.entry.cred_id == *id)) .any(|cred| cred.entry.cred_id == *id))

View File

@ -3,14 +3,30 @@
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
#[cfg(feature = "api-types")] #[cfg(feature = "api-types")]
use proxmox_schema::{api, Updater}; use proxmox_schema::{api, Updater, UpdaterType};
use webauthn_rs::crypto::COSEKey; use url::Url;
use webauthn_rs::proto::{Credential, CredentialID};
use webauthn_rs::proto::{COSEKey, Credential, CredentialID, UserVerificationPolicy};
use super::IsExpired; use super::IsExpired;
#[cfg_attr(feature = "api-types", api)] #[derive(Clone, Deserialize, Serialize)]
/// Origin URL for WebauthnConfig
pub struct OriginUrl(Url);
#[cfg(feature = "api-types")]
impl UpdaterType for OriginUrl {
type Updater = Option<Self>;
}
#[cfg_attr(feature = "api-types", api(
properties: {
rp: { type: String },
origin: { type: String },
id: { type: String },
}
))]
#[cfg_attr(feature = "api-types", derive(Updater))] #[cfg_attr(feature = "api-types", derive(Updater))]
/// Server side webauthn server configuration. /// Server side webauthn server configuration.
#[derive(Clone, Deserialize, Serialize)] #[derive(Clone, Deserialize, Serialize)]
@ -25,7 +41,7 @@ pub struct WebauthnConfig {
/// users type in their browsers to access the web interface. /// users type in their browsers to access the web interface.
/// ///
/// Changing this *may* break existing credentials. /// Changing this *may* break existing credentials.
pub origin: String, pub origin: OriginUrl,
/// Relying part ID. Must be the domain name without protocol, port or location. /// Relying part ID. Must be the domain name without protocol, port or location.
/// ///
@ -38,16 +54,16 @@ pub struct WebauthnConfig {
/// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by /// Note that we may consider changing this so `get_origin` returns the `Host:` header provided by
/// the connecting client. /// the connecting client.
impl webauthn_rs::WebauthnConfig for WebauthnConfig { impl webauthn_rs::WebauthnConfig for WebauthnConfig {
fn get_relying_party_name(&self) -> String { fn get_relying_party_name(&self) -> &str {
self.rp.clone() &self.rp
} }
fn get_origin(&self) -> &String { fn get_origin(&self) -> &Url {
&self.origin &self.origin.0
} }
fn get_relying_party_id(&self) -> String { fn get_relying_party_id(&self) -> &str {
self.id.clone() &self.id
} }
} }
@ -132,6 +148,7 @@ pub struct WebauthnCredential {
pub counter: u32, pub counter: u32,
} }
/// ignores verified and registration_policy fields for now
impl From<Credential> for WebauthnCredential { impl From<Credential> for WebauthnCredential {
fn from(cred: Credential) -> Self { fn from(cred: Credential) -> Self {
Self { Self {
@ -142,12 +159,15 @@ impl From<Credential> for WebauthnCredential {
} }
} }
/// always sets verified to false and registration_policy to Discouraged for now
impl From<WebauthnCredential> for Credential { impl From<WebauthnCredential> for Credential {
fn from(val: WebauthnCredential) -> Self { fn from(val: WebauthnCredential) -> Self {
Credential { Credential {
cred_id: val.cred_id, cred_id: val.cred_id,
cred: val.cred, cred: val.cred,
counter: val.counter, counter: val.counter,
verified: false,
registration_policy: UserVerificationPolicy::Discouraged,
} }
} }
} }