Re: [bfcpbis] Comments on draft-ietf-bfcpbis-rfc4582bis-13

["Charles Eckel (eckelcu)" <[e-mail]>]

A couple comments inline.

On 9/22/15, 11:26 PM, "bfcpbis on behalf of Paul E. Jones"
<[e-mail] on behalf of [e-mail]> wrote:

>Ben,
>
>Following up on this email (and building a growing to-do list)....
>
>------ Original Message ------
>From: "Ben Campbell" <[e-mail]>
>To: "[e-mail]"
><[e-mail]>
>Cc: "Richard Barnes" <[e-mail]>; [e-mail]
>Sent: 3/2/2015 10:23:58 PM
>Subject: [bfcpbis] Comments on draft-ietf-bfcpbis-rfc4582bis-13
>
>>(no hats‹please treat as any other IETF last call feedback)
>>
>>Hi,
>>
>>Here are some comments on draft-ietf-bfcpbis-rfc4582bis-13.
>>
>>I've divided them into substantive and editorial comments. I've tried
>>to limit comments to the actual "bis" parts, but I don't know how
>>successful I've been at that.
>>
>>Thanks!
>>
>>Ben.
>>
>>
>>*** substantive comments ***
>>
>>
>>‹ 5.1, "ver"
>>
>>Am I correct that the guidance for receiving an unsupported version
>>applies when you get the wrong version for the transport? (even if you
>>might have supported that version on a different transport?) If so, it
>>would be good to mention that.
>
>Valid point.  I'll add it to the to-do list.
>
>
>>Also it would be nice to elaborate a bit more on the case of a device
>>only supporting 4582. Given that you aren't allowed to send version 1
>>over UDP, how would this ever be an issue? Are you worried about early
>>UDP implementations that still use version 1? If so, that would be
>>worth a note.
>
>Devices supporting 4582 will use a reliable transport.  Personally, I
>would not worry about the "why".  If it's the wrong/unexpected value at
>the wrong/unexpected place, return an error.

There are implementations of BFCP/UDP that were done at various points in
the standardization process. At some point in time, the numbering of some
BFCP/UDP specific primitives changed. Then the version field was defined.
Some people use the version number to guess if an implementation is using
the old/new primitive numbering. Numerous interoperability events have
resulted in fine tuning of this black magic. I don¹t think we can
recommend this. We can forbid it, but no one will listen.

>
>
>>‹ 5.1, payload length:
>>
>>How is a TCP recipient expected to know a payload length was wrong?
>>(Other than the parser failing on the next message?)
>
>This is an issue in framing any protocol over TCP.  If it's wrong,
>subsequent messages just start looking like garbage and messages are
>rejected.
>
>
>>Also, in the "note", what is the "message size" for a fragmented
>>message over UDP? Is it the size of a fragment, or the size of the
>>message before fragmentation plus the additional common headers? In
>>either case, the REQUIRED part seems more a statement of fact (i.e. you
>>mathematically can't express a size that won't fit) than a normative
>>requirement.
>
>This refers to the BFCP message generally, before it's framed and
>transmitted.  Agree on REQUIRED ... make it lowercase.
>
>
>>5.3.11:
>>
>>It seems like "Hello" does quite a bit more than liveness checks.
>>(e.g., establishing the connection for UDP, capabilities query). Also,
>>if Hello is used for liveness checks, I missed any discussion about the
>>procedures for using it for that purpose. (Note that 6.2.4 says to use
>>STUN for liveness checks for non-reliable transports.)
>
>Yep, valid points.  I'll add it to my list.
>
>
>>-- 6.2.1:
>>
>>Is this section only applicable to non-reliable transports? Also, the
>>bit about only sending one message until you get an ack is not quite
>>true even for UDP if you consider fragmented messages.
>
>I think this is just for UDP (since it's in the unreliable transport
>sub-section); TCP handles congestion itself.  There is a single message,
>but it might be fragmented over several packets.  That's really no
>different than if fragmentation happened at the IP layer.  Do you want
>something else stated here?
>
>
>>  -- 6.2.1, last paragraph:
>>
>>Can you elaborate on what it means to say the "default MUST be" a
>>certain value? Do you mean an implementation MUST use that for a
>>default if no other value is selected? Or is this more a statement of
>>fact (the default value _is_...)?
>
>Given Gorry's comments, I think we need to revisit this assignment.
>
>
>>-- 6.2.3, paragraph 5: "The Fragment Offset field denotes the number of
>>bytes..."
>>
>>The definition said the unit was 4-octet units.
>
>Yeah, that needs to be clarified.  Will put it on the list.
>
>
>>  -- 8:
>>
>>If a request times out completely on a non-reliable transport, does
>>that impact the state of the "bfcp connection"?
>Also needs to be clarified.  Thanks.
>
>
>>  -- 9.1:
>>
>>The text mentions sending self-signed certs or fingerprints in an
>>"initial integrity protected channel". I assume that means the
>>signaling protocol? The language does not seem to allow for a CA issued
>>certs validated with PKI techniques. Is that the intent? If so, can
>>they be mixed and matched? (i.e. a self-signed client cert and a CA
>>issued server cert?)
>
>We'll need to revisit this.  The protocol itself does not care about the
>certificates, only that we need some way to ensure we're talking to the
>right endpoint.  These certs could be CA-issued.  I'll put this on the
>list.
>
>
>>13.1.1, last paragraph:
>>
>>What if the Ack is never received? What if it is received after T2
>>expires? (In the latter case, does the device need to be prepared to
>>receive an Ack for which it no longer has transaction state?)
>
>For some reason, I'm not matching 13.1.1 against the question you are
>asking, but I understand your point.  If a message sender sends a
>message and does not get a reply, it sends it again a few times using
>exponential back-off.  If a response is not received, then ... that
>speaks to your question of "is this a connection failure?"  That's on my
>list.
>
>
>>13.7, paragraph 2:
>>
>>Is the normative requirement to send an error 12 if you don't support
>>the version redundant with similar normative language in the definition
>>of "ver"?
>
>It is redundant.
>
>
>>  -- section 14, last sentence:
>>
>>I think you mean to require the use of encryption if TLS/DTLS is used,
>>but the language can be read to globally require TLS/DTLS, which is not
>>consistent with guidance elsewhere.
>
>I'm not quite sure what the issue is.  TLS or DTLS are optional, but
>they are the only means through which authentication and confidentiality
>are provided in the protocol.
>
>
>>-- Appendix A:
>>
>>For non-reliable transports, do both endpoints share a transaction ID
>>sequence, or does each end keep it's own independent sequence? I would
>>have guessed the latter, but in step 5 of the example, the server
>>originated transaction has a transaction ID that is one higher than the
>>latest value sent by the client.
>
>The transaction number spaces are independent and the "R" bit is used to
>help make it clear that a message relates to a server-initiated
>transaction or a client-initiated transaction.  What I think is needed
>in that section is showing the R bit value.
>
>
>>  -- Note following Figure 48: "... does not and SHOULD not"
>>
>>That's an odd construction. Do you mean a statement of fact, or a
>>normative requirement? If the latter, why is this a SHOULD and not a
>>MUST? Can you imagine a situation where it might be valid for the
>>client to ack the initial FloorRequestStatus message?
>
>Given this is just an example to illustrate the protocol, I would
>suggest we change the sentence to read:
>"As such, it is not followed by a ..."
>
>There is nothing new here.
>
>
>>
>>*** editorial comments ***
>>
>>‹ section 3, 1st paragraph:
>>
>>The bis version considers new requirements beyond those mentioned,
>>doesn't it? (e.g., UDP support?) It would be nice to mention that here.
>
>I'm personally not sure what other requirements are considered.  It
>appears to be an enhancement only to add UDP.  We need to get clarity
>from the co-authors.  That said, exactly where were you looking for new
>text?  And you want a list of "what's new"?

We tried to change as little other stuff as possible, fixing only things
we encountered that we thought would be real problems. These are listed in
16.2.

>
>
>>‹ 4.1, paragraph 7:
>>
>>I'm a bit surprised to see the unreliable transport message flows
>>buried in an appendix. Aren't those the whole point of the update?
>
>Given they are examples, I personally thought this was reasonably,
>especially since the area there the TCP-based flows are would get really
>cluttered.  We could swap the flows or move the reliable transport
>examples to an appendix, too, but I like having examples up front.  If
>anything, I'd suggest swapping the flows (moving the reliable flows to
>an appendix).  Actually, given the list of things on the to-do list, I'm
>inclined to say "leave it" to avoid introducing instability.
>
>
>>  ‹ 5.1, Res:
>>
>>The phrase "At this point," doesn't seem to add anything, and may
>>confuse readers.
>Yeah, others noted that and I agree we should remove it.
>
>
>>
>>‹5.2, "M" bit:
>>
>>The phrase "In all other cases,..." is a little  ambiguous. I assume
>>you mean "Attributes specified in future extensions that do not have
>>the "M" bit set..."
>
>Agree this needs adjustment, but it's not necessarily attributes
>specified in the future, but just ones that are not recognized.  Perhaps
>this?
>
>     Unrecognized attributes, such as those that might be specified in
>future extensions, that do
>     not have the "M" bit set are ignored, but the message is processed.
>
>Paul

Cheers,
Charles

>
>_______________________________________________
>bfcpbis mailing list
>[e-mail]
>https://www.ietf.org/mailman/listinfo/bfcpbis