Subject: [netmod] Working group Last Call: draft-ietf-netmod-acl-model-06

From: Nadeau Thomas <[email-masked]>
Date: Wed, 9 Dec 2015 08:27:04 -0800

This email initiates a NETMOD WG Last call for draft-ietf-netmod-acl-model-06. 
Please review the draft and make any comments to the list by Wednesday, 16 December, 2015
by 8AM EST.

	Tom and Kent (as NETMOD Co-chairs)

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 11 Dec 2015 12:31:58 +0100

> Please review the draft and make any comments to the list by Wednesday, 16 December, 2015

Technical

- I am not sure I personally like the acl-oper-data and ace-oper-data
  containers in the middle of the config true tree. I understand that
  operational state in this case is likely tightly bound to the
  lifetime of the config state but I also generally prefer to have a
  clean separation of config from state. But since I am not
  implementing this, I am happy to leave the decision to those who
  write the code.

- I would probably have used src-ipv4-prefix and dst-ipv4-prefix
  instead of source-ipv4-network and destination-ipv4-network and
  I would probably have used src-mac-address and src-mac-address-mask
  and dst-mac-address and dst-mac-address-mask. Generally simple
  src instead source and dst instead destination - lines up all
  much more nicely.

- I would have put the acl-name and acl-type first followed by the
  entries list.

- I also wonder why we use both the term 'entry' and 'rule', why not
  pick one of them? You have for example rule-name (which is the key
  of the list but again comes last).

- Should there be features for ace-eth and ace-ipv4 and ace-ipv6 or do
  we expect that all implementations will always support all three?
  Another option would be to have the generic model completely without
  any protocol specific elements and to have separate models to
  augment in ipv4, ipv6, and ethernet specific ace types. I actually
  think this would make much more sense since you would not have a
  module 'packet fields' but instead a clear extension of the
  ietf-access-control-list module. You could also drop the examples
  how to extend the core model since the ip and ethernet extensions
  would already serve the purpose. You also would not need features
  since an implementation advertises the extension modules.

- The 'uses packet-fields:metadata' resolves to a leaf
  'input-interface' which is of type string. Is this the only metadata
  field we ever envision to be there? If so, why not make it part of
  the base model? If not, what is the extensibility model here?  And
  should the interface reference not use a more specific type than
  'string'?

- Some implementations support the action to jump or goto other
  acls. Is this not common enough to include it as a base action,
  perhaps protected by a feature?

- In the example in section 4.3, does the CLI shown really help much?
  The syntax is pretty system specific. In the XML shown, can you not
  leave out all the fields that are not set? This would remove a lot
  of noise. I do not understand what having both actions deny and
  permit at the same time means. Did you validate the example against
  the data model? (I also find the keys at the end somewhat strange
  and not that NETCONF XML serialization actually requires the keys to
  be sent first.)

- The clarification whether the boundary port numbers are included in
  a port range should be in the YANG model. The YANG model should also
  say what it means if one of the ranges is not present. We should not
  define the semantics by examples.

- I do not understand section 5. It shows some nftables syntax but
  does not really shed a light on what the example shown does or how
  that maps to the YANG data model. So I am left somewhat clueless.

- Can I trigger multiple actions from an ace? Or do I have to
  replicate the ace to do that? In general, there is extension of
  a choice (means one of several) and there are extension of a
  choice already present.

- Empty description statements or descriptions statements "(null)"
  need to be fixed.

- I am a bit surprised that we do not define how to attach an ACL to
  an interfaces or a set of interfaces given that we do have an
  interfaces YANG data model. The example given in A.3 makes me wonder
  why there should be a counter in the interfaces config tree. The
  targets/interface-name back-reference seems to indicate that I can
  attach an ACL only to a single interface.

- I do not understand A.4 at all. Defining an identity does not make
  the choice work differently.

- Has someone tried to implement this?

Organization

- Section 3 to a large extend is a repetition of section 1. Perhaps
  make section 1 shorter and leave the details to section 3?

- Some empty lines in the YANG definition would have pleased my eyes.

Editorial

- there are a couple of missing articles throughout the text
- sometimes singular/plural is at odds
- s/this draft/this document/
- you may add another 'e' to my name ;-)

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Eliot Lear <[email-masked]>
Date: Thu, 17 Dec 2015 14:38:31 +0100

Hi everyone,

Please forgive the newbie question, but I would like to understand
whether the group has considered an ACL type that matches against a host
name.

Eliot

On 12/9/15 5:27 PM, Nadeau Thomas wrote:
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod
>

---

From: Nadeau Thomas <[email-masked]>
Date: Thu, 17 Dec 2015 08:45:35 -0500

Do you mean an ASCII DNS name (versus an IP address w a mask)?

	—Tom

> On Dec 17, 2015:8:38 AM, at 8:38 AM, Eliot Lear <[email-masked]> wrote:
> 
> Hi everyone,
>

---

From: Eliot Lear <[email-masked]>
Date: Thu, 17 Dec 2015 15:44:08 +0100

> 	Do you mean an ASCII DNS name (versus an IP address w a mask)?

I was thinking of "host" in RFC 6021.

Eliot

---

From: Nadeau Thomas <[email-masked]>
Date: Thu, 17 Dec 2015 10:53:13 -0500

You raise a good point. Do the contributors/editors have any thoughts on
this suggestion?

	—Tom

> On Dec 17, 2015:9:44 AM, at 9:44 AM, Eliot Lear <[email-masked]> wrote:
> 
> 
>

---

From: "Einar Nilsen-Nygaard (einarnn)" <[email-masked]>
Date: Fri, 18 Dec 2015 12:42:49 +0000

I have been involved in some discussions within Cisco relating to our contributions to this draft, and I can say that we did not spend much time discussing such an addition within Cisco. I am not sure what discussions the ACL model design team may have had as a group.

However, I think the addition of hostname-based matching would be a great one, but I think that, if added, it should be made optional via a feature due to the potential complexities of implementation. It would be great to see if we can get this added.

Cheers,

Einar

> On Dec 17, 2015, at 3:53 PM, Nadeau Thomas <[email-masked]> wrote:
> 
>

---

From: Eliot Lear <[email-masked]>
Date: Fri, 18 Dec 2015 14:32:03 +0100

I should point out that there's some related IPR in this space that
Cisco has.  Depending on how this gets resolved, we'll file appropriately.

---

From: "Sterne, Jason (Jason)" <[email-masked]>
Date: Fri, 18 Dec 2015 22:21:07 +0000

I'm not a fan of adding something like that in the base model.  Let's get a basic model done and then we can consider an extension draft.  I'd think that things like TCP flags, for example, would be a more natural & common thing to add to an ACL model than a host name match so I can't see host name being in there before TCP flags (which I'm not advocating for in the base model).

I also don't think the metadata interface match should be in this base model either.  That is out of place IMO.  The base model provides an ACL that can then get associated with objects like interfaces (as in the example in section A.3).

I'd also suggest we consider making the actions 'deny' and 'permit' presence containers instead of empty leafs.  That would allow easier augmentations (e.g. additional 'permit' parameters for policy based forwarding for example).

Regards,
Jason

-----Original Message-----
From: netmod [mailto:[email-masked]] On Behalf Of Nadeau Thomas
Sent: Thursday, December 17, 2015 10:53
To: Lear Eliot
Cc: Benoit Claise; RTG YANG Design Team; netmod WG
Subject: Re: [netmod] Working group Last Call: draft-ietf-netmod-acl-model-06

	You raise a good point. Do the contributors/editors have any thoughts on this suggestion?

	—Tom

---

From: Dean Bogdanovic <[email-masked]>
Date: Sat, 19 Dec 2015 07:50:58 -0500

Juergen,

Please see answers inline

Dean

> On Dec 11, 2015, at 12:31 PM, Juergen Schoenwaelder <[email-masked]> wrote:
> 

I know that src and dst are pretty standard abbreviations, but have preference to use full words.

> - I would have put the acl-name and acl-type first followed by the

Can you please expand on this? Is there any major difference? OR just design choice?
>  of the list but again comes last).
This is a bit of compromise between Cisco and Juniper terminology. In Cisco it is ACE and in Juniper term. But in the yang model and in the text we are using consistently one or the other word.

>  since an implementation advertises the extension modules.

We started early with such design, but then the WG feedback moved us to the current  design.

>  the base model? If not, what is the extensibility model here?  

It can be used for route filtering 

> And
>  'string’?

Interface references can be many things, from standard naming we are familiar, e.g. ge-1/0/0.1 to a numerical value like 13276. Leaving it as string gives us most flexibility in that regards.

>  perhaps protected by a feature?

Yes among vendors, but it can be very specific. Example, in Juniper implementation there are two types of filters, classical and Fast Update Filters (FUF). Classic filters support (on specific HW platforms) filter as action, but FUF does not. Not all terms and not all actions are supported on FUFs. So you have to see what filter type and what platform it is, in order to such a specific action.
>  The syntax is pretty system specific.

OTH, it is widely understood and self explanatory. 

> In the XML shown, can you not
We used pyang sample xml skeleton to create the xml example.

Leaving both actions in the document is my fault. In the model, action is a choice, and it was a bad copy paste job. Didn’t choose :)
>  define the semantics by examples.
I’m loosing you here. We are stating in the model if the boundary port numbers are included in a port rang.

   container source-port-range {
      presence "Enables setting source port range";
      description 
        "Inclusive range representing source ports to be used.
        When only lower-port is present, it represents a single port.";
      leaf lower-port {
        type inet:port-number;
        mandatory true;
        description
          "Lower boundary for port.";
      }
      leaf upper-port {
        type inet:port-number;
        must ". >= ../lower-port" {
          error-message
          "The upper-port must be greater than or equal to lower-port";
        }
        description
          "Upper boundary for port . If existing, the upper port
          must be greater or equal to lower-port.";
      }
    }

Same is done for the upper port
>  that maps to the YANG data model. So I am left somewhat clueless.

Just wanted to point out that the basic structure of iptables is similar to ACL yang model, so a translation from one to the other should be a  simple process. If someone wants to make it compliant.
>  a choice (means one of several) and there are extension of a
Again, this is very platform dependent, so didn’t want to include it in the base model.

> - Empty description statements or descriptions statements "(null)"

There are no empty descriptions. You are probably referring to ietf-packet-fields.yang model. There is a bug in pyang which was reported to Martin and fixed since.

pyang --ietf --strict ietf-packet-fields.yang
ietf-packet-fields.yang:67: warning: RFC 6087: 4.12: statement "must" should have a "description" substatement
ietf-packet-fields.yang:89: warning: RFC 6087: 4.12: statement "must" should have a "description” substatement

The code is correct, just the compiler is reporting an error.
          error-message
          "The upper-port must be greater than or equal to lower-port";
        }
        description
          "Upper boundary for port . If existing, the upper port
          must be greater or equal to lower-port.";
      }
>  attach an ACL only to a single interface.

I wasn’t the author of that section

> - Has someone tried to implement this?

Was working on implementation while at my previous employer, but am not aware of any other implementation.

>  make section 1 shorter and leave the details to section 3?

OK
> - Some empty lines in the YANG definition would have pleased my eyes.

OK
> - you may add another 'e' to my name ;-)

or ü

Will fix those
> Rtg-dt-yang-arch mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtg-dt-yang-arch

---

From: Dean Bogdanovic <[email-masked]>
Date: Sat, 19 Dec 2015 08:05:25 -0500

The basic design idea for the base model is structure that all vendors support. Some of the examples mentioned below, like FQDN, are not supported by all vendors and are protected by IPR (which I wasn’t aware of it). There are many possible match conditions that could be added to the base model, like Auth header in IPSec or IPSec encapsulation security payload to keep it with security. There are many match conditions in Class of Services as well. All these match conditions would have created more issues to come to consensus about the base model, so for that reason we went with the minimal model that would be easy for all vendors to implement.

Dean

> On Dec 18, 2015, at 5:21 PM, Sterne, Jason (Jason) <[email-masked]> wrote:
> 
> I also don't think the metadata interface match should be in this base model either.  That is out of place IMO.  The base model provides an ACL that can then get associated with objects like interfaces (as in the example in section A.3)

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 21 Dec 2015 16:33:13 +0100

> I know that src and dst are pretty standard abbreviations, but have preference to use full words.

My eyes prefer the shorter versions but it might be subjective where
the line between arconyms and words is drawn. For me, things are not
done consistently according to my personal preferences. ;-)

> Can you please expand on this? Is there any major difference? OR just design choice?

Technically you can define key leafs anywhere but I think there are some
benefits of defining them at the beginning of a list:

a) I find it nice if the leafs listed in the key statement follow the
   key statement and not N pages down the document.
b) If you put them at the end and you have to add additional definitions,
   you will end up with leafs somewhere in the middle.
c) The XML encoding requires to ship key leafs first and it might be
   simpler if the key leafs are also defined first in the data model
   (so if you use the same order, you actually get things correct).

Now you tell me what the advantages are to put them at the end...

> This is a bit of compromise between Cisco and Juniper terminology. In Cisco it is ACE and in Juniper term. But in the yang model and in the text we are using consistently one or the other word.

For the sake of clarity, I personally would prefer to have a single
term. I think Linux packet filters call things rules. I think BSD
packet filtering calls things rules. Wikipedia seem say that packet
filters consist of filtering rules... So I guess I have a preference
but I will not get a sleepless night over this.
 
> We started early with such design, but then the WG feedback moved us to the current  design.

Can you provide pointers to minutes etc? I think the routing model
also has a core model where even unicast routing is augmented in. This
seems to make the boundary between the generic infrastructure and the
addins very clear.
 
> It can be used for route filtering

Yeah, but the questions were:

- Is this the only metadata field we ever envision to be there?
- If so, why not make it part of the base model?
- If not, what is the extensibility model here?

> Interface references can be many things, from standard naming we are familiar, e.g. ge-1/0/0.1 to a numerical value like 13276. Leaving it as string gives us most flexibility in that regards.

I disagree that the goal here is most flexibility. We do have an
interfaces data model in the IETF. Why are we avoiding to refer to it
here?

> Yes among vendors, but it can be very specific. Example, in Juniper implementation there are two types of filters, classical and Fast Update Filters (FUF). Classic filters support (on specific HW p…

What speaks against making jump and/or call actions a feature?

> OTH, it is widely understood and self explanatory.

The question was whether it is needed to show system specific CLI.
BTW, I note that the textual description says "deny ... from
_leaving_".  How is the 'from leaving' translated into the XML? Is
there any distinction between input and output filters (or forwarding
filters or whatever your engine supports)?

> We used pyang sample xml skeleton to create the xml example.

Whatever, the noise does not really help and the example might even
mislead people to believe they have to write down all unused leafs.
 
> I’m loosing you here. We are stating in the model if the boundary port numbers are included in a port rang.

Ack, I found it in the description of container source-port-range (and
previously I looked at lower-port and upper-port).

> Just wanted to point out that the basic structure of iptables is similar to ACL yang model, so a translation from one to the other should be a  simple process. If someone wants to make it compliant.

It escapes me how the iptables fragment shown in page 18 maps to the
YANG model. What is the substance behind the statement "It should be
fairly easy to do translation between ACL YANG model described in this
draft and Linux nftables."? Has someone done a mapping? It is not
obvious to me how this works.

> Again, this is very platform dependent, so didn’t want to include it in the base model.

So what would I do if I want to 'log' and 'deny'? Even if the base
model does not do that, how can I augment this in? The base model must
provide the necessary extensibility hooks.

> There are no empty descriptions. You are probably referring to ietf-packet-fields.yang model. There is a bug in pyang which was reported to Martin and fixed since.

I am talking about this (page 25):

      description "(null)";
 
> I wasn’t the author of that section

This is a WG document so this needs to be resolved.

> Was working on implementation while at my previous employer, but am not aware of any other implementation.

I believe it will help a lot to have prototype implementations of this
data model in order to make sure the model does actually work and
provides the extensibility hooks that are necessary. In particular, I
would feel much more convinced if it would be clear (as a proof of
concept) how the model maps to say Linux packet filters. I would
actually not mind if mapping examples from the data model to various
filtering engines would be in the appendix. This would give evidence
that people have actually worked through a couple of concrete
examples.

/js