w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.19k stars 172 forks source link

Unspecified CBOR encoding of high-valued integer numbers due to unspecified threshold #1044

Closed peteroupc closed 5 years ago

peteroupc commented 6 years ago

Section 9 contains the following text:

When the JavaScript value is a non-integer number, it is converted to a 64-bit CBOR floating point number. Otherwise, when the JavaScript type corresponds to a JSON type, the conversion is done using the rules defined in Section 4.2 of [RFC7049] (Converting from JSON to CBOR), but operating on inputs of JavaScript type values rather than inputs of JSON type values.

However, because section 4.2 of the RFC says the following:

JSON numbers without fractional parts (integer numbers) are represented as integers ...; integers longer than an implementation-defined threshold (which is usually either 32 or 64 bits) may instead be represented as floating-point values.

and section 9 doesn't specify a threshold in this sense, then CBOR canonicalization is not possible if those integers are present — as a result it is not specified unambiguously whether certain integers higher than a threshold (232? 231? 264? some other threshold?) should be encoded as integers as or as floating-point numbers, since the relevant threshold is "implementation-defined".

agl commented 6 years ago

I think we should write something to state the obvious here, that implementations are not allowed to arbitrarily change integers into floating-point values. However, it seems unlikely that we'll tidy this up prior to PR.

agl commented 5 years ago

(From the call of 2019-02-06:)

It's also possible for a floating-point value in Javascript to happen to be an integer. Thus the type of the translated CBOR will depend on the values of the input, which seems like it would break things.

However, since we don't have any implementations of the generic transform of extensions that I'm aware of, we could change things in level two without breaking any existing systems.

selfissued commented 5 years ago

Note that if values used by an extension are floating point, the extension can specify that they always be represented as floating point - even if values happen to be integers. The location extension https://www.w3.org/TR/webauthn/#sctn-location-extension does this for the coordinate values, for instance.

nadalin commented 5 years ago

@jcjones Please review

selfissued commented 5 years ago

Unless @jcjones disagrees, I believe that we should close this with no action.

jcjones commented 5 years ago

I agree with closing this no action. Ultimately, this has to be some other spec's fix.

peteroupc commented 5 years ago

Cross-referencing the issue for the draft revision of the CBOR specification.

peteroupc commented 5 years ago

I should make an observation: Strictly speaking there are no "integer numbers" (that is, integer number types) in JavaScript (until perhaps recently with BigInt). JavaScript until recently has only one number type, namely a 64-bit floating-point number (of the class Number), and no integer types such as 16-, 32- or 64-bit signed integers. In that case, the phrase "When the JavaScript value is a non-integer number..." should perhaps be changed to "When the JavaScript value is of the type Number..." if that is what section 9 means.

peteroupc commented 5 years ago

I see that currently, of the defined WebAuthn extensions, only biometricPerfBounds includes number types, namely float, in client extension inputs. This suggests to me that—

With these changes, this issue becomes resolved as moot.