On Mon, Mar 21, 2016 at 10:54:59AM -0400, Russell Bryant wrote: > Prior to this commit, once a connection had been committed to the > connection tracker, the connection would continue to be allowed, even > if the policy defined in the ACL table changed. This patch changes > the implementation so that existing connections are affected by policy > changes. > > The implementation is based on the suggested approach in this mailing > list thread: > > http://openvswitch.org/pipermail/dev/2016-February/065716.html > > Instead of always allowing packets associated with an established > connection, we now put all packets in the request direction through > the flows generated based on OVN ACLs. If a packet associated with an > established connection hits a "drop" ACL, that means we have > encountered a policy change and should drop packets associated with > this connection from now on. We handle this by setting "ct_label" on > the associated connection tracking entry. > > These changes also account for re-allowing a known connection after > ct_label had been set on it. This can happen if you delete an ACL and > then re-create it while connection state is still known. > > The proposal on the mailing list also discussed the idea that > ovn-controller could periodically sweep the connection tracker and > delete entries with ct_label set. That is not implemented in this > patch. Instead, we rely on connections dying since we're dropping > its packets and then allowing the connection tracking entry to > eventually time out. More proactively clearing them out could be a > future enhancement. > > As a realistic example of how this works, consider this security policy > from an OpenStack+OVN development environment. > > +---------+-----------------------+ > | name | security_group_rules | > +---------+-----------------------+ > | default | egress, IPv4 | > | | egress, IPv6 | > | | ingress, IPv4, 22/tcp | > | | ingress, IPv4, icmp | > +---------+-----------------------+ > > The OpenStack Neutron plugin creates ACLs that drop traffic by default > and higher priority ACLs for each type of traffic that is allowed. In > this case, the ACLs for a port using the "default" security group are: > > from-lport 1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4) > allow-related > from-lport 1002 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6) > allow-related > from-lport 1001 (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip) > drop > to-lport 1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 > && icmp4) allow-related > to-lport 1002 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 > && tcp && tcp.dst == 22) allow-related > to-lport 1001 (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip) > drop > > which results in the following logical flows: > > ... > table=1( ls_in_pre_acl), priority= 100, match=(ip), action=(ct_next;) > table=1( ls_in_pre_acl), priority= 0, match=(1), action=(next;) > table=2( ls_in_acl), priority=65535, match=(!ct.est && ct.rel && > !ct.new && !ct.inv && ct_label[0] == 0), action=(next;) > table=2( ls_in_acl), priority=65535, match=(ct.est && !ct.rel && > !ct.new && !ct.inv && ct.rpl && ct_label[0] == 0), action=(next;) > table=2( ls_in_acl), priority=65535, match=(ct.inv || (ct.est && > ct.rpl && ct_label[0] == 1)), action=(drop;) > table=2( ls_in_acl), priority= 2002, match=(!ct.new && ct.est && > !ct.rpl && ct_label[0] == 0 && (inport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)), action=(next;) > table=2( ls_in_acl), priority= 2002, match=(!ct.new && ct.est && > !ct.rpl && ct_label[0] == 0 && (inport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)), action=(next;) > table=2( ls_in_acl), priority= 2002, match=(((ct.new && !ct.est) || > (!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (inport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4)), > action=(ct_commit(ct_label=0); next;) > table=2( ls_in_acl), priority= 2002, match=(((ct.new && !ct.est) || > (!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (inport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip6)), > action=(ct_commit(ct_label=0); next;) > table=2( ls_in_acl), priority= 2001, match=((!ct.est || (ct.est && > ct_label[0] == 1)) && (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && > ip)), action=(drop;) > table=2( ls_in_acl), priority= 2001, match=(ct.est && ct_label[0] == > 0 && (inport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip)), > action=(ct_commit(ct_label=1);) > table=2( ls_in_acl), priority= 1, match=(ip && (!ct.est || (ct.est > && ct_label[0] == 1))), action=(ct_commit(ct_label=0); next;) > table=2( ls_in_acl), priority= 0, match=(1), action=(next;) > ... > table=0( ls_out_pre_acl), priority= 100, match=(ip), action=(ct_next;) > table=0( ls_out_pre_acl), priority= 0, match=(1), action=(next;) > table=1( ls_out_acl), priority=65535, match=(!ct.est && ct.rel && > !ct.new && !ct.inv && ct_label[0] == 0), action=(next;) > table=1( ls_out_acl), priority=65535, match=(ct.est && !ct.rel && > !ct.new && !ct.inv && ct.rpl && ct_label[0] == 0), action=(next;) > table=1( ls_out_acl), priority=65535, match=(ct.inv || (ct.est && > ct.rpl && ct_label[0] == 1)), action=(drop;) > table=1( ls_out_acl), priority= 2002, match=(!ct.new && ct.est && > !ct.rpl && ct_label[0] == 0 && (outport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && icmp4)), action=(next;) > table=1( ls_out_acl), priority= 2002, match=(!ct.new && ct.est && > !ct.rpl && ct_label[0] == 0 && (outport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && tcp && tcp.dst == 22)), > action=(next;) > table=1( ls_out_acl), priority= 2002, match=(((ct.new && !ct.est) || > (!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (outport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && icmp4)), > action=(ct_commit(ct_label=0); next;) > table=1( ls_out_acl), priority= 2002, match=(((ct.new && !ct.est) || > (!ct.new && ct.est && !ct.rpl && ct_label[0] == 1)) && (outport == > "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip4 && tcp && tcp.dst == 22)), > action=(ct_commit(ct_label=0); next;) > table=1( ls_out_acl), priority= 2001, match=((!ct.est || (ct.est && > ct_label[0] == 1)) && (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && > ip)), action=(drop;) > table=1( ls_out_acl), priority= 2001, match=(ct.est && ct_label[0] == > 0 && (outport == "0a7409c8-d179-4915-9eb2-f53426ae16dd" && ip)), > action=(ct_commit(ct_label=1);) > table=1( ls_out_acl), priority= 1, match=(ip && (!ct.est || (ct.est > && ct_label[0] == 1))), action=(ct_commit(ct_label=0); next;) > table=1( ls_out_acl), priority= 0, match=(1), action=(next;) > ... > > One way I tested this by leaving ping running, ensuring that it was > blocked when the rule for ICMP was deleted, and then re-allowed when > the rule allowing ICMP was restored. In this case, the ICMP > connection is still known by the connection tracker, but the flows > ensure that ct_label gets reset back to 0. > > Reported-by: Xiao Li Xu <xiaol...@cn.ibm.com> > Reported-at: https://bugs.launchpad.net/networking-ovn/+bug/1536080 > Suggested-by: Justin Pettit <jpet...@ovn.org> > Signed-off-by: Russell Bryant <russ...@ovn.org> > Acked-by: Han Zhou <zhou...@gmail.com>
Thanks for doing all this work! Acked-by: Ben Pfaff <b...@ovn.org> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
