Relaying Jörg's review sent to the authors of the draft 2013-11-30.

Please, step forward with any reactions or input regarding the two 
reviews we received recently. My plan is to go through them and provide 
feedback to the list ASAP.

(Jörg is added to the copy list, since he's not following the BFCPbis 
mailing list)

-- Tom

-------- Original Message --------

Hi Gonzalo and all,

thanks a lot for the diff, this is really useful!

So, I have gone in some detail through the document and found a
bunch of issues to be addressed.  Most of those revolve around
the use of unreliable transport, which appears to be underspecified
in a number of ways.

See below for the comments and some suggestions.  I don't think the
document is ready for any IESG steps yet.  But I also don't think it's
going to take long to get it there.

Cheers,
Jörg



5.1. General

      SHOULD ->  MUST be cleared by the sender

      When moving from a 16- or 32-bit "value" to an unsigned
      integer, the byte order must be specified.

5.1. R bit

      Question: Can it happen that transport changes in a relay?  If
      my dim memory serves me right, then STUN relay may change the
      transport and thus the reliability.  They won't interpret BFCP,
      however, and so the bit would suddenly be wrong.  Or is this
      scenario prohibited?

5.2. Use of SHOULD

      Quite a few places state a SHOULD requirement, e.g., for sending
      error messages.  But this doesn't make sense.  Based upon which
      grounds would an endpoint _not_ send an error message?

5.2.6 Generic error

      Is it specified what the receiver of a non-specific error is
      supposed to do?  Apparently, it cannot talk to the server.  So,
      abandon floor control?

5.3.14, 5.3.15., and 5.3.17

      It seems that FloorRequestStatusAck, FloorStatusAck, and GoodbyeAck
      are essentially packet-level acks.  So why not have just a single
      code point for those, since they all carry a transaction id anyway?

6.2  Unreliable Transport

      The text often talks about UDP datagrams, even though DTLS
      transport appears assumed.

      "Entities MUST have at most one outstanding request transaction at
      any one time."

      This probably wants the addition of a "per peer", otherwise a floor
      control server would be in trouble.  (Section 6.2.1 has this right)

6.2 Hello

    Clients MUST announce their presence to the floor control server by
    sending a Hello message.  The floor control server responds to the
    Hello message with a HelloAck message.  The client considers the
    floor control service as present and available only upon receiving
    the HelloAck message.

    When does the server consider the client to have disappeared so that
    it can discard state?

    It seems I can run a client, send a Hello message, get the HelloAck,
    and then send a floor control request with a spoofed IP address, and
    predict and fake the response for the first message I am expecting
    from the server in response.  Then, I have state installed that will
    generate packets and retransmissions to a random target until the
    server discards the client state.  But there is no mention when the
    state would be discarded.

    Maybe this is less of an attack problem since this is bound to a
    conference somewhere, but I still don't have a mechanism to get
    rid of the transport state -- could be important particularly when
    new connections are instantiated.  While it is stated that the old
    state is lost (for the new instance of the client), will it be
    discarded?  There is no guidance.

6.2.3 Fragmentation

    Is there any guidance on transmitting fragments?  (any pacing?)

    How is N defined?  One would assume N = ceil (msgsize/MTUsize), but
    an implementer might decide to use a larger N.  In any case, when
    we use N in the spec, it must be said how it is established.


8.2  Transaction number re-use.

      The text currently states that a transaction number "MUST NOT be
      reused in another message from the floor control server checks
      whether until the appropriate response from the client sending is
      received for the transaction".

      If the ID is monotonically increasing (modulo wrap-around) why not
      make re-use illegal?  Allowing re-use seems to be calling for
      trouble, especially in case some transactions may refer to others
      by their ID or if transactions last longer.

8.3.2. T2 is unclear

9.1  Authentication

    The text states:

    BFCP messages received over an authenticated TLS/DTLS connection are
    considered authenticated.  A floor control server that receives a
    BFCP message over TCP/UDP (no TLS/DTLS) can request the use of TLS/
    DTLS by generating an Error message, as described in Section 13.8,
    with an Error code with a value of 9 (Use TLS) or a value of 11 (Use
    DTLS) respectively.  Clients SHOULD simply ignore unauthenticated
    messages.

    "can" ->  MUST or MAY (this is normative behavior)

    But does this work?  A server has one connection to a client.  So,
    if the server has at its disposition to accept TCP/UDP but the client
    should ignore such messages, with the connection bidirectional, then
    this de-facto mandates TLS/DTLS, doesn't it?  Or the client would
    never react.

Appendix A: Wouldn't the monoticity of the transaction IDs become
    clearer if increments or 1 in numbers were used?