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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Rafa

My attempt to read the text keeps running into a brick wall (lockdown:-( but I have a few new comments. I said sometime ago that the IESG would likely find plenty to say and I see that Benjamin has lived up to my expectations. I wish I could be as productive as him:-(

For completeness, I include what I have found so far but suspect that they will all be overtaken by IESG comments.

RFC2247 I don't find good for DN; I find RFC4519 better but suspect that X.520 would be better still but do not have a copy of that to hand; this is the sort of thing I expect the IESG to have definite views on

given that the model has a boolean for fragmentation should is also have a leaf or some such for PMTU? Doing the former requires the latter or is that latter expected to be elsewhere in a different YANG model?

Abstract; PAD, SPD, SAD need expanding or else leaving out if the reference to IPsec databases is adequate for the Abstract.

I know you added the comment about no protocol (the comment surprised me:-) but I think the YANG modules matter more so I would say
'It defines three YANG modules but no new protocol.'

1
/Inspired in the SDN/Inspired by the SDN/
3
/state date/state data/

4.1
'a YANG data model  .. MUST be defined'
it would be good to reference where they are, ie the Appendices

4.2
ditto

[which is as far as I got reading but ..]

8
/should adhere/SHOULD adhere/  ??

Tom Petch

On 02/11/2020 12:06, tom petch wrote:
Rafa

As you may know, the submission window closes in the week before the
IETF meeting which is perhaps why over 150 new I-D have just appeared:-(
which may or may not affect your plans. I expect to look some more this
week but the country is going into lockdown for four weeks, some are
saying for six months, so I need to work out what I have to get done
before all the places I need to go shut:-(  Some things pre-empt IETF work!

On the formatting, my hope was that the paragraphs of text as in YANG
description would spread out to occupy fewer lines, so fixing the
indentation is an improvement but not quite all I had hoped for - it
certainly makes it easier to read.  I would not reissue a new version
just for this

And inline twice


On 30/10/2020 17:43, Rafa Marin-Lopez wrote:
Hi Tom:

Thank you very much again. See our comments inline:

El 29 oct 2020, a las 18:06, tom petch <daedulus@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
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.

I still see eight - the one that lacks units is
leaf established

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.

I see it a second time for
leaf integrity-algorithm


Tom Petch

/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@xxxxxxxxxxx. 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@xxxxxxxx
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@xxxxxxxxxxx>, "Rafael Lopez" <rafa@xxxxx>,
"Gabriel Lopez-Millan" <gabilm@xxxxx>, "Rafa Marin-Lopez"
<rafa@xxxxx>


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:

--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call




[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux