I've reviewed draft-ietf-xcon-bfcp-05.txt. Overall, I was impressed with
the clarity of this document, and with how easy it was to understand and
analyze without a deep understanding of SIP or conferencing protocols.
This document is long (70 pages) but not difficult; about half the length
is fairly mechanical descriptions of the various message formats.
I do have a few specific comments, included below. Some of these are
fairly minor, but I'd like to call particular attention to point 8, which
describes a potentially serious security issue which, while it should not
require protocol changes, does need to be called out and explained in
detail.
** BFCP comments
1. Section 5.1 states that the "reserved" bits in the common header
SHOULD be set to zero. If the intent is to allow these bits to be
used in a backwards-compatible way by future revisions to the
protocol (possibly including extending the length of the version
field), then this needs to be a MUST. Otherwise it will not be
possible to "newer" implementations to tell if a non-zero bit has
the meaning specified by the future protocol revision, or is simply
set because an "older" implementation disregarded the SHOULD.
2. It seems that a maximum attribute length of 255 bytes could be
somewhat limiting, particularly for attributes containing free-form
text fields, and for grouped attributes containing such attributes
(consider a FLOOR-REQUEST-INFORMATION, which may contain two user
display names, two URI's, plus a free-form PARTICIPANT-PROVIDED-INFO).
3. The next-to-last paragraph on page 17 states that if the 'M' bit
is set on an unrecognized attribute, "the message is rejected".
This is a key requirement for proper interoperability of future
extensions, and IMHO is an appropriate place to use requirements
language. I suggest changing "is rejected" to "MUST be rejected".
4. In section 5.2.6, the Queue Position field is 8 bits wide. What
value is reported for requests whose queue position is greater
than 255?
5. In section 5.2.12, why does the SUPPORTED-ATTRIBUTES attribute list
attributes as 16-bit numbers? Since attribute codes are only 7 bits
wide, why not list them in the same format used for error code 7?
6. In section 9.2.1, the third full paragraph on page 40 suggests that
the error code for "Invalid Nonce" is 6. In section 5.2.8, this
value is listed as 4.
7. In section 9.2.1, it is specified that if a client receives an
"authentication failed" error, it MUST NOT send further messages to
the server before obtaining a new shared secret. This suggests that
an attacker may be able to cause a client repeatedly to perform the
out-of-band exchange used to obtain such a secret, either by sending
spoofed errors to the client or by sending spoofed messages to the
server with improper authentication. Depending on the protocol used
to obtain the shared secret, this may make it easier to attack the
mechanism used to protect that exchange. This issue should at least
be called out in security considerations.
8. This document suggests that if TLS is used, then only the first
message need be signed, and that it can then be assumed that any
further messages received along the same TLS session are coming
from the same client. This is true only if a client knows in advance
that it is supposed to be using TLS, refuses to talk to a server
that does not use TLS, and checks the server's certificate.
If a client is not using TLS, or does not check the server's identity,
then it is possible for an attacker to capture a client's signed
message, then open his own TLS-protected connection to the server,
replay the captured message, and then send requests of his choosing.
9. The IANA considerations section establishes registries for five sets
of parameters, and specifies registration policies of "Specification
Required" for each of these. All of the namespaces involved are
quite small (8 bits), and thus may lead to scarcity. I would suggest
that the registration policies be at least "Expert Review", and that
at least one value be reserved to allow for future expansion of the
namespace if that ever becomes necessary (if it doesn't, then it won't
be a great loss to reserve one value). Also, is the value 0 intended
to be reserved in these registries?
-- Jeffrey T. Hutzelman (N3NHS) <jhutz+@xxxxxxx>
Sr. Research Systems Programmer
School of Computer Science - Research Computing Facility
Carnegie Mellon University - Pittsburgh, PA
_______________________________________________
Ietf@xxxxxxxx
https://www1.ietf.org/mailman/listinfo/ietf