Subject: Genart last call review of draft-ietf-rtcweb-security-arch-18

From: Russ Housley <[email-masked]>
Date: Sat, 09 Feb 2019 10:50:48 -0800

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.

Section 4.2: Please add a sentence or two that defines Interactive
Connectivity Establishment (ICE) data and non-ICE data.

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.

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.

Section 10: since this is a standards track document, the IESG should
be responsible for this new codepoint, not the document author.

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.

Section 6.2 uses [email-masked]  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).

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.

Nits:

Section 1 includes: "... SDP-based like SIP."  Please add a reference
for SDP.

Section 4.1: s/ permissions till later/ permissions until later/

Section 4.4: please add a reference for STUN.

Section 6.2: s/(though see Section 6.3/(See Section 6.3/

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.

Section 6.4: s/non-turn candidates/non-TURN candidates/

Section 6.5: the phrase "Implementations MUST implement" seems awkward.
Perhaps "Implementations MUST support".  This appears several places.

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.

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.

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.

---

From: Sean Turner <[email-masked]>
Date: Wed, 20 Feb 2019 22:09:00 -0500

I generated PR for these:
https://github.com/rtcweb-wg/security-arch/pull/85

> On Feb 9, 2019, at 13:50, Russ Housley <[email-masked]> wrote:
> 
> Reviewer: Russ Housley

There is s9.1 that addresses this :)

> 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.

> 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.

> too awkward to actually get the information on the screen while the

Would something like this work:

  These are compared by the peers to authenticate one another.

> be responsible for this new codepoint, not the document author.

changed

> 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

> statement on examples (the URL is above).

Changed it to [email-masked]

> (the URL is above).  It seems best to use the same names here as are

I changed to “a poker site” to match that phrase, which is used in the 1st para of that section.

> Section 1 includes: "... SDP-based like SIP."  Please add a reference

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

> the end of the paragraph.

fixed (I just remove the [ ]).

> Section 6.4: s/non-turn candidates/non-TURN candidates/

fixed

> Perhaps "Implementations MUST support".  This appears several places.

fixed

> 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.

> quotes) are using different conventions.  Please use quotes in both

I think I fixed this.

> of this word and consider alternatives.

I’ll leave this for ekr.