Subject: Re: [Gen-art] Gen-art reiew of draft-ietf-rohc-ipsec-extensions-hcoipsec-05

From: Elwyn Davies <[email-masked]>
Date: Thu, 19 Nov 2009 13:55:52 +0000

Ertekin, Emre [USA] wrote:
> Hi Elwyn,
>
> Thank you for taking the time to review and comment on our draft; sorry for our late reply.  Please see our responses below.
>  
>   
>> Summary: Almost ready.  There should probably be a formal ASN.1
>> description
>> of the additional fields in the 'Processing' component of the SPD as
>> well as
>> the informal textual description to match RFC 4301. I believe this
>> document
>> should be specified as updatinbg RFC 4301 (IPsec).
>>
>> Major issues: None.
>> Minor issues:
>> Status:  Presumably this document should be classified as updating RFC
>> 4301
>> since it modifies the SPD etc.
>>     
>
> Although we are modifying the SPD, I would consider these as (optional) extensions rather than updates to the base IPsec RFC.  Further, we are not changing any of the text of RFC 4301.  Therefore, we would prefer to leave the draft as-is.
>
> Please let us know if this is ok with you.
>   
The term 'optional' seems inappropriate.  The draft uses 'must'  (should 
thus be 'MUST'?) in s2.1 when describing things that need to be added  
to the SPD if ROHC is in use.  Be that as it may, this is indeed an 
extension.  From the point of view of implementors, having an explicit 
pointer to the RFC which this draft will become from RFC 4301 would help 
people find things they might need to implement.  Also if one was making 
RFC 4301bis it would probably be good to merge in this work.  It would 
probably be better if one had an 'extends' category rather than just 
'updates' but we don't.  If your AD is happy that this is not an 
'updates' then I'll live with it.
>  
>   
>> SPD additions:
>> In Section 2.1 some additional fields to be added to the SPD
>> Processing component are described informally.  RFC 4301 which has the
>> unmodified version of the  Processing component also has a formal ASN.1
>> description.  I think this document needs an updated  ASN.1 desciption
>> also.
>>
>> Further the description talks about adding 'processing info' fields if
>> the
>> 'processing info field is set to PROTECT'.  Although this partly
>> reflects the
>> wording in the body of RFC 4301, it is rather confusing when looking at
>> the
>> ASN.1.  The wording of para 3 of S2.1 might be better as:
>>   If the SPD entry is an IPsecEntry (to PROTECT traffic) then the
>> Processing
>>   item of the IPsecEntry must be extended with the following items:
>>     
>
>   
On refelection, I agree that using IPsecEntry in s2.1 wouldn't be 
appropriate.  However I think the wording could still be improved.  Retry:
   If the processing action associated with the selector sets is 
PROTECT, then the processing info must be
   extended with the following ROHC channel parameters:
> A while back, we were thinking of adding ASN.1 notation to the draft...however we opted not to.  This is because Appendix C in RFC 4301 is only an example ASN.1 definition of a SPD and is referred to as "merely illustrative".  Since our extensions would've expanded upon/augmented this non-normative text, we decided to leave it out.
>   
Sure, it is illustrative, but it also seems to have specified a fully 
qualified OID.  This indicates to me that at least some people may be 
using this as an access mechanism for the SPD. In that case  formally 
specifying the ASN.1 would be highly desirable even if it is not 
normative.  Doubtless there are other people who know if the SPD is 
accessed this way (e.g. as a MIB) even if I don't.

Regards,
Elwyn
> BR,
> Emre

---

From: Elwyn Davies <[email-masked]>
Date: Mon, 30 Nov 2009 11:07:55 +0000

Hi.

This seems good to me.  One minor point on the ASN.1:  I think the extra
'rohc' piece should it be specified as 'OPTIONAL' so that not it doesn't
have to be in every implementation - and should it come after the tunnel
params as this change will alter the OID of tunnel params as it stands
(if my understanding of ASN.1 is right)?

Thanks.
Elwyn

Ertekin, Emre [USA] wrote:
>>>> Processing component are described informally.  RFC 4301 which has
>>>>         
>> the
>>     
>>>> unmodified version of the  Processing component also has a formal
>>>>         
>> ASN.1
>>     
>>>> description.  I think this document needs an updated  ASN.1
>>>>         
>> desciption
>>     
>>>> also.
>>>>
>>>> Further the description talks about adding 'processing info' fields
>>>>         
>> if
>>     
>>>> the
>>>> wording in the body of RFC 4301, it is rather confusing when looking
>>>>         
>> at
>>     
>>>> the
> OK; sounds good!
>>> A while back, we were thinking of adding ASN.1 notation to the
>>>       
>> draft...however we opted not to.  This is because Appendix C in RFC
>> 4301 is only an example ASN.1 definition of a SPD and is referred to as
>> "merely illustrative".  Since our extensions would've expanded
>> upon/augmented this non-normative text, we decided to leave it out.
>>     
> OK.  We developed the ASN.1 representation of the ROHC parameters and plan to incorporate it as an Appendix to the draft.  The text reads as follows:
>
> Appendix A.  ASN.1 representation for ROHCoIPsec
>
>    This appendix is included as an additional way to describe the
>    ROHCoIPsec parameters that are included in the IPsec SPD.  It uses
>    portions of the ASN.1 syntax provided in Appendix C of RFC 4301
>    [IPSEC].  In addition, several new structures are defined.
>
>    This syntax has been successfully compiled.  However, it is merely
>    illustrative and need not be employed in an implementation to achieve
>    compliance.
>
>    The "Processing" data structure, defined in Appendix C of RFC 4301,
>    is augmented to include a ROHC parameters element as follows:
>
>          Processing ::= SEQUENCE {
>              extSeqNum   BOOLEAN, -- TRUE 64 bit counter, FALSE 32 bit
>              seqOverflow BOOLEAN, -- TRUE rekey, FALSE terminate & audit
>              fragCheck   BOOLEAN, -- TRUE stateful fragment checking,
>                                   -- FALSE no stateful fragment checking
>              lifetime    SALifetime,
>              spi         ManualSPI,
>              algorithms  ProcessingAlgs,
>              rohc        RohcParams,
>              tunnel      TunnelOptions OPTIONAL
>          }
>
>    The following data structures describe these ROHC parameters:
>
>        RohcParams ::= SEQUENCE {
>            rohcEnabled         BOOLEAN, --  TRUE, hdr compression is enabled
>                                         -- FALSE, hdr compression is disabled
>            maxCID              INTEGER (0..16383),
>            mrru                INTEGER,
>            profiles            RohcProfiles,
>            rohcIntegAlg        RohcIntegAlgs,
>            rohcIntegICVLength  INTEGER
>            }
>
>        RohcProfiles ::= SET OF RohcProfile
>
>        RohcProfile ::= INTEGER {
>            rohcv1-rtp           (1),
>            rohcv1-udp           (2),
>            rohcv1-esp           (3),
>            rohcv1-ip            (4),
>
>            rohcv1-tcp           (6),
>            rohcv1-rtp-udpLite   (7),
>            rohcv1-udpLite       (8),
>
>            rohcv2-rtp         (257),
>            rohcv2-udp         (258),
>            rohcv2-esp         (259),
>            rohcv2-ip          (260),
>
>            rohcv2-rtp-udpLite (263),
>            rohcv2-udpLite     (264)
>            }
>
>        RohcIntegAlgs ::= SEQUENCE {
>            algorithm   RohcIntegAlgType,
>            parameters  ANY -- DEFINED BY algorithm -- OPTIONAL }
>
>        RohcIntegAlgType ::= INTEGER {
>            none                    (0),
>            auth-HMAC-MD5-96        (1),
>            auth-HMAC-SHA1-96       (2),
>            auth-DES-MAC            (3),
>            auth-KPDK-MD5           (4),
>            auth-AES-XCBC-96        (5),
>            auth-HMAC-MD5-128       (6),
>            auth-HMAC-SHA1-160      (7),
>            auth-AES-CMAC-96        (8),
>            auth-AES-128-GMAC       (9),
>            auth-AES-192-GMAC      (10),
>            auth-AES-256-GMAC      (11),
>            auth-HMAC-SHA2-256-128 (12),
>            auth-HMAC-SHA2-384-192 (13),
>            auth-HMAC-SHA2-512-256 (14)
>            --  tbd (15..65535)
>            }
>
> I think that does it!  Let me know if this looks good to you. 
>
> V/R,
> Emre
>

---

From: Elwyn Davies <[email-masked]>
Date: Mon, 30 Nov 2009 19:30:31 +0000

Hi.

Last minute thought... the profile identifiers tie up with the profile 
numbers in RFC 4996 and RFC 5225 - it would be worth noting that.  I 
can't see where the authentication algorithm identifiers tie up with - I 
presume there must be some place these various algorithms are specified 
for ROHC.  It ought to be referenced.

Regards,
Elwyn

Ertekin, Emre [USA] wrote:
> Hi Elwyn,
>
> Great.  I agree with both of your points.  I will make the updates to the draft.  
>
> Once I make these changes, I will consider this thread/comment as resolved.
>
> BR,
> Emre
>
>   
>> -----Original Message-----
>> From: Elwyn Davies [mailto:[email-masked]]
>> Sent: Monday, November 30, 2009 3:08 AM
>> To: Ertekin, Emre [USA]
>> Cc: General Area Reviwing Team; Mary Barnes; [email-masked];
>> [email-masked]; [email-masked]; Christou, Christos [USA]
>> Subject: Re: Gen-art reiew of draft-ietf-rohc-ipsec-extensions-
>> hcoipsec-05
>>
>> Hi.
>>
>>>>>> Further the description talks about adding 'processing info'
>>>>>>             
>> fields
>>     
>>>> if
>>>>
>>>>         
>>>>>> the
>>>>>> wording in the body of RFC 4301, it is rather confusing when
>>>>>>             
>> looking
>>     
>>>> at
>>>>
>>>>         
>>>>>> the
>>>>>>   item of the IPsecEntry must be extended with the following
>>>>>>             
>> items:
>>     
>>>>>>             
>>>> 4301 is only an example ASN.1 definition of a SPD and is referred to
>>>>         
>> as
>>     
>>>> Sure, it is illustrative, but it also seems to have specified a
>>>>         
>> fully
>>     
>>>> qualified OID.  This indicates to me that at least some people may
>>>>         
>> be
>>     
>>>> using this as an access mechanism for the SPD. In that case
>>>>         
>> formally
>>     
>>> OK.  We developed the ASN.1 representation of the ROHC parameters and
>>>       
>> plan to incorporate it as an Appendix to the draft.  The text reads as
>> follows:
>>     
>>>    illustrative and need not be employed in an implementation to
>>>       
>> achieve
>>     
>>>    compliance.
>>>
>>>    The "Processing" data structure, defined in Appendix C of RFC
>>>       
>> 4301,
>>     
>>>              extSeqNum   BOOLEAN, -- TRUE 64 bit counter, FALSE 32
>>>       
>> bit
>>     
>>>              seqOverflow BOOLEAN, -- TRUE rekey, FALSE terminate &
>>>       
>> audit
>>     
>>>                                   -- FALSE no stateful fragment
>>>       
>> checking
>>     
>>>              lifetime    SALifetime,
>>>              spi         ManualSPI,
>>>            rohcEnabled         BOOLEAN, --  TRUE, hdr compression is
>>>       
>> enabled
>>     
>>>                                         -- FALSE, hdr compression is
>>>       
>> disabled
>>