Re: [IPsec] AD review comments for draft-ietf-ipsecme-traffic-visibility

"Grewal, Ken" <ken.grewal@intel.com> Fri, 18 September 2009 23:06 UTC

Return-Path: <ken.grewal@intel.com>
X-Original-To: ipsec@core3.amsl.com
Delivered-To: ipsec@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 2292B3A67EE for <ipsec@core3.amsl.com>; Fri, 18 Sep 2009 16:06:25 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -6.599
X-Spam-Level:
X-Spam-Status: No, score=-6.599 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, RCVD_IN_DNSWL_MED=-4]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id duqu6+XHnRPv for <ipsec@core3.amsl.com>; Fri, 18 Sep 2009 16:06:24 -0700 (PDT)
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by core3.amsl.com (Postfix) with ESMTP id 44FD53A677E for <ipsec@ietf.org>; Fri, 18 Sep 2009 16:06:23 -0700 (PDT)
Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 18 Sep 2009 16:02:54 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.44,412,1249282800"; d="scan'208";a="728268162"
Received: from rrsmsx603.amr.corp.intel.com ([10.31.0.57]) by fmsmga001.fm.intel.com with ESMTP; 18 Sep 2009 16:10:17 -0700
Received: from rrsmsx601.amr.corp.intel.com (10.31.0.151) by rrsmsx603.amr.corp.intel.com (10.31.0.57) with Microsoft SMTP Server (TLS) id 8.1.358.0; Fri, 18 Sep 2009 17:07:18 -0600
Received: from rrsmsx505.amr.corp.intel.com ([10.31.0.36]) by rrsmsx601.amr.corp.intel.com ([10.31.0.151]) with mapi; Fri, 18 Sep 2009 17:07:17 -0600
From: "Grewal, Ken" <ken.grewal@intel.com>
To: "Pasi.Eronen@nokia.com" <Pasi.Eronen@nokia.com>, "ipsec@ietf.org" <ipsec@ietf.org>
Date: Fri, 18 Sep 2009 17:06:17 -0600
Thread-Topic: AD review comments for draft-ietf-ipsecme-traffic-visibility
Thread-Index: Aco3l3kkjbL+0N0PT/mqG6eTSIFVVABGuivQ
Message-ID: <C49B4B6450D9AA48AB99694D2EB0A483325793BA@rrsmsx505.amr.corp.intel.com>
References: <808FD6E27AD4884E94820BC333B2DB773C06B22D72@NOK-EUMSG-01.mgdnok.nokia.com>
In-Reply-To: <808FD6E27AD4884E94820BC333B2DB773C06B22D72@NOK-EUMSG-01.mgdnok.nokia.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [IPsec] AD review comments for draft-ietf-ipsecme-traffic-visibility
X-BeenThere: ipsec@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Discussion of IPsec protocols <ipsec.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/ipsec>, <mailto:ipsec-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/ipsec>
List-Post: <mailto:ipsec@ietf.org>
List-Help: <mailto:ipsec-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ipsec>, <mailto:ipsec-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 18 Sep 2009 23:06:25 -0000

Hi Pasi, 
Many thanks for the great feedback. I will incorporate all these items as part of the WESP update during the next virtual interim meeting on Sept 22. Furthermore, I have opened multiple tickets to ensure these are tracked and resolved. 

Some comments inline...and others will result from the discussion during the interim meeting.

Thanks, 
- Ken
 

>-----Original Message-----
>From: ipsec-bounces@ietf.org [mailto:ipsec-bounces@ietf.org] On Behalf
>Of Pasi.Eronen@nokia.com
>Sent: Thursday, September 17, 2009 6:05 AM
>To: ipsec@ietf.org
>Subject: [IPsec] AD review comments for draft-ietf-ipsecme-traffic-
>visibility
>
>I've now done my AD review for draft-ietf-ipsecme-traffic-visibility-08.
>I have two substantive comments, and a bunch of minor
>clarifications/nits.
>The substantive comments first:
>
>- A question: did the WG discuss the pros and cons of integrity
>protecting the WESP header? (This does make WESP more complex to
>implement, and currently the WESP header does not contain any data
>that would benefit from integrity protection in any way.)
[Ken] This change was the result of a discussion on threats posed by 'malware', which could modify the WESP headers to obfuscate the payload from inspection by intermediate nodes such as IDS/IPS systems. 
The issue (ticket #104) was raised and closed some time back after lengthy discussions on the topic. 
http://trac.tools.ietf.org/wg/ipsecme/trac/ticket/104


>
>- IPv6 requires extension headers to be aligned on 8-octet boundaries,
>and I believe this requirement applies to ESP, too (see e.g. RFC 4303
>Section 2.3, 2nd paragraph). All current ESP specs (all encryption
>algorithms, UDP encapsulation, etc.) meet the 8-octet alignment
>requirement -- but adding a new four-octet header there obviously
>breaks it.

[Ken] Great point! Yes, this will need to be addressed and we do need to provide an extension of the header for alignment purposes in IPv6 usages. 
I have opened a new ticket (#109) to track this. 

>
>Some minor clarifications/editorial nits:
>
>- The text currently uses "using ESP-NULL [RFC2410]" and "unencrypted"
>as synonyms. This was accurate before RFC4543, but is not any more.
>This needs some clarifying text somewhere (perhaps Section 1).
>
>- Section 1 needs a sentence or two motivating the existence of the
>"E" bit -- currently it comes as a surprise to the reader later.

[Ken] I have reopened the related ticket #84 and we will generate / vet additional text to elaborate on the motivation.


[Ken] I have captured the items below in a single new ticket (#110) as most are minor editorial changes. 

>
>- Section 2/2.1: In Figures 1, 2, and 3, the bit numbers should be
>shifted one character to the right.
>
[Ken] OK

>- Section 2: In some parts of the text, the last 8 bits of the WESP
>header are called "Flags"; in other parts, the last 5 bits are called
>"Flags". I'd suggest changing e.g. Figure 2 so that last 5 bits are
>called "Rsvd" or something.

[Ken] Agreed, using Rsvd is better than Flags. 
>
>- Section 2: "The bits are defined LSB first, so bit 0 would be the
>least significant bit of the flags octet." This doesn't match the bit
>numbering in Figure 2 (where bit 0 is the most significant bit).
>However, the bit numbers are not really used anywhere, so I would just
>suggest deleting this sentence.
>
[Ken] Agree. We already have new text in the next rev of the draft using MSB, as Tero had separately raised this point. 

>- Section 2: It would be helpful if the text explicitly said that
>HdrLen values less than 12 are invalid (and probably HdrLen values
>that are not multiple of 4 are invalid, and multiple of 8 for IPv6
>case).
>
[Ken] OK. 

>- Section 2: the text about TrailerLen is a bit unclearly written --
>the offset from the end of the packet to the last byte of the payload
>would be a negative number. I'd suggest phrasing this something like
>the "The number of bytes following the payload data in the packet, in
>octets: i.e. the total length of TFC Padding (normally not present for
>unencrypted packets), ESP Trailer (Padding, Pad Length, Next Header),
>and ESP ICV."
>
[Ken] Will discuss the scope of this, based on separate comment from Tero. 

>- Section 2: "the packet must be dropped" -> "the packet MUST be
>dropped"
>
>- The figures in 2.2.1 and 2.2.2 are very confusing, since they
>suggest WESP could be applied as a separate step after ESP processing
>(that was possible in some earlier versions of the draft, but not any
>more since ICV covers the WESP header). Since they don't really
>present much new compared to the figures in RFC 4303 Section
>3.1.1/3.1.2, perhaps they could be omitted? Or if we want to keep
>them, they probably should show the packet before applying ESP?

[Ken] Someone (do not recall who) had asked for these figures. But, I agree that it is misleading. So, propose we either remove these figures or change the 'before' diagrams to raw, instead of ESP.

>
>- Section 3: s/IPSec/IPsec/
>
>- Section 4: this section is missing the allocation of SPI value 2
>to indicate WESP from the "SPI Values" registry.
>
>- Section 4 should say that for the WESP Version Number, the
>unassigned values are 1, 2, and 3.
>
>- Section 6: [RFC4306], [RFC3948], and [RFC5226] should be normative
>references, not informative.

[Ken] OK.
>
>Best regards,
>Pasi
>_______________________________________________
>IPsec mailing list
>IPsec@ietf.org
>https://www.ietf.org/mailman/listinfo/ipsec