I generated PR for these: https://github.com/rtcweb-wg/security-arch/pull/85 > On Feb 9, 2019, at 13:50, Russ Housley <housley@xxxxxxxxxxxx> 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@xxxxxxxx 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@xxxxxxxxxxx > 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.
