-
Notifications
You must be signed in to change notification settings - Fork 23
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
Support for uint64 #20
Comments
@matthiasgeihs This is by design because JCS (RFC 8785) requires data to confirm to the JSON subset specified by JavaScript (aka I-JSON). No IETF standards based on JSON does (AFAIK...) go outside of this limit. Numbers outside of float64 must thus be put in quotes. |
Thank you for the reply and the clarification. Would it make sense to check whether precision was lost in translation and throw an error in that case? Also note that there exists an unofficial rust implementation, serde_jcs, which des not seem to conform with the standard as it encodes |
I don't know how to do that for parsing unless you write a parser from scratch. One idea behind JCS was to not require such measures. There are also a bunch of edge cases that would have to be taken in consideration if strict compatibility is to be preserved: Precision issues and data rewrites are things that a JCS user must deal with. I'm not particularly fluent in Go but maybe there are ways to control serialization that makes dealing with int64 simple? In my Java implementation I defined an int53 type which flags values that do not fit. I wasn't aware of serde_jcs, maybe I should give them a call? 😁 |
Could you point me to that
Hehe, you don't have to. But I created an issue (l1h3r/serde_jcs#3) at their repo in any case. |
I was about to open an issue related to this, but with this comment, it sounds like I shouldn't. The issue I see is:
Which IMO should throw an error. Expecting the user to provide a wrapper around JCS to do a inp == out check seems less than ideal. JCS is for use in secure contexts and the defaults should be secure. Allowing the input to be slightly different, but still verify correctly is dangerous. Often programmers will not realize that they should reparse/use the output from the canonicalized form that matches the signature, and not doing so could result in miss behavior as the used data may not match the signed data. IMO, canonicalize should default to secure behavior, and via optional arguments allow for more generous serializations. It doesn't look too hard to ensure that convert2Es6Format output is equal to it's input, and raise an error otherwise. This isn't truly ideal, as it allows some integers that are "out of range" to pass through, but would help ensure that expected behavior happens. |
Hi Guys, |
For int53 overflows it would help to use a more strict JSON encoder. For example with https://github.com/ijl/orjson you get: import orjson
orjson.dumps(2**53, option=orjson.OPT_STRICT_INTEGER)
Traceback (most recent call last):
File "...example.py", line 3, in <module>
orjson.dumps(2**53, option=orjson.OPT_STRICT_INTEGER)
TypeError: Integer exceeds 53-bit range Not sure about handling floats. |
I will do a short test, today or tomorrow. |
Just had a look at https://datatracker.ietf.org/doc/html/rfc8785#appendix-B |
@titusz I have taken a new look at the problem and my conclusion is (how strange it may sound), that it indeed works as it should. The reason is simply that there are no integers in JSON. As long as the (potentially rounded) value fits in a 64-bit double all is good. This is of course not perfect but this is what you get with RFC 8785 (and JavaScript). Numbers that needs exact representation and do not fit in a 64-bit double, must use String-based encoding schemes. If both sides do not adhere to the same rules, signatures will not validate which is more of a nuisance than a security problem. 2**600 worked as expected since it throwed an exception. In case you build an entire JSON tool-chain from scratch you can deal with this problem in a better way: |
The python implementation does not seem to throw an exception for 2**600
I think generally it would be good practice to throw an exception if a python int/float is NOT serialized into a rfc8785 conformant representation or cannot be deserialized into the same value provided for canonicalization (full round-trip support) What do you think? |
Pardon me, it should have been 2**6000 which doesn't fit in a 64-bit double. What you mention is a possibility but it is not compliant with the the RFC: https://datatracker.ietf.org/doc/html/rfc8785#section-3.2.2 The Python canonicalizer is not state of the art because that requires a combined parser and canonicalizer like my Java-based system which (optionally) does what you suggest: However, for a community solution it is hard to justify the costs given the limited benefits. A JSON -based "schema" must anyway deal with ugly things like date objects. Adding an integer specific test would though be very simple. Should I add this? FWIW, I'm currently occupied with a CBOR implementation that has no such issues since CBOR doesn't mix floats, ints and big ints, as well as offering deterministic serialization eliminating all problems of the kind we discuss here: |
Ok, I see it is tricky. Yes an integer specific test would be welcome. Off-Topic: I was also looking into CBOR lately and I liked what I saw. The specification explicitly includes a section about requirements for deterministic encoding... :) |
@titusz Done! Thanx, it was a good idea and best of all, it was simple :) |
Thanks for the quick response! I'll look at adding test cases to make sure that this is working. |
@jmgurney It surely worked but it broke the RFC so I had to revoke it :( The problem is in JSON or more specifically in the I-JSON subset that JavaScript natively supports. |
Can you explain how/why it broke the RFC? My reading of https://datatracker.ietf.org/doc/html/rfc7493#section-2.2 doesn't say that raising an exception for out of range values is not allowed. JSON signing is exactly:
and raising an exception to note that this is happening, allowing the author to know to use a string encoding is a good thing IMO. |
@jmgurney Indeed. For RGC 8785 flagging out-of-range Numbers would be quite useful. |
json-canonicalization/go/src/webpki.org/jsoncanonicalizer/jsoncanonicalizer.go
Line 237 in dc406ce
This method does not work for uint64 because float64 precision is insufficient.
Suggested fix, insert before line 236:
The text was updated successfully, but these errors were encountered: