Rather than parsing the byte structures ourselves by hand, we should determine if nom is safer and cleaner for this task.

https://www.w3.org/TR/webauthn-2/#dictionary-client-data

This will change in the future to include crossOrigin, so we must be ready for this to occur and handle it.

Hi,

I want to understand the architectural decisions in this crate and would like to confirm that the choice of reimplementing core functionality in Rust for this crate has the same goals that rustls has vs one its peers, OpenSSL. ie: fewer api pitfalls, memory safety, etc.

This is my current understanding, let me know if I'm completly off tracks :)

Best

https://pr-preview.s3.amazonaws.com/alanwaketan/webauthn/pull/1491.html#sctn-apple-anonymous-attestation

This has been documented now, so we should add support for this.

I'm in the process of migrating a web server using U2F to Webauthn using this crate, and while new Webauthn registrations have been working great in my limited testing, I've hit a snag migrating existing U2F registrations to the new Webauthn format:

Some snippets of the code I used for this migration are:

/// Migrating the credentials saved in the database

use u2f::register::Registration;
use webauthn_rs::proto::*;

let reg: Registration = get_user_registration();

let x: [u8; 32] = reg.pub_key[1..33].try_into().unwrap();
let y: [u8; 32] = reg.pub_key[33..65].try_into().unwrap();

let cred = COSEKey {
    type_: COSEContentType::ECDSA_SHA256,
    key: COSEKeyType::EC_EC2(COSEEC2Key {
        curve: ECDSACurve::SECP256R1,
        x,
        y,
    }),
};

let new_reg = Credential {
    counter: reg.counter,
    verified: false,
    cred: cred,
    cred_id: reg.key_handle.clone(),
};

/// Setting the appId extension to allow migrated credentials to be recognized
let mut ext: BTreeMap<String, String> = BTreeMap::new();
ext.insert("appid".into(), "https://domain.tld/app-id.json".into());
let (response, state) = webauthn.generate_challenge_authenticate_extensions(creds, Some(ext))?;

The problem I'm encountering right now is trying to authenticate with this credential will cause an InvalidRPIDHash error inside the verify_credential_internal function:

Is it possible to add some optional compatibility mode to this crate so it can check the U2F AppId as well as the Webauthn RP? Is there any other solution I'm not seeing right now? I'm not very knowledgeable on either U2F or Webauthn, so maybe I'm missing something.

Thanks!

Edit: The exact crate versions I'm using:

u2f = "0.2.0"
webauthn-rs = "0.3.0-alpha.7"

This will allow running of a demo site with less risk of DoS.

Update these to the latest versions, and improve the json debugging capability.

Looking at Github the project was updated to 0.2.0 in the last day (hooray!) but this git repo hasn't been changed since May(?).

Where can I find the source code for the 0.2.0?

I tested the axtix example on Fedora 32.
Tested key: HyperFIDO Pro Mini, which is a Feitian K8 rebrand/OEM device with a LED and button.

But it is not working on Firefox 82.
I can click register, Firefox asks about to make the keydata more anonymous.
But after that, my key LED doesn't blink.

But when I try Chromium:
Username "Test" and click Register
Chomium asks for a PIN.
After entering the PIN to Unlock
The LED of the key blinks and after pressing the button, registeration is compleed.

Then when I go back to firefox.
Only login works.

So I wonder why Firefox doesn't support a PIN.
Second why does your example needs that PIN? Maybe can we turn that off?
Why does https://demo.yubico.com/webauthn-technical/registration need a pin to register on Firefox?

This crate depends on an old version of lru, which might stop working as the compiler is better able to detect soundness problems (rust-lang/rust#71274). Would be great if you could update the dependency to the latest version. :)

These should probably be options to the registration or authentication functions given the way they work, rather than site wide settings.

When attempting to implement WebAuthnConfig, it has values whose return values are &str. This makes it difficult for my application to provide values that are instantiated from configuration.

I have not been able to work out how to trigger "user verification" = true on challenges. We should determine how to request these values, and check these are respected properly.

Test that we pass these! it will likely force us to implement other attestation formats, but may also shake out any other assumptions we make (especially around the JS). This is a pretty big task, so it would also be good to add more test cases for different types first to move us closer to this baseline.

Hi! Really nice library. I'd like to support WebAuthn in some of my projects.
Sadly the example currently doesn't work.

I have the Nitrokey FIDO2, Ubuntu 20.04 LTS and Firefox 80.

After clicking "Register" my key doesn't show me its indication that I should touch it. This works as expected with my key/browser combination on https://webauthn.io/.
The console shows no errors.

Let me know how I can help you!

Many attestation statements provide trust paths for attestation certificates. We should verify these trust paths, as stated in step 21 of the webauthn spec on registering a new credential.

Support the authentication attachment policy for registration challenges. See also:

https://www.w3.org/TR/webauthn/#authenticatorSelection

https://www.w3.org/TR/webauthn/#platform-attachment

Moved from #73:

Some other minor things I came across is COSEContentType: this is called COSE algorithm in WebAuthn and COSE specs. Also ECDSA_SHA256/ECDSA_SHA384/ECDSA_SHA512 are called ES256, ES384 and ES512 in the specs.

The algorithm simultaneously defines the message digest algorithm (e.g. SHA256) and the signature algorithm (ECDSA, EDDSA, RSA-PSS, etc.). Implicitly this also restricts things like the type of key and curve used.

An open question is the ergonomics of the public API and how we expose datastorage traits and other methods of application integration. As this library hasn't been used in "production" yet, it's still an open question if the decisions I made were really correct or ergonomic to use for deployments.

If you have comments or feedback on the current API design, I'd love to know!

This check https://github.com/kanidm/webauthn-rs/blob/master/src/core.rs#L297 compares the client data origin with the relying party config origin. I've bumped into the scenario where the check fires when the webauthn frontend listens on a different port than the backend. Should the check include the port, or should this be host only? (Browser/Client seems to send the port in clientData)

According to the L2 version of webauthn, "If user verification is required for this assertion, verify that the User Verified bit of the flags in authData is set.". We currently reject authentication/registration if discouraged has UV=true, which is incorrect. We should allow this.

Additionally, we should check that UV || UP == true, as UV implies UP and I think some authenticators only set one or the other.

At the moment I have glossed over extensions, but they should be supported.

Allow these via the registration policy. See also: https://www.w3.org/TR/webauthn/#resident-credential

Similar to TryFrom which has a Self::Error type, we could consider the same for WebauthnConfig implementors. A key issue is how we proxy that Error type back to the caller via the WebauthnError type without over-complication of the type system.

I want to use a Webauthn struct as app data in an actix-web backend which needs to share the struct between the worker threads of the tokio runtime. Whenever I try to use an Arc<Mutex<Webauthn>> as the type to put in App::data(), I get:

error[E0277]: `NonNull<rand::rngs::adapter::reseeding::ReseedingRng<rand_chacha::chacha::ChaCha20Core, rand_core::os::OsRng>>` cannot be sent between threads safely
  --> src/main.rs:67:5
   |
67 |     HttpServer::new(move || {
   |     ^^^^^^^^^^^^^^^ `NonNull<rand::rngs::adapter::reseeding::ReseedingRng<rand_chacha::chacha::ChaCha20Core, rand_core::os::OsRng>>` cannot be sent between threads safely
   |
   = help: within `Webauthn<config::Config>`, the trait `std::marker::Send` is not implemented for `NonNull<rand::rngs::adapter::reseeding::ReseedingRng<rand_chacha::chacha::ChaCha20Core, rand_core::os::OsRng>>`
   = note: required because it appears within the type `rand::rngs::thread::ThreadRng`
   = note: required because it appears within the type `Webauthn<config::Config>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `futures::lock::Mutex<Webauthn<config::Config>>`
   = note: 1 redundant requirements hidden
   = note: required because of the requirements on the impl of `std::marker::Send` for `Arc<futures::lock::Mutex<Webauthn<config::Config>>>`
   = note: required because it appears within the type `[closure@src/main.rs:67:21: 130:6]`
   = note: required by `HttpServer::<F, I, S, B>::new`

New to this library and Rust. How do I run the examples?

I made a mistake when declaring the privacy on the Challenge's wrapped byte vector in a recent refactor. It is currently possible to put a Vec<u8> into the Challenge wrapper, but it is not possible to get it out again.

The two solutions to this are to make the inner type pub again, or to offer a fn into_inner(self) -> Vec<u8> on this type. I prefer the latter.

https://fidoalliance.org/specs/fido-v2.1-rd-20210309/fido-client-to-authenticator-protocol-v2.1-rd-20210309.html#sctn-credProtect-extension

What would be required to finalise v0.3? Are there any other breaking api changes we want to make?

X509PublicKey takes a public key (actually this struct is a X509 certificate, not just the public key) but also a COSE algorithm identifier. However, X509 certificates already contain a signature algorithm themselves, so the COSE algorithm is redundant and leaves room for inconsistencies.

The algorithm does needs to match the COSE algorithm in some cases, for example in the attestation certificate but that can be done by converting pubk.signature_algorithm() to a COSE algorithm and comparing it.

There are already a few places in the code where this algorithm causes confusion:
crypto.rs, the algorithm follows from the certificate, it is redundant

pubk: x509::X509::from_pem(APPLE_X509_PEM).unwrap(),
// This content type was obtained statically (this certificate is static data)
t: COSEAlgorithm::ES384,

attestation.rs, it might very well not be right, the certificates are supplied by the device

// FIXME: this is determined empirically, not sure if it's always right.
std::iter::once(alg).chain(std::iter::repeat(COSEAlgorithm::ES384)),

attestation.rs, same algorithm used for all certificates in the chain, they do not have to use the same algorithm

.and_then(|b| crypto::X509PublicKey::try_from((b.as_slice(), alg)))

I would suggest removing t from the X509 struct and replacing it by a method that returns the COSE algorithm identifier based on the X509 signature algorithm. Or determining it on creation and caching it in a member variable.

What's the sequence of API calls to register a user and the 2FA device and then logging in and using the 2FA device for auth?

We are looking to potentially expose the error messages defined in this project in our product and I noticed a few things were misspelled. We would like to have confidence returning these error messages to users knowing that they conform to standard English.

Not only are there some misspelled words in the error messages, some error enum variants are misspelled as well, namely:

• AttestationCredentialSubjectKeyMistmatch -> AttestationCredentialSubjectKeyMismatch
• ParseInsufficentBytesAvailable -> ParseInsufficientBytesAvailable
  It might make sense to change these too, but that would constitute a breaking change.

The doc comment above the default implementation of policy_verify_trust says

/// The default implementation of this method rejects None and 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.

The implementation, however, appears to accept the None attestation type

fn policy_verify_trust(&self, at: AttestationType) -> Result<Credential, ()> {
    log::debug!("policy_verify_trust -> {:?}", at);
    match at {
        AttestationType::Basic(credential, _attest_cert) => Ok(credential),
        AttestationType::Self_(credential) => Ok(credential),
        AttestationType::AttCa(credential, _attest_cert, _ca_chain) => Ok(credential),
        AttestationType::None(credential) => Ok(credential),
        _ => {
            // We don't know how to assert trust in this yet, or we just
            // don't trust it at all (Uncertain, None).
            Err(())
        }
    }
}

There are a few cases in this crate's API where a function takes an argument by value, when it could easily take it by reference instead, without additional cloning.

The effect of this is that code that uses the library often needs to clone the items with which it calls the functions defined in this crate. Ideally, this could be minimized or eliminated.

This, of course, would necessitate some breaking changes.

The level 3 release will occur in the future and when it does, we should update to support it.

This was started as part of getting ios+5ci support working, however it is blocked on sfackler/rust-openssl#1095 as we need to access cert extensions and oids to complete the process.

When #66 was authored, no browser seemed to have implemented the credBlob extension. This issue serves as a reminder that we should get some example vectors to test the extensions deserialization on when this is no longer the case.

We currently deviate from the standard by base64 encoding some JSON members in challenges and responses. This may confuse some consumers, but this difficulty comes in from ArrayBuffers being a 'a [u8] in a struct, which causes lifetime issues with actix. I have also noted odd behaviours where the output of navigator.credentials methods can't be sent to clients either, also causing us to do some odd js tricks.

Basically, we need to improve the js parts, and work out better ways to handle this aspect of the system.

Per the README, I'm reaching out to flag that TouchID on the Macbook's with the touch sensor doesn't seem to work? I'm testing this through kanidm so it could be related to that 😇

Found this while attempting to register with TouchID.

Info on the checks required to support this.

There may be time in my near future to PR this.

https://w3c.github.io/webauthn/#extensions should be implemented for this server.

Situation:

Two frontends:

• https://kanidm.example.com
• https://internal.kanidm.example.com

Origin on kanidmd: https://kanidm.example.com

Registering token only works when client's pointed at https://kanidm.example.com

Authenticating against https://kanidm.example.com works OK with webauthn.

Authenticating against https://internal.kanidm.example.com fails:

TRACE    tide-request [ 156µs | 100.000% ]
INFO     ┝━ 💬 [request.info]: Request received | http.method: "POST" | path: "/v1/auth"
INFO     ┝━ 💬 [security.info]: Begin auth event | sessionid: Some(00000000-616b-5205-34c5-891ea36cd0a9) | req: AuthRequest { step: Cred(Webauthn(_)) }
INFO     ┝━ 💬 [security.info]: Handler::Webauthn -> Result::Denied - webauthn error | e: InvalidRPOrigin
INFO     ┝━ 💬 [security.info]: Credentials denied | reason: invalid webauthn authentication
INFO     ┝━ 💬 [security.info]: Sending auth result | res: Ok(AuthResult { sessionid: 00000000-616b-5205-34c5-891ea36cd0a9, state: Denied("invalid webauthn authentication"), delay: None })
INFO     ┕━ 💬 [request.info]: --> Response sent | status: 200 - OK

As discussed, it seems to relate to: https://github.com/kanidm/webauthn-rs/blob/master/src/core.rs#L316

When dealing with authentication and security code it's important to be checked for correctness. Have an external review of the code conducted to see if any issues are found. Potentially this could involve running other compliance suites if they are easily available.

Hi!

$ cargo audit

error: Vulnerable crates found!

ID:       RUSTSEC-2019-0025
Crate:    serde_cbor
Version:  0.9.0
Date:     2019-10-03
URL:      https://rustsec.org/advisories/RUSTSEC-2019-0025
Title:    Flaw in CBOR deserializer allows stack overflow
Solution:  upgrade to >= 0.10.2
Dependency tree: 
serde_cbor 0.9.0
└── webauthn-rs 0.2.2

Affected versions of this crate did not properly check if semantic tags were nested excessively during deserialization.
This allows an attacker to craft small (< 1 kB) CBOR documents that cause a stack overflow.
The flaw was corrected by limiting the allowed number of nested tags.

It might be a good idea to update it