Thanks Donald, it looks good to me.
Best regards
Jon

From: Donald Eastlake [mailto:[email protected]]
Sent: 13 June 2016 04:34
To: Jonathan Hardwick <[email protected]>
Cc: <[email protected]> ([email protected]) <[email protected]>; 
[email protected]; [email protected]; [email protected]; 
[email protected]
Subject: Re: Routing directorate review of draft-ietf-trill-channel-tunnel

Hi Jonathan,

draft-ietf-trill-channel-tunnel-09 has been posted which I believe resolves 
your comments. It also has other changes in the security portion.

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [email protected]<mailto:[email protected]>

On Wed, May 4, 2016 at 7:29 AM, Jonathan Hardwick 
<[email protected]<mailto:[email protected]>> 
wrote:
Hi Donald,

Thanks for the replies - I agree with the changes you propose.  Please see 
discussion below (look for [JEH]).

Best regards
Jon

-----Original Message-----
From: Donald Eastlake [mailto:[email protected]<mailto:[email protected]>]
Sent: 01 May 2016 21:46
To: Jonathan Hardwick 
<[email protected]<mailto:[email protected]>>
Cc: <[email protected]<mailto:[email protected]>> 
([email protected]<mailto:[email protected]>) 
<[email protected]<mailto:[email protected]>>; 
[email protected]<mailto:[email protected]>; 
[email protected]<mailto:[email protected]>; 
[email protected]<mailto:[email protected]>;
 [email protected]<mailto:[email protected]>
Subject: Re: Routing directorate review of draft-ietf-trill-channel-tunnel

On Thu, Apr 28, 2016 at 9:26 AM, Jonathan Hardwick 
<[email protected]<mailto:[email protected]>> 
wrote:
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this
> draft. The Routing Directorate seeks to review all routing or
> routing-related drafts as they pass through IETF last call and IESG
> review, and sometimes on special request. The purpose of the review is
> to provide assistance to the Routing ADs. For more information about
> the Routing Directorate, please see
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs,
> it would be helpful if you could consider them along with any other
> IETF Last Call comments that you receive, and strive to resolve them
> through discussion or by updating the draft.
>
> Best regards
> Jon
> ===
>
> Document: draft-ietf-trill-channel-tunnel
> Reviewer: Jon Hardwick
> Review Date: 28 April 2016
> Intended Status: Standards Track
>
>
> Summary
>
> I have some concerns about this document and recommend that the
> Routing ADs discuss these issues further with the authors.
>
>
> Comments
>
> The draft is overall well written and the specification is quite easy
> to understand,

Thanks.

>                      but I found some of the terminology and rationale
> to be confusing.  I would prefer to see this clarified before the
> document is published as RFC.  Note that this is the first TRILL
> document I’ve reviewed, so my context comes largely from mailing list
> searches and the shepherd’s report.
>
>
> Major Comments
>
> The motivations for this draft are quite obscure from the perspective
> of the outsider J which makes it hard for me to evaluate the proposed
> mechanism.
>
> I think the problems that the draft solves should be more clearly
> spelled out.  From the introduction:
>
>    This document updates [RFC7178] and specifies extensions to RBridge
>    Channel that provide two additional facilities as follows:
>
>       (1) A standard method to tunnel a variety of payload types by
>           encapsulating them in an RBridge Channel message.
>
>       (2) A method to provide security facilities for RBridge Channel
>           messages.
>
> I think that number (1) requires more explanation because the RBridge
> channel already provides a standard method for a variety of payload
> types to be transmitted without needing the current draft.
> What tunneling capability is this draft adding?

Good point.

The RBridge Channel facility does provide a "protocol number" which is, in 
essence, the "type" of its payload. However, there are three limitations of 
RBridge Channel: (1) No security; (2) No way to leverage the many existing 
defined Ethertypes as a payload type; and
[JEH] OK, now I understand (2), thank you.  I thought maybe you'd allocate 
Chanel protocol numbers to match Ethertypes as needed, though I now see that 
this would be quite tedious process-wise (not to mention that Ethertype has 4 
additional bits). [/JEH]

(3) RBridge Channel can only send typed messages either (3a) between RBridges 
in a campus and (3b) between end stations and RBridges on the same link. 
Earlier versions of this draft included mechanisms extensions in area 3, for 
example, for sending RBridge Channel messages between end stations and RBridges 
not on the same link; however, this added significant complexity and there 
appears to be no current need for such extensions so they were dropped, leaving 
only extensions in areas 1 and 2.

[JEH] OK.  Number 3 does sound a bit more like tunnelling than 1 or 2.  Helps 
to have the history, thanks. [/JEH]

How about the following change on additional facility 1 in the draft:

OLD
      (1) A standard method to tunnel a variety of payload types by
          encapsulating them in an RBridge Channel message.
NEW
      (1) A standard method to tunnel payloads whose type may be
          indicated by Ethertype through encapsulation in RBridge  Channel 
messages.

[JEH] Yes, looks good. [/JEH]

> A significant amount of text in the draft discusses number (2), which
> secures the channel payload, presumably to cover cases where the
> payload has no in-built security mechanism.  This appears to be the
> major purpose of the draft.  The draft achieves number (2) by adding a
> security shim header between the RBridge channel header and the
> payload.  One consideration in doing this is to remain backwards
> compatible with RFC 7178, and it looks like the working group has
> decided to achieve backwards compatibility by defining a new RBridge
> channel protocol type called “channel tunnel” – where this effectively
> means the RBridge channel payload contains an additional security shim
> which in turn contains an identifier that determines the real payload
> protocol type.
>
> I find the term “channel tunnel” misleading, as the draft does not
> appear to add any additional tunnelling capability above and beyond
> the tunnelling that can already be done using RFC 7178.  The draft
> actually describes an RBridge channel with enhanced security, so a
> term like “secure channel” would make more sense to me than “channel
> tunnel”.

OK, I understand why you think that term is misleading. While it seems quite 
reasonable to called the added fields a "shim", note that the facility 
currently called "Channel Tunnel" is quite closely integrated with the existing 
RFC 7178 RBridge Channel facility. For example, there is only one error 
reporting mechanism. Errors in the "Channel Tunnel" facility added by this 
draft are reported as if they were errors in the RBridge Channel messages to 
which the "shim" was added.

I don't actually like your suggestion of "secure channel" as a new name.  How 
about re-naming the facility being added by this draft as the "RBridge Channel 
Header Extension"?

[JEH] OK, I like RBridge Channel Header Extension. [/JEH]

I believe that RFC 7783 and only that RFC that references this draft using the 
term "Channel Tunnel" but this is a very minor informational passing reference. 
There are drafts in the publication requested state that reference this draft 
using the term "Channel Tunnel" but it seems that it would be relatively 
straightforward to change the name to "RBridge Channel Header Extension" or 
some other new name in those drafts and even easier to change it in drafts 
still under the control of the TRILL WG.

[JEH] Thanks, this works for me. [/JEH]

> Minor Comments
>
> Section 3.1 – “Any particular use of the Null Payload should specify
> what VLAN or priority should be used when relevant.” – is unclear and
> no context for this statement is given.  Should be used by what and
> for what purpose?

OK. How about:

   Any particular use of the Null Payload should specify what VLAN or
   FGL and what priority should be used in the inner data label of the
   RBridge Channel message (or in an outer VLAN tag for the native
   RBridge Channel message case) when those values are relevant.

[JEH] Fine [/JEH]

> Section 4.3 feels like a corollary to section 4.5 and so may be better
> placed as a subsection of 4.5.

The method of deriving keying material given in Section 4.3 is also used in 
DTLS security as mentioned in Section 4.6 so I think it should remain a 
separate section.

[JEH] OK [/JEH]

> Section 4.6 “The PType indicates the nature of the application_data.”
> - is potentially open to misinterpretation.  At face value it sounds
> like you are leaking some potentially sensitive information about the
> “nature” of the encrypted payload.  I think all you are actually
> saying is that it indicates whether the payload is an Ethertype, an
> Ethernet frame etc.  Suggest instead “In this case, the PType value in
> the RBridge Channel Tunnel Protocol Specific Data applies to the
> decrypted application_data.”

OK.

> Section 5.2 “with a payload type (PType) indicating a nested RBridge
> Channel message” – strictly all the PType can indicate is that the
> payload is Ethertyped; on its own it cannot indicate a nested RBridge
> Chanel message.  Suggest “and it contains a nested RBridge Chanel
> message”.

OK.

> Section 6.2
>
> “Section xxx of [RFC 7178]” should be “Section 3.2 of [RFC 7178]”.

Right. Sorry about that.

> Don’t you also need a new IANA registry for the “Rbridge Channel Error
> Subcodes” listed in table 5.2?

My opinion is that, for the first document in which you specify a field and 
some values, it is a judgment call whether you should create an IANA registry 
or not.  If you expect multiple groups to start requesting values to multiple 
purposes, then creating a registry from the start is the way to go. On the 
other hand, if a field is internal to a particular protocol and you don't 
expect any new field values to be assigned until there is a significant 
extension of that protocol, I don't see any problem in deferring the registry 
creation to the second document. This is the second document assigning values 
for RBridge Channel Error Codes so it creates a registry for them. It does not 
create a registry for SubERR field values.

[JEH] OK, just checking, and happy to defer to your judgment here. [/JEH]

> Nits
>
> Section 3.2
>
> “as describe in” -> “as described in”

OK.

> Section 4
>
> “not to met” -> “not to meet”

OK.

> 2nd paragraph – this sentence is quite long and hard to parse.

You're right. Looking at the sentence, it seems fairly easy to simplify and 
split into two sentences. How about the following
replacement:

   The Channel Tunnel DTLS based security specified in Section 4.6
   below is intended for pairwise (known unicast) use. That is, the
   case where the M bit in the TRILL Header is zero and any
   Outer.MacDA is individually addressed.

[JEH] Looks good. [/JEH]

> Section 4.2 & Section 5.1
>
> “As show in” - > “As shown in”

OK.

> Section 4.3
>
> “The use Derived Material” -> “The use of the Derived Material”

OK.

> Does Derived Material really need to be capitalized in this section?

Well, it is capitalized in the equation. Seems to me reasonable to capitalize 
in both cases to indicate that a specific type of Derived Material is being 
talked about.

[JEH] OK. [/JEH]

> Section 4.5
>
> “can reasonable be” -> “can reasonably be”

OK.

> Section 4.6
>
> “minimum MTU Sz” -> “minimum MTU size”

Sz is a standard TRILL symbol widely used in TRILL documents and defined in 
Section 1.1 of this draft. I would prefer to make the following change in 
Section 4.6: "the TRILL campus wide minimum MTU Sz" -> "Sz".

[JEH] OK - sorry, I missed the definition! [/JEH]

> “Actual application_data sent with Channel Tunnel” -> “Actual
> application_data sent within the Channel Tunnel”

OK.

> Why do you say “application_data” not “application data”?

"application_data" is the name of the field type in DTLS.

[JEH] OK. [/JEH]

> Appendix Z should presumably be removed prior to IETF last call.

While I'm not sure how much help it would be to IESG members or IETF 
participants in reviewing the draft, I don't see any reason for Appendix Z to 
be removed until RFC publication. But at least a notation asked the RFC Editor 
to delete it should be added.

[JEH] OK - please add the RFC editor note. [/JEH]

Thanks,
Donald
=============================
 Donald E. Eastlake 3rd   +1-508-333-2270<tel:%2B1-508-333-2270> (cell)
 155 Beaver Street, Milford, MA 01757 USA  
[email protected]<mailto:[email protected]>

_______________________________________________
trill mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/trill

Reply via email to