From 5e11578cf755cca7e842443f41c353f2479ff820 Mon Sep 17 00:00:00 2001 From: William Brown Date: Fri, 15 Dec 2023 14:05:18 +1000 Subject: [PATCH] Clean up core interfaces, hide some apis, move some others around --- attestation-ca/src/lib.rs | 2 - compat_tester/webauthn-rs-demo/src/actors.rs | 9 +- fido-key-manager/src/main.rs | 2 +- sshkey-attest/src/lib.rs | 5 +- .../examples/authenticate.rs | 4 +- webauthn-authenticator-rs/src/softpasskey.rs | 4 +- webauthn-authenticator-rs/src/softtoken.rs | 8 +- webauthn-rs-core/src/attestation.rs | 182 ++++++++- webauthn-rs-core/src/core.rs | 366 +++--------------- webauthn-rs-core/src/crypto.rs | 221 +---------- webauthn-rs-core/src/interface.rs | 2 +- webauthn-rs-core/src/lib.rs | 12 +- webauthn-rs/Cargo.toml | 4 +- webauthn-rs/src/lib.rs | 129 +++--- 14 files changed, 338 insertions(+), 612 deletions(-) diff --git a/attestation-ca/src/lib.rs b/attestation-ca/src/lib.rs index 9b8288f1..e7cb00f2 100644 --- a/attestation-ca/src/lib.rs +++ b/attestation-ca/src/lib.rs @@ -125,7 +125,6 @@ impl AttestationCa { if self.blanket_allow || other.blanket_allow { self.blanket_allow = true; self.aaguids.clear(); - return; } else { self.blanket_allow = false; for (o_aaguid, o_device) in other.aaguids.iter() { @@ -142,7 +141,6 @@ impl AttestationCa { // more restrictive, or we also are a blanket allow if other.blanket_allow() { // Do nothing - return; } else if self.blanket_allow { // Just set our aaguids to other, and remove our blanket allow. self.blanket_allow = false; diff --git a/compat_tester/webauthn-rs-demo/src/actors.rs b/compat_tester/webauthn-rs-demo/src/actors.rs index 26f57a1f..1843b6d7 100644 --- a/compat_tester/webauthn-rs-demo/src/actors.rs +++ b/compat_tester/webauthn-rs-demo/src/actors.rs @@ -326,7 +326,7 @@ impl WebauthnActor { let user_unique_id = Uuid::new_v4(); - let (ccr, rs) = self.wan.generate_challenge_register_options( + let (ccr, rs) = self.wan.generate_challenge_register( user_unique_id.as_bytes(), &username, &username, @@ -360,7 +360,6 @@ impl WebauthnActor { // If use_cred_id is set, only allow this cred to be used. This also allows // some extra "stuff". - let (acr, st) = match use_cred_id { Some(use_cred_id) => { let cred = creds @@ -369,9 +368,11 @@ impl WebauthnActor { .ok_or(WebauthnError::CredentialNotFound)?; self.wan - .generate_challenge_authenticate_credential(cred, uv, extensions) + .generate_challenge_authenticate(vec![cred], uv, extensions, None) } - None => self.wan.generate_challenge_authenticate(creds, extensions), + None => self + .wan + .generate_challenge_authenticate(creds, None, extensions, None), }?; debug!("complete ChallengeAuthenticate -> {:?}", acr); diff --git a/fido-key-manager/src/main.rs b/fido-key-manager/src/main.rs index 77d9cc46..179d1731 100644 --- a/fido-key-manager/src/main.rs +++ b/fido-key-manager/src/main.rs @@ -22,7 +22,7 @@ use webauthn_authenticator_rs::{ ui::Cli, SHA256Hash, }; -use webauthn_rs_core::interface::COSEKeyType; +use webauthn_rs_core::proto::COSEKeyType; /// Parses a Base-16 encoded string. /// diff --git a/sshkey-attest/src/lib.rs b/sshkey-attest/src/lib.rs index 9922e342..118c0a02 100644 --- a/sshkey-attest/src/lib.rs +++ b/sshkey-attest/src/lib.rs @@ -25,9 +25,10 @@ use uuid::Uuid; pub use webauthn_rs_core::error::WebauthnError; use webauthn_rs_core::{ attestation::{ - validate_extension, verify_attestation_ca_chain, AttestationFormat, FidoGenCeAaguid, + assert_packed_attest_req, validate_extension, verify_attestation_ca_chain, + AttestationFormat, FidoGenCeAaguid, }, - crypto::{assert_packed_attest_req, compute_sha256, verify_signature}, + crypto::{compute_sha256, verify_signature}, internals::AuthenticatorData, proto::{ AttestationCaList, AttestationMetadata, COSEAlgorithm, COSEKey, COSEKeyType, diff --git a/webauthn-authenticator-rs/examples/authenticate.rs b/webauthn-authenticator-rs/examples/authenticate.rs index 89707eec..92fa002f 100644 --- a/webauthn-authenticator-rs/examples/authenticate.rs +++ b/webauthn-authenticator-rs/examples/authenticate.rs @@ -248,7 +248,7 @@ async fn main() { let name = "william"; let (chal, reg_state) = wan - .generate_challenge_register_options( + .generate_challenge_register( &unique_id, name, name, @@ -290,11 +290,13 @@ async fn main() { let (chal, auth_state) = wan .generate_challenge_authenticate( vec![cred.clone()], + None, Some(RequestAuthenticationExtensions { appid: Some("example.app.id".to_string()), uvm: None, hmac_get_secret: None, }), + None, ) .unwrap(); diff --git a/webauthn-authenticator-rs/src/softpasskey.rs b/webauthn-authenticator-rs/src/softpasskey.rs index 0dfb7576..a12232a8 100644 --- a/webauthn-authenticator-rs/src/softpasskey.rs +++ b/webauthn-authenticator-rs/src/softpasskey.rs @@ -549,7 +549,7 @@ mod tests { let name = "william"; let (chal, reg_state) = wan - .generate_challenge_register_options( + .generate_challenge_register( &unique_id, name, name, @@ -578,7 +578,7 @@ mod tests { let cred = wan.register_credential(&r, ®_state, None).unwrap(); let (chal, auth_state) = wan - .generate_challenge_authenticate(vec![cred], None) + .generate_challenge_authenticate(vec![cred], None, None, None) .unwrap(); let r = wa diff --git a/webauthn-authenticator-rs/src/softtoken.rs b/webauthn-authenticator-rs/src/softtoken.rs index 6c41c2ea..2fa9bff1 100644 --- a/webauthn-authenticator-rs/src/softtoken.rs +++ b/webauthn-authenticator-rs/src/softtoken.rs @@ -900,7 +900,7 @@ mod tests { let name = "william"; let (chal, reg_state) = wan - .generate_challenge_register_options( + .generate_challenge_register( &unique_id, name, name, @@ -938,7 +938,7 @@ mod tests { info!("Credential -> {:?}", cred); let (chal, auth_state) = wan - .generate_challenge_authenticate(vec![cred], None) + .generate_challenge_authenticate(vec![cred], None, None, None) .unwrap(); let r = wa @@ -980,7 +980,7 @@ mod tests { let name = "william"; let (chal, reg_state) = wan - .generate_challenge_register_options( + .generate_challenge_register( &unique_id, name, name, @@ -1032,7 +1032,7 @@ mod tests { let mut wa = WebauthnAuthenticator::new(soft_token); let (chal, auth_state) = wan - .generate_challenge_authenticate(vec![cred], None) + .generate_challenge_authenticate(vec![cred], None, None, None) .unwrap(); let r = wa diff --git a/webauthn-rs-core/src/attestation.rs b/webauthn-rs-core/src/attestation.rs index 414451d2..6f874d07 100644 --- a/webauthn-rs-core/src/attestation.rs +++ b/webauthn-rs-core/src/attestation.rs @@ -1,16 +1,15 @@ -//! Attestation information and verifications procedures. +//! Attestation information and verification procedures. //! This contains a transparent type allowing callbacks to -//! make attestation decisions. See the WebauthnConfig trait -//! for more details. +//! make attestation decisions. use std::convert::TryFrom; use crate::crypto::{ - assert_packed_attest_req, assert_tpm_attest_req, compute_sha256, only_hash_from_type, - verify_signature, + check_extension, compute_sha256, only_hash_from_type, verify_signature, TpmSanData, }; use crate::error::WebauthnError; use crate::internals::*; +use crate::internals::{tpm_device_attribute_parser, TpmVendor}; use crate::proto::*; use base64urlsafedata::Base64UrlSafeData; use openssl::hash::MessageDigest; @@ -21,6 +20,8 @@ use openssl::x509::store; use openssl::x509::verify; use uuid::Uuid; use x509_parser::oid_registry::Oid; +use x509_parser::prelude::GeneralName; +use x509_parser::x509::X509Version; /// x509 certificate extensions are validated in the webauthn spec by checking /// that the value of the extension is equal to some other value @@ -536,6 +537,93 @@ pub(crate) fn verify_packed_attestation( } } +/// Verify that attestnCert meets the requirements in +/// [§ 8.2.1 Packed Attestation Statement Certificate Requirements][0] +/// +/// [0]: https://www.w3.org/TR/webauthn-2/#sctn-packed-attestation-cert-requirements +pub fn assert_packed_attest_req(pubk: &x509::X509) -> Result<(), WebauthnError> { + // https://w3c.github.io/webauthn/#sctn-packed-attestation-cert-requirements + let der_bytes = pubk.to_der()?; + let x509_cert = x509_parser::parse_x509_certificate(&der_bytes) + .map_err(|_| WebauthnError::AttestationStatementX5CInvalid)? + .1; + + // The attestation certificate MUST have the following fields/extensions: + // Version MUST be set to 3 (which is indicated by an ASN.1 INTEGER with value 2). + if x509_cert.version != X509Version::V3 { + trace!("X509 Version != v3"); + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + + // Subject field MUST be set to: + // + // Subject-C + // ISO 3166 code specifying the country where the Authenticator vendor is incorporated (PrintableString) + // Subject-O + // Legal name of the Authenticator vendor (UTF8String) + // Subject-OU + // Literal string “Authenticator Attestation” (UTF8String) + // Subject-CN + // A UTF8String of the vendor’s choosing + let subject = &x509_cert.subject; + + let subject_c = subject.iter_country().take(1).next(); + let subject_o = subject.iter_organization().take(1).next(); + let subject_ou = subject.iter_organizational_unit().take(1).next(); + let subject_cn = subject.iter_common_name().take(1).next(); + + if subject_c.is_none() || subject_o.is_none() || subject_cn.is_none() { + trace!("Invalid subject details"); + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + + match subject_ou { + Some(ou) => match ou.attr_value().as_str() { + Ok(ou_d) => { + if ou_d != "Authenticator Attestation" { + trace!("ou != Authenticator Attestation"); + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + } + Err(_) => { + trace!("ou invalid"); + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + }, + None => { + trace!("ou not found"); + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + } + + // If the related attestation root certificate is used for multiple authenticator models, + // the Extension OID 1.3.6.1.4.1.45724.1.1.4 (id-fido-gen-ce-aaguid) MUST be present, + // containing the AAGUID as a 16-byte OCTET STRING. The extension MUST NOT be marked as critical. + // + // We already check that the value matches the AAGUID in attestation + // verification, so we only have to check the critical requirement here. + // + // The problem with this check, is that it's not actually required that this + // oid be present at all ... + check_extension( + &x509_cert.get_extension_unique(&FidoGenCeAaguid::OID), + false, + |fido_gen_ce_aaguid| !fido_gen_ce_aaguid.critical, + )?; + + // The Basic Constraints extension MUST have the CA component set to false. + check_extension(&x509_cert.basic_constraints(), true, |basic_constraints| { + !basic_constraints.value.ca + })?; + + // An Authority Information Access (AIA) extension with entry id-ad-ocsp and a CRL + // Distribution Point extension [RFC5280] are both OPTIONAL as the status of many + // attestation certificates is available through authenticator metadata services. See, for + // example, the FIDO Metadata Service [FIDOMetadataService]. + + Ok(()) +} + // https://w3c.github.io/webauthn/#fido-u2f-attestation // https://medium.com/@herrjemand/verifying-fido-u2f-attestations-in-fido2-f83fab80c355 pub(crate) fn verify_fidou2f_attestation( @@ -900,6 +988,90 @@ pub(crate) fn verify_tpm_attestation( )) } +pub(crate) fn assert_tpm_attest_req(x509: &x509::X509) -> Result<(), WebauthnError> { + let der_bytes = x509.to_der()?; + let x509_cert = x509_parser::parse_x509_certificate(&der_bytes) + .map_err(|_| WebauthnError::AttestationStatementX5CInvalid)? + .1; + + // TPM attestation certificate MUST have the following fields/extensions: + + // Version MUST be set to 3. + if x509_cert.version != X509Version::V3 { + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + + // Subject field MUST be set to empty. + let subject_name_ref = x509.subject_name(); + if subject_name_ref.entries().count() != 0 { + return Err(WebauthnError::AttestationCertificateRequirementsNotMet); + } + + // The Subject Alternative Name extension MUST be set as defined in [TPMv2-EK-Profile] section 3.2.9. + // https://www.trustedcomputinggroup.org/wp-content/uploads/Credential_Profile_EK_V2.0_R14_published.pdf + check_extension( + &x509_cert.subject_alternative_name(), + true, + |subject_alternative_name| { + // From [TPMv2-EK-Profile]: + // In accordance with RFC 5280[11], this extension MUST be critical if + // subject is empty and SHOULD be non-critical if subject is non-empty. + // + // We've already returned if the subject is non-empty, so we can just + // check that the extension is critical. + if !subject_alternative_name.critical { + return false; + }; + + // The issuer MUST include TPM manufacturer, TPM part number and TPM + // firmware version, using the directoryName form within the GeneralName + // structure. + subject_alternative_name + .value + .general_names + .iter() + .any(|general_name| { + if let GeneralName::DirectoryName(x509_name) = general_name { + TpmSanData::try_from(x509_name) + .and_then(|san_data| { + tpm_device_attribute_parser(san_data.manufacturer.as_bytes()) + .map_err(|_| WebauthnError::ParseNOMFailure) + }) + .and_then(|(_, manufacturer_bytes)| { + TpmVendor::try_from(manufacturer_bytes) + }) + .is_ok() + } else { + false + } + }) + }, + )?; + + // The Extended Key Usage extension MUST contain the "joint-iso-itu-t(2) internationalorganizations(23) 133 tcg-kp(8) tcg-kp-AIKCertificate(3)" OID. + check_extension( + &x509_cert.extended_key_usage(), + true, + |extended_key_usage| { + extended_key_usage + .value + .other + .contains(&der_parser::oid!(2.23.133 .8 .3)) + }, + )?; + + // The Basic Constraints extension MUST have the CA component set to false. + check_extension(&x509_cert.basic_constraints(), true, |basic_constraints| { + !basic_constraints.value.ca + })?; + + // An Authority Information Access (AIA) extension with entry id-ad-ocsp and a CRL Distribution + // Point extension [RFC5280] are both OPTIONAL as the status of many attestation certificates is + // available through metadata services. See, for example, the FIDO Metadata Service [FIDOMetadataService]. + + Ok(()) +} + pub(crate) fn verify_apple_anonymous_attestation( acd: &AttestedCredentialData, att_obj: &AttestationObject, diff --git a/webauthn-rs-core/src/core.rs b/webauthn-rs-core/src/core.rs index 6c667a7b..4635023b 100644 --- a/webauthn-rs-core/src/core.rs +++ b/webauthn-rs-core/src/core.rs @@ -33,8 +33,9 @@ use crate::internals::*; use crate::proto::*; use base64urlsafedata::Base64UrlSafeData; -/// This is the core of the Webauthn operations. It provides 4 interfaces that you will likely -/// use the most: +/// The Core Webauthn handler. +/// +/// It provides 4 interfaces methods for registering and then authenticating credentials. /// * generate_challenge_register /// * register_credential /// * generate_challenge_authenticate @@ -77,8 +78,8 @@ impl WebauthnCore { /// If you still choose to continue, and use this directly, be aware that: /// /// * This function signature MAY change WITHOUT NOTICE and WITHIN MINOR VERSIONS - /// * That you are responsible for UPHOLDING many invariants within webauthn that are NOT DOCUMENTED /// * You MUST understand the webauthn specification in excruciating detail to understand the traps within it + /// * That you are responsible for UPHOLDING many invariants within webauthn that are NOT DOCUMENTED in the webauthn specification /// /// Seriously. Use `webauthn-rs` instead. pub fn new_unsafe_experts_only( @@ -114,44 +115,6 @@ impl WebauthnCore { Challenge::new(rng.gen::<[u8; CHALLENGE_SIZE_BYTES]>().to_vec()) } - /// Generate a new challenge for client registration. - /// Same as `generate_challenge_register_options` but with simple, default options - pub fn generate_challenge_register( - &self, - user_unique_id: &[u8], - user_name: &str, - user_display_name: &str, - user_verification_required: bool, - ) -> Result<(CreationChallengeResponse, RegistrationState), WebauthnError> { - let policy = if user_verification_required { - Some(UserVerificationPolicy::Required) - } else { - // I'm so mad about ctap2.0 you have no idea. - Some(UserVerificationPolicy::Preferred) - }; - - let attestation = AttestationConveyancePreference::None; - let exclude_credentials = None; - let extensions = None; - let credential_algorithms = COSEAlgorithm::secure_algs(); - let require_resident_key = false; - let authenticator_attachment = None; - - self.generate_challenge_register_options( - user_unique_id, - user_name, - user_display_name, - attestation, - policy, - exclude_credentials, - extensions, - credential_algorithms, - require_resident_key, - authenticator_attachment, - false, - ) - } - /// Generate a new challenge for client registration. This is the first step in /// the lifecycle of a credential. This function will return the /// creationchallengeresponse which is suitable for serde json serialisation @@ -163,7 +126,7 @@ impl WebauthnCore { /// persist. It is strongly advised you associate this RegistrationState with the /// UserId of the requester. #[allow(clippy::too_many_arguments)] - pub fn generate_challenge_register_options( + pub fn generate_challenge_register( &self, user_unique_id: &[u8], user_name: &str, @@ -171,13 +134,11 @@ impl WebauthnCore { attestation: AttestationConveyancePreference, policy: Option, exclude_credentials: Option>, - extensions: Option, - credential_algorithms: Vec, require_resident_key: bool, authenticator_attachment: Option, - experimental_reject_passkeys: bool, + experimental_reject_synchronised_authenticators: bool, ) -> Result<(CreationChallengeResponse, RegistrationState), WebauthnError> { let policy = policy.unwrap_or(UserVerificationPolicy::Preferred); @@ -187,25 +148,6 @@ impl WebauthnCore { let user_id: UserId = user_unique_id.to_vec(); - // Setup our extensions. - // unimplemented!(); - - // resident key needs cred props - - // user verification needs credProtect (?) - - // Have a UV strict? - - // minPinLength - - // hmacSecret - - // credProps - - // userVerificationMethod - - // - let challenge = self.generate_challenge(); let resident_key = if require_resident_key { @@ -268,7 +210,8 @@ impl WebauthnCore { require_resident_key, authenticator_attachment, extensions: extensions.unwrap_or_default(), - experimental_allow_passkeys: !experimental_reject_passkeys, + experimental_allow_synchronised_authenticators: + !experimental_reject_synchronised_authenticators, }; // This should have an opaque type of username + chal + policy @@ -305,7 +248,7 @@ impl WebauthnCore { require_resident_key: _, authenticator_attachment: _, extensions, - experimental_allow_passkeys, + experimental_allow_synchronised_authenticators, } = state; let chal: &ChallengeRef = challenge.into(); @@ -319,7 +262,7 @@ impl WebauthnCore { attestation_cas, false, extensions, - *experimental_allow_passkeys, + *experimental_allow_synchronised_authenticators, )?; // Check that the credentialId is not yet registered to any other user. If registration is @@ -350,7 +293,7 @@ impl WebauthnCore { attestation_cas: Option<&AttestationCaList>, danger_disable_certificate_time_checks: bool, req_extn: &RequestRegistrationExtensions, - experimental_allow_passkeys: bool, + experimental_allow_synchronised_authenticators: bool, ) -> Result { // Internal management - if the attestation ca list is some, but is empty, we need to fail! if attestation_cas @@ -636,8 +579,8 @@ impl WebauthnCore { return Err(WebauthnError::CredentialAlteredAlgFromRequest); } - // OUT OF SPEC - Allow rejection of passkeys if desired by the caller. - if !experimental_allow_passkeys && credential.backup_eligible { + // OUT OF SPEC - Allow rejection of synchronised credentials if desired by the caller. + if !experimental_allow_synchronised_authenticators && credential.backup_eligible { error!("Credential counter is 0 - may indicate that it is a passkey and not bound to hardware."); return Err(WebauthnError::CredentialMayNotBeHardwareBound); } @@ -843,87 +786,37 @@ impl WebauthnCore { Ok(data.authenticator_data) } - /// Authenticate a set of credentials, deriving the correct user verification policy - /// for them in a secure manner. - pub fn generate_challenge_authenticate( - &self, - creds: Vec, - extensions: Option, - ) -> Result<(RequestChallengeResponse, AuthenticationState), WebauthnError> { - // Should this filter on cred.verified instead rather than the policy? - // Or do we need to send Preferred instead of Discouraged due to ctap2.0? - // - // https://github.com/kanidm/webauthn-rs/issues/91 - // - let mut policies = - BTreeSet::from_iter(creds.iter().map(|cred| cred.registration_policy.to_owned())); - if policies.len() > 1 { - return Err(WebauthnError::InconsistentUserVerificationPolicy); - } - let policy = policies - .pop_first() - .ok_or(WebauthnError::CredentialNotFound)?; - - self.generate_challenge_authenticate_inner(creds, policy, extensions, false) - } - - /// Authenticate a single credential, with the ability to override the userVerification - /// policy requested, or extensions in use. If userVerification is `None`, the policy from - /// registration is used. - /// - /// NOTE: Over-riding the UserVerificationPolicy may have SECURITY consequences. You should - /// understand how this interacts with the single credential in use, and how that may impact - /// your system security. - /// - /// If in doubt, do NOT use this function! - pub fn generate_challenge_authenticate_credential( - &self, - cred: Credential, - policy: Option, - extensions: Option, - ) -> Result<(RequestChallengeResponse, AuthenticationState), WebauthnError> { - let policy = policy.unwrap_or(cred.registration_policy); - self.generate_challenge_authenticate_inner(vec![cred], policy, extensions, false) - } - /// Authenticate a set of credentials allowing the user verification policy to be set. + /// If no credentials are provided, this will start a discoverable credential authentication. /// - /// NOTE: Over-riding the UserVerificationPolicy may have SECURITY consequences. You should - /// understand how this interacts with the single credential in use, and how that may impact - /// your system security. + /// Policy defines the require UserVerification policy. This MUST be consistent with the + /// credentials being used in the authentication. /// - /// If in doubt, do NOT use this function! - pub fn generate_challenge_authenticate_policy( + /// `allow_backup_eligible_upgrade` allows rejecting credentials whos backup eligibility + /// has changed between registration and authentication. + pub fn generate_challenge_authenticate( &self, creds: Vec, - policy: UserVerificationPolicy, + policy: Option, extensions: Option, - allow_backup_eligible_upgrade: bool, + allow_backup_eligible_upgrade: Option, ) -> Result<(RequestChallengeResponse, AuthenticationState), WebauthnError> { - self.generate_challenge_authenticate_inner( - creds, - policy, - extensions, - allow_backup_eligible_upgrade, - ) - } + let policy = if let Some(policy) = policy { + policy + } else { + let mut policies = + BTreeSet::from_iter(creds.iter().map(|cred| cred.registration_policy.to_owned())); + if policies.len() > 1 { + return Err(WebauthnError::InconsistentUserVerificationPolicy); + } + policies + .pop_first() + .ok_or(WebauthnError::CredentialNotFound)? + }; - /// Begin a discoverable authentication session. - pub fn generate_challenge_authenticate_discoverable( - &self, - policy: UserVerificationPolicy, - extensions: Option, - ) -> Result<(RequestChallengeResponse, AuthenticationState), WebauthnError> { - self.generate_challenge_authenticate_inner(vec![], policy, extensions, false) - } + // Defaults to false. + let allow_backup_eligible_upgrade = allow_backup_eligible_upgrade.unwrap_or_default(); - fn generate_challenge_authenticate_inner( - &self, - creds: Vec, - policy: UserVerificationPolicy, - extensions: Option, - allow_backup_eligible_upgrade: bool, - ) -> Result<(RequestChallengeResponse, AuthenticationState), WebauthnError> { let chal = self.generate_challenge(); // Get the user's existing creds if any. @@ -1178,162 +1071,6 @@ impl WebauthnCore { } } -/* -/// The WebauthnConfig type allows site-specific customisation of the Webauthn library. -/// This provides a set of callbacks which are used to supply data to various structures -/// and calls, as well as callbacks to manage data persistence and retrieval. -pub trait WebauthnConfig { - /// Returns a reference to your relying parties name. This is generally any text identifier - /// you wish, but should rarely if ever change. Changes to the relying party name may - /// confuse authenticators and will cause their credentials to be lost. - /// - /// Examples of names could be `My Awesome Site`, `https://my-awesome-site.com.au` - fn get_relying_party_name(&self) -> &str; - - /// Returns a reference to your sites origin. The origin is the URL to your site with - /// protocol and port. This should rarely, if ever change. In production usage this - /// value must always be https://, however http://localhost is acceptable for testing - /// only. We may add warnings or errors for non-https:// urls in the future. Changing this - /// may cause associated authenticators to lose credentials. - /// - /// Examples of this value could be. `https://my-site.com.au`, `https://my-site.com.au:8443` - fn get_origin(&self) -> &url::Url; - - /// Returns the relying party id. This should never change, and is used as an id - /// in cryptographic operations and credential scoping. This is defined as the domain name - /// of the service, minus all protocol, port and location data. For example: - /// `https://name:port/path -> name` - /// - /// If changed, all associated credentials will be lost in all authenticators. - /// - /// Examples of this value for the site `https://my-site.com.au/auth` is `my-site.com.au` - fn get_relying_party_id(&self) -> &str; - - /// Get the list of valid credential algorithms that this service can accept. Unless you have - /// speific requirements around this, we advise you leave this function to the default - /// implementation. - fn get_credential_algorithms(&self) -> Vec { - vec![COSEAlgorithm::ES256, COSEAlgorithm::RS256] - } - - /// Return a timeout on how long the authenticator has to respond to a challenge. This value - /// defaults to 60000 milliseconds. You likely won't need to implement this function, and should - /// rely on the defaults. - fn get_authenticator_timeout(&self) -> u32 { - AUTHENTICATOR_TIMEOUT - } - - /// Returns the default attestation type. Options are `None`, `Direct` and `Indirect`. - /// Defaults to `None`. - /// - /// DANGER: The client *may* alter this value, causing the registration to not contain - /// an attestation. This is *not* a verified property. - /// - /// You *must* also implement policy_verify_trust if you change this from `None` else - /// this can be BYPASSED. - fn get_attestation_preference(&self) -> AttestationConveyancePreference { - AttestationConveyancePreference::None - } - - /// Get the preferred policy on authenticator attachment hint. Defaults to None (use - /// any attachment method). - /// - /// Default of None allows any attachment method. - /// - /// WARNING: This is not enforced, as the client may modify the registration request to - /// disregard this, and no part of the registration response indicates attachement. This - /// is purely a hint, and is NOT a security enforcment. - fn get_authenticator_attachment(&self) -> Option { - None - } - - /// Get the site policy on if the registration should use a resident key so that - /// username and other details can be embedded into the authenticator - /// to allow bypassing that part of the interaction flow. - /// - /// Defaults to "false" aka non-resident keys. - /// See also: - /// - /// WARNING: This is not enforced as the client may modify the registration request - /// to disregard this, and no part of the registration process indicates residence of - /// the credentials. This is not a security enforcement. - fn get_require_resident_key(&self) -> bool { - false - } - - /// If the attestation format is not supported, should we ignore verifying the attestation - fn ignore_unsupported_attestation_formats(&self) -> bool { - false - } - - /// Decides the verifier must error on invalid counter values - fn require_valid_counter_value(&self) -> bool { - true - } - - /// A callback to allow trust decisions to be made over the attestation of the - /// credential. It's important for your implementation of this callback to follow - /// the advice of the w3c standard, notably: - /// - /// Assess the attestation trustworthiness using the outputs of the verification procedure in step 19, as follows: - /// * If no attestation was provided, verify that None attestation is acceptable under Relying Party policy. - /// * If self attestation was used, verify that self attestation is acceptable under Relying Party policy. - /// * Otherwise, use the X.509 certificates returned as the attestation trust path from the verification - /// procedure to verify that the attestation public key either correctly chains up to an acceptable - /// root certificate, or is itself an acceptable certificate (i.e., it and the root certificate - /// obtained previously may be the same). - /// - /// The default implementation of this method rejects Uncertain attestation, and - /// will "blindly trust" self attestation and the other types as valid. - /// If you have strict security requirements we strongly recommend you implement this function, - /// and we may in the future provide a stronger default relying party policy. - fn policy_verify_trust(&self, pad: ParsedAttestationData) -> Result<(), ()> { - debug!("policy_verify_trust -> {:?}", pad); - match pad { - ParsedAttestationData::Basic(_attest_cert) => Ok(()), - ParsedAttestationData::Self_ => Ok(()), - ParsedAttestationData::AttCa(_attest_cert, _ca_chain) => Ok(()), - ParsedAttestationData::AnonCa(_attest_cert, _ca_chain) => Ok(()), - ParsedAttestationData::None => Ok(()), - // TODO: trust is unimplemented here - ParsedAttestationData::ECDAA => Err(()), - // We don't trust Uncertain attestations - ParsedAttestationData::Uncertain => Err(()), - } - } - - /// Get the site policy on whether cross origin credentials are allowed. - /// - /// A credential is cross origin if the ECMAScript context in which the - /// credential creation functions were invoked belonged to a different - /// origin than that of the RP the credential is being created for. - /// - /// WARNING: Most browsers do not currently send the `crossOrigin` value so - /// we assume where the key is absent that the credential was not created in - /// a cross-origin context. - fn allow_cross_origin(&self) -> bool { - false - } - - /// Allow subdomains of origin to be valid to use credentils from the parent origin. This - /// exists due to a subtle confusion in the webauthn specification. In - /// https://www.w3.org/TR/webauthn-2/#scope we see that the relying party ID is intended - /// to allow effective domains to be validated by the client for the origin that we are - /// using, however in https://www.w3.org/TR/webauthn-2/#sctn-registering-a-new-credential step - /// 9 it is requested that origin *equality* is performed. This would disallow subdomains - /// of the effective domain from being use. - /// - /// By default we take the "strict" behaviour to only allow exact origin matches, but some - /// applications may wish subdomains of origin to valid. Consider idm.example.com and - /// host.idm.example.com where a credential should be valid for either. - /// - /// In most cases you DO NOT need to enable this option. - fn allow_subdomains_origin(&self) -> bool { - false - } -} -*/ - #[cfg(test)] mod tests { #![allow(clippy::panic)] @@ -2483,8 +2220,8 @@ mod tests { fn register_userid( user_unique_id: &[u8], - name: &str, - display_name: &str, + user_name: &str, + user_display_name: &str, ) -> Result<(CreationChallengeResponse, RegistrationState), WebauthnError> { #![allow(clippy::unwrap_used)] @@ -2497,9 +2234,28 @@ mod tests { None, ); - let policy = true; + let policy = Some(UserVerificationPolicy::Required); + + let attestation = AttestationConveyancePreference::None; + let exclude_credentials = None; + let extensions = None; + let credential_algorithms = COSEAlgorithm::secure_algs(); + let require_resident_key = false; + let authenticator_attachment = None; - wan.generate_challenge_register(user_unique_id, name, display_name, policy) + wan.generate_challenge_register( + user_unique_id, + user_name, + user_display_name, + attestation, + policy, + exclude_credentials, + extensions, + credential_algorithms, + require_resident_key, + authenticator_attachment, + false, + ) } #[test] @@ -2970,7 +2726,7 @@ mod tests { // Ensure we get a bad result. assert!( - wan.generate_challenge_authenticate(creds.clone(), None) + wan.generate_challenge_authenticate(creds.clone(), None, None, None) .unwrap_err() == WebauthnError::InconsistentUserVerificationPolicy ); @@ -2995,7 +2751,7 @@ mod tests { .unwrap(); } - let r = wan.generate_challenge_authenticate(creds.clone(), None); + let r = wan.generate_challenge_authenticate(creds.clone(), None, None, None); debug!("{:?}", r); assert!(r.is_ok()); @@ -3019,7 +2775,7 @@ mod tests { .unwrap(); } - let r = wan.generate_challenge_authenticate(creds.clone(), None); + let r = wan.generate_challenge_authenticate(creds.clone(), None, None, None); debug!("{:?}", r); assert!(r.is_ok()); } diff --git a/webauthn-rs-core/src/crypto.rs b/webauthn-rs-core/src/crypto.rs index 880f5130..4c213f52 100644 --- a/webauthn-rs-core/src/crypto.rs +++ b/webauthn-rs-core/src/crypto.rs @@ -7,14 +7,11 @@ use core::convert::TryFrom; use openssl::{bn, ec, hash, nid, pkey, rsa, sha, sign, x509}; -use x509_parser::x509::X509Version; -// use super::constants::*; use super::error::*; -use crate::attestation::{AttestationX509Extension, FidoGenCeAaguid}; use crate::proto::*; -use crate::internals::{tpm_device_attribute_parser, TpmVendor}; +use x509_parser::prelude::{X509Error, X509Name}; // Why OpenSSL over another rust crate? // - The openssl crate allows us to reconstruct a public key from the @@ -24,9 +21,6 @@ use crate::internals::{tpm_device_attribute_parser, TpmVendor}; // has resources and investment into it's maintenance, so we can a least // assert a higher level of confidence in it that . -// Object({Integer(-3): Bytes([48, 185, 178, 204, 113, 186, 105, 138, 190, 33, 160, 46, 131, 253, 100, 177, 91, 243, 126, 128, 245, 119, 209, 59, 186, 41, 215, 196, 24, 222, 46, 102]), Integer(-2): Bytes([158, 212, 171, 234, 165, 197, 86, 55, 141, 122, 253, 6, 92, 242, 242, 114, 158, 221, 238, 163, 127, 214, 120, 157, 145, 226, 232, 250, 144, 150, 218, 138]), Integer(-1): U64(1), Integer(1): U64(2), Integer(3): I64(-7)}) -// - fn pkey_verify_signature( pkey: &pkey::PKeyRef, stype: COSEAlgorithm, @@ -62,42 +56,6 @@ fn pkey_verify_signature( .map_err(WebauthnError::OpenSSLError) } -/* -impl TryFrom<(&[u8], COSEAlgorithm)> for X509PublicKey { - type Error = WebauthnError; - - // Must be DER bytes. If you have PEM, base64decode first! - fn try_from((d, t): (&[u8], COSEAlgorithm)) -> Result { - let pubk = - x509::X509::from_der(d).map_err(WebauthnError::OpenSSLError) - - #[allow(clippy::single_match)] - match &t { - COSEAlgorithm::ES256 => { - let pk = pubk.public_key().map_err(WebauthnError::OpenSSLError)?; - - let ec_key = pk.ec_key().map_err(WebauthnError::OpenSSLError)?; - - ec_key.check_key().map_err(WebauthnError::OpenSSLError)?; - - let ec_grpref = ec_key.group(); - - let ec_curve = ec_grpref - .curve_name() - .ok_or(WebauthnError::OpenSSLErrorNoCurveName)?; - - if ec_curve != nid::Nid::X9_62_PRIME256V1 { - return Err(WebauthnError::CertificatePublicKeyInvalid); - } - } - _ => {} - } - - Ok(X509PublicKey { pubk, t }) - } -} -*/ - /// Validate an x509 signature is valid for the supplied data pub fn verify_signature( alg: COSEAlgorithm, @@ -110,9 +68,7 @@ pub fn verify_signature( pkey_verify_signature(&pkey, alg, signature, verification_data) } -use x509_parser::prelude::{GeneralName, X509Error, X509Name}; - -fn check_extension( +pub(crate) fn check_extension( extension: &Result, X509Error>, must_be_present: bool, f: F, @@ -144,7 +100,7 @@ where } } -struct TpmSanData<'a> { +pub(crate) struct TpmSanData<'a> { pub manufacturer: &'a str, pub _model: &'a str, pub _version: &'a str, @@ -219,177 +175,6 @@ impl<'a> TryFrom<&'a X509Name<'a>> for TpmSanData<'a> { } } -pub(crate) fn assert_tpm_attest_req(x509: &x509::X509) -> Result<(), WebauthnError> { - let der_bytes = x509.to_der()?; - let x509_cert = x509_parser::parse_x509_certificate(&der_bytes) - .map_err(|_| WebauthnError::AttestationStatementX5CInvalid)? - .1; - - // TPM attestation certificate MUST have the following fields/extensions: - - // Version MUST be set to 3. - if x509_cert.version != X509Version::V3 { - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - - // Subject field MUST be set to empty. - let subject_name_ref = x509.subject_name(); - if subject_name_ref.entries().count() != 0 { - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - - // The Subject Alternative Name extension MUST be set as defined in [TPMv2-EK-Profile] section 3.2.9. - // https://www.trustedcomputinggroup.org/wp-content/uploads/Credential_Profile_EK_V2.0_R14_published.pdf - check_extension( - &x509_cert.subject_alternative_name(), - true, - |subject_alternative_name| { - // From [TPMv2-EK-Profile]: - // In accordance with RFC 5280[11], this extension MUST be critical if - // subject is empty and SHOULD be non-critical if subject is non-empty. - // - // We've already returned if the subject is non-empty, so we can just - // check that the extension is critical. - if !subject_alternative_name.critical { - return false; - }; - - // The issuer MUST include TPM manufacturer, TPM part number and TPM - // firmware version, using the directoryName form within the GeneralName - // structure. - subject_alternative_name - .value - .general_names - .iter() - .any(|general_name| { - if let GeneralName::DirectoryName(x509_name) = general_name { - TpmSanData::try_from(x509_name) - .and_then(|san_data| { - tpm_device_attribute_parser(san_data.manufacturer.as_bytes()) - .map_err(|_| WebauthnError::ParseNOMFailure) - }) - .and_then(|(_, manufacturer_bytes)| { - TpmVendor::try_from(manufacturer_bytes) - }) - .is_ok() - } else { - false - } - }) - }, - )?; - - // The Extended Key Usage extension MUST contain the "joint-iso-itu-t(2) internationalorganizations(23) 133 tcg-kp(8) tcg-kp-AIKCertificate(3)" OID. - check_extension( - &x509_cert.extended_key_usage(), - true, - |extended_key_usage| { - extended_key_usage - .value - .other - .contains(&der_parser::oid!(2.23.133 .8 .3)) - }, - )?; - - // The Basic Constraints extension MUST have the CA component set to false. - check_extension(&x509_cert.basic_constraints(), true, |basic_constraints| { - !basic_constraints.value.ca - })?; - - // An Authority Information Access (AIA) extension with entry id-ad-ocsp and a CRL Distribution - // Point extension [RFC5280] are both OPTIONAL as the status of many attestation certificates is - // available through metadata services. See, for example, the FIDO Metadata Service [FIDOMetadataService]. - - Ok(()) -} - -/// Verify that attestnCert meets the requirements in -/// [§ 8.2.1 Packed Attestation Statement Certificate Requirements][0] -/// -/// [0]: https://www.w3.org/TR/webauthn-2/#sctn-packed-attestation-cert-requirements -pub fn assert_packed_attest_req(pubk: &x509::X509) -> Result<(), WebauthnError> { - // https://w3c.github.io/webauthn/#sctn-packed-attestation-cert-requirements - let der_bytes = pubk.to_der()?; - let x509_cert = x509_parser::parse_x509_certificate(&der_bytes) - .map_err(|_| WebauthnError::AttestationStatementX5CInvalid)? - .1; - - // The attestation certificate MUST have the following fields/extensions: - // Version MUST be set to 3 (which is indicated by an ASN.1 INTEGER with value 2). - if x509_cert.version != X509Version::V3 { - trace!("X509 Version != v3"); - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - - // Subject field MUST be set to: - // - // Subject-C - // ISO 3166 code specifying the country where the Authenticator vendor is incorporated (PrintableString) - // Subject-O - // Legal name of the Authenticator vendor (UTF8String) - // Subject-OU - // Literal string “Authenticator Attestation” (UTF8String) - // Subject-CN - // A UTF8String of the vendor’s choosing - let subject = &x509_cert.subject; - - let subject_c = subject.iter_country().take(1).next(); - let subject_o = subject.iter_organization().take(1).next(); - let subject_ou = subject.iter_organizational_unit().take(1).next(); - let subject_cn = subject.iter_common_name().take(1).next(); - - if subject_c.is_none() || subject_o.is_none() || subject_cn.is_none() { - trace!("Invalid subject details"); - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - - match subject_ou { - Some(ou) => match ou.attr_value().as_str() { - Ok(ou_d) => { - if ou_d != "Authenticator Attestation" { - trace!("ou != Authenticator Attestation"); - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - } - Err(_) => { - trace!("ou invalid"); - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - }, - None => { - trace!("ou not found"); - return Err(WebauthnError::AttestationCertificateRequirementsNotMet); - } - } - - // If the related attestation root certificate is used for multiple authenticator models, - // the Extension OID 1.3.6.1.4.1.45724.1.1.4 (id-fido-gen-ce-aaguid) MUST be present, - // containing the AAGUID as a 16-byte OCTET STRING. The extension MUST NOT be marked as critical. - // - // We already check that the value matches the AAGUID in attestation - // verification, so we only have to check the critical requirement here. - // - // The problem with this check, is that it's not actually required that this - // oid be present at all ... - check_extension( - &x509_cert.get_extension_unique(&FidoGenCeAaguid::OID), - false, - |fido_gen_ce_aaguid| !fido_gen_ce_aaguid.critical, - )?; - - // The Basic Constraints extension MUST have the CA component set to false. - check_extension(&x509_cert.basic_constraints(), true, |basic_constraints| { - !basic_constraints.value.ca - })?; - - // An Authority Information Access (AIA) extension with entry id-ad-ocsp and a CRL - // Distribution Point extension [RFC5280] are both OPTIONAL as the status of many - // attestation certificates is available through authenticator metadata services. See, for - // example, the FIDO Metadata Service [FIDOMetadataService]. - - Ok(()) -} - impl TryFrom for ECDSACurve { type Error = WebauthnError; fn try_from(nid: nid::Nid) -> Result { diff --git a/webauthn-rs-core/src/interface.rs b/webauthn-rs-core/src/interface.rs index e687b08a..4043079a 100644 --- a/webauthn-rs-core/src/interface.rs +++ b/webauthn-rs-core/src/interface.rs @@ -38,7 +38,7 @@ pub struct RegistrationState { pub(crate) require_resident_key: bool, pub(crate) authenticator_attachment: Option, pub(crate) extensions: RequestRegistrationExtensions, - pub(crate) experimental_allow_passkeys: bool, + pub(crate) experimental_allow_synchronised_authenticators: bool, } /// The in progress state of an authentication attempt. You must persist this associated to the UserID diff --git a/webauthn-rs-core/src/lib.rs b/webauthn-rs-core/src/lib.rs index 36bec5dc..3fed6719 100644 --- a/webauthn-rs-core/src/lib.rs +++ b/webauthn-rs-core/src/lib.rs @@ -4,12 +4,14 @@ //! to allow strong, passwordless, cryptographic authentication to be performed. Webauthn //! is able to operate with many authenticator types, such as U2F. //! -//! This library aims to provide a secure Webauthn implementation that you can -//! plug into your application, so that you can provide Webauthn to your users. +//! ⚠️ ⚠️ ⚠️ THIS IS UNSAFE. AVOID USING THIS DIRECTLY ⚠️ ⚠️ ⚠️ //! -//! To use this library yourself, you will want to reference the `WebauthnConfig` trait to -//! develop site specific policy and configuration, and the `Webauthn` struct for Webauthn -//! interactions. +//! If possible, use the `webauthn-rs` crate, and it's safe wrapper instead! +//! +//! Webauthn as a standard has many traps that in the worst cases, may lead to +//! bypasses and full account compromises. Many of the features of webauthn are +//! NOT security policy, but user interface hints. Many options can NOT be +//! enforced. `webauthn-rs` handles these correctly. USE `webauthn-rs` INSTEAD. #![cfg_attr(docsrs, feature(doc_cfg))] #![cfg_attr(docsrs, feature(doc_auto_cfg))] diff --git a/webauthn-rs/Cargo.toml b/webauthn-rs/Cargo.toml index a0b31f70..9da9e9c2 100644 --- a/webauthn-rs/Cargo.toml +++ b/webauthn-rs/Cargo.toml @@ -16,7 +16,9 @@ features = ["danger-allow-state-serialisation", "danger-user-presence-only-secur rustdoc-args = ["--cfg", "docsrs"] [features] -preview-features = ["conditional-ui", "attestation"] +default = ["attestation"] + +preview-features = ["conditional-ui"] resident-key-support = [] conditional-ui = [] attestation = [] diff --git a/webauthn-rs/src/lib.rs b/webauthn-rs/src/lib.rs index 28f1510b..62f4f53d 100644 --- a/webauthn-rs/src/lib.rs +++ b/webauthn-rs/src/lib.rs @@ -16,6 +16,9 @@ //! In the simplest case where you just want to replace passwords with strong self contained multifactor //! authentication, you should use our passkey flow. //! +//! Remember, no other authentication factors are needed. A passkey combines inbuilt user +//! verification (pin, biometrics, etc) with a hardware cryptographic authenticator. +//! //! ``` //! use webauthn_rs::prelude::*; //! @@ -41,12 +44,10 @@ //! After this point you then need to use `finish_passkey_registration`, followed by //! `start_passkey_authentication` and `finish_passkey_authentication` //! -//! No other authentication factors are needed! A passkey combines inbuilt user verification (pin, biometrics, etc) -//! with a hardware cryptographic authenticator. -//! //! # Tutorial //! -//! Tutorials and examples on how to use this library in your website project is on the project github +//! Tutorials and examples on how to use this library in your website project is on the project +//! github //! //! # What is a "Passkey"? //! @@ -54,18 +55,22 @@ //! the terminology. There are at least four definitions that we are aware of. A passkey is: //! //! * any possible webauthn authenticator - security key, tpm, touch id, etc -//! * a platform authenticator - built into a device such as touch id, tpm, etc -//! * a synchronised credential - backed by a cloud keychain like Apple iCloud -//! * a resident key - a stored, discoverable credential allowing usernameless flows +//! * a platform authenticator - built into a device such as touch id, tpm, etc. This excludes security keys. +//! * a synchronised credential - backed by a cloud keychain like Apple iCloud or Bitwarden. +//! * a resident key (rk) - a stored, discoverable credential allowing usernameless flows //! -//! The issue is each of these definitions have different pros/cons and different implications. For +//! Each of these definitions have different pros and cons, and different usability implications. For //! example, passkeys as resident keys means you can accidentally brick many ctap2.0 devices by exhausting -//! their storage. Passkeys as platform authenticators means only certain devices can use them. -//! Passkeys as synced credentials means only certain devices with specific browser combinations can -//! use them. +//! their storage (forcing them to require a full reset, wiping all credentials). Passkeys as platform +//! authenticators means only certain laptops and phones can use them. +//! Passkeys as synced credentials means only certain devices with specific browser combinations +//! or extensions can use passkeys. //! -//! In this library we chose to define passkey's as "any possible authenticator". If the device -//! opportunistically creates rk (such as Apple iCloud Keychain) then in the future we *may* allow usernameless +//! In this library we chose to define passkey's as "any possible authenticator". This conforms to the +//! w3c webauthn workgroup goal to enable all users to choose their authenticator, without any negative +//! penalities or traps. +//! If the device opportunistically creates a resident key (such as Apple iCloud Keychain) then in +//! the future we *may* allow usernameless //! flows once we are satisfied with the state of these ui's in browsers. //! //! # Features @@ -106,21 +111,20 @@ //! [Credential] type is exposed via the [prelude] when this feature is enabled. //! //! However, you should be aware that manipulating the internals of a [Credential] may affect the usage -//! of that [Credential] in certain use cases. You should be careful when enabling this feature that -//! you do not change internal [Credential] values. +//! any security properties of that [Credential] in certain cases. You should be careful when +//! enabling this feature that you do not change internal [Credential] values without understanding +//! the implications. //! //! ## User-Presence only SecurityKeys //! //! By default, SecurityKeys will opportunistically enforce User Verification (Such as a PIN or -//! Biometric). This can cause issues with Firefox which only supports CTAP1. An example of this -//! is if you register a SecurityKey on chromium it will be bound to always perform UserVerification -//! for the life of the SecurityKey precluding it's use on Firefox. +//! Biometric). This prevent UV bypass attacks and allows upgrade of the SecurityKey to a Passkey. //! //! Enabling the feature `danger-user-presence-only-security-keys` changes these keys to prevent //! User Verification if possible. However, newer keys will confusingly force a User Verification //! on registration, but will then not prompt for this during usage. Some user surveys have shown //! this to confuse users to why the UV is not requested, and it can lower trust in these tokens -//! when they are elevated to be self-contained MFA as the user believes these UV prompts to be +//! when they are elevated to be self-contained MFA (passkey) as the user believes these UV prompts to be //! unreliable and not verified correctly - in other words it trains users to believe that these //! prompts do nothing and have no effect. In these cases you MUST communicate to the user that //! the UV *may* occur on registration and then will not occur again, and that is *by design*. @@ -146,7 +150,7 @@ //! To enable the registration call that triggers the 'Google Passkey stored in Google Password //! Manager' key flow, you can enable the feature `workaround-google-passkey-specific-issues`. This //! flow can only be used on Android devices with GMS Core, and you must have a way to detect this -//! ahead of time. +//! ahead of time. This flow must NEVER be triggered on any other class of device or browser. //! //! ## Conditional UI / Username Autocompletion //! @@ -155,7 +159,7 @@ //! single step. //! //! Not all devices support this, nor do all browsers. As a result you must always support the -//! full passkey flow, and conditional-ui is only opportunistically itself. +//! full passkey flow, and conditional-ui is only opportunistic in itself. //! //! User testing has shown that these conditional UI flows in most browsers are hard to activate //! and may be confusing to users, as they attempt to force users to use caBLE/hybrid. We don't @@ -456,18 +460,18 @@ impl Webauthn { } /// Initiate the registration of a new passkey for a user. A passkey is any cryptographic - /// authenticator acting as a single factor of authentication, far stronger than a password - /// or email-reset link. + /// authenticator which internally verifies the user's identity. As these are self contained + /// multifactor authenticators, they are far stronger than a password or email-reset link. Due + /// to how webauthn is designed these authentications are unable to be phished. /// /// Some examples of passkeys include Yubikeys, TouchID, FaceID, Windows Hello and others. /// /// The keys *may* exist and 'roam' between multiple devices. For example, Apple allows Passkeys /// to sync between devices owned by the same Apple account. This can affect your risk model - /// related to these credentials, but generally in all cases passkeys are better than passwords! + /// related to these credentials, but generally in most cases passkeys are better than passwords! /// - /// You *should* NOT pair this authentication with another factor. A passkey may opportunistically - /// allow and enforce user-verification (MFA), but this is NOT guaranteed with all authenticator - /// types. + /// You *should* NOT pair this authentication with any other factor. A passkey will always + /// enforce user-verification (MFA) removing the need for other factors. /// /// `user_unique_id` *may* be stored in the authenticator. This may allow the credential to /// identify the user during certain client side work flows. @@ -475,7 +479,7 @@ impl Webauthn { /// `user_name` and `user_display_name` *may* be stored in the authenticator. `user_name` is a /// friendly account name such as "claire@example.com". `user_display_name` is the persons chosen /// way to be identified such as "Claire". Both can change at *any* time on the client side, and - /// MUST NOT be used as primary keys. They *may not* be present in authentication, these are only + /// MUST NOT be used as primary keys. They *are not* present in authentication, these are only /// present to allow client facing work flows to display human friendly identifiers. /// /// `exclude_credentials` ensures that a set of credentials may not participate in this registration. @@ -500,7 +504,6 @@ impl Webauthn { /// /// ``` /// # use webauthn_rs::prelude::*; - /// /// # let rp_id = "example.com"; /// # let rp_origin = Url::parse("https://idm.example.com") /// # .expect("Invalid URL"); @@ -508,7 +511,7 @@ impl Webauthn { /// # .expect("Invalid configuration"); /// # let webauthn = builder.build() /// # .expect("Invalid configuration"); - /// + /// # /// // you must store this user's unique id with the account. Alternatelly you can /// // use an existed UUID source. /// let user_unique_id = Uuid::new_v4(); @@ -554,7 +557,7 @@ impl Webauthn { }); self.core - .generate_challenge_register_options( + .generate_challenge_register( user_unique_id.as_bytes(), user_name, user_display_name, @@ -610,7 +613,7 @@ impl Webauthn { }); self.core - .generate_challenge_register_options( + .generate_challenge_register( user_unique_id.as_bytes(), user_name, user_display_name, @@ -668,11 +671,11 @@ impl Webauthn { ) -> WebauthnResult<(RequestChallengeResponse, PasskeyAuthentication)> { let extensions = None; let creds = creds.iter().map(|sk| sk.cred.clone()).collect(); - let policy = UserVerificationPolicy::Required; - let allow_backup_eligible_upgrade = true; + let policy = Some(UserVerificationPolicy::Required); + let allow_backup_eligible_upgrade = Some(true); self.core - .generate_challenge_authenticate_policy( + .generate_challenge_authenticate( creds, policy, extensions, @@ -743,7 +746,7 @@ impl Webauthn { /// your site, then you can provide the Yubico Root CA in this list, to validate that all /// registered devices are manufactured by Yubico. /// - /// Extensions may ONLY be accessed if an `attestation_ca_list` is provided, else they can + /// Extensions may ONLY be accessed if an `attestation_ca_list` is provided, else they /// ARE NOT trusted. /// /// # Returns @@ -871,7 +874,7 @@ impl Webauthn { let reject_passkeys = true; self.core - .generate_challenge_register_options( + .generate_challenge_register( user_unique_id.as_bytes(), user_name, user_display_name, @@ -943,7 +946,7 @@ impl Webauthn { ) -> WebauthnResult<(RequestChallengeResponse, SecurityKeyAuthentication)> { let extensions = None; let creds = creds.iter().map(|sk| sk.cred.clone()).collect(); - let allow_backup_eligible_upgrade = false; + let allow_backup_eligible_upgrade = Some(false); let policy = if self.user_presence_only_security_keys { UserVerificationPolicy::Discouraged_DO_NOT_USE @@ -952,9 +955,9 @@ impl Webauthn { }; self.core - .generate_challenge_authenticate_policy( + .generate_challenge_authenticate( creds, - policy, + Some(policy), extensions, allow_backup_eligible_upgrade, ) @@ -990,7 +993,7 @@ impl Webauthn { /// biometric or other factor. Only if this verification passes, is the signature released /// and provided. /// - /// As a result, the server *only* requires this attested_passkey key to authenticator the user + /// As a result, the server *only* requires this attested_passkey key to authenticate the user /// and assert their identity. Because of this reliance on the authenticator, attestation of /// the authenticator and it's properties is strongly recommended. /// @@ -998,11 +1001,11 @@ impl Webauthn { /// devices, and must be bound to a single authenticator. This precludes the use of certain types /// of authenticators (such as Apple's Passkeys as these are always synced). /// - /// Additionally, these credentials can have an attestation or certificate of authenticity - /// validated to give you stronger assertions in the types of devices in use. + /// Additionally, these credentials must provide an attestation certificate of authenticity + /// which will be cryptographically validated to stricly enforce that only certain devices may be used. /// /// You *should* recommend to the user to register multiple attested_passkey keys to their account on - /// seperate devices so that they have fall back authentication. + /// seperate devices so that they have fall back authentication in the case of device failure or loss. /// /// You *should* have a workflow that allows a user to register new devices without a need to register /// other factors. For example, allow a QR code that can be scanned from a phone, or a one-time @@ -1023,7 +1026,7 @@ impl Webauthn { /// You *should* provide the list of credentials that are already registered to this user's account /// to prevent duplicate credential registrations. /// - /// `attestation_ca_list` contains an optional list of Root CA certificates of authenticator + /// `attestation_ca_list` contains a required list of Root CA certificates of authenticator /// manufacturers that you wish to trust. For example, if you want to only allow Yubikeys on /// your site, then you can provide the Yubico Root CA in this list, to validate that all /// registered devices are manufactured by Yubico. @@ -1034,9 +1037,6 @@ impl Webauthn { /// device are used such as a TPM or TouchId. If set to Cross-Platform, only devices that are /// removable from the device can be used such as yubikeys. /// - /// Currently, extensions are *not* possible to request due to webauthn not properly supporting - /// them in broader contexts. - /// /// # Returns /// /// This function returns a `CreationChallengeResponse` which you must serialise to json and @@ -1055,7 +1055,6 @@ impl Webauthn { /// ``` /// # use webauthn_rs::prelude::*; /// use webauthn_rs_device_catalog::Data; - /// /// # let rp_id = "example.com"; /// # let rp_origin = Url::parse("https://idm.example.com") /// # .expect("Invalid url"); @@ -1063,7 +1062,7 @@ impl Webauthn { /// # .expect("Invalid configuration"); /// # let webauthn = builder.build() /// # .expect("Invalid configuration"); - /// + /// # /// // you must store this user's unique id with the account. Alternatelly you can /// // use an existed UUID source. /// let user_unique_id = Uuid::new_v4(); @@ -1140,14 +1139,16 @@ impl Webauthn { // only viable on FIDO2/CTAP2 creds that actually support this. enforce_credential_protection_policy: Some(true), }), + // https://www.w3.org/TR/webauthn-2/#sctn-uvm-extension uvm: Some(true), cred_props: Some(true), + // https://fidoalliance.org/specs/fido-v2.1-rd-20210309/fido-client-to-authenticator-protocol-v2.1-rd-20210309.html#sctn-minpinlength-extension min_pin_length: Some(true), - hmac_create_secret: None, + hmac_create_secret: Some(true), }); self.core - .generate_challenge_register_options( + .generate_challenge_register( user_unique_id.as_bytes(), user_name, user_display_name, @@ -1221,11 +1222,11 @@ impl Webauthn { hmac_get_secret: None, }); - let policy = UserVerificationPolicy::Required; - let allow_backup_eligible_upgrade = false; + let policy = Some(UserVerificationPolicy::Required); + let allow_backup_eligible_upgrade = Some(false); self.core - .generate_challenge_authenticate_policy( + .generate_challenge_authenticate( creds, policy, extensions, @@ -1274,15 +1275,21 @@ impl Webauthn { pub fn start_discoverable_authentication( &self, ) -> WebauthnResult<(RequestChallengeResponse, DiscoverableAuthentication)> { - let policy = UserVerificationPolicy::Required; + let policy = Some(UserVerificationPolicy::Required); let extensions = Some(RequestAuthenticationExtensions { appid: None, uvm: Some(true), hmac_get_secret: None, }); + let allow_backup_eligible_upgrade = Some(false); self.core - .generate_challenge_authenticate_discoverable(policy, extensions) + .generate_challenge_authenticate( + vec![], + policy, + extensions, + allow_backup_eligible_upgrade, + ) .map(|(mut rcr, ast)| { // Force conditional ui - this is not a generic discoverable credential // workflow! @@ -1382,7 +1389,7 @@ impl Webauthn { }); self.core - .generate_challenge_register_options( + .generate_challenge_register( user_unique_id.as_bytes(), user_name, user_display_name, @@ -1440,11 +1447,11 @@ impl Webauthn { hmac_get_secret: None, }); - let policy = UserVerificationPolicy::Required; - let allow_backup_eligible_upgrade = false; + let policy = Some(UserVerificationPolicy::Required); + let allow_backup_eligible_upgrade = Some(false); self.core - .generate_challenge_authenticate_policy( + .generate_challenge_authenticate( creds, policy, extensions,