Skip to content

Commit

Permalink
245 disable rs1 (#250)
Browse files Browse the repository at this point in the history
* Remove insecure rsa-sha1 support
  • Loading branch information
Firstyear authored Jan 4, 2023
1 parent 696a7b0 commit c761181
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 64 deletions.
2 changes: 0 additions & 2 deletions webauthn-rs-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ categories = ["authentication", "web-programming"]
license = "MPL-2.0"

[features]
insecure-rs1 = []

default = []

[dependencies]
Expand Down
37 changes: 8 additions & 29 deletions webauthn-rs-core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2434,14 +2434,10 @@ mod tests {
true,
);
trace!("{:?}", result);
if cfg!(feature = "insecure-rs1") {
assert!(result.is_ok());
} else {
assert!(matches!(
result,
Err(WebauthnError::CredentialInsecureCryptography)
))
}
assert!(matches!(
result,
Err(WebauthnError::CredentialInsecureCryptography)
))
}

fn register_userid(
Expand Down Expand Up @@ -3802,27 +3798,10 @@ mod tests {
true,
);

if cfg!(feature = "insecure-rs1") {
assert!(result.is_ok());
match result.unwrap().attestation.metadata {
AttestationMetadata::Tpm {
aaguid,
firmware_version,
} => {
assert!(firmware_version == 1390997124431944132);
assert!(
aaguid
== uuid::Uuid::parse_str("08987058cadc4b81b6e130de50dcbe96").unwrap()
);
}
_ => panic!("invalid metadata"),
}
} else {
assert!(matches!(
result,
Err(WebauthnError::CredentialInsecureCryptography)
))
}
assert!(matches!(
result,
Err(WebauthnError::CredentialInsecureCryptography)
))
}

#[test]
Expand Down
20 changes: 4 additions & 16 deletions webauthn-rs-core/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,8 @@ fn pkey_verify_signature(
Ok(verifier)
}
COSEAlgorithm::INSECURE_RS1 => {
if cfg!(feature = "insecure-rs1") {
warn!("INSECURE SHA1 USAGE DETECTED");
let mut verifier = sign::Verifier::new(hash::MessageDigest::sha1(), pkey)
.map_err(WebauthnError::OpenSSLError)?;
verifier
.set_rsa_padding(rsa::Padding::PKCS1)
.map_err(WebauthnError::OpenSSLError)?;
Ok(verifier)
} else {
error!("INSECURE SHA1 USAGE DETECTED");
Err(WebauthnError::CredentialInsecureCryptography)
}
error!("INSECURE SHA1 USAGE DETECTED");
Err(WebauthnError::CredentialInsecureCryptography)
}
c_alg => {
debug!(?c_alg, "WebauthnError::COSEKeyInvalidType");
Expand Down Expand Up @@ -432,15 +422,13 @@ impl EDDSACurve {

pub(crate) fn only_hash_from_type(
alg: COSEAlgorithm,
input: &[u8],
_input: &[u8],
) -> Result<Vec<u8>, WebauthnError> {
match alg {
COSEAlgorithm::INSECURE_RS1 => {
// sha1
warn!("INSECURE SHA1 USAGE DETECTED");
hash::hash(hash::MessageDigest::sha1(), input)
.map(|dbytes| Vec::from(dbytes.as_ref()))
.map_err(WebauthnError::OpenSSLError)
Err(WebauthnError::CredentialInsecureCryptography)
}
c_alg => {
debug!(?c_alg, "WebauthnError::COSEKeyInvalidType");
Expand Down
1 change: 0 additions & 1 deletion webauthn-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ features = ["danger-allow-state-serialisation", "danger-user-presence-only-secur
[features]
resident-key-support = []
preview-features = []
danger-insecure-rs1 = ["webauthn-rs-core/insecure-rs1"]
danger-allow-state-serialisation = []
danger-credential-internals = []
danger-user-presence-only-security-keys = []
Expand Down
16 changes: 0 additions & 16 deletions webauthn-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,6 @@
//! Enabling the feature `danger-allow-state-serialisation` allows you to re-enable serialisation
//! of these types, provided you accept and understand the handling risks associated.
//!
//! ## Allow Insecure RSA_SHA1
//!
//! Many Windows Hello credentials are signed with RSA and SHA1. SHA1 is considered broken
//! and should not be trusted in cryptographic contexts. These signatures are used only
//! during attestation, but the credentials themself are generally RSA-SHA256. In some
//! cases this may allow forgery of a credentials attestation, meaning you are unable to
//! trust the integrity of the authenticator.
//!
//! For the broadest compatibility, and if you do not use attestation (such as passkey only users)
//! then you do not need to enable this feature since attestation is not requested.
//!
//! If you require attestation of authenticators,
//! you may choose to use RSA SHA1 attestation signed credentials with `danger-insecure-rs1`.
//!
//! If in doubt, do not enable this feature.
//!
//! ## Credential Internals and Type Changes
//!
//! By default the type wrappers around the keys are opaque. However in some cases you
Expand Down

0 comments on commit c761181

Please sign in to comment.