Hi Robert,
Thanks for your review. Please see inline <Ram>
-----Original Message-----
From: Robert Sparks <rjsparks(_at_)nostrum(_dot_)com>
Date: Thursday, 5 January 2017 at 4:05 AM
To: "gen-art(_at_)ietf(_dot_)org" <gen-art(_at_)ietf(_dot_)org>
Cc: "draft-ietf-bfcpbis-bfcp-websocket(_dot_)all(_at_)ietf(_dot_)org"
<draft-ietf-bfcpbis-bfcp-websocket(_dot_)all(_at_)ietf(_dot_)org>,
"bfcpbis(_at_)ietf(_dot_)org" <bfcpbis(_at_)ietf(_dot_)org>,
"ietf(_at_)ietf(_dot_)org" ><ietf(_at_)ietf(_dot_)org>
Subject: Review of draft-ietf-bfcpbis-bfcp-websocket-13
Resent-From: <alias-bounces(_at_)ietf(_dot_)org>
Resent-To: <anton(_dot_)roman(_at_)quobis(_dot_)com>,
<stephane(_dot_)cazeaux(_at_)orange(_dot_)com>,
<gsalguei(_at_)cisco(_dot_)com>,
<sergio(_dot_)garcia(_dot_)murillo(_at_)gmail(_dot_)com>,
<rmohanr(_at_)cisco(_dot_)com>,
<victor(_dot_)pascual(_dot_)avila(_at_)oracle(_dot_)com>,
<keith(_dot_)drage(_at_)alcatel-lucent(_dot_)com>,
<eckelcu(_at_)cisco(_dot_)com>, <ben(_at_)nostrum(_dot_)com>,
<alissa(_at_)cooperw(_dot_)in>, ><aamelnikov(_at_)fastmail(_dot_)fm>, Charles
Eckel <eckelcu(_at_)cisco(_dot_)com>
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