Perfect, thanks a lot! I've pushed the update https://gerrit.fd.io/r/#/c/6858/10 which codifies all of our discussion.
With that change, both applying an inexistent ACL and deleting the ACL that is applied somewhere will be an error. One can still create an ACL with no rules, in which case applying that ACL will follow the lookup logic with the default-deny in the end of the vector of ACLs lookup. --a On 6/17/17, Luke, Chris <chris_l...@comcast.com> wrote: > That was going to be one of my queries, but forgot to ask. Do we allow that > currently? Probably shouldn't, as you say, for symmetry. > > Chris. > >> -----Original Message----- >> From: Andrew Yourtchenko [mailto:ayour...@gmail.com] >> Sent: Friday, June 16, 2017 17:51 >> To: Luke, Chris <chris_l...@cable.comcast.com> >> Cc: Marco Varlese <marco.varl...@suse.com>; vpp-dev@lists.fd.io >> Subject: Re: [vpp-dev] Bind / Unbind of ACL >> >> Ok! So what do you think if then we were to also disallow applying the >> ACL >> that doesn't exist yet ? >> >> It feels like it would be a matching symmetric behavior "from the other >> side". >> ? >> >> --a >> >> On 16 Jun 2017, at 15:38, Luke, Chris <chris_l...@comcast.com> wrote: >> >> >> From: Marco Varlese [mailto:marco.varl...@suse.com] >> >> Sent: Friday, June 16, 2017 9:23 >> >>> On Fri, 2017-06-16 at 15:12 +0200, Andrew 👽 Yourtchenko wrote: >> >>>> On 6/16/17, Marco Varlese <marco.varl...@suse.com> wrote: >> >>>> >> >>>>> On Thu, 2017-06-15 at 14:22 +0200, Andrew 👽 Yourtchenko wrote: >> >>>>> >> >>>>> After a bit more thinking - there is a way that should take care of >> >>>>> both: >> >>>>> >> >>>>> 1) What Chris wrote: have consistent behaviour with non-existent >> >>>>> ACL as if the ACL matching fell off the end of the ACL: if an >> >>>>> empty ACL is referenced, treat it as if it had entries but none of >> >>>>> them had matched. Then we still hit the "default deny" if none of >> >>>>> the applied ACLs match, and drop the packets - so it will be >> >>>>> logical. >> >>>>> >> >>>>> 2) What Marco wrote: when deleting an already referenced ACL, >> >>>>> unapply it from all the places where it is applied. >> >>>>> >> >>>>> It's a change in the behaviour in that the current behaviour is to >> >>>>> have the empty ACL act as if it was "deny any any", and block the >> >>>>> matching even if there is another ACL after it - but on the other >> >>>>> hand this would take both viewpoints in mind... >> >>>> I think this approach would still leave the system in an >> >>>> inconsistent >> state: >> >>>> an >> >>>> interface is basically assigned an ACL which does not exist in the >> system. >> >>>> Also, the risk I see is if I later on create an ACL with the >> >>>> previously used index then an interface would see that ACL being >> >>>> applied automatically (since it's referenced). However, I may not >> >>>> want to have that ACL assigned to that particular interface. >> >>>> Correct? >> >>>> >> >>>> I think that the "deletion" of an ACL could see one of the two >> >>>> behaviours >> >>>> below: >> >>>> 1) the deletion of an ACL should be DENIED if that ACL is assigned >> >>>> to any interface (probably the easier and safer approach); >> >>>> 2) the deletion of an ACL should see a CASCADING effect onto the >> >>>> interfaces which would be "cleaned up" of any references to that >> >>>> ACL; >> >>>> >> >>> >> >>> Right, the (2) was what I was trying to suggest to do... >> >>> >> >>>> >> >>>> I think (1) is a very good way of solving the initial problem since >> >>>> it works nicely if you manage VPP directly (e.g. via command-line) >> >>>> and if you use a controller. In the latter, the controller can >> >>>> react on the "error" returned by the acl_del API because that ACL >> >>>> is assigned somewhere. >> >>>> >> >>> >> >>> ...but the (1) is another good option to me. >> >>> >> >>> So it seems we are converging on (1) ? >> >> I would go with (1)... >> > >> > I feel I have a slight preference for this (1) also; In general I don't >> > like the >> implicit actions such as in (2). >> > >> > Chris >> > >> >>> >> >>> --a >> >>> >> >>> >> >>> >> >>>> >> >>>> >> >>>> Cheers, >> >>>> Marco >> >>>> >> >>>>> >> >>>>> >> >>>>> What do you think ? >> >>>>> >> >>>>> --a >> >>>>> >> >>>>>> On 6/9/17, Andrew 👽 Yourtchenko <ayour...@gmail.com> wrote: >> >>>>>> >> >>>>>> >> >>>>>> Assuming the only change is to effectively have >> >>>>>> "unbind_acl_from_everywhere; delete_acl" instead of "delete_acl", >> >>>>>> maybe it would be best to tackle that post-17.07 with a separate >> >>>>>> API message acl_del_and_unbind or similar ? >> >>>>>> >> >>>>>> I feel a beet wary of adding more hidden state (even though the >> >>>>>> reflected sessions table does provide already plenty of it :) >> >>>>>> >> >>>>>> --a >> >>>>>> >> >>>>>>> On 6/9/17, Luke, Chris <chris_l...@comcast.com> wrote: >> >>>>>>> >> >>>>>>> >> >>>>>>> Would it make sense to have a flag on the interface (or >> >>>>>>> globally), set when applying the ACL, that indicates the desired >> >>>>>>> behavior when the ACL is empty or non-existent? At the moment >> to >> >>>>>>> me it seems logical that this is the same behavior as when >> >>>>>>> matching falls off the end of the ACL. >> >>>>>>> >> >>>>>>> Chris. >> >>>>>>> >> >>>>>>>> >> >>>>>>>> >> >>>>>>>> -----Original Message----- >> >>>>>>>> From: vpp-dev-boun...@lists.fd.io >> >>>>>>>> [mailto:vpp-dev-boun...@lists.fd.io] >> >>>>>>>> On >> >>>>>>>> Behalf Of Andrew ?? Yourtchenko >> >>>>>>>> Sent: Friday, June 9, 2017 7:53 >> >>>>>>>> To: Marco Varlese <marco.varl...@suse.com> >> >>>>>>>> Cc: vpp-dev@lists.fd.io >> >>>>>>>> Subject: Re: [vpp-dev] Bind / Unbind of ACL >> >>>>>>>> >> >>>>>>>> Hi Marco, >> >>>>>>>> >> >>>>>>>> Yes, this works as expected, assuming after deletion *all* the >> >>>>>>>> traffic is denied, rather than just the SSH traffic. >> >>>>>>>> >> >>>>>>>> If you apply to an interface the ACL# that does not exist, that >> >>>>>>>> is the same as if there was an ACL with just the "deny all" >> >>>>>>>> semantics, to avoid the perception that a given policy is >> >>>>>>>> enforced when it isn't - so I erred on the side of caution. >> >>>>>>>> >> >>>>>>>> The way to remove the ACL: you would ensure the ACL is not >> >>>>>>>> applied to the >> >>>>>>>> interface(s) first, then remove the ACL (or replace it with a >> >>>>>>>> different policy in- place). >> >>>>>>>> >> >>>>>>>> Alternatively, you can just replace the existing ACL in-place >> >>>>>>>> with "permit any" >> >>>>>>>> for IPv4 and IPv6 - this way you explicitly state that there is >> >>>>>>>> a policy to permit all the traffic. >> >>>>>>>> >> >>>>>>>> I've been bitten myself and seen several times in my career >> >>>>>>>> when an applied but non-existent ACL caused problems later on, >> >>>>>>>> in the worst possible moment. The current behaviour IMHO >> makes >> >>>>>>>> the config discrepancy clear - what do you think ? >> >>>>>>>> >> >>>>>>>> --a >> >>>>>>>> >> >>>>>>>>> On 6/9/17, Marco Varlese <marco.varl...@suse.com> wrote: >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> Hi, >> >>>>>>>>> >> >>>>>>>>> I am trying the ACL functionality and I found a "strange" >> >>>>>>>>> behaviour. >> >>>>>>>>> >> >>>>>>>>> The steps I follow to use an ACL are: >> >>>>>>>>> * I create an ACL to deny SSH traffic between VMs (via the >> >>>>>>>> 'acl_add_replace' >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> function) >> >>>>>>>>> * Set that ACL to the interfaces involved (via the >> >>>>>>>>> 'acl_interface_set_acl_list' >> >>>>>>>>> function) >> >>>>>>>>> >> >>>>>>>>> After performing the above steps the traffic was correctly >> >>>>>>>>> being blocked. >> >>>>>>>>> >> >>>>>>>>> However, when I decided to enable the SSH traffic again, I >> >>>>>>>>> simply deleted the ACL (via the 'acl_del' function) with the >> >>>>>>>>> consequence though that the traffic was still being denied. >> >>>>>>>>> >> >>>>>>>>> Is this behaviour correct? >> >>>>>>>>> If so what would be the right way to unset hence disable a >> >>>>>>>>> given ACL from an interface (or multiple)? >> >>>>>>>>> >> >>>>>>>>> >> >>>>>>>>> Thanks, >> >>>>>>>>> Marco >> >>>>>>>>> >> >>>>>>>>> _______________________________________________ >> >>>>>>>>> vpp-dev mailing list >> >>>>>>>>> vpp-dev@lists.fd.io >> >>>>>>>>> https://lists.fd.io/mailman/listinfo/vpp-dev >> >>>>>>>> _______________________________________________ >> >>>>>>>> vpp-dev mailing list >> >>>>>>>> vpp-dev@lists.fd.io >> >>>>>>>> https://lists.fd.io/mailman/listinfo/vpp-dev >> >>>>>>> >> >>>>>>> >> >>>>>> >> >>>>> >> >>>> >> >>> >> > > > _______________________________________________ vpp-dev mailing list vpp-dev@lists.fd.io https://lists.fd.io/mailman/listinfo/vpp-dev
