| Message ID | 20180308013922.103884-1-vsairam@vmware.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] datapath-windows: Block established TCP connections from creating new CT Entry | expand |
On Wed, Mar 07, 2018 at 05:39:22PM -0800, Sairam Venugopal wrote: > Established TCP Connections could potentially form invalid Conntrack > Entries when OVS is getting installed or the Conntrack Flows are applied. > Prevent this from happening by explicitly requiring SYN packets to be > present for creating new Conntrack entries. > > Signed-off-by: Sairam Venugopal <vsairam@vmware.com> I think that this is the opposite of the policy direction that we're pursuing in the userspace connection tracker. Darrell, do I understand that correctly?
Hi Sai "loose mode" is the default for Linux/BSD/userpsace DP. I think this default makes sense because the most common use cases are: moving a connection from one instance of a firewall (stateless or stateful) to another. With loose mode disabled, we would drop the subsequent packets. Moving from no protection to some form of protection is less common and in that case, everything that happened before and the existing endpoint states are all tainted anyways. Thanks Darrell On Thu, Mar 8, 2018 at 10:43 AM, Ben Pfaff <blp@ovn.org> wrote: > On Wed, Mar 07, 2018 at 05:39:22PM -0800, Sairam Venugopal wrote: > > Established TCP Connections could potentially form invalid Conntrack > > Entries when OVS is getting installed or the Conntrack Flows are applied. > > Prevent this from happening by explicitly requiring SYN packets to be > > present for creating new Conntrack entries. > > > > Signed-off-by: Sairam Venugopal <vsairam@vmware.com> > > I think that this is the opposite of the policy direction that we're > pursuing in the userspace connection tracker. Darrell, do I understand > that correctly? >
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath-windows/ovsext/Conntrack-tcp.c index 8cbab24..ea43df1 100644 --- a/datapath-windows/ovsext/Conntrack-tcp.c +++ b/datapath-windows/ovsext/Conntrack-tcp.c @@ -455,9 +455,15 @@ OvsConntrackValidateTcpPacket(const TCPHdr *tcp) return FALSE; } + /* Block pre-established connections from going through */ + if (!(tcp_flags & TCP_SYN)) { + OVS_LOG_TRACE("Pre-established TCP packet detected, non-SYN flags not allowed," + "tcp_flags %hu", tcp_flags); + return FALSE; + } + /* A syn+ack is not allowed to create a connection. We want to allow - * totally new connections (syn) or already established, not partially - * open (syn+ack). */ + * totally new connections (syn), not partially open (syn+ack). */ if ((tcp_flags & TCP_SYN) && (tcp_flags & TCP_ACK)) { OVS_LOG_TRACE("Invalid TCP packet detected, SYN+ACK flags not allowed," "tcp_flags %hu", tcp_flags);
Established TCP Connections could potentially form invalid Conntrack Entries when OVS is getting installed or the Conntrack Flows are applied. Prevent this from happening by explicitly requiring SYN packets to be present for creating new Conntrack entries. Signed-off-by: Sairam Venugopal <vsairam@vmware.com> --- datapath-windows/ovsext/Conntrack-tcp.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)