Re: [secdir] secdir review of draft-ietf-json-text-sequence-11

[Nico Williams <[e-mail]>]

On Tue, Dec 16, 2014 at 07:51:32AM -0500, Carl Wallace wrote:
> On 12/15/14, 7:01 PM, "Nico Williams" <[e-mail]> wrote:
> ><snip>
> >> This document describes a means of encoding and parsing arbitrarily
> >>large
> >> sequences of JSON texts. The document claims no new security
> >> considerations beyond those in RFC 7159.  I have some concern that the
> >
> >Though it turns out that RFC7159's security considerations section was
> >probably too brief.  Anyways.
> 
> Seems a good place to augment since you have noticed shortcomings.

Indeed.

> >> [...]
> >That's a good point.  Signing individual JSON text sequence elements is
> >outside the scope of this document.  Either a complete sequence is
> >signed, or individual texts are signed with a wrapper.  I wouldn't
> >recommend transferring a sequence of texts and a parallel sequence of
> >signatures, say.
> 
> It’s probably worth stating up front the process of encoding a JSON text
> sequence alters the JSON text sequence elements and that signing
> individual elements is outside the scope.

OK, that will be section 3 (security considerations text), something
like:

   Parsing and re-encoding a JSON text sequence need not produce the
   same sequence of octets.  Do not rely on being able to reproduce the
   same inputs to a cryptographic integrity protection function.

> >> - Section 2.3 should clarify that malformed JSON text sequences are also
> >> not fatal (I.e., arriving at an RS without having seen a LF).
> >
> >That's covered in detail in section 2.4.  I wouldn't want to replicate
> >any part of 2.4 in 2.3.
> 
> I don’t recall seeing any mention of malformed JSON text sequences. I
> think you either get to a next <RS> or end of the stream.

Section 2.3 actually says "malformed" in its first sentence.  It
mentions truncation only as an example of why a JSON text might be
malformed, in the second sentent.

There is also mention of malformed JSON texts in section 2.4 (explaining
why 'true' must not be produce upon merely consuming the 'e' in it, as
the next octet might not be a ws, nor RS).

Here's the proposed change:

OLD:
2.3.  Incomplete JSON texts are not fatal

NEW:
2.3.  Incomplete or malformed JSON texts are not fatal

(and s/textm/text,/)


> >> - The second paragraph of Section 2.3 suggests that an incremental JSON
> >> parser may be used across content from multiple sequence elements.  The
> >> ABNF for encoders does not seem to support this.
> >
> >Are you thinking of a a sequence like <RS> <JSON-text-w/o-trailing-LF>
> ><JSON-text> ... being parsed as two JSON texts?
> 
> I was thinking more like <RS> <some fraction of JSON possibly a single
> character>.

The ABNF for encoding assumes proper operation, yes.  The parser ABNF
takes care of malformed JSON texts.  This may be a simplification, but I
think it's a useful one.

> >Or are you thinking of a <RS> <JSON-text-containing-RS> <LF>?
> >
> >The latter must yield a parse error from the JSON text parser.
> >
> >As to the former... I think I'd thought about it and left it as-is,
> >though I can't recall why, with the security considerations text about
> >smuggling being sufficient (except it's not quite).
> >
> >How about this new text:
> >
> >   After reading and parsing a JSON text, the sequence parser MUST skip
> >   to the RS byte following the JSON text (or terminate if at end of
> >   input).
> 
> I think this text would be a nice addition. I noticed the lack of calling
> out "read until end if no <RS>" but it seemed implied.

OK.

> [extensive discussion of the LF elided]
>
> OK, though as noted above I still don’t see the need for adding the <LF>
> in the encoder without removing it in the corresponding parser.

There is no need to remove it in the sequence parser, though the
sequence parser may do it.  The sequence parser need only reject
top-level number/true/false/null values whose text did not end in a
whitespace.  A sequence parser could do this by insisting that the
sequence element end in an LF, and it can remove the trailing LF as well
as leaving it in, as the trailing LF's presence (or absence) does not
affect the validity of the JSON text to be parsed.

Adding the LF in the sequence encoder does not hurt, since all JSON
texts can end with arbitrary amounts of ws.  It merely helps delimit
sequence elements, both to ensure that top-level numbers/true/false/null
are delimited, and to help keep lines shorter for users using $PAGER and
$EDITOR to view JSON text sequences.

Delimiting otherwise non-self- delimiting texts is an important
function.

Nico
--