Re: [Gen-art] Review of draft-ietf-bfcpbis-bfcp-websocket-13

["Ram Mohan R (rmohanr)" <[e-mail]>]

Hi Robert,

Thanks for your review. Please see inline <Ram>

>-----Original Message-----
>From: Robert Sparks <[e-mail]>
>Date: Thursday, 5 January 2017 at 4:05 AM
>To: "[e-mail]" <[e-mail]>
>Cc: "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" ><[e-mail]>
>Subject: Review of draft-ietf-bfcpbis-bfcp-websocket-13
>Resent-From: <[e-mail]>
>Resent-To: <[e-mail]>, <[e-mail]>, <[e-mail]>, <[e-mail]>, <[e-mail]>, ><[e-mail]>, <[e-mail]>, <[e-mail]>, <[e-mail]>, <[e-mail]>, ><[e-mail]>, Charles Eckel <[e-mail]>
>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