Re: [netmod] WG Last Call: draft-ietf-netmod-acl-model-15

[Mahesh Jethanandani <[e-mail]>]

Eliot,

I am not sure about what is a basic function, and what is not. Tomorrow, somebody can argue that TCP SYN flag is a basic function and should be broken out from under the TCP header.

I would rather that port definition remain under TCP/UDP with the feature statement ‘match-on-tcp’ and ‘match-on-udp’. The proposed change then looks like what is here <https://github.com/netmod-wg/acl-model/pull/23/commits/dc4ed556bd71cb9e648b0babdebe05a3a5b7bf68>, and all the changes in the PR are captured here <https://github.com/netmod-wg/acl-model/pull/23>.

Thanks

> On Jan 22, 2018, at 11:14 AM, Eliot Lear <[e-mail]> wrote:
> 
> Hi Kent and Mahesh and Sonal,
> 
> Thanks very much for working on this draft.  I have noted one problem that I think needs correcting.  I come prepared with a diff.
> 
> The current model has {source,dest}-port-or-range hanging off ipv4 or ipv6.  This is a transport parameter and is not appropriate for protocols that do not use ports (ie, ICMP, ESP, etc).  A better locale would be to hang these components underneath l4 underneath their respective tcp and udp branches.
> 
> Because this is so basic a function, I propose that this not be included in match-on-tcp or match-on-udp.  Instead, the contents of both tcp and udp be moved to new containers "tcp-all" and "udp-all", respectively, and the ports hang as peers to that.  Thus, if a very simple device can understand TCP and UDP ports but cannot understand more detailed information, that is supported.
> 
>  And so from a tree perspective, it would look like this:
> 
>        |        |  +--rw (l4)?
>        |        |  |  +--:(tcp)
>        |        |  |  |  +--rw tcp
>        |        |  |  |     +--rw source-port-range-or-operator
>        |        |  |  |     |  +--rw (port-range-or-operator)?
>        |        |  |  |     |     +--:(range)
>        |        |  |  |     |     |  +--rw lower-port    inet:port-number
>        |        |  |  |     |     |  +--rw upper-port    inet:port-number
>        |        |  |  |     |     +--:(operator)
>        |        |  |  |     |        +--rw operator?     operator
>        |        |  |  |     |        +--rw port          inet:port-number
>        |        |  |  |     +--rw destination-port-range-or-operator
>        |        |  |  |     |  +--rw (port-range-or-operator)?
>        |        |  |  |     |     +--:(range)
>        |        |  |  |     |     |  +--rw lower-port    inet:port-number
>        |        |  |  |     |     |  +--rw upper-port    inet:port-number
>        |        |  |  |     |     +--:(operator)
>        |        |  |  |     |        +--rw operator?     operator
>        |        |  |  |     |        +--rw port          inet:port-number
>        |        |  |  |     +--rw tcp-all {match-on-tcp}?
>        |        |  |  |        +--rw sequence-number?          uint32
>        |        |  |  |        +--rw acknowledgement-number?   uint32
>        |        |  |  |        +--rw data-offset?              uint8
>        |        |  |  |        +--rw reserved?                 uint8
>        |        |  |  |        +--rw flags?                    bits
>        |        |  |  |        +--rw window-size?              uint16
>        |        |  |  |        +--rw urgent-pointer?           uint16
>        |        |  |  |        +--rw options?                  uint32
>        |        |  |  +--:(udp)
>        |        |  |  |  +--rw udp
>        |        |  |  |     +--rw source-port-range-or-operator
>        |        |  |  |     |  +--rw (port-range-or-operator)?
>        |        |  |  |     |     +--:(range)
>        |        |  |  |     |     |  +--rw lower-port    inet:port-number
>        |        |  |  |     |     |  +--rw upper-port    inet:port-number
>        |        |  |  |     |     +--:(operator)
>        |        |  |  |     |        +--rw operator?     operator
>        |        |  |  |     |        +--rw port          inet:port-number
>        |        |  |  |     +--rw destination-port-range-or-operator
>        |        |  |  |     |  +--rw (port-range-or-operator)?
>        |        |  |  |     |     +--:(range)
>        |        |  |  |     |     |  +--rw lower-port    inet:port-number
>        |        |  |  |     |     |  +--rw upper-port    inet:port-number
>        |        |  |  |     |     +--:(operator)
>        |        |  |  |     |        +--rw operator?     operator
>        |        |  |  |     |        +--rw port          inet:port-number
>        |        |  |  |     +--rw udp-all {match-on-udp}?
>        |        |  |  |        +--rw length?   uint16
> 
> A diff ietf-packet-fields.yang and ietf-access-control-lists.yang is attached.
> 
> Eliot
> 
> 
> 
> On 17.01.18 22:55, Kent Watsen wrote:
>> All,
>> 
>> This starts a two-week working group last call on
>> draft-ietf-netmod-acl-model-15.
>> 
>> This working group last call ends on January 31st.
>> Please send your comments to the NETMOD mailing list.
>> 
>> Positive comments, e.g., "I've reviewed this document
>> and believe it is ready for publication", are welcome!
>> This is useful and important, even from authors.
>> 
>> Also, could the authors, explicitly CC-ed on this email,
>> please confirm at this time that they are unaware of
>> any IPR related to this draft.
>> 
>> Thank you,
>> NETMOD Chairs
>> 
>> _______________________________________________
>> netmod mailing list
>> [e-mail] <mailto:[e-mail]>
>> https://www.ietf.org/mailman/listinfo/netmod <https://www.ietf.org/mailman/listinfo/netmod>
>> 
> 
> *** [e-mail]	Mon Jan 22 12:58:08 2018
> --- [e-mail]	Mon Jan 22 13:10:57 2018
> ***************
> *** 190,205 ****
>           payload. In IPv6, this field is known as 'next-header.";
>        reference "RFC 719, RFC 2460.";
>      }
> -     container source-port-range-or-operator {
> -       uses port-range-or-operator;
> -       description
> -         "Source port definition.";
> -     }
> -     container destination-port-range-or-operator {
> -       uses port-range-or-operator;
> -       description
> -         "Destination port definition.";
> -     }
>    }
> 
>    grouping acl-ipv4-header-fields {
> --- 190,195 ----
> *** [e-mail]	Mon Jan 22 10:16:17 2018
> --- [e-mail]	Mon Jan 22 13:09:06 2018
> ***************
> *** 440,457 ****
> 
>              choice l4 {
>                container tcp {
> !                 if-feature match-on-tcp;
>                  uses packet-fields:acl-tcp-header-fields;
> !                   description
>                      "Rule set that matches TCP headers.";
> !               }
> ! 
>                container udp {
> !                 if-feature match-on-udp;
> !                 uses packet-fields:acl-udp-header-fields;
> !                 description
> !                   "Rule set that matches UDP headers.";
> !               }
> 
>                container icmp {
>                  if-feature match-on-icmp;
> --- 440,482 ----
> 
>              choice l4 {
>                container tcp {
> ! 		container source-port-range-or-operator {
> ! 		   uses packet-fields:port-range-or-operator;
> ! 		      description
> ! 		        "Source port definition.";
> !                 }			
> ! 		container destination-port-range-or-operator {
> ! 		   uses packet-fields:port-range-or-operator;
> ! 		   description
> !  		     "Destination port definition.";
> ! 		}
> ! 		container tcp-all {
> !                    if-feature match-on-tcp;
>                  uses packet-fields:acl-tcp-header-fields;
> !                    description
>                      "Rule set that matches TCP headers.";
> !                 }
> ! 	      	description "TCP matchable characteristics";
> ! 	      }
>                container udp {
> ! 		container source-port-range-or-operator {
> ! 		   uses packet-fields:port-range-or-operator;
> ! 		      description
> ! 		        "Source port definition.";
> !                 }			
> ! 		container destination-port-range-or-operator {
> ! 		   uses packet-fields:port-range-or-operator;
> ! 		   description
> !  		     "Destination port definition.";
> ! 		}
> ! 		container udp-all {
> !                    if-feature match-on-udp;
> !                    uses packet-fields:acl-udp-header-fields;
> !                    description
> !                      "Rule set that matches UDP headers.";
> !                 }
> ! 	      	description "UDP matchable characteristics";
> ! 	      }
> 
>                container icmp {
>                  if-feature match-on-icmp;
> _______________________________________________
> netmod mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/netmod

Mahesh Jethanandani
[e-mail]