Re: [rtcweb] Opsdir last call review of draft-ietf-rtcweb-security-11

[Joe Clarke <[e-mail]>]

On 2/14/19 14:23, Sean Turner wrote:
> Hi! Doc Shepherd here ;)
> 
>> On Feb 12, 2019, at 14:44, Joe Clarke <[e-mail]> wrote:
>>
>> Reviewer: Joe Clarke
>> Review result: Not Ready
>>
>> I have been assigned to review this document on behalf of the Ops directorate. 
>> In general, I found the document well-written, but the reason I marked it as
>> not ready as I was confused as to its standards track trajectory.  I do not see
>> any kind of inter-operable standard being defined here.  On my reading --
>> before I noticed it was standards track -- it felt informational.  While it
>> does set out a threat model for the browser, I struggle to see how that needs
>> to be standardized.
> 
> The rationale I provided in the Shepherd write was this:
>    This draft is bound standards track because it includes all of the WebRTC
>    security considerations and will referred to from all WebRTC WG drafts.
> 
> There are also 8 2119-MUSTs/MUST NOTs is the document that affect browser behavior, which (I think) gets it over the informational level hurdle.

Not sure, TBH.  The way it read to me was more informational, which is
why I was surprised to see it on the standards track after the
read-through.  But given this extra bit of context about its intent,
perhaps standard is the way to go.  I'm glad it's been
considered/discussed, and I would defer to ADs on that.

> 
>> On that threat model note, the abstract indicates that the WebRTC threat model
>> will be laid out, but section 3 defines a more general browser threat model.
> 
> It does, but the 1st sentence explains why they are the same.  I guess we could rename the section, but it’s just a layer of indirection.

It is.  But while the requirements follow directly, there are additional
considerations.  I think renaming would make it clearer.

> 
>> Beyond those items, I noticed various nits and other small items when reading
>> the document.  Most broadly, I feel this document would benefit from a
>> terminology section to define acronyms such as ICE, TURN, STUN, VoIP, etc. 
>> Additionally, in section 3.1, the document refers to "scripts" in a general
>> way.  While the implication is JavaScript code that will run in a browser, I
>> think that kind of context setting might be made more explicit in a terminology
>> section.
>>
>> Other nits are mentioned below on a section-by-section basis.
> 
> I addressed these in the following PR:
> https://github.com/rtcweb-wg/security/pull/13

Thanks!

>> ===
>>
>> Section 4.3.2.1:
>>
>> OLD:
>>
>>  (a) the browser to trusted UI to provide the name and
>>
>> I don't grok this sentence fragment.  There seems to be a verb missing, and I'm
>> not sure what your intent is here.
> 
> I suggest “the browser has trusted UI …”. if that’s wrong I can amend the PR.

To that correction, perhaps, "the browser has a trusted UI"

Joe