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

Add note about use of floating point in extensions passed through that clients do not recognize #1307

Merged
merged 2 commits into from Oct 9, 2019

Conversation

selfissued
Copy link
Contributor

@selfissued selfissued commented Sep 24, 2019

Fixes #1273


Preview | Diff

@selfissued selfissued added this to the L2-WD-02 milestone Sep 24, 2019
@selfissued selfissued self-assigned this Sep 24, 2019
@peteroupc
Copy link

peteroupc commented Sep 24, 2019

My concern was that if different clients use different integer thresholds for passing through unrecognized extensions (and neither WebAuthn nor RFC 7049 specify a default threshold), then that will hinder bit-for-bit comparison or hashing of CTAP2 messages with the same content. This concern is not relevant if the only purpose of canonicalizing CTAP2 messages (both in WebAuthn and elsewhere) is, as the CTAP2 protocol states, "[t]o reduce the complexity of the messages and the resources required to parse and validate them", and not also to enable bit-for-bit comparison or hashing of CTAP2 messages with semantically equal content.

My concern is not whether clients will convert some floating-point values to integers when they pass through unrecognized extensions, but at what integer threshold they do so.

@emlun
Copy link
Member

emlun commented Sep 25, 2019

@peteroupc I don't think that should be a problem, at least for WebAuthn ceremonies. The RP has no direct control of the exact bytes being sent to the authenticator anyway (because the client can add random extra stuff to the CollectedClientData, for example), so you can't rely on the same input producing the exact same outputs even with deterministic signatures.

@peteroupc
Copy link

peteroupc commented Sep 25, 2019

If that's the case, then there should be an additional note that different clients might choose different integer thresholds for converting floating-point values to integers, and that this is not a problem because canonicalized CTAP2 messages generated by the protocol are not meant to be compared bit-for-bit with each other (either directly or via hash codes). This is unlike other protocols like DKIM and XML Signature, which do use canonicalization for bit-for-bit data comparison.

Perhaps the CTAP2 protocol can also clarify that the canonical form it specifies is not intended to enable bit-for-bit comparisons either.

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this addresses the issue, tho could use some polishing...

index.bs Outdated
Comment on lines 4838 to 4843
Note that the numeric conversion rules have the consequence that
when a client passes through an extension it does not recognize,
if the extension uses floating point values,
[=authenticators=] need to be prepared to receive those values as [=CBOR=] integers
when the floating point values used happen to be integers,
should the [=authenticator=] want the extension to always work without actual [=client=] support for it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all one long sentence -- suggested rewrite (plus I'd move this Note to under line 4834 where numeric conversion is discussed):

Note: Due to Javascript's numeric conversion rules, any floating point values may be converted to integers. Thus, [=authenticators=] need to be prepared to receive those values as either [=CBOR=] floating-point or as [=CBOR=] integers. If this is not feasible, the extension may be designed with the [=client=] performing any necessary value type conversions.

@peteroupc
Copy link

For your information: See cbor-wg/CBORbis#117, which modifies the CBOR revision to include defaults for encoding JSON numbers as CBOR.

@selfissued selfissued merged commit 36fe4ed into w3c:master Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certain issues in client extension pass-through specification
4 participants