Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing stored data in server #95

Closed
akanass opened this issue Feb 1, 2021 · 11 comments
Closed

Missing stored data in server #95

akanass opened this issue Feb 1, 2021 · 11 comments
Labels
package:server @simplewebauthn/server

Comments

@akanass
Copy link
Contributor

akanass commented Feb 1, 2021

As described in the Yubico developer Guide to backing up authenticator data, userHandle, metadata, and attestation data should be stored in the database.

For the attestation, see my comment

For the userHandle, the value exists and it's the user.id field during attestation generation phase - we have to store it during attestation verification phase inside AuthenticatorDevice object with others data and to do it, we have to keep it like challenge between both phases. see comment also.

For the metadata, the value should be returned by MDS but if it's not included, we should have at least this {"authenticator_attachment":"cross-platform"} or {"authenticator_attachment":"platform"} regarding the type of the authenticator registered. This can be very helpful to display dedicated section in the UI regarding the authenticator type.

All this new values should be stored inside AuthenticatorDevice object and in the database as well, like this they can be used during verification process.

@MasterKale
Copy link
Owner

I'm glad you brought this up, it encapsulates something I've been musing over lately, one whether I need to think about providing additional date out of attestation verification. I've heard some one or two people that aaguid would be useful output, and now you're bringing up things like authenticator attachment.

Let me take a look at that developer guide and see if that can't become the basis for returning more info from an attestation verification.

@akanass
Copy link
Contributor Author

akanass commented Feb 1, 2021

In the guide, they are saying that:

  • If you care about attestation, it is recommended to also store the raw attestation object as part of the credential. This enables you to retroactively inspect credential attestations in response to policy changes and/or compromised authenticators.
  • If you care about attestation, use the attestationTrusted, attestationType and attestationMetadata fields to enforce your attestation policy.

And the utility to store attestation data is describe here

@akanass
Copy link
Contributor Author

akanass commented Feb 1, 2021

And User Handle is very important as well because it can allow usernameless/passwordless verification so we have to store it during attestation phase and validate it during assertion phase.

This should be a stable, unique identifier for the user - equivalent to a username. However due to privacy considerations it is recommended to set the user handle to a random byte array rather than a username and not using the user primary key store in the db.

@akanass akanass changed the title Missing returned data in server Missing stored data in server Feb 1, 2021
@MasterKale MasterKale added the package:server @simplewebauthn/server label Feb 1, 2021
@MasterKale
Copy link
Owner

PR #97 will introduce a feature that sets a new, random 64-byte Buffer as user.id in the return value from generateAttestationOptions() in the name of better privacy. It'll be up to the RP to store this value while attestation occurs in the browser, but at least this way it'll be possible to save the handle with the credential after attestation verification to match up with the value of userHandle returned within an assertion credential.

@MasterKale
Copy link
Owner

Ah, I got my issues mixed up. PR #97 is targeting fixes for both this issue and #96 hence my getting all turned around.

In addition to my comment above, that PR also updates verifyAttestationResponse() to return more information closer to what Yubico recommends, including aaguid as well as the full, raw attestation object.

@akanass
Copy link
Contributor Author

akanass commented Feb 3, 2021

@MasterKale thanks and can we have the metadata field as well to store the value of the authenticator as describe in Yubico credentials table and the example I gave to you

@MasterKale
Copy link
Owner

@akanass I don't see in Yubico's docs what the "metadata" table is supposed to contain, but based on your comments I'm thinking the way I'll return metadata is either A) undefined if MetadataService is not enabled, or B) whatever statement MetadataService might have for the authenticator's aaguid (otherwise undefined).

As for authenticator_attachment, nothing in the WebAuthn spec provides the actual attachment in the response, so this would have to be something persisted at the RP's application layer between options generation and verification.

@akanass
Copy link
Contributor Author

akanass commented Feb 3, 2021

@MasterKale , Here the metadata in credentials table.

fido2_data_model

And in Yubico demo website the call to list authenticators is this one:

fetch("https://demo.yubico.com/api/v1/user/5420a14a-4a0e-4a61-af56-c8fa8cb7213b/authenticator", {
  "headers": {
    "accept": "*/*",
    "accept-language": "fr-FR,fr;q=0.9,en-US;q=0.8,en;q=0.7",
    "sec-fetch-dest": "empty",
    "sec-fetch-mode": "cors",
    "sec-fetch-site": "same-origin",
    "sec-gpc": "1"
  },
  "referrer": "https://demo.yubico.com/webauthn/",
  "referrerPolicy": "strict-origin-when-cross-origin",
  "body": null,
  "method": "GET",
  "mode": "cors",
  "credentials": "include"
});

RESPONSE

{
  "data":{
  "authenticators":{
    "webauthn":[
      {
      "deviceType":"yk5series",
      "id":"f4b6c018-d8f9-4d3e-825f-406b76895448",
      "lastUsed":"2021-01-30T15:25:39.147510+00:00",
      "metadata":{"authenticator_attachment":"cross-platform"},
      "name":"YubiKey 5 Series",
      "registeredAt":"2021-01-30T15:25:39.147510+00:00",
      "type":"webauthn"
      },
      
      {
      "deviceType":"unknown",
      "id":"9c5c1bfc-a845-43f0-9c4a-56770d59c4a1",
      "lastUsed":"2021-01-30T15:32:07.304554+00:00",
      "metadata":{"authenticator_attachment":"platform","form_factor":"pc","os_family":"macos"},
      "name":"macOS computer",
      "registeredAt":"2021-01-30T15:32:07.304554+00:00",
      "type":"webauthn"
      }
    ]
    }
  },
"status":"success"
}

As we can see, at least we have AuthenticatorAttachment from AuthenticatorSelectionCriteria

@MasterKale
Copy link
Owner

Thank you for the example. Looking closely at that, it all appears to be metadata specifically for Yubico's Relying Party implementation. "id" looks like it could be the authenticator's aaguid, but aside from that there's nothing in there that would be returned by SimpleWebAuthn as part of attestation verification. That'd all come down to how the RP collects and persists such data for a given WebAuthn attestation.

@MasterKale
Copy link
Owner

MasterKale commented Feb 4, 2021

Going down the list of columns in that Yubico table definition:

  • PR feature/better-passwordless-usernameless #97 will cover aaguid and attestation (via the new attestationObject property) as part of additional return values from verifyAttestationResponse()
  • metadata can be retrieved by aaguid above from MetadataService after attestation verification via MetadataService.getStatement()
  • userHandle can be retrieved from user.id in the options returned by generateAttestationOptions() or, when available, an assertion's response.userHandle prior to calling verifyAssertionResponse()

The retrieval and storage of these values will be left up to the RP. Orchestrating all of this would be better as entries on the SimpleWebAuthn docs site, which will be work addressed in Issue #96.

@MasterKale
Copy link
Owner

PR #97 has been merged, so I'm going to close out this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:server @simplewebauthn/server
Projects
None yet
Development

No branches or pull requests

2 participants