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

JFV + ECMA-262 #225

Closed
igrigorik opened this issue Aug 10, 2016 · 21 comments
Closed

JFV + ECMA-262 #225

igrigorik opened this issue Aug 10, 2016 · 21 comments
Labels

Comments

@igrigorik
Copy link
Member

Based on the various discussion at IETF, HTTP Workshop, and mailing lists, JFV may not be the right long-term "replacement" header encoding format. However, while sketches are being pushed around, it's not clear if and when such a format will come either… In the meantime, I believe we can shape JFV (with minor modifications) into a format that is immediately useful and addresses many of the current pain points for new header fields. Specifically:

  • UA's want a reusable and tested parser. Today we end up hand-rolling parsers for most every field (due to ABNF quirks, past implementation mistakes, etc), which leads to interop problems, security issues, and so on. Also, this is not a Chrome specific problem, all major UA's have the same pain points, and we all need to do better.
  • At the same time, all UA's have well-tested JSON parsers. Reusing existing code helps cut down on attack surface area, helps with interop, etc. As such, JFV format is a strong plus.
  • JSON parsers are easy to come by on the server side and all the same benefits and motivation is just as applicable.

That said, as others have already pointed out, we do need additional restrictions on JFV beyond what is currently in the spec. Also, it just so happens that browsers already follow the ECMA-262 spec, which addresses most of the raised concerns:

Are there other issues that were brought up that are not addressed above?

My proposal is that we update JFV to spec above ECMA-262 restrictions and requirements. Doing so would enable UA's to adopt JFV with minimal work.

p.s. in progress specs and browser features that plan on leveraging JFV: Clear-Site-Data, Feature Policy, Reporting, Network Error Logging, … and I'm sure we'll see more soon.

/cc @reschke @mnot @mcmanus

@igrigorik igrigorik added the jfv label Aug 10, 2016
@mnot
Copy link
Member

mnot commented Aug 11, 2016

Hmm. The number issues are around parsing, not generation, AIUI. So, your solution is good for response headers, but not request headers -- because the browsers haven't yet taken over the server side implementations.

@clelland
Copy link
Contributor

The pointer to 11.8.3 was intended to tackle parsing; 11.8.3.1 defines the parsing algorithm for numeric literals (precisely, down to potentially rounding in the 20th significant digit). The examples where actually significant digits get ignored would indicate a bug in parsing, according to the ES spec.

@mnot
Copy link
Member

mnot commented Aug 11, 2016

Right. The pushback against doing something like this that I've heard is that no matter what we say in the spec, people are going to just point the JSON parser they have at hand at the header field value, and trust what it gives them.

As such, using JSON is a bit of an attractive nuisance.

It may be that we can specify a profile of JSON that is safe to use, and create a set of easy-to-use libraries and get them into the relevant server-side platforms, but that's a substantial amount of effort (any volunteers?), and the natural lag of many distros/platforms means that they might not be available for some time (leading people to use plain JSON parsers).

The most serious issue is undoubtedly the number format. Perhaps cautioning against minting headers that use non-integer numbers would be a workable stopgap (although the temptation might be quite strong in some use cases).

@igrigorik
Copy link
Member Author

Well, the alternatives are: hand-rolled ABNF parsers with buggy substring matches (because that seems to be preferred "optimized" way of doing things); or completely new parses for each platform for some yet-to-be-defined format... shrug, pick your poison.

UA's can enforce the JSON subset ("strict mode") we define here, which will nudge all the other parties in the right direction.

@mnot
Copy link
Member

mnot commented Aug 11, 2016

I do agree that not inventing something new is pretty attractive. How you feel about disallowing (or just strongly warning) against non-integer numbers in request headers?

@igrigorik
Copy link
Member Author

Strongly warning seems reasonable. I'm not aware of any existing efforts that are planning to use non-integer values + JFV, but making that a requirement doesn't feel right either.

@rousskov
Copy link

A JSON profile, strict modes, wrapper API suggestions, and/or strong warnings simply document attack vectors without solving the underlying problem. If JSON is the answer, then it comes with a "fuzzy numbers" problem that does not have reliable solutions. You can sugar-coat it or hope that it will go away in 20 years, but you cannot solve it.

@kazuho
Copy link
Contributor

kazuho commented Aug 12, 2016

@igrigorik @mnot
If we are to going to use a modified variant of JSON, I strongly support disallowing non-integer numbers.

Using ECMA262's definition of number would not help us, neither in terms of the definition or the ease of implementation.

Regarding the definition, my understanding is that ECMA262 merely specifies how the numbers should look like as strings, but does not specify how the strings should be converted to IEEE754 numbers. Lack of a formal definition of such procedure is what leads us to disagreements between the implementation (that in turn becomes a cradle of vulnerabilities).

Regarding the ease of implementation, ECMA262 is a superset of JSON in terms of how numbers can be represented (i.e. binary, octal and hexadecimal representations), which seems unnecessary complex to me.

@mnot

It may be that we can specify a profile of JSON that is safe to use, and create a set of easy-to-use libraries and get them into the relevant server-side platforms, but that's a substantial amount of effort (any volunteers?)

It might be possible to filter out JSON containing floating point representations before applying a JSON parser, and in that regard, using json2.js (that uses eval to parse JSON after filtering out dangerous strings) as a basis might be worth considering. I anticipate that just modifying the number representation in this regexp would be sufficient for the matter.

@igrigorik
Copy link
Member Author

Parallel thread: https://lists.w3.org/Archives/Public/ietf-http-wg/2016JulSep/thread.html#msg436

@rousskov I appreciate where you're coming from, but (personally, at least) I think that paints an overly black and white prognosis. I think we could make reasonable progress here with a restricted set + browsers enforcement.

@kazuho @mnot I can't shake the feeling that restricting to integer values will come back to bite us, but if you think otherwise.. shrug, we can give it a shot.

@clelland
Copy link
Contributor

Presumably, we'll also have to restrict the range of integers to Number.MIN_SAFE_INTEGER <= n <= Number.MAX_SAFE_INTEGER, to be sure that the value parsed is the same as the number presented in the JSON.

@rousskov
Copy link

@igrigorik I agree -- reasonable progress is very possible, for some definition of reasonable, especially if your primary focus is "browsers". That reasonable progress will not solve the problem -- fuzzy JSON numbers will cause CVEs. If reasonable progress [with browsers] is good enough, and a few design-triggered CVEs are not important enough, then this JSON problem is not a big deal at all.

@reschke
Copy link
Contributor

reschke commented Oct 7, 2016

FWIW, another plausible thing to do would be to require conformance to the I-JSON profile (see https://tools.ietf.org/html/rfc7493).

@igrigorik -- is that something you would consider implementing (essentially adding a stricter mode to the existing JSON parser?)

@igrigorik
Copy link
Member Author

FWIW, another plausible thing to do would be to require conformance to the I-JSON profile (see https://tools.ietf.org/html/rfc7493).

@reschke that's effectively what we proposed above.. ECMA-262 and I-JSON dictate same rules for number encoding?


@clelland @esprehn fyi... phk put together a rough draft of his (alternative) proposal:

@reschke
Copy link
Contributor

reschke commented Oct 12, 2016

@igrigorik it's different with respect to duplicate keys...

@igrigorik
Copy link
Member Author

@reschke true, but what's the benefit of I-JSON over the existing ECMA-262 behavior in browsers?

@reschke
Copy link
Contributor

reschke commented Oct 13, 2016

easier to specify from an IETF spec :-).

At the end of the day, we'll forbid sending duplicate keys anyway, right? The difference is just how the recipient handles those (complain vs last wins)

@igrigorik
Copy link
Member Author

At the end of the day, we'll forbid sending duplicate keys anyway, right? The difference is just how the recipient handles those (complain vs last wins)

Right. I got internal pushback earlier on special-casing this in our JSON parser (to ignore+complain).. last-wins is simple and consistent with the rest of the (browser) platform.

@esprehn
Copy link

esprehn commented Oct 17, 2016

Yeah we don't want to have two versions of the JSON parser. So whatever this spec says should agree with 262.

@reschke
Copy link
Contributor

reschke commented Oct 20, 2016

@igrigorik - would something like 9928ebd work for you?

@igrigorik
Copy link
Member Author

@reschke yes, I think that looks reasonable.

Furthermore, ordering of object members is not significant and can not be relied upon.

Paging @clelland @esprehn for sanity check..

@mnot
Copy link
Member

mnot commented Dec 25, 2016

Closing, since the WG has abandoned JFV in favour of PHK's proposal.

@mnot mnot closed this as completed Dec 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

7 participants