Re: [Gen-art] [rtcweb] Genart last call review of draft-ietf-rtcweb-security-arch-18

Russ, thank you for your review. Sean, thank you for your responses. I entered a Yes ballot.

The use of “settings” reads ok to me.

Alissa

> On Feb 20, 2019, at 10:09 PM, Sean Turner <sean@sn3rd.com> wrote:
> 
> I generated PR for these:
> https://github.com/rtcweb-wg/security-arch/pull/85
> 
>> On Feb 9, 2019, at 13:50, Russ Housley <housley@vigilsec.com> wrote:
>> 
>> Reviewer: Russ Housley
>> Review result: Almost Ready
>> 
>> 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
>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>> 
>> Document: draft-ietf-rtcweb-security-arch-18
>> Reviewer: Russ Housley
>> Review Date: 2019-02-07
>> IETF LC End Date: 2019-02-15
>> IESG Telechat date: unknown
>> 
>> Summary: Almost Ready
>> 
>> 
>> Major Concerns:
>> 
>> Section 4.1 says "... preferably over TLS ...", but it does not tell
>> what the consequences are if TLS is not used.  Since this is the
>> security architecture, I would expect these consequences to be
>> described.
> 
> There is s9.1 that addresses this :)
> 
>> Section 4.2: Please add a sentence or two that defines Interactive
>> Connectivity Establishment (ICE) data and non-ICE data.
> 
> Since s4.2 of this document points to s4.2 of security-arch and there’s an entire subsection on ICE I am hoping that the references are enough.
> 
>> Section 6.5 includes a contradiction.  One place it says, " MUST NOT
>> negotiate cipher suites with NULL encryption", and another place it
>> says, "if Null ciphers are used ...".  Please make these consistent.
> 
> I deleted the display requirements section because I think the prohibiting on negotiating NULL drives the display requirement.
> 
>> Section 6.5 requires implementation of certificate fingerprints or a
>> Short Authentication String (SAS).  Please add a sentence to tell how
>> they are used to provide out-of-band verification.  Without such a
>> sentence, it is easy to imagine an implementation with a UI that is
>> too awkward to actually get the information on the screen while the
>> call is in progress.
> 
> Would something like this work:
> 
>  These are compared by the peers to authenticate one another.
> 
>> Section 10: since this is a standards track document, the IESG should
>> be responsible for this new codepoint, not the document author.
> 
> changed
> 
>> Minor Concerns:
>> 
>> Section 3.1 uses https://www.evil.org/ as an example.  However, this is
>> a registered domain.  It would be better to follow the IESG statement on
>> examples: https://www6.ietf.org/iesg/statement/examples.html.
> 
> I was really hoping a Dr. Evil included their info the DNS.  It wasn’t there.
> I changed to http://example.org
> 
>> Section 6.2 uses customerservice@ford.com  as an example.  Of course,
>> ford.com is a registered domain. It would be better to follow the IESG
>> statement on examples (the URL is above).
> 
> Changed it to customerservice@example.org
> 
>> Section 7 uses Poker Galaxy  as an example.  Of course, this is a real
>> web site. It would be better to follow the IESG statement on examples
>> (the URL is above).  It seems best to use the same names here as are
>> used in Section 7.2.
> 
> I changed to “a poker site” to match that phrase, which is used in the 1st para of that section.
> 
>> Nits:
>> 
>> Section 1 includes: "... SDP-based like SIP."  Please add a reference
>> for SDP.
> 
> I have to admit that I’d probably be confused if there was a reference to SDP after "SDP-based like SIP [RFC4566]” and it reads a little awkward if we do "SDP-based [RFC4566[ like SIP.  RFC 4566 is referred to in s3 when the SDP attribute is defined and there’s a reference tor SIP, which also refers to SDP,  earlier.  I tend think the reader won’t be that confused ;)
> 
>> Section 4.1: s/ permissions till later/ permissions until later/
> 
> Fixed
> 
>> Section 4.4: please add a reference for STUN.
> 
> The reference is a sentence later.
> 
>> Section 6.2: s/(though see Section 6.3/(See Section 6.3/
> 
> fixed
> 
>> Section 6.4: please do not enclose the note is '[' and ']'.  Avoid
>> confusion with reference syntax.  One solution is to put the note at
>> the end of the paragraph.
> 
> fixed (I just remove the [ ]).
> 
>> Section 6.4: s/non-turn candidates/non-TURN candidates/
> 
> fixed
> 
>> Section 6.5: the phrase "Implementations MUST implement" seems awkward.
>> Perhaps "Implementations MUST support".  This appears several places.
> 
> fixed
> 
>> Section 6.5 ought to begin with "All data channels MUST be secured via
>> DTLS."  This appears half way through the section, but the material that
>> comes before is really in support of this sentence.
> 
> Eh - when I read that I thought - generic requirements and then ones for media and the data channels.
> 
>> Section 8.1 discusses "<user>@<domain>", but the discussion of "user"
>> (note the quotes) and the discussion of domain (note the absense of
>> quotes) are using different conventions.  Please use quotes in both
>> places or neither place.
> 
> I think I fixed this.
> 
>> There are places in this document where "settings" is confusing.  It is
>> unclear whether the word is referring to configuration settings or it
>> is referring to an environment or situation.  Please look at each use
>> of this word and consider alternatives.
> 
> I’ll leave this for ekr.
> 
> _______________________________________________
> rtcweb mailing list
> rtcweb@ietf.org
> https://www.ietf.org/mailman/listinfo/rtcweb