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-boun...@ietf.org [mailto:ipsec-boun...@ietf.org] On Behalf >Of pasi.ero...@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 _______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
