From c7b42808a722e0222ccacd3fdb69cab5ca3d96e5 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Fri, 24 Nov 2023 10:05:28 +1000 Subject: [PATCH 1/5] Move `DEFAULT_AUTHENTICATOR_TIMEOUT` into `webauthn-rs`. This fixes a documentation build breakage caused by #385, and shifts default timeouts into our recommended interface. --- compat_tester/webauthn-rs-demo/src/actors.rs | 4 +- fido-key-manager/src/main.rs | 1 + .../examples/authenticate.rs | 2 +- webauthn-rs-core/src/constants.rs | 3 - webauthn-rs-core/src/core.rs | 69 ++++++++++--------- webauthn-rs/src/lib.rs | 11 +-- 6 files changed, 47 insertions(+), 43 deletions(-) diff --git a/compat_tester/webauthn-rs-demo/src/actors.rs b/compat_tester/webauthn-rs-demo/src/actors.rs index ef2436b9..a7b1440b 100644 --- a/compat_tester/webauthn-rs-demo/src/actors.rs +++ b/compat_tester/webauthn-rs-demo/src/actors.rs @@ -6,7 +6,7 @@ use webauthn_rs_core::proto::{ }; use webauthn_rs_core::proto::{AuthenticationState, RegistrationState}; -use webauthn_rs::{prelude::Uuid, Webauthn, WebauthnBuilder}; +use webauthn_rs::{prelude::Uuid, Webauthn, WebauthnBuilder, DEFAULT_AUTHENTICATOR_TIMEOUT}; use webauthn_rs_core::WebauthnCore; use webauthn_rs_demo_shared::*; @@ -61,7 +61,7 @@ impl WebauthnActor { &rp_name, &rp_id, vec![rp_origin.to_owned()], - None, + DEFAULT_AUTHENTICATOR_TIMEOUT, None, None, ); diff --git a/fido-key-manager/src/main.rs b/fido-key-manager/src/main.rs index d8f41276..c484bbc6 100644 --- a/fido-key-manager/src/main.rs +++ b/fido-key-manager/src/main.rs @@ -13,6 +13,7 @@ use std::time::Duration; use tokio_stream::StreamExt; #[cfg(feature = "solokey")] use webauthn_authenticator_rs::ctap2::SoloKeyAuthenticator; +#[cfg(feature = "solokey")] use webauthn_authenticator_rs::prelude::WebauthnCError; use webauthn_authenticator_rs::{ ctap2::{ diff --git a/webauthn-authenticator-rs/examples/authenticate.rs b/webauthn-authenticator-rs/examples/authenticate.rs index d74d4416..89707eec 100644 --- a/webauthn-authenticator-rs/examples/authenticate.rs +++ b/webauthn-authenticator-rs/examples/authenticate.rs @@ -237,7 +237,7 @@ async fn main() { "https://localhost:8080/auth", "localhost", vec![url::Url::parse("https://localhost:8080").unwrap()], - Some(Duration::from_millis(1)), + Duration::from_secs(60), None, None, ); diff --git a/webauthn-rs-core/src/constants.rs b/webauthn-rs-core/src/constants.rs index 0ba5661b..1ae36a88 100644 --- a/webauthn-rs-core/src/constants.rs +++ b/webauthn-rs-core/src/constants.rs @@ -1,5 +1,2 @@ -use std::time::Duration; - // Can this ever change? pub const CHALLENGE_SIZE_BYTES: usize = 32; -pub const DEFAULT_AUTHENTICATOR_TIMEOUT: Duration = Duration::from_millis(60000); diff --git a/webauthn-rs-core/src/core.rs b/webauthn-rs-core/src/core.rs index 285a3eee..3ff1c732 100644 --- a/webauthn-rs-core/src/core.rs +++ b/webauthn-rs-core/src/core.rs @@ -26,7 +26,7 @@ use crate::attestation::{ verify_apple_anonymous_attestation, verify_attestation_ca_chain, verify_fidou2f_attestation, verify_packed_attestation, verify_tpm_attestation, AttestationFormat, }; -use crate::constants::{CHALLENGE_SIZE_BYTES, DEFAULT_AUTHENTICATOR_TIMEOUT}; +use crate::constants::CHALLENGE_SIZE_BYTES; use crate::crypto::compute_sha256; use crate::error::WebauthnError; use crate::internals::*; @@ -85,7 +85,7 @@ impl WebauthnCore { rp_name: &str, rp_id: &str, allowed_origins: Vec, - authenticator_timeout: Option, + authenticator_timeout: Duration, allow_subdomains_origin: Option, allow_any_port: Option, ) -> Self { @@ -95,7 +95,7 @@ impl WebauthnCore { rp_id: rp_id.to_string(), rp_id_hash, allowed_origins, - authenticator_timeout: authenticator_timeout.unwrap_or(DEFAULT_AUTHENTICATOR_TIMEOUT), + authenticator_timeout, require_valid_counter_value: true, ignore_unsupported_attestation_formats: true, allow_cross_origin: false, @@ -1345,6 +1345,7 @@ mod tests { use crate::{internals::*, AttestationFormat}; use base64::{engine::general_purpose::STANDARD, Engine}; use base64urlsafedata::Base64UrlSafeData; + use std::time::Duration; use url::Url; use webauthn_rs_device_catalog::data::{ @@ -1355,6 +1356,8 @@ mod tests { yubico::YUBICO_U2F_ROOT_CA_SERIAL_457200631_PEM, }; + const AUTHENTICATOR_TIMEOUT: Duration = Duration::from_secs(60); + // Test the crypto operations of the webauthn impl #[test] @@ -1364,7 +1367,7 @@ mod tests { "http://127.0.0.1:8080/auth", "127.0.0.1", vec![Url::parse("http://127.0.0.1:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1415,7 +1418,7 @@ mod tests { "webauthn.io", "webauthn.io", vec![Url::parse("https://webauthn.io").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1460,7 +1463,7 @@ mod tests { "localhost:8443/auth", "localhost", vec![Url::parse("https://localhost:8443").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1505,7 +1508,7 @@ mod tests { "localhost:8080/auth", "localhost", vec![Url::parse("http://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1550,7 +1553,7 @@ mod tests { "webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au/compat_test").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1595,7 +1598,7 @@ mod tests { "webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au/compat_test").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1643,7 +1646,7 @@ mod tests { "localhost:8080/auth", "localhost", vec![Url::parse("http://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1765,7 +1768,7 @@ mod tests { "https://testing.local", "testing.local", vec![Url::parse("https://testing.local").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -1888,7 +1891,7 @@ mod tests { "https://172.20.0.141:8443/auth", "172.20.0.141", vec![Url::parse("https://172.20.0.141:8443").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2015,7 +2018,7 @@ mod tests { "https://etools-dev.example.com:8080/auth", "etools-dev.example.com", vec![Url::parse("https://etools-dev.example.com:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2159,7 +2162,7 @@ mod tests { "https://etools-dev.example.com:8080/auth", "etools-dev.example.com", vec![Url::parse("https://etools-dev.example.com:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2475,7 +2478,7 @@ mod tests { "https://etools-dev.example.com:8080/auth", "etools-dev.example.com", vec![Url::parse("https://etools-dev.example.com:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2509,7 +2512,7 @@ mod tests { "https://spectral.local:8443/auth", "spectral.local", vec![Url::parse("https://spectral.local:8443").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2711,7 +2714,7 @@ mod tests { "https://spectral.local:8443/auth", "spectral.local", vec![Url::parse("https://spectral.local:8443").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -2854,7 +2857,7 @@ mod tests { "http://127.0.0.1:8080/auth", "127.0.0.1", vec![Url::parse("http://127.0.0.1:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3005,7 +3008,7 @@ mod tests { "rp_name", "idm.example.com", vec![Url::parse("https://idm.example.com:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, Some(true), None, ); @@ -3177,7 +3180,7 @@ mod tests { "http://localhost:8080/auth", "localhost", vec![Url::parse("http://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3221,7 +3224,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3267,7 +3270,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3311,7 +3314,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3353,7 +3356,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3396,7 +3399,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3444,7 +3447,7 @@ mod tests { "https://webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3491,7 +3494,7 @@ mod tests { "http://localhost:8080/auth", "localhost", vec![Url::parse("http://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3611,7 +3614,7 @@ mod tests { "webauthn.io", "webauthn.io", vec![Url::parse("https://webauthn.io").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3671,7 +3674,7 @@ mod tests { "webauthn.io", "webauthn.io", vec![Url::parse("https://webauthn.io").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3730,7 +3733,7 @@ mod tests { "webauthn.org", "webauthn.org", vec![Url::parse("https://webauthn.org").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3799,7 +3802,7 @@ mod tests { "webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3866,7 +3869,7 @@ mod tests { "webauthn.firstyear.id.au", "webauthn.firstyear.id.au", vec![Url::parse("https://webauthn.firstyear.id.au").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -3920,7 +3923,7 @@ mod tests { "localhost", "localhost", vec![Url::parse("http://localhost:8080/").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); diff --git a/webauthn-rs/src/lib.rs b/webauthn-rs/src/lib.rs index 034c0625..f25b60f4 100644 --- a/webauthn-rs/src/lib.rs +++ b/webauthn-rs/src/lib.rs @@ -217,6 +217,9 @@ pub mod prelude { pub use webauthn_rs_core::AttestationFormat; } +/// The default authenticator interaction timeout, if none is otherwise specified. +pub const DEFAULT_AUTHENTICATOR_TIMEOUT: Duration = Duration::from_millis(60000); + /// A constructor for a new [Webauthn] instance. This accepts and configures a number of site-wide /// properties that apply to all webauthn operations of this service. #[derive(Debug)] @@ -226,7 +229,7 @@ pub struct WebauthnBuilder<'a> { allowed_origins: Vec, allow_subdomains: bool, allow_any_port: bool, - timeout: Option, + timeout: Duration, algorithms: Vec, user_presence_only_security_keys: bool, } @@ -282,7 +285,7 @@ impl<'a> WebauthnBuilder<'a> { allowed_origins: vec![rp_origin.to_owned()], allow_subdomains: false, allow_any_port: false, - timeout: None, + timeout: DEFAULT_AUTHENTICATOR_TIMEOUT, algorithms: COSEAlgorithm::secure_algs(), user_presence_only_security_keys: false, }) @@ -320,9 +323,9 @@ impl<'a> WebauthnBuilder<'a> { /// Set the timeout value to use for credential creation and authentication challenges. /// - /// If not set, defaults to [webauthn_rs_core::constants::DEFAULT_AUTHENTICATOR_TIMEOUT]. + /// If not set, defaults to [DEFAULT_AUTHENTICATOR_TIMEOUT]. pub fn timeout(mut self, timeout: Duration) -> Self { - self.timeout = Some(timeout); + self.timeout = timeout; self } From 21cd72da4a7ca15575f92f7e582d2859b06caab1 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Fri, 24 Nov 2023 10:29:23 +1000 Subject: [PATCH 2/5] Add PhantomData to stubs with type parameters, to fix a build issue on nightly --- webauthn-authenticator-rs/src/stubs.rs | 40 +++++++++++++++++++------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/webauthn-authenticator-rs/src/stubs.rs b/webauthn-authenticator-rs/src/stubs.rs index 68db60df..51883e27 100644 --- a/webauthn-authenticator-rs/src/stubs.rs +++ b/webauthn-authenticator-rs/src/stubs.rs @@ -54,15 +54,21 @@ pub mod tokio { } pub mod sync { pub mod mpsc { - pub struct Sender {} - pub struct Receiver {} + pub struct Sender { + _phantom: std::marker::PhantomData, + } + pub struct Receiver { + _phantom: std::marker::PhantomData, + } } } pub mod time { pub async fn sleep(_: std::time::Duration) {} } pub mod task { - pub struct JoinHandle {} + pub struct JoinHandle { + _phantom: std::marker::PhantomData, + } pub fn spawn(future: A) -> JoinHandle {} pub fn spawn_blocking() {} } @@ -71,7 +77,9 @@ pub mod tokio { #[cfg(not(feature = "ctap2"))] pub mod tokio_stream { pub mod wrappers { - pub struct ReceiverStream {} + pub struct ReceiverStream { + _phantom: std::marker::PhantomData, + } } } @@ -85,8 +93,12 @@ pub mod tokio_tungstenite { } } } - pub struct MaybeTlsStream {} - pub struct WebSocketStream {} + pub struct MaybeTlsStream { + _phantom: std::marker::PhantomData, + } + pub struct WebSocketStream { + _phantom: std::marker::PhantomData, + } } #[cfg(not(any(feature = "bluetooth", feature = "cable")))] @@ -120,9 +132,13 @@ pub mod openssl { pub struct BigNumContext {} } pub mod ec { - pub struct EcKey {} + pub struct EcKey { + _phantom: std::marker::PhantomData, + } pub struct EcGroup {} - pub struct EcKeyRef {} + pub struct EcKeyRef { + _phantom: std::marker::PhantomData, + } pub struct EcPoint {} pub struct EcPointRef {} pub enum PointConversionForm {} @@ -132,8 +148,12 @@ pub mod openssl { } pub mod pkey { pub struct Id {} - pub struct PKey {} - pub struct PKeyRef {} + pub struct PKey { + _phantom: std::marker::PhantomData, + } + pub struct PKeyRef { + _phantom: std::marker::PhantomData, + } pub struct Private {} pub struct Public {} } From 6f3dad7d0101aff7d04d3627ce7bcc60389a2ce6 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Fri, 24 Nov 2023 11:13:25 +1000 Subject: [PATCH 3/5] fix softtoken --- webauthn-authenticator-rs/src/softtoken.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/webauthn-authenticator-rs/src/softtoken.rs b/webauthn-authenticator-rs/src/softtoken.rs index af1e3332..78a778a3 100644 --- a/webauthn-authenticator-rs/src/softtoken.rs +++ b/webauthn-authenticator-rs/src/softtoken.rs @@ -852,7 +852,7 @@ impl AuthenticatorBackendHashedClientData for SoftTokenFile { mod tests { use super::*; use openssl::{hash::MessageDigest, rand::rand_bytes, sign::Verifier, x509::X509}; - use std::collections::BTreeSet; + use std::{collections::BTreeSet, time::Duration}; use tempfile::tempfile; use webauthn_rs_core::{ proto::{AttestationCa, AttestationCaList, COSEKey}, @@ -876,6 +876,8 @@ mod tests { softtoken::SoftToken, }; + const AUTHENTICATOR_TIMEOUT: Duration = Duration::from_secs(60); + #[test] fn webauthn_authenticator_wan_softtoken_direct_attest() { let _ = tracing_subscriber::fmt::try_init(); @@ -883,7 +885,7 @@ mod tests { "https://localhost:8080/auth", "localhost", vec![url::Url::parse("https://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); @@ -962,7 +964,7 @@ mod tests { "https://localhost:8080/auth", "localhost", vec![url::Url::parse("https://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); From 4215dd8a0c651d49f6e3298bb351dd93165442d1 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Fri, 24 Nov 2023 11:18:38 +1000 Subject: [PATCH 4/5] Fix softpasskey --- webauthn-authenticator-rs/src/softpasskey.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/webauthn-authenticator-rs/src/softpasskey.rs b/webauthn-authenticator-rs/src/softpasskey.rs index bac60840..0dfb7576 100644 --- a/webauthn-authenticator-rs/src/softpasskey.rs +++ b/webauthn-authenticator-rs/src/softpasskey.rs @@ -523,11 +523,14 @@ impl U2FToken for SoftPasskey { mod tests { use super::SoftPasskey; use crate::prelude::{Url, WebauthnAuthenticator}; + use std::time::Duration; use webauthn_rs_core::WebauthnCore as Webauthn; use webauthn_rs_proto::{ AttestationConveyancePreference, COSEAlgorithm, UserVerificationPolicy, }; + const AUTHENTICATOR_TIMEOUT: Duration = Duration::from_secs(60); + #[test] fn webauthn_authenticator_wan_softpasskey_self_attest() { let _ = tracing_subscriber::fmt::try_init(); @@ -535,7 +538,7 @@ mod tests { "https://localhost:8080/auth", "localhost", vec![url::Url::parse("https://localhost:8080").unwrap()], - None, + AUTHENTICATOR_TIMEOUT, None, None, ); From 6eeef0e4d0874b313afd30f51d404eb3dfd7fd40 Mon Sep 17 00:00:00 2001 From: Michael Farrell Date: Fri, 24 Nov 2023 12:25:51 +1000 Subject: [PATCH 5/5] Update timeouts per Webauthn-3 recommendation, and add verbiage --- webauthn-rs/src/lib.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/webauthn-rs/src/lib.rs b/webauthn-rs/src/lib.rs index f25b60f4..c691985c 100644 --- a/webauthn-rs/src/lib.rs +++ b/webauthn-rs/src/lib.rs @@ -217,8 +217,10 @@ pub mod prelude { pub use webauthn_rs_core::AttestationFormat; } -/// The default authenticator interaction timeout, if none is otherwise specified. -pub const DEFAULT_AUTHENTICATOR_TIMEOUT: Duration = Duration::from_millis(60000); +/// The [Webauthn recommended authenticator interaction timeout][0]. +/// +/// [0]: https://www.w3.org/TR/webauthn-3/#ref-for-dom-publickeycredentialcreationoptions-timeout +pub const DEFAULT_AUTHENTICATOR_TIMEOUT: Duration = Duration::from_secs(300); /// A constructor for a new [Webauthn] instance. This accepts and configures a number of site-wide /// properties that apply to all webauthn operations of this service. @@ -323,7 +325,28 @@ impl<'a> WebauthnBuilder<'a> { /// Set the timeout value to use for credential creation and authentication challenges. /// - /// If not set, defaults to [DEFAULT_AUTHENTICATOR_TIMEOUT]. + /// If not set, this defaults to [`DEFAULT_AUTHENTICATOR_TIMEOUT`], per + /// [Webauthn Level 3 recommendations][0]. + /// + /// Short timeouts are difficult for some users to meet, particularly if + /// they need to physically locate and plug in their authenticator, use a + /// [hybrid authenticator][1], need to enter a PIN and/or use a fingerprint + /// reader. + /// + /// This may take even longer for users with cognitive, motor, mobility + /// and/or vision impairments. Even a minor skin condition can make it hard + /// to use a fingerprint reader! + /// + /// Consult the [Webauthn specification's accessibilty considerations][2], + /// [WCAG 2.1's "Enough time" guideline][3] and + /// ["Timeouts" success criterion][4] when choosing a value, particularly if + /// it is *shorter* than the default. + /// + /// [0]: https://www.w3.org/TR/webauthn-3/#ref-for-dom-publickeycredentialcreationoptions-timeout + /// [1]: https://www.w3.org/TR/webauthn-3/#dom-authenticatortransport-hybrid + /// [2]: https://www.w3.org/TR/webauthn-3/#sctn-accessiblility-considerations + /// [3]: https://www.w3.org/TR/WCAG21/#enough-time + /// [4]: https://www.w3.org/WAI/WCAG21/Understanding/timeouts.html pub fn timeout(mut self, timeout: Duration) -> Self { self.timeout = timeout; self