Re: [trill] Routing directorate review of draft-ietf-trill-channel-tunnel

[Alia Atlas <[e-mail]>]

Jonathan,

Thank you very much for your review and for following up with Donald to
resolve the concerns.

Donald,

Thanks for resolving these quickly.

Regards,
Alia

On Wed, May 4, 2016 at 11:57 AM, Donald Eastlake <[e-mail]> wrote:

> Hi Jonathan,
>
> Thanks for agreeing that the suggested changes resolve our comments.  I
> think they are pretty much editorial/clarification so I'll go ahead and
> post a revision incorporating them.
>
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  155 Beaver Street, Milford, MA 01757 USA
>  [e-mail]
>
> On Wed, May 4, 2016 at 7:29 AM, Jonathan Hardwick <
> [e-mail]> 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:[e-mail]]
>> Sent: 01 May 2016 21:46
>> To: Jonathan Hardwick <[e-mail]>
>> Cc: <[e-mail]> ([e-mail]) <[e-mail]>;
>> [e-mail]; [e-mail];
>> [e-mail]; [e-mail]
>> Subject: Re: Routing directorate review of draft-ietf-trill-channel-tunnel
>>
>> On Thu, Apr 28, 2016 at 9:26 AM, Jonathan Hardwick <
>> [e-mail]> 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   [phone] (cell)
>>  155 Beaver Street, Milford, MA 01757 USA  [e-mail]
>>
>
>