Re: [tsvwg] I-D Action: draft-ietf-tsvwg-rlc-fec-scheme-10.txt

[Benjamin Kaduk <[e-mail]>]

Sorry for the slow reply; these generally look good but there were a couple
of places where you asked for feedback.  (I'll trim the other parts for
ease of reading.)

On Fri, Jan 18, 2019 at 08:34:11AM +0100, Vincent Roca wrote:
> Dear IESG members, all,
> 
> PART II:  ANSWERS TO DETAILED COMMENTS
> 
> 
> > =====================================================================
> > Benjamin Kaduk Discuss 
> > Discuss (2018-10-10) 
> > 
> 
> > The security considerations need to be checked about whether only DoS or
> > data corruption is possible in a few situations.
> 
> [VR] Those are the two risks that come to my mind.

Thank you for double-checking.

> 
> > (More details on almost all of these in the COMMENT section.)
> > 
> > =====================================================================
> > Comment (2018-10-10) 
> > 
> >                                                                For
> >       instance, at session start, upon receiving new source ADUs, the
> >       ew_size progressively increases until it reaches its maximum
> >       value, ew_max_size.  [...]
> > 
> > This seems to preclude the window shrinking due to ADUs aging out.  Under
> > the constant bitrate assumption this is justified, but a somewhat
> > artificial limitation.
> 
> [VR] Yes, I agree, but as explained above we give non normative hints that
> could also be totally absent from the doc.

I don't really read this as non-normative, though.  Even though it starts
with "For instance," that could just mean that we are considering one
way to think about the behavior, but the rest of the text is written as if
it is a statement of fact, unqualified to a specific example trace.  I
think it would be better to say something like "For example, in the common
case at session start, [...]"

> 
> > Section 3.4
> > 
> >                                                      In order to
> >    validate an implementation of this PRNG, using seed 1, the 10,000th
> >    value returned by: tinymt32_rand(s, 0xffff) MUST be equal to 0x7c37.
> > 
> > "seed 1" sounds like an index into an enumerated list; it's probably better
> > to say "a seed value of 0x00000001".
> 
> [VR] changed for: "using a seed value of 1,..." (I don't like using hexa notation
> if useless). Several occurences changed.

This is definitely subject to your judgement; I'll just note that I
suggested the full hex to reiterate to the reader that this is a 32-bit
field.

> > It looks like this tinymt_init() implementation probably generates and
> > discards enough output to get past the initial highly correlated stage,
> > though I'd appreciate if that was explictily confirmed.
> 
> [VR] You're right and that's something essential.
> 1st part of the answer: the core prng itself seems good if we believe the
> authors and related publications. 
> 2nd part of the answer: we added several tests to check its adequacy,
> including the mapping approach to a smaller range, in Appendix B. It's
> made by non specialists (us), but tailored to our needs.

Thank you for reassuring me on this point.

> 
> > Section 8.4
> > 
> > I found where RFC 6363 Section 9 claims that the following subsections will
> > define a mandatory-to-implement security scheme, but not where IPsec/ESP
> > was actually specified as such.  Can you give me a pointer?
> 
> [VR] RFC 6363, section 9.5. "Baseline Secure FEC Framework Operation", I quote,
> "describes a baseline mode of secure FEC Framework operation based on the
> application of the IPsec protocol".
> It does not refer to IPsec/ESP explicitly, in this sentence, but this is 
> largely mentionned before and after. The RFC 6363 "Security Considearations"
> text could probably be improved (I'm also responsible of it).
> 
> The section 8.4 "clarifies" this in a sense but may go beyond what is said
> in RFC 6363. Is that acceptable?

Thanks for the pointer.  I guess, the Section 8.4 here is just reiterating
the intent of RFC 6363, so this document is fine as-is.  It might be worth
filing an errata report against 6363 to clarify that the "baseline mode" is
the mandatory-to-implement-but-not-mandatory-to-use functionality indicated
by the Section 9 (of that document) introductory paragraph.

> > =====================================================================
> > Eric Rescorla Discuss 
> > Discuss (2018-10-09) 
> > 
> > Rich version of this review at:
> > https://mozphab-ietf.devsvcdev.mozaws.net/D3423
> > 
> > 
> > S 12.2.
> > >        s->status[0] = s->status[1];
> > >        s->status[1] = s->status[2];
> > >        s->status[2] = x ^ (y << TINYMT32_SH1);
> > >        s->status[3] = y;
> > >        s->status[1] ^= -((int32_t)(y & 1)) & s->mat1;
> > >        s->status[2] ^= -((int32_t)(y & 1)) & s->mat2;
> > 
> > You also can't assume that negative numbers have any particular
> > representation (e.g., twos complement) so this XOR is not
> > deterministic.
> 
> [VR] Here on the opposite this part of the core PRNG and we cannot change
> nor remove anything. 
> We didn't notice determinism problems on our target test platforms, and
> provide validation tables. If there's an issue in one particular environment,
> it should be quickly identified and a work-around will have to be found then.
> I'm sorry but here there's little we can do IMHO.

C99 is quite clear that both twos-complement, ones-complement, and
sign+magnitude are permitted representations for signed integers (see,
e.g., Section 6.2.6.2 paragraph 2 in n1256.pdf, the last public draft
version before the final spec, which is paywall'd).  Alternately, consult
n1570.pdf (same section/paragraph), the draft C11 spec with same caveat.

Wikipedia (https://en.wikipedia.org/wiki/Ones%27_complement) has a small
list of ones-complement architectures, including the PDP-1 and certain
UNIVAC series machines.  Just because the authors of this document have not
encountered any such machines, it does not mean they do not exist; it still
seems needlessly restrictive to limit the applicability artificially like
this.

-Benjamin