Hi Robert,
Thanks for your review. Please see inline <Ram>
>-----Original Message-----
>From: Robert Sparks <rjsparks@xxxxxxxxxxx>
>Date: Thursday, 5 January 2017 at 4:05 AM
>To: "gen-art@xxxxxxxx" <gen-art@xxxxxxxx>
>Cc: "draft-ietf-bfcpbis-bfcp-websocket.all@xxxxxxxx" <draft-ietf-bfcpbis-bfcp-websocket.all@xxxxxxxx>, "bfcpbis@xxxxxxxx" <bfcpbis@xxxxxxxx>, "ietf@xxxxxxxx" ><ietf@xxxxxxxx>
>Subject: Review of draft-ietf-bfcpbis-bfcp-websocket-13
>Resent-From: <alias-bounces@xxxxxxxx>
>Resent-To: <anton.roman@xxxxxxxxxx>, <stephane.cazeaux@xxxxxxxxxx>, <gsalguei@xxxxxxxxx>, <sergio.garcia.murillo@xxxxxxxxx>, <rmohanr@xxxxxxxxx>, ><victor.pascual.avila@xxxxxxxxxx>, <keith.drage@xxxxxxxxxxxxxxxxxx>, <eckelcu@xxxxxxxxx>, <ben@xxxxxxxxxxx>, <alissa@xxxxxxxxxx>, ><aamelnikov@xxxxxxxxxxx>, Charles Eckel <eckelcu@xxxxxxxxx>
>Resent-Date: Thursday, 5 January 2017 at 4:05 AM
> Reviewer: Robert Sparks
> Review result: Ready with Issues
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair. Please treat these comments just
> like any other last call comments.
> For more information, please see the FAQ at
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>
> Document: draft-ietf-bfcpbis-bfcp-websocket13-
> Reviewer: Robert Sparks
> Review Date: 2017-01-04
> IETF LC End Date: 2017-01-11
> IESG Telechat date: 2017-01-19
>
> Summary: Basically ready but with issues that need to be addressed
> before publication as a Proposed Standard
>
> Issues:
>
> The BFCP spec (at draft-ietf-bfcpbis-rfc4582bis) relies heavily on
> recommendations it makes about the use of TLS or DTLS, and even goes
> to
> the point of specifyig a particular set of cipher suites to use wih
> those protocols when using them with BFCP. The security
> considerations
> section of that document details some specific attacks and how the
> use
> of TLS/DTLS mitigates them (providing some justification for the
> cipher
> suites that the document specifies).
>
> This document provides a _COMPLETELY DIFFERENT_ security mechanism
> (essentially punting entirely to whatever a websocket library
> provides
> with the expectation that that will also be rooted in TLS) when it
> substitutes websockets as the layer under BFCP. The security
> considerations section needs to make this much more obvious -
> implementers and deployers need to be see this as a strong-primary
> point to avoid anyone thinking all the thinking that went into
> securing
> BFCP as captured in draft-ietf-bfcpbis-rfc4582bis still applies.
<Ram> I will add the below line in security consideration section. Is this sufficient?
NEW:
“The security considerations described in draft-ietf-bfcpbis-rfc4582bis are applicable here as well.”
> There should be more discussion about what a BFCP implementation that
> cares about the attacks discussed in section 14 of
> draft-ietf-bfcpbis-rfc4582bis requires of its library. The current
> document gets most of the way there, but there are things missing.
>Shouldn't there be some discussion of ensuring the websocket library
> used supports and will use the cipher suites called out in the core
> BFCP document? If not, this document needs to be very explicit that
> you
> are only going to get the confidentiality protection the library
> provides.
<Ram> Good point. I would prefer to add a text something to the effect of:
“The security considerations mentioned in section 14 of [draft-ietf-bfcpbis-rfc4582bis] are applicable here. In order to mitigate
the attacks mentioned in section 14 of [draft-ietf-bfcpbis-rfc4582bis], it is RECOMMENDED that the clients and server use secure WebSocket
with an encryption algorithm according to Section 7 of [draft-ietf-bfcpbis-rfc4582bis]”
> The current consideration section calls out relying on "a
> typical webserver-client model" and talks about server
> authentication,
> but not client authentication. Section 8 shows most of what you're
> expecting the server to do to authenticate the client, but you need
> more text about what you expect the client libraries to be doing to
> let
> the server do its job (and you should point back to that from the
> security considerations section).
<Ram> section 8 second para has text on what client should do. Also 4th para has some text. Is there anything else you would like to see in that?
I will add a line in security considerations
EXISTING:
The security model here is a typical webserver-
client model where the client validates the server certificate and
then connects to the server
NEW:
The security model here is a typical webserver-
client model where the client validates the server certificate and
then connects to the server. Section 8 describes the authentication procedures between client and server.
> I strongly recommend simply walking through the cases again in the
> security considerations section of this document, explaining how the
> websocket library and the bfcp implementation are going to interact
> to
> mitigate the attacks.
>
> Nits/editorial comments:
>
> The 3rd paragraph of section 3 speaks generally about how the
> websocket
> protocol works - you call out it can carry text or binary data and
> that
> it supports split frames. But then you go on to constrain the use of
> the protocol in this document to a particular bit of binary data and
> constrain using the protocol to not split frames (and to only put one
> BFCP message in each frame). This is confusing. I suggest deleting
> the
> second sentence of that paragraph and the indented call-out below it.
> If the observation about the API callbacks is important, work it in
> where you talk about the one-messsage-per-frame restriction.
<Ram> I am ok to delete the second line and the indented call-out.
>
> The last sentence of the second paragraph of section 5 relies on an
> inference that you should make explicit. Instead of "is a server on
> the
> Internet", say "will have a globally routable address".
<Ram> Ok will fix it.
>
> The last paragraph of 6.1 is not logically sound - it falls apart at
> "So". Please restructure it. As it stands, it says something like:
> 'Some soda manufacturers don't provide sugar-free variants of their
> soda. Therefore, we recommend always drinking sugar-laden soda, but
> we
> allow drinking sugar-free.' What were you actually trying to say?
How about changing to this?
EXISTING:
Some web browsers do not allow non-secure WebSocket connections to be
made. So, while this document recommends the use of Secure
WebSockets (i.e.TCP/WSS) for security reasons, TCP/WS is also
permitted so as to achieve maximum compatibility among clients.
NEW:
While this document recommends the use of Secure
WebSockets (i.e.TCP/WSS) for security reasons, TCP/WS is also
permitted so as to achieve maximum compatibility among clients.
Regards,
Ram