Re: [tsvwg] feedback request: draft-thornburgh-adobe-rtmfp

[Michael Thornburgh <[e-mail]>]

hi Richard. thanks for your feedback.  replies inline.

> From: Scheffenegger, Richard [mailto:[e-mail]]
> Sent: Thursday, April 18, 2013 7:15 AM
> 
> 
> Hi Michael,
> 
> I'm currently reviewing the draft-06 in more details. The following is a list of nits and comments
> while doing a first close pass.
> 
> 
> A few high level remarks first:
> 
> As RTMFP seems to use many of the same concepts existing in SCTP, an expert in that field would
> probably also be an ideal reviewer.

agreed.  i have discussed RTMFP and this draft with at least one SCTP expert.

> Security considerations: There are a couple of fields, that seem to be mostly at the same relative
> offset in a packet, and are initialized to identical values. A thorough security analysis would
> probably need to look if the chosen encryption becomes (significantly) weaker due to this aspect
> (chosen plaintext attack, even though only partial).

i don't believe this is an issue, particularly when RTMFP is used with the recommended cipher mode (CBC), and where CBC is done properly.  can you give some specific examples where you believe this might be an issue?  designing/choosing a cipher suite to be resistant to various cryptanalysis techniques is a general cryptography problem and is beyond the scope of this specification.
 
> Swapping sections 2 and 3 around would probably make it easier to read into the mechanisms of the
> protocol; the variable length encoded wire protocol can be defined, once the architecture of the
> protocol is described.

i will consider this; however, i prefer the current organization both for reference purposes and for bottom-up explanation (which is my personal style of explanation).  since i composed this specification with this order in mind, i believe swapping the syntax and operation sections could require significant editorial reworking beyond simply reordering the sections.

> I noted that the expected initialization for reserved bits is sometimes left out; and there are no
> explicit default behaviors defined for unexpected values (running out of data for variable length
> integers, missing "necessary" fields in SACK, ...

thanks. i'll add a note about treatment of reserved fields and parse errors.

> One other observation: Some graphs use +~+~ vs. +-+- or +~~~ vs. +---, if this is of importance, the
> difference might be declared (like the variable length fields with /\/).

the "~~~" vs "---" differentiates fields that might not be present (for example, the type and value of an option might not be present depending on the length).  since the C-like procedural descriptions are definitive, and since the present-or-not disposition of a field usually can't be determined only from the diagram, it's not critical to understanding if that aspect of the diagrams isn't inferred from context.  it is a minor and non-critical decoration.

> 2.1.3 uppercase MAY for the marker

that "may" does not indicate a normative requirement level according to the intent of RFC 2119.  lower-case "may" (or "can") is appropriate here as it describes a possible scenario, not a "truly optional" item or aspect.  i'll change it to "can".

> 2.1.5 a short statement on reserved bits (SHOULD be zero, MUST be ignored/session terminated...)?

ok.

> 2.2.2 the graph is slightly confusing me (I guess the two lines of +-+-+-+ btwn SSID and the union set
> me off).

that diagram style is consistent throughout the document.  in the case of this specific diagram, removing one of the "+-+-+" lines may be clearer while not diverging too far from the general style.  in general, not running the boxes together was the best way i found to illustrate the variable-length fields (particularly VLUs).

> If the encrypted packet is of length zero, does that mean that scrambled session ID is in the clear
> (well, unscrambled)? But is a packet of length zero valid for any reason?

you can't have a 0-length encrypted packet because a plain packet is always at least one byte long.  but yes, if the encrypted packet was somehow empty, then SSID == SID XOR 0 XOR 0 == SID.

> 2.2.4 again, reserved bits; the first regular paragraphs after padding talks about mode - that could
> be moved upwards to the discussion of the mode field; The chunks sentence (last in this section) could
> go to the chunk description. Also, "MUST ONLY" is not RFC2119 - I'd suggest that sentence to "Chunks
> that are not for session startup MUST NOT be in packets with mode 3."

noted. while "MUST ONLY" is not an RFC 2119 keyword, it is correct english modified with a requirement keyword and emphasis.  i will add a definition of "MUST ONLY" in the "Terminology" section.

> One other thing about padding. When reading the text, it sounds like a chunk could span multiple
> packets:
> 
> *  The chunkLength field of the current chunk indicates that the
>          chunk payload wouldn't fit in the remaining bytes of the
>          packet.
> 
> Does that mean, that chunkpayload may be transported in a following packet by itself? If so, a
> reference to the section where this is explained would be good.

interesting interpretation. chunks do not span packets. i will clarify that language.  the procedural description should be unambiguous though.

> 2.3: I assume the list of type codes is exhaustive; but why is 0x08 and 0x48 reserved, when all the
> non-defined codes would be reserved as well? One wonders why the order was done in that particular way
> (there are apparently request-response type of code pairs scattered), but why not list them in roughly
> ascending order, or explain why these codes were selected in that particular way (are there legacy
> uses for other codepoints, that cann't be used any more, or stuff you don't want to disclose
> publicly?)
> 
> Grouping them by uses (ie. those that are only valid during session startup / mode 3, vs other modes
> might be beneficial?

the chunk type codes are arranged in the order one might encounter them in normal operation, starting with session startup (in the handshake order) and then normal flow lifetime (send data, ack, close), and session close.

0x08 and 0x48 are reserved with a specific (but unstated) future use in mind. the intent of calling them out is to keep any derivative or supplement to this specification, which might define new chunk types, from using 0x08 and 0x48.  i will also add language describing that unrecognized chunk types (or chunk types inappropriate to the packet's mode) MUST be ignored.

> 2.3.1 reserved again :) this time, the expected value is also not stated in the struct definition (as
> in 2.1.5)
> 
> 2.3.2 MUST ONLY -> MUST (I'll stop reporting these).

the "ONLY" is important and is used correctly.  describing the packet constraints avoiding "ONLY" (with just MUST and MUST NOT) i think would be less clear and involve an annoying degree of constraint combinations.  as i mentioned above, i will be adding a definition for "MUST ONLY".

> 2.3.11 forwaresequencenumber: this appears to be an indication of the sender buffer? What happens to a
> reliable transfer, if the fsnoffset no longer includes the oldest, lost packet... (wondering).
> Sequence numbers map to fragments. As TCP person, this keeps confusing me... (adding "fragment
> sequence number" or such would probably be a good constant remaineder).

the flow sender will not send (or resend) any fragment with a sequence number less than or equal to the forward sequence number. this is how partial reliability works.  the sender does not move the forward sequence number past unacknowledged sequence numbers that have not been abandoned.  see section 3.6.2.3 and the steps for "Trim abandoned messages from the front of the queue and find the Forward Sequence Number FSN".

i will try to clarify sequence numbers relating to fragments.

> 2.3.11.1 the Text refers to the option number in decimal, while the list is in hexadecimal... (nit)

ok. i will address this.

> 2.3.11.1.1 presumably, there is an API call for the App to set/retrieve the metadata?

see section 3.6.3.1.  this specification imposes very few requirements on the API. a practical implementation might find it advantageous to allow an application to retrieve the metadata for a receiving flow after the initial open notification; however, that is not required.  a recommended API could be the subject of a different specification.  

> 2.3.11.1.2 what about the other error states - should a flow be terminated, the option ignored
> subsequently, API disgression?

i'm not sure what you mean by "other error states".  section 3.6.3.1 describes all required operational semantics for flow association.

> 2.3.12: in figure 3, the use of the user data chunk (plus the proper additional header fields) would
> be also valid; however, I don't quite get why there would be multiple fragments - non-interlaced - to
> the same message in the same packet; That picture seems to be too artificial to depict a real use
> case, not? I can imagine multiple, independent messages (FRA=0) in the same packet, using the next
> user data chunk, or multiple fragmens, sent in individual packets but those would need to use the user
> data chunk, correct? Also, the FSN remains identical in this example, that means the fsnoffset encoded
> on the wire increases...

you're right. i contrived this example to illustrate both next user data and a multi-fragment message. that was a mistake. i'll revise this to show independent messages using "Next User Data" which is how it would be used in nearly all real-life cases.  if User Data had been used to encode sequential fragments in the same packet, then the fsnOffset would increase in each chunk to keep the FSN the same.

> 2.3.13: bufAvailable would map to TCP receive window, scaled by 10 bits; Is it feasible to have a
> receiver perform a window reduction during retransmissions (ie. bufferBytesAvailable reduce to 1023
> bytes, therefore bufferBlocksAvailable = 0, while packets are outstanding? Is a RTMFP sender following
> section 4.2.2.16 of [RFC1122]? If not, that might limit the retransmission of outstanding user data...

the bufferBlocksAvailable is CEIL(bufferBytesAvailable / 1024), so 1023 bytes is still one block.  RTMFP allows the receive window to shrink while packets are outstanding. that would limit retransmission of outstanding user data.  this is allowed and desirable.  note that the "DISCUSSION" in RFC 1122 Section 4.2.2.16 states that the window should not shrink because "many TCP implementations become confused".  buggy TCP window handling implementations are not a concern for RTMFP.  :)  RTMFP sections 3.6.2.3 and 3.6.2.9 give the normative behavior for a sender regarding the receive window.

> Apparently, as much of the concepts seem to be related to SCTP, a reviewer with SCTP background might
> be interested in this draft as well..
> 
> 
> 2.3.14: the (selective ack) remainder must be at least 2 bytes (in the pseudo-code while check), but a
> miscoded packet could still be crafted. What should be the default option? Also, in 2.1.2, the pseudo-
> function is IMHO not defined properly for miscoded VLUs (ie. a single byte 0x80 - which may be valid
> but unnecessary encoding - , or any 0b1xxxxxx value followed by no additional bytes). From TCP stacks,
> malformed SACK pairs are silently ignored (only the cumulative ACK is retained, plus all the valid
> SACKs decoded).

the while check (remainder() > 0) would be correct and sufficient if there was language describing what to do when there is a parse error/truncation.  i will add that language.

> I assume the RTMFP library has proper range checks and bounds implemented, which are simply not stated
> explicitly in the draft.

yes.

> For the two data ACKs, it's stated that the one that can be encoded in fewer bits should be sent...
> Obviously, there is no difference when there are no losses/unordered deliveries. But to fully adhere
> to that SHOULD, the receiver would have to built both, and then choose the smaller one?  I'm just
> curious, for a thin stream (ie. realtime) I'd expect that in most cases, the bitmap encoding will be
> more efficient, while for bulk transfers, the range encoding, unless there is heavy reordering/heavy
> random loss (congestion loss usually is correlated). Do you have any data showing which variant is
> more often used in a strictly compliant receiver?

our implementation understands both kinds, but currently only sends range acks.  we have no data to show which variant would be used more often.

> I'll continue my review of the document tomorrow...
> 
> 
> Best regards,
> 
> Richard Scheffenegger

thanks!

-michael thornburgh