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

[Eliot Lear <[e-mail]>]

I'm ok with this as well, Mahesh.

Thanks,

Eliot


On 22.01.18 22:48, Mahesh Jethanandani wrote:
> 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]
>> <mailto:[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]
>>> https://www.ietf.org/mailman/listinfo/netmod
>>>
>>
>> *** [e-mail]
>> <mailto:[e-mail]>Mon Jan 22 12:58:08 2018
>> --- [e-mail]
>> <mailto:[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]
>> <mailto:[e-mail]>Mon Jan 22
>> 10:16:17 2018
>> --- [e-mail]
>> <mailto:[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] <mailto:[e-mail]>
>> https://www.ietf.org/mailman/listinfo/netmod
>
> Mahesh Jethanandani
> [e-mail] <mailto:[e-mail]>
>