On Sat, 2017-06-17 at 11:27 +0200, Andrew 👽 Yourtchenko wrote: > 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. +1
> > --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
