Subject: Re: [netmod] IETF ACL model

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 21 Nov 2017 08:25:10 -0800

[Taking the discussion to the mailing list]

The summary of the discussion happening on a private thread has to do with the ‘any’ container (now leaf) definition in the ACL model for something that matches anything, much like a ‘*’ would do in regex. The discussion has come down to:

- leave the definition as is, so users would have to explicitly define it
  pro: It is explicit and there would be no confusion about its presence or absence. 
  con: It is verbose, in that every access list entry would have to define it.

- remove the any definition, so an absence of the definition implies match on all. The text in the draft would have to be updated to state this, and YANG parsers would need to watch out for the non-definition in the match container.

Opinion? Preferences?

p.s. Current CLI configurations require explicit declaration of ‘any’.

> On Nov 20, 2017, at 2:20 PM, Jeff Haas <[email-masked]> wrote:
> 
>> 
>> On Nov 20, 2017, at 4:54 PM, Kristian Larsson <[email-masked]> wrote:
>> 
>> 
>> 
>> On 2017-11-20 18:26, Jeff Haas wrote:
>>> IMO, the contention here is a consequence of proof by assertion. >
>>>> On Nov 17, 2017, at 5:56 AM, Kristian Larsson <[email-masked] <mailto:[email-masked]>> wrote:
>>>> 
>>>>>> 
>>>>>> Changes I'd like to see to this PR:
>>>>>> * remove any-acl completely as it serves no purpose (we achieve
>>>>>>  this through a ACE with no match conditions)
>>>>> Will let Jeff respond as he articulated this requirement. Agree that most platforms have deny as the default. But what if the ace entry is something like this:
>>>>> access-list 11 permit tcp any any
>>>>> Would the absence of match rules for IP addresses imply any any?
>>>> 
>>>> An absence of any match means it matches on everything. If there are no IP matches at all, then yes, it implies any source or destination address. I don't see why we would need to explicitly have an any-acl container or leaf to point this out. With the same concept applied uniformly you would explicitly call out "any" matches for all types of matches. That would be extremely verbose.
>>> Where in the model does it say that the absence of a matches container implies a match all vs. match none?
>>> Where in the normative text of the internet-draft does it say that?
>>> In the absence of either of the above, an "any" removes ambiguity.
>>> FWIW, I'm totally fine with removing the any; however you *MUST* clarify the above.  Preferably both in the matches container and also in the normative text.  For my part, I think it's easier on the parser if you basically require at least one item from the container so you don't have to deal with optional contents.  But I'm not the one writing yang parsers for a living.
>> 
>> I think it's clear that if no match condition is defined for a particular (header) field then that means any value will match. It is only an extension of this that no match conditions at all means we match any and all packets. I think that is logical.
>> 
>> This can naturally be spelled out in the text :)
> 
> You think it's clear, but where is it in the document or model?
> 
> Of such "implied clarity" is many interop bugs made. :-P
> 
> -- Jeff
> 
>> 
>> Kind regards,
>>  Kristian.

Mahesh Jethanandani
[email-masked]

---

From: Robert Wilton <[email-masked]>
Date: Tue, 21 Nov 2017 16:39:45 +0000

On 21/11/2017 16:25, Mahesh Jethanandani wrote:
> The summary of the discussion happening on a private thread has to do 
> with the ‘any’ container (now leaf) definition in the ACL model for 
> something that matches anything, much like a ‘*’ would do in regex. 
> The discussion has come down to:
>
What would it mean if the client configured an ACL type of "any" but 
didn't configure the "any" leaf?

> - remove the any definition, so an absence of the definition implies 
> match on all. The text in the draft would have to be updated to state 
> this, and YANG parsers would need to watch out for the non-definition 
> in the match container.
>
> Opinion? Preferences?
>

If we have an "any" leaf that it seems that:
- it only applies for an ACL of type "any-acl", and can't be used for 
other ACL types.
- it has to be applied for an ACL of type "any-acl" (to have any use).
- it makes is ambiguous if the leaf isn't configured, which would sort 
of force the any leaf to be mandatory for ACLs of type "any-acl".

So it seems redundant to me.  I.e. I'm not sure what extra information 
an "any" leaf imparts beyond knowing that the ACL is of type"any-acl".

Thanks,
Rob

>
>> On Nov 20, 2017, at 2:20 PM, Jeff Haas <[email-masked] 
>> <mailto:[email-masked]>> wrote:
>>
>>>
>>> On Nov 20, 2017, at 4:54 PM, Kristian Larsson 
>>> <[email-masked] <mailto:[email-masked]>> wrote:
>>>
>>>
>>>
>>>>> On Nov 17, 2017, at 5:56 AM, Kristian Larsson 
>>>>> <[email-masked] <mailto:[email-masked]> 
>>>>> <mailto:[email-masked]>> wrote:
>>>>>
>>>>>>>
>>>>>> Will let Jeff respond as he articulated this requirement. Agree 
>>>>>> that most platforms have deny as the default. But what if the ace 
>>>>>> entry is something like this:
>>>>> An absence of any match means it matches on everything. If there 
>>>>> are no IP matches at all, then yes, it implies any source or 
>>>>> destination address. I don't see why we would need to explicitly 
>>>>> have an any-acl container or leaf to point this out. With the same 
>>>>> concept applied uniformly you would explicitly call out "any" 
>>>>> matches for all types of matches. That would be extremely verbose.
>>>> Where in the model does it say that the absence of a matches 
>>>> container implies a match all vs. match none?
>>>> FWIW, I'm totally fine with removing the any; however you *MUST* 
>>>> clarify the above.  Preferably both in the matches container and 
>>>> also in the normative text.  For my part, I think it's easier on 
>>>> the parser if you basically require at least one item from the 
>>>> container so you don't have to deal with optional contents.  But 
>>>> I'm not the one writing yang parsers for a living.
>>>
>>> I think it's clear that if no match condition is defined for a 
>>> particular (header) field then that means any value will match. It 
>>> is only an extension of this that no match conditions at all means 
>>> we match any and all packets. I think that is logical.
>>>
> [email-masked] <mailto:[email-masked]>
>

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 22 Nov 2017 09:39:04 +0100 (CET)

Mahesh Jethanandani <[email-masked]> wrote:
> The summary of the discussion happening on a private thread has to
> do with the ‘any’ container (now leaf) definition in the ACL model
> for something that matches anything, much like a ‘*’ would do in
> regex. The discussion has come down to: 
> 

>   match on all.

It wouldn't be the *absence* that implies match on all, but rather it
is the explicit setting of the type to 'any-acl' that means match on
all:

  <acl>
    <name>my-any-acl</name>
    <type>any-acl</type>
  </acl>

Having to also define the leaf "any" as well would just be redundant:

  <acl>
    <name>my-any-acl</name>
    <type>any-acl</type>
    <aces>
      <ace>
        <name>my-any-rule</name>
        <any/>
      </ace>
    </aces>
  </acl>

BTW, I support Kristian's suggestion of have just the "name" as the
key in the "acl" list, and making "type" mandatory.

I also suggest you remove redundant prefixes in the leaf names,
e.g. as above s/acl-type/type/ s/acl-name/name/ and
s/rule-name/name/.  This is how we usually name nodes in the IETF YANG
models.

> The text in the draft would have to be updated to
>   state this, and YANG parsers would need to watch out for the
>   non-definition in the match container.
> 
> Opinion? Preferences?
> 

But do they also have the acl type definition?

/martin

---

From: Robert Wilton <[email-masked]>
Date: Wed, 22 Nov 2017 14:01:09 +0000

Thinking about this some more.  I'm not sure what it means for the "ACL 
Type" to be "any-acl".  It seems that the "match any packet" should be a 
type of ACE, e.g. perhaps as the last entry of an ACL, rather than a 
type of ACL.

Otherwise if the ACL type is "any-acl" then this only allows two types 
of ACLs to be defined, neither of which seem to be particularly useful:
(1) An ACL that matches all traffic and permits it, i.e. the same as 
having no ACL at all.
(2) An ACL that matches all traffic and drops.

So I think perhaps the answer here is to define neither ACL type 
"any-acl" nor leaf "any".  The presumption could be that any ACE that is 
configured to match no fields implicitly matches all packets (because 
all non specified fields are treated as wildcards), and then applies the 
permit/deny rule associated with the ACE.  This logic can apply to all 
ACL types.

Thanks,
Rob

On 22/11/2017 08:39, Martin Bjorklund wrote:
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: Kristian Larsson <[email-masked]>
Date: Mon, 27 Nov 2017 14:17:55 +0100

> type of ACE, e.g. perhaps as the last entry of an ACL, rather than a 

Yes, I agree as so far that any-acl makes no sense as an acl-type. The
way I understood acl-type, and the way that vendors have told me it will
be used, is to say "this is an IPv4 ACL" and then on an attachment point
you can specify that only ACLs of acl-type ipv4-acl can be attached to
the interface. That makes perfect sense. I do not see how any-acl can
map into this.

I agree that any-acl is logically a type of ACE but we don't have an
ace-type and the exact same information can IMHO already be conveyed
WITHOUT the any-acl type and thus it has no reason to exist. Nor do we
need a feature for it.

>From what I can tell the any-acl container in the ACE should be used to
explicitly signify a match on "any". Think of IOS style ipv4 acl:
  permit ip any any

We have to provide a source and destination so this would be a rather
explicit mapping of that. However, our structure in this YANG model is
just completely different than an IOS command so I don't see why we
should try and mimic IOS in the YANg model.

Not specifying a destination IP address means we match on any
destination IP address. The same is true for any other field we can
match on. Not setting a match implies we don't try to match on that
field, thus we allow "any" value. I think the logical continuation of
this is that for an ACE with no matches defined at all, we match any
packet. I think we can update the text to better explain this.

> permit/deny rule associated with the ACE. This logic can apply to all 

Yes yes yes :)

   Kristian.

---

From: Robert Wilton <[email-masked]>
Date: Mon, 27 Nov 2017 15:00:38 +0000

>> permit/deny rule associated with the ACE. This logic can apply to all
Another area of the current model that I'm not quite convinced about is 
about the L4 matches (UDP, TCP, ICMP).

Currently the model assumes that if a device can support matching on say 
UDP fields that they can apply to any type of ACL.  However, I think 
that it is plausible that the UCP, TCP and ICMP fields might only apply 
to IP ACLs and not Ethernet ACLs.

Hence, I was wondering if there should be "acl match type" identity 
defined for the "l4" field matches?

This would then cause there to be twice as many ACL types identities and 
features defined, e.g.

eth,
ipv4,
ipv6,
l4
eth-l4,
eth-ipv4,
eth-ipv4-l4,
eth-ipv6,
eth-ipv6-l4,
eth-ipv4-ipv6,
eth-ipv4-ipv6-l4
ipv4
ipv4-l4
ipv6
ipv6-l4,
ipv4-ipv6
ipv4-ipv6-l4

So, trading off my ACL type combinations for more expressiveness as to 
what can actually be supported.

Thanks,
Rob

> .

---

From: Mahesh Jethanandani <[email-masked]>
Date: Mon, 27 Nov 2017 09:47:16 -0800

An updated version of the model has been posted as part of the PR here <https://github.com/netmod-wg/acl-model/commit/2477cd400cce6d39933c908ad97da27ff759588b>.

The particular change removes any-acl from the model, expands on eth (to ethernet), removes acl- prefix for things like acl-type and acl-name. Please review.

> On Nov 27, 2017, at 5:17 AM, Kristian Larsson <[email-masked]> wrote:
> 
> 

Mahesh Jethanandani
[email-masked]

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 28 Nov 2017 11:17:15 +0100 (CET)

Mahesh Jethanandani <[email-masked]> wrote:
> An updated version of the model has been posted as part of the PR here
> <https://github.com/netmod-wg/acl-model/commit/2477cd400cce6d39933c908ad97da27ff759588b>.
> 
> The particular change removes any-acl from the model, expands on eth
> (to ethernet), removes acl- prefix for things like acl-type and
> acl-name. Please review.

I think 99% of the changes in this PR look good.  The one
exception is the typedef that used to be called "acl-type".  I think
it should still be called "acl-type".  "type" is too broad.  NOTE,
this is just the typedef; the leaf /access-lists/acl/type should keep
its name ("type").

/martin

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 29 Nov 2017 12:11:17 -0800

The updated commit here <https://github.com/netmod-wg/acl-model/pull/19/commits/37e4c030180ae052a5fae26ca86813970fc6b4bf> takes care of restoring “type" to "acl-type", fixes some indentation issues, adds a choice for “l3" where either “ipv4" or “ipv6" can be selected, and a similar choice at “l4" that allows either “tcp", “udp" or “icmp" to be selected, and removes changes for “global" attachment point. Will add the last item as a separate commit.

Unless I hear objections, I will roll the pr/18 changes into the master branch in 48 hours.

> On Nov 28, 2017, at 2:17 AM, Martin Bjorklund <[email-masked]> wrote:
> 

Mahesh Jethanandani
[email-masked]

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 29 Nov 2017 16:19:45 -0800

For now.

Kristian and I discussed this, and agreed that we will pull it in in a new pull request.

> On Nov 29, 2017, at 4:08 PM, Sonal Agarwal (agarwaso) <[email-masked]> wrote:
> 
> Are you removing the definition  of “global” ACL?
> 
> -
> leaf global {
> -
> if-feature global-attachment;
> -
> type
> empty;
> -
> description
> -
> "ACL rule is global";
> - }
> 
> The remaining changes look fine to me.
> 
> Thanks,
> ---
> Sonal Agarwal
> 
> From: Mahesh Jethanandani <[email-masked] <mailto:[email-masked]>>
> Date: Wednesday, November 29, 2017 at 12:11 PM
> To: NetMod WG <[email-masked] <mailto:[email-masked]>>
> Cc: "Robert Wilton -X (rwilton - ENSOFT LIMITED at Cisco)" <[email-masked] <mailto:[email-masked]>>, Jeffrey Haas <[email-masked] <mailto:[email-masked]>>, Cisco Employee <[email-masked] <mailto:[email-masked]>>, Kristian Larsson <[email-masked] <mailto:[email-masked]>>, Kristian Larsson <[email-masked] <mailto:[email-masked]>>, Martin Bjorklund <[email-masked] <mailto:[email-masked]>>
> Subject: Re: [netmod] IETF ACL model
> 
>> On Nov 28, 2017, at 2:17 AM, Martin Bjorklund <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> Mahesh Jethanandani <[email-masked] <mailto:[email-masked]>> wrote:
>>> <https://github.com/netmod-wg/acl-model/commit/2477cd400cce6d39933c908ad97da27ff759588b <https://github.com/netmod-wg/acl-model/commit/2477cd400cce6d39933c908ad97da27ff759588b>>.
>>> 
>>>> On Nov 27, 2017, at 5:17 AM, Kristian Larsson <[email-masked] <mailto:[email-masked]>>
>>>> wrote:
>>>> 
>>>> 
>>>> Robert Wilton <[email-masked] <mailto:[email-masked]>> writes:
>>>> 

Mahesh Jethanandani
[email-masked]

---

From: Kristian Larsson <[email-masked]>
Date: Thu, 30 Nov 2017 09:30:51 +0100

Right, it's not about the concept as such, I think we've agreed that
having a global attachment point is fine. It's just that we need to
polish the actual implementation and seeing how it be good to merge the
current PR I suggested we put the global attachment point in a separate
PR. Eases the workflow :)

   kll

---

From: Kristian Larsson <[email-masked]>
Date: Thu, 30 Nov 2017 10:00:40 +0100

> to IP ACLs and not Ethernet ACLs.

Indeed, I agree with you. It's IMHO not just plausible but probable.
I think the model is a compromise between reality and perfectly
expressed constraints.

As I've alluded to I think the matches you can use are really not a
condition of the device but of the attachment point. Depending on where
you attach the ACL you will have different capabilities. Accurately
modeling this however, is very difficult and so the use of YANG features
limiting the ACLs themselves (and not what ACL can be attached
somewhere) to convey roughly what things are supported by the device is
a reasonable approximation.

For example, if we have two types of attachment points and one supports
matching on everything whereas the other does not, naturally our YANG
feature set must be the superset of the features supported by the two
attachment points or we could not even define those matches in the ACLs.
Trying to attach an ACL that matches on things that this attachment
point doesn't support will result in a run-time error. Similarly, the
match conditions supported is likely different based on what type of
linecard we have installed, which is AFAIK impossible to express in a
YANG model. We cannot condition this on the type of line card, since
that is state data and we can't do constraints from config to state.

I bet a bunch of the potential match conditions on IP header fields are
not supported by most devices today (think cheap commodity chips). Or
being able to match on TCP and a given port but not on TCP-flags...
there are lots of cases. The question is which ones are important and
"large" enough to distinguish through different acl-types.

Would it be reasonable to put L4 in the acl-type as well? Yes, I think
so, although I'll admit I don't feel strongly about it :)

   kll