Re: [I2nsf] [Last-Call] New Version Notification for draft-ietf-i2nsf-sdn-ipsec-flow-protection-11.txt

Hi Tom:

Thank you very much again. See our comments inline:

> El 29 oct 2020, a las 18:06, tom petch <daedulus@btconnect.com> escribió:
> 
> Back to routine comments on this I-D; I don't think that any of my earlier ones are unresolved.
> 
> On 29/10/2020 11:23, Rafa Marin-Lopez wrote:
>> Hi Tom:
>> 
>>> El 28 oct 2020, a las 19:02, tom petch <daedulus@btconnect.com> escribió:
>>> 
>>> On 28/10/2020 10:42, Rafa Marin-Lopez wrote:
>>>> Hi Tom:
>>>> 
>>>> Thank you very much for your insight. It is very helpful. Please see our comments/questions inline.
>>>> 
>>>>> El 27 oct 2020, a las 13:42, tom petch <daedulus@btconnect.com> escribió:
>>>>> 
>>>>> I think that the IESG will find a number of problems with this I-D.
>>>>> 
>>>>> YANG module references RFC822 which is several years out of date
> 
> Rafa
> 
> Several YANG leaf are in seconds, eight I think, but none use the YANG
> units seconds;
> statement.  I suggest adding it.

Added. They are seven.
> 
> typedef ipsec-spd-action
> capitalises the actions as does RFC4301 but later uses of this have lower case actions; probably ok as it is

Ok.
> 
> protocol number appears in several places but technically, IPv6, which we must all love and promote, has 'next header' instead.  Probably worth a mention at least once in the YANG module

We have included this note:

NOTE: In case of IPv6, the protocol in the IP packet payload is
specified in the Next Header field of the IPv6 
packet.";
> 
> On the start >= end, I was looking at nat-yang and it has
> must ". >= ../start-port-number"
> which is the sort of YANG constraint I had in mind

Done. 

must '. >= ../start’

> 
> leaf dscp-mapping
> I do not understand, a hex string of indeterminate length for DSCP values (plural).  I expect DSCP to be six bit and values to be a leaf-list

Right. Since we have inet:dscp type , we have used that one in a leaf-list as you recommend.

leaf-list dscp-mapping { 
       type inet:dscp;
       default 0;
       description 
         "DSCP values allowed for packets carried over
         this IPsec SA."; 
       reference
         "Section 4.4.2.1. in RFC 4301.";
     }

> 
> leaf ecn
> /Annex C/Appendix C/

Fixed.
> 
> I see that RFC8247 gives AEAD status which is a good reference to have except that as my other post today says there is no requirement for any future additions to the IANA data to include this information; IANA for IKEv2 needs updating iun a separate I-D!
> 
> leaf mask
> hex-string with no length specified

Since this masked is after all 32-bits mask we have used uint32 instead.
> 
> leaf ds-algorithm
> default 1
> which I think is RSA; worth stating as it might raise some AD eyebrows

Clarified in the description.
> 
> pfs-groups
> /it is required perfect forward secrecy/perfect forward secrecy is required/

Fixed.

> and a reference to Transform Type 4?

As we mention earlier, this pfs-groups used the type pfs-group, which includes the reference to Transform Type 4.
> 
> leaf ext-seq-num
> /FALSE/false/ or /False/?
> 
> leaf encryption-algorithm (twice, in separate containers)

Mmmmm… we only see in one place.

> /the integrity leaf/ the integrity-algorithm leaf/

Fixed.
> 
> I have yet to look at the text or the examples, probably next week.

Thank you very much. We are going to post -12 with all these comments and the previous ones so far. If there are pending comments we can prepare -13 right away.

Best Regards.
> 
> Tom Petch
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>>> 
>>> On boiler plate, I mean the reference to RFC2119 which now must use the language from RFC8174 in the body of the I-D; sorry for the confusion.
>> 
>> Ah ok. I guess you are referring to change that paragraph with this:
>> 
>> 
>> The key words "MUST", "MUST NOT", "REQUIRED",
>> "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT",
>> "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
>> "OPTIONAL" in this document are to be interpreted
>> as described in BCP 14 RFC2119 RFC8174 when,
>> and only when, they appear in all capitals,
>> as shown here.
>> 
>>> 
>>> On XXXX, you have XXXX standing in for more than one RFC-to-be which confuses.  The convention is to use XXXX for this I-D and then AAAA BBBB etc for any others such as the netconf I-D in this instance.
>> 
>> Ah ok. In any case, we have now only XXXX for our RFC-to-be so problem fixed.
>>> 
>>> Two big issues, for me (perhaps not for others).  The convention with YANG is for each successive line to be indented two characters, you have four, which creates a lot of white space and pushes the text to the right hand margin.  I think that two characters is the default when you use pyang to format a YANG module.
>> 
>> We can try to reduce it to two characters.
>> 
>>> 
>>> And references.  I have had to work harder than I want to to make sense of the IANA references.  I think you should have five separate references in the I-D for IANA for
>>> Transform Type 1
>>> Transform Type 3
>>> Transform Type 4
>>> Authentication Method
>>> Protocol Numbers
>>> and each reference in the I-D should have a URL pointing to the specific section of IANA web site.
>> 
>> Ok, good. This follows what we thought.
>> 
>>> In the YANG, it is harder to know what to do.  Those first three references are in the third tier i.e.
>>> Group - Internet Key Exchange V2 (IKEv2) Parameters
>>> Registry  -Transform Attribute Types
>>> and then Type 1, 3, 4 as the third tier as I am calling it
>>> and I think that every reference in the YANG should give me all three tiers after IANA in that order perhaps
>>> IANA; IKEv2 Parameters; Transform Atribute Types; Transform Type 1
>> 
>> Great. We also considered this as a possible solution after sending our e-mail. Let’s do it.
>> 
>>> If the syntax needs tweeking, then the RFC Editor will do a good job of that but at present the references are inconsistent in which elements are specified in what order and that is something the RFC Editor probably cannot cope with.
>>> Authentication Method is a registry so that just needs Group name and Registry name after IANA.
>> 
>> Ok, good.
>>> 
>>> Some minor glitches.
>>> I-D appears twice in the body of the I-D - perhaps document or memo.
>> 
>> Ok.
>> 
>>> objetives/objectives/
>> 
>> Fixed.
>>> end port number perhaps /must/MUST/
>> 
>>> and YANG is very good at including such checks with a must ....
>>> 'If AEAD is used .. where? this occurs in several places and I think that you need to specify the leaf where AEAD will be specified or implied.
>> 
>> Ok we have changed it to:
>> 
>> If Authenticated Encryption with Associated Data (AEAD) is used (leaf esp-algorithms/encryption/algorithm-type)
>> this flag MUST be false."
>> 
>>> And is it possible to make that a YANG 'must' statement - looking at the IKEv2 registries it is not obvious which are AEAD so that might be more complexity than it is worth.
>>> 'only available on linux kernels' Um implementation detail, you may get asked to remove that altogether or at least to a Informative Appendix - I would leave it in for now.
>> 
>> Ok.
>> 
>>> 'import ietf-i2nsf-ikec' the reference needs to be to a RFC and if it is not yet an RFC then RFC XXXX <title> in both Appendix B and C
>> 
>> Yes, we have changed that to:
>> 
>> RFC XXXX: Software-Defined Networking
>>                (SDN)-based IPsec Flow Protection.
>> 
>>> leaf-list pfs-groups could do with a reference - Transform Type 4?
>> 
>> The leaf list is referring to type pfs-group and we have now
>> 
>> typedef pfs-group {
>>             type uint16;
>>             description
>>                 "DH groups for IKE and IPsec SA rekey.";
>>             reference
>>                 "IANA; Internet Key Exchange V2 (IKEv2) Parameters;
>> 				Transform Atribute Types; Transform Type 4 -
>>                 Diffie-Hellman Group Transform IDs.
>> 				Section 3.3.2 in RFC 7296.";
>>         }
>> 
>> 
>>> 
>>> I am still working my way through the YANG so may have some more comments tomorrow.
>> 
>> Ok, do not worry we will work in -12 with these comments so far to have a quick response. We can prepare a another version later with the rest of them.
>> 
>> Thank you very much for your effort.
>>> 
>>> Tom Petch
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>>> We have realized that we missed to change this, even though we discussed it. We will change it right away in the following way (bold):
>>>> 
>>>> case rfc822-address-string {
>>>>      leaf rfc822-address-string {
>>>>           type string;
>>>>           description
>>>>               "Specifies the identity as a
>>>>                fully-qualified RFC5322 email
>>>>                address string. An example is,
>>>>                jsmith@example.com. The string
>>>>                MUST NOT contain any
>>>>                terminators e.g., NULL, CR,
>>>>                etc.).";
>>>>            reference
>>>>                   "RFC 5322.";
>>>>      }
>>>> }
>>>> 
>>>> Btw, we already used in the past “case rfc822-address-string” and “leaf rfc822-address-string” since this is coming from IKEv2 standard. Do you think we should change that name as well?
>>>> 
>>>> 
>>>>> 
>>>>> YANG module references IANA Protocol Numbers which is not in the I-D references
>>>> 
>>>> We have included the following reference:
>>>> 
>>>> [IANA-Protocols-Number]
>>>>               Internet Assigned Numbers Authority (IANA), "Protocol
>>>>               Numbers", January 2020.
>>>> 
>>>> 
>>>>> 
>>>>> s.2 boiler plate is out of date
>>>> 
>>>> What we see is the I-D has the second choice stated in https://www.ietf.org/standards/ids/guidelines/
>>>> 
>>>> This Internet-Draft is submitted in full conformance with the
>>>>    provisions of BCP 78 and BCP 79.
>>>> 
>>>>    Internet-Drafts are working documents of the Internet Engineering
>>>>    Task Force (IETF).  Note that other groups may also distribute
>>>>    working documents as Internet-Drafts.  The list of current Internet-
>>>>    Drafts is at https://datatracker.ietf.org/drafts/current/.
>>>> 
>>>>    Internet-Drafts are draft documents valid for a maximum of six months
>>>>    and may be updated, replaced, or obsoleted by other documents at any
>>>>    time.  It is inappropriate to use Internet-Drafts as reference
>>>>    material or to cite them other than as "work in progress."
>>>> 
>>>> Could you refer what is out of date?
>>>> 
>>>>> 
>>>>> XXXX is standing in for more than one RFC
>>>> 
>>>> Yes, XXXX has been used because we do not know the future number assigned to our I-D.
>>>> 
>>>> Also we realized we also included this to refer to crypto-types I-D but this has been solved now in a new version -12 that we are preparing to include your comments. We noticed we can replace the type of rw cert-data?, ca-data*, crl-data? for binary without any problem.
>>>> 
>>>> |           |     +--rw cert-data?        binary
>>>> |           +--rw private-key?            binary
>>>> |           +--rw ca-data*                binary
>>>> |           +--rw crl-data?               binary
>>>> 
>>>>> 
>>>>> but the show stopper that makes a proper review of this too costly is the references.  Those to IANA of which there are several I want to pursue.  The I-D reference is to IKEv2 parameters. Sadly, this is a three tier structure and noone agrees on what to call the third tier so I will call it tier3 here.  Top level is Group, as per RFC8126, second level is Registry.  The I-D reference is to the Group only which is fine if the actual reference then specifies the Registry and Tier3 but they never do, usually just Tier3 e.g. Transform Type 3 which makes for a lot of work for the reader, too much for this one.  You have to go hunting in all the second level Registry until you can find a match for the Tier3 identifier. And there are no URL.  If you want an example that I find easy to use, go look at RFC8407 (as usual).
>>>> 
>>>> You’re right. Could you point the exact part at RFC 8407 with that example? We would really appreciate it.
>>>> 
>>>> On the other hand, would it be enough to include the URL for Transform Type 3 https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-7 ?
>>>> 
>>>> (Same for Transform Type 1, Transform Type 4)
>>>> 
>>>>> 
>>>>> The reference for import of i2nsf-ikec gives a YANG module name; this needs to be the name of the RFC to be
>>>> 
>>>> Fixed.
>>>> 
>>>> import ietf-i2nsf-ikec {
>>>>             prefix nsfikec;
>>>>             reference
>>>>                 "RFC XXXX: Software-Defined Networking
>>>>                (SDN)-based IPsec Flow Protection.";
>>>> }
>>>> 
>>>> We still use XXXX because we do not know the number assigned to the RFC to be.
>>>> 
>>>>> 
>>>>> The example IPv6 address in the YANG module has :0:0: which is usually just ::
>>>> 
>>>> Fixed.
>>>> 
>>>> If you have any further comments, please let us know so we can include them in -12
>>>> 
>>>> Best Regards.
>>>>> 
>>>>> And I have some way to go still.
>>>>> 
>>>>> Tom Petch
>>>>> 
>>>>> On 22/10/2020 18:39, Rafa Marin-Lopez wrote:
>>>>>> Dear all:
>>>>>> 
>>>>>> After receiving a suggestion to make things clearer in the feature ikeless-notification description, we have just uploaded a new version -11 with a minor change to add the following text:
>>>>>> 
>>>>>> feature ikeless-notification {
>>>>>>             description
>>>>>>                 "This feature indicates that the server supports
>>>>>>                 generating notifications in the ikeless module.
>>>>>> 
>>>>>>                 To ensure broader applicability of this module,
>>>>>>                 the notifications are marked as a feature.
>>>>>>                 For the implementation of ikeless case,
>>>>>>                 the NSF is expected to implement this
>>>>>>                 feature.";
>>>>>>         }
>>>>>> 
>>>>>> Best Regards.
>>>>>> 
>>>>>>> Inicio del mensaje reenviado:
>>>>>>> 
>>>>>>> De: internet-drafts@ietf.org
>>>>>>> Asunto: New Version Notification for draft-ietf-i2nsf-sdn-ipsec-flow-protection-11.txt
>>>>>>> Fecha: 22 de octubre de 2020, 15:32:50 CEST
>>>>>>> Para: "Fernando Pereniguez-Garcia" <fernando.pereniguez@cud.upct.es>, "Rafael Lopez" <rafa@um.es>, "Gabriel Lopez-Millan" <gabilm@um.es>, "Rafa Marin-Lopez" <rafa@um.es>
>>>>>>> 
>>>>>>> 
>>>>>>> A new version of I-D, draft-ietf-i2nsf-sdn-ipsec-flow-protection-11.txt
>>>>>>> has been successfully submitted by Rafa Marin-Lopez and posted to the
>>>>>>> IETF repository.
>>>>>>> 
>>>>>>> Name:		draft-ietf-i2nsf-sdn-ipsec-flow-protection
>>>>>>> Revision:	11
>>>>>>> Title:		Software-Defined Networking (SDN)-based IPsec Flow Protection
>>>>>>> Document date:	2020-10-22
>>>>>>> Group:		i2nsf
>>>>>>> Pages:		92
>>>>>>> URL:            https://www.ietf.org/archive/id/draft-ietf-i2nsf-sdn-ipsec-flow-protection-11.txt
>>>>>>> Status:         https://datatracker.ietf.org/doc/draft-ietf-i2nsf-sdn-ipsec-flow-protection/
>>>>>>> Htmlized:       https://datatracker.ietf.org/doc/html/draft-ietf-i2nsf-sdn-ipsec-flow-protection
>>>>>>> Htmlized:       https://tools.ietf.org/html/draft-ietf-i2nsf-sdn-ipsec-flow-protection-11
>>>>>>> Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-i2nsf-sdn-ipsec-flow-protection-11
>>>>>>> 
>>>>>>> Abstract:
>>>>>>>   This document describes how to provide IPsec-based flow protection
>>>>>>>   (integrity and confidentiality) by means of an Interface to Network
>>>>>>>   Security Function (I2NSF) controller.  It considers two main well-
>>>>>>>   known scenarios in IPsec: (i) gateway-to-gateway and (ii) host-to-
>>>>>>>   host.  The service described in this document allows the
>>>>>>>   configuration and monitoring of IPsec Security Associations (SAs)
>>>>>>>   from a I2NSF Controller to one or several flow-based Network Security
>>>>>>>   Functions (NSFs) that rely on IPsec to protect data traffic.
>>>>>>> 
>>>>>>>   The document focuses on the I2NSF NSF-facing interface by providing
>>>>>>>   YANG data models for configuring the IPsec databases (SPD, SAD, PAD)
>>>>>>>   and IKEv2.  This allows IPsec SA establishment with minimal
>>>>>>>   intervention by the network administrator.  It does not define any
>>>>>>>   new protocol.
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> Please note that it may take a couple of minutes from the time of submission
>>>>>>> until the htmlized version and diff are available at tools.ietf.org.
>>>>>>> 
>>>>>>> The IETF Secretariat
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>>> -------------------------------------------------------
>>>>>> Rafa Marin-Lopez, PhD
>>>>>> Dept. Information and Communications Engineering (DIIC)
>>>>>> Faculty of Computer Science-University of Murcia
>>>>>> 30100 Murcia - Spain
>>>>>> Telf: +34868888501 Fax: +34868884151 e-mail: rafa@um.es
>>>>>> -------------------------------------------------------
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> -------------------------------------------------------
>>>> Rafa Marin-Lopez, PhD
>>>> Dept. Information and Communications Engineering (DIIC)
>>>> Faculty of Computer Science-University of Murcia
>>>> 30100 Murcia - Spain
>>>> Telf: +34868888501 Fax: +34868884151 e-mail: rafa@um.es
>>>> -------------------------------------------------------
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>> 
>> -------------------------------------------------------
>> Rafa Marin-Lopez, PhD
>> Dept. Information and Communications Engineering (DIIC)
>> Faculty of Computer Science-University of Murcia
>> 30100 Murcia - Spain
>> Telf: +34868888501 Fax: +34868884151 e-mail: rafa@um.es
>> -------------------------------------------------------
>> 
>> 
>> 
>> 
>> 
> 
> _______________________________________________
> I2nsf mailing list
> I2nsf@ietf.org
> https://www.ietf.org/mailman/listinfo/i2nsf

-------------------------------------------------------
Rafa Marin-Lopez, PhD
Dept. Information and Communications Engineering (DIIC)
Faculty of Computer Science-University of Murcia
30100 Murcia - Spain
Telf: +34868888501 Fax: +34868884151 e-mail: rafa@um.es
-------------------------------------------------------