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

["Grewal, Ken" <[e-mail]>]

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: [e-mail] [mailto:[e-mail]] On Behalf
>Of [e-mail]
>Sent: Thursday, September 17, 2009 6:05 AM
>To: [e-mail]
>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
>[e-mail]
>https://www.ietf.org/mailman/listinfo/ipsec