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

[Elwyn Davies <[e-mail]>]

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:
> Hi Elwyn,
>
>   
>>>> 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:
>>     
>
> 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.
>>     
>> 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.
>>     
>
> 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
>