Subject: [tsvwg] Benjamin Kaduk's Discuss on draft-ietf-tsvwg-rlc-fec-scheme-09: (with DISCUSS and COMMENT)

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 10 Oct 2018 21:28:43 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-tsvwg-rlc-fec-scheme-09: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tsvwg-rlc-fec-scheme/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thank you for making updates in response to the secdir review; they do
help.

That said, while I think this is a useful technology to specify and a clear
improvement over the existing block-based schemes, this document still has
several flaws that need to be fixed before it is ready for publication.

Most notably, it is internally inconsistent in several places, about
whether bitrate must be fixed or can vary, or whether the encoding/decoding
window size ratio is 3/4, or whether the encoding window size only changes
during startup as new ADUs are added to reach the max encoding window size,
etc.  I attempt to note these in my COMMENTs (noting that they were written
as I go through the document and I may not have caught all the places where
text later in the document clarifies the situation).

It's also implied but not entirely clear that the C code is part of the
normative algorithm specification, and the C code itself incurs
implementation-defined behavior (so as to not be usable as a deterministic
protocol standard element), as Ekr notes.

The recommendation to bound ls_max_size in section 3.1.1 does not seem
strong enough to allow this scheme to meet its stated constraints for
maximum latency, since solving very large linear systems can exceed the
provisioned timeslot.

The SDP encoding for the fssi parmeter should explicitly state that the
name value "E" is used to convey the E parameter.

Multi-byte integer protocol fields need to specify the endianness of
encoding (i.e., network byte order).  (E.g., for ESI and E values.)

The security considerations need to be checked about whether only DoS or
data corruption is possible in a few situations.

(More details on almost all of these in the COMMENT section.)

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Section 1.1

   With FECFRAME, there is a single FEC encoding point (either a end-
   host/server (source) or a middlebox) and a single FEC decoding point
   (either a end-host (receiver) or middlebox).  [...]

"single decoding point" seems inconsistent with "multicast/broadcast"

   size, the higher the robustness (e.g., in front of long packet
   erasure bursts), but also the higher the maximum decoding latency

Is this supposed to be "in the face of" instead of "in front of"?

Section 1.2

   This document introduces two fully-specified FEC Schemes that follow
   a totally different approach: [...]

The first time I read this I parsed it as "totally different from each
other", as opposed to "totally different from the existing block codes".

Section 2

It seems like a potentially artificial limitation to assume constant
input/output bitrate.

Section 3.1

   Maximum FEC-related latency budget, max_lat (in seconds) with real-
   time flows:

A reader might assume that this is limited to an integral number of
seconds, which seems like an artifical limitation.  Omitting the
parenthetical might actually be an improvement, as whatever mechanism is
used to specify the limit would necessarily include units.

                                                               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.

   Symbol size, E (in bytes):  the E parameter determines the source and
      repair symbol sizes (necessarily equal).  This is an input
      parameter that enables a FECFRAME sender to derive other internal
      parameters, as explained below.  An implementation at a sender
      SHOULD fix the E parameter and communicate it as part of the FEC
      Scheme-Specific Information (Section 4.1.1.2).

Under what condition would a sender not fix the E parameter?  (That is, why
is this a SHOULD and not a MUST?  The communication in FEC Scheme-Specific
Information is clearly a SHOULD, though.)

                              The source ADU flow may be a Constant
   Bitrate (CBR) or Variable BitRate (VBR) flow.  The flow's minimum/
   maximum bitrate might or might not be known.  The FEC Schemes can

This seems to contradict the previous assumption of constant bitrate,
absent grossly inefficient padding schemes.

Section 3.1.1

                                                  For the FEC Schemes
   specified in this document, in line with [Roca17], the ew_max_size
   SHOULD be computed with:

      ew_max_size = dw_max_size * 0.75

This is just self-citing to a document that itself makes an arbitrary
choice of parameter.  It would seem that the reference should be limited to
something like "being able to reuse analysis from [Roca17]" rather than
attempting to use it as a definitive explanation for this parameter
selection.

   The ew_max_size is the main parameter at a FECFRAME sender.  It is
   RECOMMENDED to check that the ew_max_size value stays within
   reasonnable bounds in order to avoid hazardous behaviours.

Can you provide concrete values or guidelines for "reasonable bounds", or
describe the spectrum of possible hazardous behaviors?

                                                 A receiver can then
   easily compute dw_max_size:

      dw_max_size = max_NSS_observed / 0.75

This formula only holds when the "SHOULD" above is adhered to.
It may be more appropriate to just fix this parameter at 0.75 as a property
of this FEC Scheme and not include text admitting the possibility for other
values.  (Other values could be specified later with different FEC encoding
IDs.)

                 It is RECOMMENDED to check that the ls_max_size value
   stays within reasonnable bounds in order to avoid hazardous
   behaviours.

It's unclear that this can only be RECOMMENDED, as the time needed to solve
the system is included in the max_lat latency budget (and the encoder does
not necessarily know the details of the decoder system!).

Section 3.1.2

   Beyond these general guidelines, the details of how to manage these
   situations at a FECFRAME sender and receiver can depend on additional
   considerations that are out of scope of this document.

It would be nice to give some indication of what sorts of things might go
into these additional considerations, like the nature of loss along the
path, the computational capabilities of the receiver(s), if the input
bitrate has predictable structure, etc.

Section 3.2

   At a sender, an ADU coming from the application cannot directly be
   mapped to source symbols.  [...]

It might be better to say "is not" than "cannot", as the latter leaves the
reader searching for an explanation which is not yet available, and to some
extent this is just a design choice made in the protocol specification (a
consequence of allowing multiple flows to use the same FEC context).

   ADU can be assigned to the right source flow (note that transport
   port numbers and IP addresses cannot be used to that purpose as they
   are not recovered during FEC decoding).

Please note that the repair packets are going over a different 5-tuple than
the source packets, to help the reader make the connection about the lack
of address/port information.

Section 3.3

This would be a fine place to mention how symbols are numbered/indexed
and forward-reference to Section 4.1.4

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".

   This PRNG MUST first be initialized with a 32-bit unsigned integer,
   used as a seed.  The following function is used to this purpose:

      void tinymt32_init (tinymt32_t * s, uint32_t seed);

This seems to imply that the C code is going to be a normative part of the
specification.  Is that intended?

Section 3.5

                                          When DT is between 0 (minimum
   value) and strictly inferior to 15, the average probability of having
   a non zero coefficient equals (DT +1) / 16.

nit: "between 0 (the minimum value) and 14"
nit: "DT + 1"

   With the RLC over GF(2) FEC Scheme (Section 5), m MUST be equal to 1.
   With RLC over GF(2^^8) FEC Scheme (Section 4), m MUST be equal to 8.

I think "is equal to" is more appropriate than "MUST be equal to", here --
it's just a statement of fact.

    * (in) repair_key    key associated to this repair symbol. This
    *                    parameter is ignored (useless) if m=2 and dt=15

'm' of 2 is not defined in this document.  I assume this is supposed to be
"the GF(2) case", i.e., m == 1.

    * (out)              returns an error code

What value indicates success?

The tinymt usage here is "fragile" in a sense that I would not like if I
was performing a code review on this code, namely which successive
invocation gets used for which output coefficient depends on previous
tinymt output values.  That said, the scheme is deterministic (modulo Ekr's
Discuss points), which is the only feature needed for interoperability.

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.

Section 4.1.1.2

   When SDP is used to communicate the FFCI, this FEC Scheme-specific
   information is carried in the 'fssi' parameter in textual
   representation as specified in [RFC6364].  For instance:

   fssi=E:1400

I think you should explicitly say that this uses the "name" of "E" for the
SDP encoding.

   If another mechanism requires the FSSI to be carried as an opaque
   octet string (for instance, after a Base64 encoding), the encoding

Wouldn't this be *prior to* Base64 encoding, not after?

      Encoding symbol length (E): 16-bit field.

Generally we still say "encoded in network byte order (big-endian)" for
16-bit integer fields.

Section 4.1.3

How does an encoder select repair_key values to put in repair payload IDs?
(I guess Section 6.1 kind-of covers this but does not explicitly say that
it's an arbitrary choice.)

Section 5.1.3

Leaving the encoder the option of setting an arbitrary repair_key for
DT==15 leaves open the possibility of a covert channel and the
corresponding privacy considerations.  Usually we just say "MUST set to
zero on transmission and ignore on receipt" for this sort of thing.

Section 8

   The FEC Framework document [RFC6363] provides a comprehensive
   analysis of security considerations applicable to FEC Schemes.

I would suggest going with "fairly comprehensive" or similarly couched
language; as my review of draft-ietf-tsvwg-fecframe-ext indicates, it is
not a fully comprehensive analysis.

Section 8.2

   o  FEC Encoding ID: changing this parameter leads a receiver to
      consider a different FEC Scheme.  The consequences are severe, the
      format of the Explicit Source FEC Payload ID and Repair FEC
      Payload ID of received packets will probably differ, leading to
      various malfunctions.  Even if the original and modified FEC
      Schemes share the same format, FEC decoding will either fail or
      lead to corrupted decoded symbols.  This will happen if an
      attacker turns value YYYY (i.e., RLC over GF(2)) to value XXXX
      (RLC over GF(2^^8)), an additional consequence being a higher
      processing overhead at the receiver.  In any case, the attack
      results in a form of Denial of Service (DoS);

I think this could lead to undetected data corruption, which is worse than
DoS.  Similarly for the next bullet 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?

Section 8.5

As with all random matrix algorithms, there is some chance that the
indicated scheme will produce poorly conditioned matrices (i.e., less than
full rank) for the decoder to process.  The coefficient generation scheme
seems to be deterministic and dependent only on the repair_key and DT
values for a given encoding window, so in principle an attacker could
precompute which inputs would produce singular linear systems under given
packet loss conditions.  That said, an attacker able to modify the
transmitted repair_key, DT, and NSS values can affect the system in more
disruptive ways already, so this sort of attack probably doesn't need to be
mentioned here.

Section 9

[same comment about "comprehensive" as for section 8]

Section 9.2

   Note that using a density threshold equal to 15 with RLC over GF(2)
   is equivalent to using an XOR code that compute the XOR sum of all
   the source symbols in the encoding window.  In that case: (1) a
   single repair symbol can be produced for any encoding window, and (2)

I think this may be more clear as "only a single repair symbol can be
produced".  (Also another occurrence earlier in the document?)

Appendix A

(As Ekr notes, the floating-point mode is not specified, so there are
several multiplications/divisions that are not deterministic.)

Appendix B

   Finally, it is worth noting that a good receiver, i.e., a receiver
   that benefits from an FEC protection significantly higher than what
   is required to recover from packet losses, can choose to reduce the
   ls_max_size.  In that case lost ADUs will be recovered without
   relying on this optimization.

nit: I'd use a different phrase than "good receiver", as the relevant
property is of the network path to the receiver and not its (implementation
or other) quality.  Perhaps, "a receiver that does not experience much
packet loss".