Subject: [Gen-art] Gen-ART Review of draft-ietf-behave-turn-uri-05

From: "Spencer Dawkins" <[email-masked]>
Date: Mon, 11 Jan 2010 17:07:41 -0600

I have been selected as the General Area Review Team (Gen-ART) reviewer for
this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html). Please resolve
these comments along with any other Last Call comments you may receive.

Document: draft-ietf-behave-turn-uri-05
Reviewer: Spencer Dawkins
Review Date: 2010-01-11
IETF LC End Date: 2010-01-13
IESG Telechat date: (not known)

Summary: This draft is almost ready for publication as a Proposed Standard. 
I have a couple of minor questions, shown below in sections 3 and 6.1.

Nits and readability comments aren't actually part of the review - they're 
for the author and editors...

Spencer

1.  Introduction

   The TURN specification [I-D.ietf-behave-turn] defines a process for a
   TURN client to find TURN servers by using DNS SRV resource records,
   but this process does not let the TURN server administrators
   provision the preferred TURN transport protocol between the client
   and the server and for the TURN client to discover this preference.

Spencer (readability): s/for/does not allow/ ?

   This document defines an S-NAPTR application [RFC3958] for this
   purpose.  This application defines "RELAY" as an application service
   tag and "turn.udp", "turn.tcp", and "turn.tls" as application
   protocol tags.

   Another usage of the resolution mechanism described in this document
   would be Remote Hosting as described in [RFC3958] section 4.4.  For
   example a VoIP provider who does not want to deploy TURN servers
   could use the servers deployed by another company but could still
   want to provide configuration parameters to its customers without
   explicitly showing this relationship.  The mechanism permits one to
   implement this indirection, without preventing the company hosting
   the TURN servers from managing them as it see fit.

Spencer (nit): s/see/sees/

   [I-D.petithuguenin-behave-turn-uri-bis] can be used as a convenient
   way of carrying the four components needed by the resolution
   mechanism described in this document.

3.  Resolution Mechanism

   An Allocate error response as specified in section 6.4 of
   [I-D.ietf-behave-turn] is processed as a failure as specified by
   [RFC3958] section 2.2.4.  The resolution stops when a TURN client
   gets a successful Allocate response from a TURN server.  After an
   allocation succeeds or all the allocations fail, the resolution
   context MUST be discarded and the resolution algorithm MUST be
   restarted from the beginning for any subsequent allocation.  Servers
   blacklisted as described in section 6.4 of [I-D.ietf-behave-turn]
   SHOULD NOT be used for the specified duration even if returned by a

Spencer (minor): I'm not sure why SHOULD NOT is appropriate here. If this 
isn't MUST NOT, I'm not sure it should be 2119 language. Is this just saying 
"if you include blacklisted servers, good things will probably not happen"?

   subsequent resolution.

   First the resolution algorithm checks that the parameters can be
   resolved with the list of TURN transports supported by the
   application:

Spencer (readability): I'm surprised that the following information is not 
shown as a table - Table 1 is showing something a lot more straightfoward, 
so I know you guys know how to create tables!

   o  If <secure> is false and <transport> is defined as "udp" but the
      list of TURN transports supported by the application does not
      contain UDP then the resolution MUST stop with an error.
   o  If <secure> is false and <transport> is defined as "tcp" but the
      list of TURN transports supported by the application does not
      contain TCP then the resolution MUST stop with an error.
   o  If <secure> is true and <transport> is defined as "udp" then the
      algorithm MUST stop with an error.
   o  If <secure> is true and <transport> is defined as "tcp" but the
      list of TURN transports supported by the application does not
      contain TLS then the resolution MUST stop with an error.
   o  If <secure> is true and <transport> is not defined but the list of
      TURN transports supported by the application does not contain TLS
      then the resolution MUST stop with an error.
   o  If <transport> is defined but unknown then the resolution MUST
      stop with an error.

   5.  If the first NAPTR query in the previous step does not return any
       result then the SRV algorithm defined in [RFC2782] is used to
       generate a list of IP address and port tuples.  The SRV algorithm
       is applied by using each transport in the filtered list of TURN
       transports supported by the application for the Protocol, <host>
       for the Name, "turn" for the Service if <secure> is false or
       "turns" for the Service if <secure> is true.  The same transport
       that was used to generate a list of tuples is used with each of
       this tuples for contacting the TURN server.  The SRV algorithm

Spencer (nit): s/this/these/

       recommends doing an A query if the SRV query returns an error or
       no SRV RR; in this case the default port declared in
       [I-D.ietf-behave-turn] for the "turn" SRV service name if
       <secure> is false, or the "turns" SRV service name if <secure> is
       true MUST be used for contacting the TURN server.  Also in this
       case, this specification modifies the SRV algorithm by
       recommending an A and AAAA query.  If the TURN client cannot
       contact a TURN server at any of the IP address and port tuples
       returned by the SRV algorithm with the transports from the
       filtered list then the resolution MUST stop with an error.

6.1.  RELAY Application Service Tag Registration

   Application Protocol Tag: RELAY

   Intended usage: See Section 3.

   Interoperability considerations: N/A

   Security considerations: See Section 5.

   Relevant publications: This document.

Spencer (minor): do you intend that the IANA replace the "This document" 
occurences with an RFC number before publication?

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Tue, 12 Jan 2010 10:19:38 -0800

Hi Spencer,

Thanks for the review.  See below for my comments.

On 01/11/2010 03:07 PM, Spencer Dawkins wrote:
> Summary: This draft is almost ready for publication as a Proposed
> Standard. I have a couple of minor questions, shown below in sections 3
> and 6.1.
> 
> Nits and readability comments aren't actually part of the review -
> they're for the author and editors...
> 
> Spencer
> 
> 1.  Introduction
> 

Applied.

> Spencer (nit): s/see/sees/

Applied.

> Spencer (minor): I'm not sure why SHOULD NOT is appropriate here. If
> this isn't MUST NOT, I'm not sure it should be 2119 language. Is this
> just saying "if you include blacklisted servers, good things will
> probably not happen"?

Hmmm.  The purpose of this text is to prevent a client to continuously try to
reach a crashed TURN server when the DNS cache will still continue to return the
RRs for the duration of the TTL.  I am not sure that this count as an
interoperability issue but I think that this is something that must be prevented
so unless someone complain I will change the "SHOULD NOT" to a "MUST NOT".

> Spencer (readability): I'm surprised that the following information is
> not shown as a table - Table 1 is showing something a lot more
> straightfoward, so I know you guys know how to create tables!
> 

Hmm, the problem is that rules #3 and #6 do not fit nicely in a table.  Actually
the purpose of "Table 1" is to be able to reference the rules easily in the
subsequent text, which is not the case for this set of rules.

> Spencer (nit): s/this/these/

Applied.

> occurences with an RFC number before publication?

No, I was thinking that the "this document" would reference the RFC after
publication, but I guess that your question means that the registration template
can be detached from the RFC and so needs to contain absolute references.

So I added the following to the text:

[Note to RFC Editor: Replace "This document" with reference to this document]

- -- 
Marc Petit-Huguenin
Personal email: [email-masked]
Professional email: [email-masked]
Blog: http://blog.marc.petit-huguenin.org

---

From: "Spencer Dawkins" <[email-masked]>
Date: Tue, 12 Jan 2010 14:11:31 -0600

Hi, Marc...

Works for me, and thank you for grokking what I was asking about (not 
replacing "this document" before PUBLICATION, but after publication and 
before creating the registry entry).

Thanks,

Spencer

----- Original Message ----- 
From: "Marc Petit-Huguenin" <[email-masked]>
To: "Spencer Dawkins" <[email-masked]>
Cc: <[email-masked]>; <[email-masked]>; "General 
Area Review Team" <[email-masked]>
Sent: Tuesday, January 12, 2010 12:19 PM
Subject: Re: Gen-ART Review of draft-ietf-behave-turn-uri-05

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Spencer,
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAktMvTcACgkQ9RoMZyVa61fRrQCeJRtQJ0rAfMOYVCPgHWZoBdba
> ha8An0uCiHQb/ijgzJwANuaskMVkzcz/
> =03SR
> -----END PGP SIGNATURE-----

---

From: "Spencer Dawkins" <[email-masked]>
Date: Mon, 18 Jan 2010 10:53:57 -0600

Russ asked me if -08 addressed my comments - it does, and does not introduce 
any new text that I needed to comment on. "Ready for publication as Proposed 
Standard", in my opinion.

Thanks!

Spencer

> isn't MUST NOT, I'm not sure it should be 2119 language. Is this just 
> saying "if you include blacklisted servers, good things will probably not 
> happen"?
>
>   subsequent resolution.
>
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Gonzalo Camarillo <[email-masked]>
Date: Mon, 18 Jan 2010 19:11:05 +0200

Thanks for your review, Spencer!

Gonzalo