-
Notifications
You must be signed in to change notification settings - Fork 120
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
Provide More Options Public Key Exporting and Argument Formatting #143
Comments
Hey @knightcode , I'm not sure if I fully understand your requirement about not being locked into this library - it's an open-source repository and we use an open standard, so if you save the result that a standardized browser responds, any library would be able to continue working with it (not that saving attestationResults really is a best practice and a migration with an intermediate browser response is a good idea). Generally best practice is not to do any validation or mutation yourself, as you might miss something that the library would've caught or prevented. In any case, we are an actively maintained community library that welcome every PR. If you can improve the Buffers and extend the format options, we'd be very happy about your contribution 👍 |
@JamesCullum , I guess there's two concerns:
|
Thanks for the feedback - I'm still not sure if I fully get the point.
Do I understand you correctly that you are not yet able to take care of the PR yourself? We can label it as "Help wanted" and see if someone has the same pain point and can implement it. |
Maybe I'll get time to work on this, but I don't think it'll be soon. Also, by no means am I an expert on packing certificates and public keys, e.g. I assume SimpleWebAuth had some argument for CBOR or anything else, but I have no idea what it was. And I couldn't find any js libraries that translated from ASN.1 to CBOR or back. Someone else could do better at that than me. But this is what I would like to be able to write: await fidoLib.assertionResult({
id: creds.id, // Buffer or TypedArray or ArrayBuffer
rawId: creds.rawId, // Buffer or TypedArray or ArrayBuffer
response: {
clientDataJSON: response.clientDataJSON, // base64 string | Buffer | TypedArray | ArrayBuffer
authenticatorData: response.authenticatorData, // Buffer | TypedArray | ArrayBuffer
signature: response.signature, // base64 string | Buffer | TypedArray | ArrayBuffer
userHandle: response.userHandle // base64 string | Buffer | TypedArray | ArrayBuffer
}
}, {
challenge: lastChallenge, // base64 string | Buffer | TypedArray | ArrayBuffer
origin: ...,
factor: "either",
publicKey: userAuthenticator.publicKey, // Object (JWK) | String (PEM) | Buffer/ArrayBuffer (COSE or DER if easy to differentiate)
prevCounter: ...,
userHandle: ... // base64 string | Buffer | TypedArray | ArrayBuffer
}); If await fidoLib.assertionResult(clientsideCredentials, {
challenge: lastChallenge,
origin: ...,
factor: "either",
publicKey: userAuthenticator.publicKey,
prevCounter: ...,
userHandle: ...
}); |
Thanks for the example, this really helped me understand what you are trying to achieve. I agree that more formats would always be nice, however I'm not sure if the difference between the code you currently need to use and the one you propose is different enough to warrant a change in the package - this sounds like a helper method would be the more appropriate solution. The reason you have the data in this shape is because you want to implement non-standard use cases specific to your application and you do proprietary changes to execute this. It would be very difficult to implement and maintain a compatible interface for each of those - and the work and time would exceed the one of a simple helper method on your side by a wide margin. We can use the voting function for this comment to see if there are enough users that would like to see exactly the same interfaces (thumbs up = yes). If it gets enough traction, there surely is a developer who can implement it. |
We have to store a public key in the interim time between the result of
attestationResult
and any subsequent calls toassertionResult
. And I'm looking to for the most flexible yet compact storage format, i.e. a format that doesn't lock me into this library if it stops getting maintained and one that plays nice withsubtle.importKey()
ornode:crypto.createPublicKey()
in case that ever becomes useful in the future. And we have the added constraint that it has be rendered into a string in PEM format for theassertionResult()
call.It's this later constraint that I find problematic because I could give you the public key in a format that doesn't require a much preprocessing, namely that we encode it to PEM so that you can decode it from PEM.
Given that we have three options for acquiring the public key, namely
result.authnrData.get( ___ )
with"credentialPublicKeyPem"
,"credentialPublicKeyCose"
, or"credentialPublicKeyJwk"
as arguments to theMap
call, my current solution is to get the PEM format, strip it down to a base64 string that can be parsed into aBuffer
(Uint8Array
) and then store the decoded bytes. Jwk would be an option that Subtle supports, but it's not as compact.Then for
assertionResult
, I have to do this ugly thing:As such, I would humbly request that
assertResult
be a little more open in the formats that it accepts. And some added documentation about what these formats can and should be would have saved me a ton of trial & error combined with digging through and instrumenting the source code. I don't think it would be too difficult to test the inputs for string or buffer types and act accordingly under some documented assumptions.Side note, raw
ArrayBuffer
s are the worst. With like any other type, I could give you a view of some segment of memory and avoid copying bytes around.The text was updated successfully, but these errors were encountered: