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

["Ben Campbell" <[e-mail]>]

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

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.


— 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?)

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.

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

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

-- 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_...)?

-- 6.2.3, paragraph 5: "The Fragment Offset field denotes the number of 
bytes..."

The definition said the unit was 4-octet units.

-- 8:

If a request times out completely on a non-reliable transport, does that 
impact the state of the "bfcp connection"?

-- 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?)

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?)

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

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

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

-- 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?


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

— 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?

— 5.1, Res:

The phrase "At this point," doesn't seem to add anything, and may 
confuse readers.

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