When RequestOptions.Rp.Id is null, we should default to Origin/effectiveDomain

[Regenhardt]

Currently the lib compares the rp id in the original PublicKeyRequestOptions to the rp id in the created credential. If the id was however not set in the request options, as is specified as valid in the spec (see https://developer.mozilla.org/en-US/docs/Web/API/PublicKeyCredentialRequestOptions), it just crashes because it just passes the id to UTF8.GetBytes, which (rightly so) doesn't accept null values.

There should be a if(originalOptions.Rp.Id != null) guard before that line to guard either that one at 7) computing the hash (here) or also the one at 9) comparing said hash.

[abergs]

To nuance this, RpID is not allowed to be null.

If is is missing the value, we should default to using the Origin or effectiveDomain: https://www.w3.org/TR/webauthn-2/#GetAssn-DetermineRpId and https://www.w3.org/TR/webauthn-2/#CreateCred-DetermineRpId

A credential will always be bound to a RpID/Origin.

[Regenhardt]

So, verify that the RpId in the attestation matches the origin of the request, or fill the options.Rp.Id from the initial request?

The API could be used from multiple frontends so we can't really use a single configured domain here.

[Regenhardt]

...which actually breeds a whole new question I might actually try to google: What goes in the RpId when a desktop or mobile app (i.e. not a browser, not via website, no domain) makes a credential?

[Regenhardt]

Turns out, apps may use whatever as RP ID, like package name or hash of the signing key for android, although google seems to prefer people linking their app to a website and using that domain. But yeah, gotta think about clients not using the same domain as the server, or not using a domain at all.

I therefore suggest to just not verify the RP ID if it doesn't exist in the creation options. Mostly because it's easier than adding an optional RP ID parameter to the API. The client can take care of filling out or deciding on the actual RP ID used for the authenticator, if any, as leaving it blank means the authenticator's implementation chooses.

[Regenhardt]

Ok with #372 maybe the extra parameter for creating the options looks better.

[abergs]

Ok with #372 maybe the extra parameter for creating the options looks better.

@Regenhardt Can you highlight what you mean with the extra parameter for creating the options and why you think it is better? I'm not sure I follow

[Regenhardt]

Say a company offers three services, A, B, and C, and an identity provider Ident. You wanna login to A, so you go to a.myCo.com and login. Now to add a fingerprint because you don't want to enter your password every time, the website gets new creation options from Ident and puts them into navigator.credentials.create - at which point, the way it currently works, the browser cancels, because the RpId (ident.myCo.com) doesn't match the current domain (a.myCo.com).

So before I'd've said just leave the RpId blank and let the client figure it out. As the spec says, the implementation of the authenticator will fill the RpId with the current domain anyway and it's not something cryptographically secure we absolutely need to verify.

But since it's just nice to actually do verify it, we need a way of putting a.myCo.com into the RpId when it's created, because after that it's gonna be immutable (once #372 goes live). So whether the controller reads it from the request or requests a parameter, the options need a way of setting a specific RpId that is not the server and can be one of multiple services that use Ident.

So in my opinion, Fido2.RequestNewCredential should take an additional parameter, either the RpId or a whole Fido2Configuration object to use instead of the one provided on construction (which usually happens via DI anyway).

Edit, demonstration via the included demo:

Edit2: Different error actually, since the RpId actually is ok here (localhost) but I forgot to add the client's full domain to the allowed origins. Can't actually locally show this problem as I only have localhost and not two domains.

[abergs]

What you are describing is already possible by creating a new Fido2-class with the appropriate config?
I'm not sure I see an advantage of moving it to an additional parameter and increasing the API surface.

[Regenhardt]

Oh I see. I must admit I totally forgot you can actually work without DI and construct the thing directly. Yes that should indeed completely remove this problem, will check it out.
What do you think about adding to the FullyQualifiedOrigins summary the information that those are the hosts/domains/origins allowed to use this ID provider, and to the Origins summary that it also sets the FullyQualifiedOrigins? I'll gladly make a PR after this journey.

[Regenhardt]

Ok I inject a factory now with default Fido2Config, with a method constructing a Fido2 service with the HttpContext.Request.Host.Host as config.ServerDomain now, this works great albeit semantically not very nice since it's not actually the domain of the server but just one of the allowed origins basically.
No idea how this would work from a mobile app though, as that isn't running on a domain.

[abergs]

I'm guessing your sending a request from the mobile app to your webservice? Then you could just pass along the RPID you wish as part of the request and instantiate the FIDO2 service with the data that the client send. This could be done using DI or without -- albeit, not using DI will be more straight forward.

What do you think about adding to the FullyQualifiedOrigins summary the information that those are the hosts/domains/origins allowed to use this ID provider, and to the Origins summary that it also sets the FullyQualifiedOrigins?

I'm sorry, I'm not completely understanding what you want to accomplish here. Could you explain it in a different way?

[Regenhardt]

I'd like to improve the documentation for someone implementing Fido2.
I read through the code and only after going through it, learned that I needed to add the origins of my webapp to the server's origins list so the server allows the webapp to use the server. Right now, it basically just says "Server origins", which is obviously the URL where the webservice is hosted. It does however not tell me that the list also has to include hosts where a webapp is hosted, which can differ from the actual server domain, especially when there's multiple clients using this service.

I'd put something like Origins from where a webapp is allowed to use this server. Validation requests from origins not in this list will fail. to make it clear that a user (of the lib) has to add all possible origins to that list.

[abergs]

I agree that documentation is in need for improvement. Would welcome a addition in either code comments or adding a small section in the readme (or creating a docs.md and linking from the readme).

If modifying existing code comments. We try to keep to the WebAuthn Spec for describing fields and reference the spec where it makes sense, but adding an additional "Friendly description" (without removing existing comments) is appreciated.

[Kevin-Andrew]

If modifying existing code comments. We try to keep to the WebAuthn Spec for describing fields and reference the spec where it makes sense, but adding an additional "Friendly description" (without removing existing comments) is appreciated.

On that note, at CredentialCreateOptions.cs line 162, for the Id property of the PublicKeyCredentialRpEntity class that is supposed to represent the RP ID, it has the following comment:

A unique identifier for the Relying Party entity, which sets the RP ID.

To me, that is very misleading and confusing and is nowhere close to what the specification says. I think it would be nice to update the XML doc of that property so that it includes a link to the official documentation and can also just duplicate the documentation right there as well.

You'll notice that the specification also briefly mentions what other non-web platforms, such as mobile applications MAY do, which I don't know if it would still be of interest to @Regenhardt . However, it still says that the RP ID syntax MUST conform to either a domain name or URI.

/// <summary>
/// See Relying Party Identifier RP ID: https://w3c.github.io/webauthn/#rp-id
/// <para>In the context of the WebAuthn API, a relying party identifier is a valid domain string identifying the WebAuthn
/// Relying Party on whose behalf a given registration or authentication ceremony is being performed. A public key credential
/// can only be used for authentication with the same entity (as identified by RP ID) it was registered with.</para>
/// <para>By default, the RP ID for a WebAuthn operation is set to the caller’s origin's effective domain. This default MAY be
/// overridden by the caller, as long as the caller-specified RP ID value is a registrable domain suffix of or is equal to the caller’s
/// origin's effective domain. See also § 5.1.3 Create a New Credential - PublicKeyCredential’s [[Create]](origin, options,
/// sameOriginWithAncestors) Method and § 5.1.4 Use an Existing Credential to Make an Assertion - PublicKeyCredential’s
/// [[Get]](options) Method.</para>
/// <para>Note: An RP ID is based on a host's domain name. It does not itself include a scheme or port, as an origin does. The
/// RP ID of a public key credential determines its scope. I.e., it determines the set of origins on which the public key credential
/// may be exercised, as follows:
/// - The RP ID must be equal to the origin's effective domain, or a registrable domain suffix of the origin's effective domain.
/// - The origin's scheme must be https.
/// - The origin's port is unrestricted.
/// </para>
/// <para>For example, given a Relying Party whose origin is https://login.example.com:1337, then the following RP IDs are
/// valid: login.example.com (default) and example.com, but not m.login.example.com and not com.</para>
/// <para>This is done in order to match the behavior of pervasively deployed ambient credentials (e.g., cookies, [RFC6265]).
/// Please note that this is a greater relaxation of "same-origin" restrictions than what document.domain's setter provides.</para>
/// <para>These restrictions on origin values apply to WebAuthn Clients.</para>
/// <para>Other specifications mimicking the WebAuthn API to enable WebAuthn public key credentials on non-Web
/// platforms (e.g. native mobile applications), MAY define different rules for binding a caller to a Relying Party Identifier.
/// Though, the RP ID syntaxes MUST conform to either valid domain strings or URIs [RFC3986] [URL].</para>
/// </summary>
[JsonPropertyName("id")]
public string Id { get; set; }

[Regenhardt]

I agree it could have more information that is already in the spec, although I'm hesitant to just put the whole spec there as it's way too big for inline docs in my opinion. After my current PR is finished, I'll gladly make some documentation improvements and take this into consideration, summarizing the most relevant information from the spec here and linking the actual spec for further reading.