Last Call Review of draft-ietf-tsvwg-rlc-fec-scheme-07
review-ietf-tsvwg-rlc-fec-scheme-07-secdir-lc-dekok-2018-09-20-00

Request Review of draft-ietf-tsvwg-rlc-fec-scheme
Requested rev. no specific revision (document currently at 16)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2018-09-24
Requested 2018-09-10
Draft last updated 2018-09-20
Completed reviews Secdir Last Call review of -07 by Alan DeKok (diff)
Genart Last Call review of -07 by Russ Housley (diff)
Assignment Reviewer Alan DeKok
State Completed
Review review-ietf-tsvwg-rlc-fec-scheme-07-secdir-lc-dekok-2018-09-20
Reviewed rev. 07 (document currently at 16)
Review result Has Nits
Review completed: 2018-09-20

Review
review-ietf-tsvwg-rlc-fec-scheme-07-secdir-lc-dekok-2018-09-20

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments.

The summary of the review is read with nits

  The document seems to clearly describe what it's doing and why. 

  One nit I have is the formulae in the document, e.g.:

   dw_max_size = (max_lat * br_out * cr) / (8 * E)

  My concern here is about numerical overflow, which could lead to improper calculations and security problems.

  Experience has shown that naive implementations will just do the calculations no matter what the inputs.  Garbage inputs then lead to garbage outputs, and we have problems.

  It would be good to:

* state the limits on each of the variables, so that implementations can know that values *outside* of these limits are erroneous, and should be ignored

* state the minimum size of each of the above variables.  e.g. "dw_max_size" is likely not going to be an 8-bit integer.  So what data types are sufficient to perform the calculations correctly?

  This information may be in another document, but it wasn't clear on cursory inspection.

3.1.2:

   Another approach consists in using ADU timing information (e.g.,
   using the timestamp field of an RTP packet header

  What happens if the timestamp is outside of reasonable bounds?  There could be a recommendation that only timestamps within a certain window are used.

  e.g. an adversary sends timestamps which are unreasonable, and a naive implementation could use those, and engage in integer overflow, etc.

  Section 8 describes protections against many adversarial attacks, which is good.

  I would suggest adding a sub-section of Section 8, discussion computation / numerical issues, as discussed above.  

  Other than that, the document looks fine.