From e090833d29738274c1d171eba53e239c1c49ea7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gustavo=20I=C3=B1iguez=20Goia?= <gooffy1@gmail.com>
Date: Sun, 23 Jul 2023 15:14:05 +0200
Subject: [PATCH] intercept packets only with the SYN flag set

Using "ct state NEW" to intercept packets causes some undesired effects:
  We intercept packets that not only have the SYN flag set, like ACK,
  ACK+PSH or SYN+ACK. Mainly response packets.

  This means that the IPs are not always in the expected order:
  443:1.1.1.1 -> 192.168.1.123:12345
  which causes sometimes not to obtain the process of the connection,
  because the connection in the system appears as
  12345:192.168.1.123 -> 1.1.1.1:443

Intercepting packets with *only* the SYN flag set seems to resolve
this problem.
---
 daemon/firewall/nftables/monitor.go |  8 ++--
 daemon/firewall/nftables/rules.go   | 58 +++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/daemon/firewall/nftables/monitor.go b/daemon/firewall/nftables/monitor.go
index 60e5c1f02b..b8a2754b0a 100644
--- a/daemon/firewall/nftables/monitor.go
+++ b/daemon/firewall/nftables/monitor.go
@@ -33,16 +33,16 @@ func (n *Nft) AreRulesLoaded() bool {
 					return false
 				}
 				nRules++
-				if c.Table.Name == exprs.NFT_CHAIN_MANGLE && rdx+1 != len(rules) {
-					log.Warning("nfables queue rule is not the latest of the list, reloading")
+				if c.Table.Name == exprs.NFT_CHAIN_MANGLE && rdx < len(rules)-2 {
+					log.Warning("nfables queue rule is not the latest of the list (%d/%d), reloading", rdx, len(rules))
 					return false
 				}
 			}
 		}
 	}
-	// we expect to have exactly 2 rules (queue and dns). If there're less or more, then we
+	// we expect to have exactly 3 rules (2 queue and dns). If there're less or more, then we
 	// need to reload them.
-	if nRules != 2 {
+	if nRules != 3 {
 		log.Warning("nfables filter rules not loaded: %d", nRules)
 		return false
 	}
diff --git a/daemon/firewall/nftables/rules.go b/daemon/firewall/nftables/rules.go
index 9fe75e1f2a..9640acf890 100644
--- a/daemon/firewall/nftables/rules.go
+++ b/daemon/firewall/nftables/rules.go
@@ -96,6 +96,12 @@ func (n *Nft) QueueConnections(enable bool, logError bool) (error, error) {
 		Table:    table,
 		Chain:    chain,
 		Exprs: []expr.Any{
+			&expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1},
+			&expr.Cmp{
+				Op:       expr.CmpOpNeq,
+				Register: 1,
+				Data:     []byte{unix.IPPROTO_TCP},
+			},
 			&expr.Ct{Register: 1, SourceRegister: false, Key: expr.CtKeySTATE},
 			&expr.Bitwise{
 				SourceRegister: 1,
@@ -113,6 +119,58 @@ func (n *Nft) QueueConnections(enable bool, logError bool) (error, error) {
 		// rule key, to allow get it later by key
 		UserData: []byte(InterceptionRuleKey),
 	})
+
+	/* nft --debug=netlink add rule inet mangle output tcp flags '& (fin|syn|rst|ack) == syn' queue bypass num 0
+	[ meta load l4proto => reg 1 ]
+	[ cmp eq reg 1 0x00000006 ]
+	[ payload load 1b @ transport header + 13 => reg 1 ]
+	[ bitwise reg 1 = ( reg 1 & 0x00000002 ) ^ 0x00000000 ]
+	[ cmp neq reg 1 0x00000000 ]
+	[ queue num 0 bypass ]
+
+	Intercept packets *only* with the SYN flag set.
+	Using 'ct state NEW' causes to intercept packets with other flags set, which
+	sometimes means that we receive outbound connections not in the expected order:
+	  443:1.1.1.1 -> 192.168.123:12345 (bits ACK, ACK+PSH or SYN+ACK set)
+	*/
+	n.Conn.AddRule(&nftables.Rule{
+		Position: 0,
+		Table:    table,
+		Chain:    chain,
+		Exprs: []expr.Any{
+			&expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1},
+			&expr.Cmp{
+				Op:       expr.CmpOpEq,
+				Register: 1,
+				Data:     []byte{unix.IPPROTO_TCP},
+			},
+			&expr.Payload{
+				DestRegister: 1,
+				Base:         expr.PayloadBaseTransportHeader,
+				Offset:       13,
+				Len:          1,
+			},
+			&expr.Bitwise{
+				DestRegister:   1,
+				SourceRegister: 1,
+				Len:            1,
+				Mask:           []byte{0x17},
+				Xor:            []byte{0x00},
+			},
+			&expr.Cmp{
+				Op:       expr.CmpOpEq,
+				Register: 1,
+				Data:     []byte{0x02},
+			},
+			&expr.Queue{
+				Num:  n.QueueNum,
+				Flag: expr.QueueFlagBypass,
+			},
+		},
+		// rule key, to allow get it later by key
+		UserData: []byte(InterceptionRuleKey),
+	})
+
 	// apply changes
 	if !n.Commit() {
 		return fmt.Errorf("Error adding interception rule "), nil