[bfcpbis] PROTO review comments on draft-ietf-bfcpbis-rfc4582bis-10

[Mary Barnes <[e-mail]>]

I have agreed to shepherd this document on behalf of the WG.  I have
reviewed this document in preparation for doing the PROTO write-up.

I think the document needs a bit of work before it's ready to progress.  I
have a few questions/comments, as well as editorial nits which I think
would improve readability and ease of understanding.

Regards,
Mary.


*General:*
1) I still have not had a response from Keith Drage nor Joerg Ott with
regards to IPR, so I cannot forward the document to the AD even after the
document is updated until I get those responses.

2) Obviously, the WG needs to agree proposed solutions to the points made
by Joerg:
http://www.ietf.org/mail-archive/web/bfcpbis/current/msg00249.html
At this time, I've not seen the authors post any proposals as to how to
deal with those comments/concerns

3) I have reviewed the document in detail - both the entire document and
the diffs between RFC4582. I have both comments and nits on text that was
also in RFC 4582, as well as on the new text, detailed below. Note, that a
number of nits are around the use of lower case normative words which ought
to be uppercase to distinguish them from the normal English usage. I think
there are a number of cases where "may"s for example would be much better
stated as "can"s.  I've identified some of those below, but I don't think
it's worth the effort to change them all (nor my effort to identify them
all). `


*Comments/questions:*

1) Section 3.4, 1st paragraph.  You need to specify what you mean by the
parenthetical "in a certain way" - that doesn't tell you anything.
 Something like "depending upon policies, privileges, etc." is probably
more appropriate.

2) Section 6.2.
a) 3rd paragraph, 1st sentence: I don't think this is normative since
you're referring to the normative section.  I suggest "shall form" ->
"forms".
b) 4th paragraph. Under what conditions would a server not send an Error
message or is that statement just specifically sending an error message
with a parameter value of 10?  I can't see why this isn't a MUST. If it's
really a should, more text is needed as to what happens if it isn't sent
(or why not sending the Error message is okay).
c) 4th para, last sentence: "shall" -> "SHALL"
d) last para, next to last sentence.  Why isn't this "MUST" request the use
of DTLS? If it's a SHOULD, then the impacts or criteria under which DTLS
isn't REQUIRED should be clarified.  I see that the paragraph has
references to  5018 Section 5.  I think it also should be clarified that
the "section 6" reference in that paragraph is also in 5018 (as I don't
think you're referring to section 6 of this document).

3) Section 6.2.1
a) 1st para, 3rd sentence. I don't think "previous paragraph" is accurate.
 I think it would be better to include an explicit reference to section 6.2
or say "previous section".
b) next to last sentence ought to be written normatively
OLD
   The default initial
   interval is set to 500ms and the interval is doubled after each
   retransmission attempt.
NEW
   The default initial
   interval MUST be set to 500ms and the interval MUST be doubled after each
   retransmission attempt.

4) Section 6.2.2, 1st sentence.  What happens if the connection isn't
treated as closed (since this is a SHOULD and not a MUST)?  Either this is
a MUST or the reasons one wouldn't treat the connection as closed and what
happens when it's not treated as closed need to be explained.

5) Section 6.2.3, indented paragraph.  What happens if the cookie exchange
mechanisms in DTLS aren't used and/or what other mechanisms could be used?
  (Note, also that the should ought to be SHOULD).

6) Section 9.1.
a) 1st para, 2nd sentence. (Similar to comment 9 on rfc4583-bis).  The
"assumes" needs to a REQUIRED.
OLD:
   BFCP assumes that there is an integrity-
   protected channel between the client and the floor control server
   that can be used to exchange their self-signed certificates or, more
   commonly, the fingerprints of these certificates.

NEW:
   An initial integrity-protected channel is REQUIRED between the client
   and the floor control server that can be used to exchange their
   self-signed certificates or, more commonly, the fingerprints of
   these certificates.

OR  this needs to be a SHOULD and then include the text that's in the
"Note…" in the 3rd paragraph that indicates if there are additional
authentication mechanisms defined that don't require the integrity
protected channel.

b) 2nd paragraph, last sentence.  Under what criteria would a client NOT
ignore unauthenticated messages or what bad things can happen if a client
doesn't ignore the messages.  It might be better to make this a MUST.

c) 4th paragraph, 2nd sentence.  "SHOULD check" -> "MUST check" or explain
what might happen if the floor control servers doesn't check that the
messages aren't using an authorized User ID.

7) Section 11.1.
a) 4th paragraph.  There's three SHOULDs in there that all ought to be
MUSTs OR the situations under which the action doesn't need to be done
needs to be specified.
b) 5th paragraph: I don't think that the use of Queue position field in the
last sentence prior to the indented text ought to be normative - i.e., I
think the "MAY use the Queue Position field" ought to be "can use the
Queue Position field".
c) Indented text (1st para) after 5th para.   There are several "may"s:
(lower case) in this paragraph (and the next note), that probably all ought
to be "can"s
d) last paragraph. It's not clear to me whether this "may" ought to be
normative. If so, it should be a MAY and perhaps reworded as I think it's
trying to say that "the floor chair MAY include STATUS-INFO attributes" in
   Otherwise, a "can" would suffice.

8) Section 12.1.2.  There is no normative language in this section.  It
needs to be revisited.  For example, 2nd para, 1st sentence.  It would seem
that "the floor control server will
   respond with a FloorStatus message or with an Error message"
ought to be "MUST respond.

9) Section 13.6, 1st para.  Under what circumstances would the server NOT
generate an Error response (i.e., why is it a SHOULD and not a MUST)?

10) Section 13.7, 1st para. Same comment as 9 above.

11) Section 16.1.  I think there should be mention of the additional text
added for handling incorrect payload length.

12) Section 16.1. "Requiring timely response".  Shouldn't there be a
reference to section 8.3 (Timers)?

13) Appendix A. Has there been anyone that has verified the call flows with
a tool or in the lab?  If so, who was that?

*Nits:*

1) Section 3.1, last paragraph.  I would prefer this be updated to be much
more precise in terms of CCMP.  I realize that this document was published
well before CCMP, so the text in RFC 4582 is based on draft versions.
OLD:
   Conference control clients using CCMP [17] can specify such floor-
   related settings by editing the floor-information section of the
   to-be created conference object provided in the body of a CCMP
   confRequest/create message issued to the conference control server.

NEW:
    Conference control clients using CCMP [17] can specify such floor-
   related settings in the <floor-information> [RFC6501]element of the
   to-be created conference object provided in the body of a CCMP
   confRequest/create message issued to the conference control server.

2) Section 3.2, 1st para, 2nd sentence.  "These data include…" -> This data
includes

3) Section 3.3, last paragraph. "<floor-information> section" ->
 "<floor-information> element"

4) Section 6.2, 4th para, last sentence: "shall" -> "SHALL"

5) Section 6.2.3:
- seventh paragraph. "must then retransmit" -> "MUST then retransmit"
- indented paragraph. "the Payload Length field may be compared" - > "can
be compared"

6) Section 8, 2nd indented paragraph. "must be non-zero" -> "MUST be
non-zero"

7) Section 10.1.1. 1st paragraph after 2nd indented para. "must insert" ->
"MUST insert"

8) Section 10.1.2, 5th & 6th paragraphs.  "MAY display" -> "can display"

9) Section 12.2.1. last paragraph. "must insert" -> "MUST insert"

10) Section 12.3.1.   last paragraph. "must insert" -> "MUST insert"

11) Section 16.1.  This is a bit hard to read.  I would suggest you use a
bulleted or numbered list rather than this hanging list format.

12) References:
a) Per the first nit, a reference to RFC 6501 (XCON Data model) should be
added to this document.
b) RFC 4582 should be a normative reference
c) I also suggest that RFC 5018 is normative as this document relies on RFC
5018 for authentication and security.