Closed MasterKale closed 3 years ago
@MasterKale I can't see the UserHandle
generation in your PR. Did you revert it? because you said that it'll be auto-generated but in the code, we have to pass it in option
And what is the difference between credentialID
and aaguid
? In Yubico
table, we have only credentialID
so I didn’t get it
Ok I saw the comment in the code
So if I understand aaguid
is used for MetadatService
I guess? and credentialID
to link the public-key
with the credential
Yes, that's correct about aaguid
. If you want to dive into a weeds a bit it's a part of the FIDO2 spec, and can be passed into MetadataService to retrieve the corresponding FIDO2 Metadata Statement.
credentialID
and credentialPublicKey
are a pair of values that must be maintained with each other. You'll pass credentialID
into allowCredentials
and excludeCredentials
, and look up a credentialPublicKey
by credentialID
when trying to find the corresponding public key to verify an assertion.
@MasterKale and what about userHandle
generation? we have to do it by ourself? because you told me last time you will auto-generate it
I made the decision not to generate a userHandle
because the combination of RP ID and user handle are how authenticators internally track credentials. If the user handle is unique on every attestation (if SimpleWebAuthn were to automatically generate a userHandle every time) then the RP would lose track of credentials and be unable to pull up that credential for use when the user completes an assertion.
This would make it impossible for the RP to support usernameless in particular, as it's the userHandle
that gets returned during assertion that helps the RP figure out which user is logging in. Therefore the RP needs to be particular about what value is provided here so that it has a chance at correctly associate an attestation credential to a specific user.
@MasterKale thanks for your explanation so I will generate it by myself but I can see the type is string
in the library and in documentation it's a buffer
. What about that?
user.id
in the spec's PublicKeyCredentialCreationOptions
is indeed a buffer
. However, generateAttestationOptions()
returns a JSON-compatible representation of options as PublicKeyCredentialCreationOptionsJSON
. userID
, then, is defined as a string so that it can be easily transmitted to the front end, at which point Browser's startAttestation()
turns it into a UInt8Array before passing it to navigator.credentials.create()
.
By making userID
a string
it leaves it up to the RP to decide how to generate and encode the user handle.
To better support WebAuthn's passwordless and usernameless capabilities, additional info should be persisted after a successful attestation verification. This PR achieves this with the following changes:
The return value of
verifyAttestationResponse()
has been enhanced with additional information:attestationInfo
credentialID
as aBuffer
(replacingbase64CredentialID
)credentialPublicKey
as aBuffer
(replacingbase64PublicKey
)aaguid
as a UUIDstring
credentialType
as an attestation/assertion's PublicKeyCredential[[type]]
slot as astring
userVerified
as theboolean
value offlags.uv
(now only when the attestation is verified)attestationObject
as the decodedresponse.attestationObject
Buffer returned from the browserAdditionally,
verifyAssertionResponse()
has had its return data structure change to align with the above changes.assertionInfo
credentialID
Buffer replaces the oldbase64CredentialID
stringnewCounter
renames the oldcounter
The
excludeCredentials
argument ofgenerateAttestationOptions()
andallowCredentials
ofgenerateAssertionOptions()
now expect the credentialid
to be a Buffer. Previously this was expected to be a string. This change was made to make it easier to feed the return value ofverifyAttestationResponse()
directly into future calls togenerateAttestationOptions()
andgenerateAssertionOptions()
. This also reduces the amount of encoding/decoding between Buffer and Base64URL throughout attestation and assertion verification, which should be more performant.Finally
MetadataService
picked up some preliminary log statements that had to be commented out at the last second. Enabling this logging will be the target of the next release pending a solution to anylogger Issue #17.Related Issues