From 92c67a52b28704c086dbccd302b576c6290cafb2 Mon Sep 17 00:00:00 2001
From: Aryan Patil <aryanpatil0700@gmail.com>
Date: Wed, 21 Oct 2026 20:53:10 +0530
Subject: [PATCH] firewall: validate that conntrack timeouts are strictly
 positive (#1984)

---
 firewall.go      | 20 +++++++++++++++++---
 firewall_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/firewall.go b/firewall.go
index 6029d888..2e1df4f8 100644
--- a/firewall.go
+++ b/firewall.go
@@ -206,11 +206,25 @@ func NewFirewallFromConfig(l *slog.Logger, cs *CertState, c *config.C) (*Firewal
 		panic("No certificate available to reconfigure the firewall")
 	}
 
+	tcpTimeout := c.GetDuration("firewall.conntrack.tcp_timeout", time.Minute*12)
+	udpTimeout := c.GetDuration("firewall.conntrack.udp_timeout", time.Minute*3)
+	defaultTimeout := c.GetDuration("firewall.conntrack.default_timeout", time.Minute*10)
+
+	if tcpTimeout <= 0 {
+		return nil, fmt.Errorf("firewall.conntrack.tcp_timeout must be greater than 0, got: %s", tcpTimeout)
+	}
+	if udpTimeout <= 0 {
+		return nil, fmt.Errorf("firewall.conntrack.udp_timeout must be greater than 0, got: %s", udpTimeout)
+	}
+	if defaultTimeout <= 0 {
+		return nil, fmt.Errorf("firewall.conntrack.default_timeout must be greater than 0, got: %s", defaultTimeout)
+	}
+
 	fw := NewFirewall(
 		l,
-		c.GetDuration("firewall.conntrack.tcp_timeout", time.Minute*12),
-		c.GetDuration("firewall.conntrack.udp_timeout", time.Minute*3),
-		c.GetDuration("firewall.conntrack.default_timeout", time.Minute*10),
+		tcpTimeout,
+		udpTimeout,
+		defaultTimeout,
 		certificate,
 		//TODO: max_connections
 	)
diff --git a/firewall_test.go b/firewall_test.go
index b8727915..3e4bcee0 100644
--- a/firewall_test.go
+++ b/firewall_test.go
@@ -58,6 +58,31 @@ func TestNewFirewall(t *testing.T) {
 	assert.Equal(t, 3602, conntrack.TimerWheel.wheelLen)
 }
 
+func TestNewFirewall_ZeroOrNegativeTimeout(t *testing.T) {
+	l := test.NewLogger()
+	c := &dummyCert{}
+	cs := &CertState{v2Cert: c, initiatingVersion: cert.Version2}
+
+	cases := map[string]string{
+		"tcp_timeout zero":         "firewall:\n  conntrack:\n    tcp_timeout: 0s\n",
+		"tcp_timeout negative":     "firewall:\n  conntrack:\n    tcp_timeout: -5s\n",
+		"udp_timeout zero":         "firewall:\n  conntrack:\n    udp_timeout: 0s\n",
+		"udp_timeout negative":     "firewall:\n  conntrack:\n    udp_timeout: -5s\n",
+		"default_timeout zero":     "firewall:\n  conntrack:\n    default_timeout: 0s\n",
+		"default_timeout negative": "firewall:\n  conntrack:\n    default_timeout: -5s\n",
+	}
+
+	for name, cfgStr := range cases {
+		t.Run(name, func(t *testing.T) {
+			cfg := config.NewC(l)
+			require.NoError(t, cfg.LoadString(cfgStr))
+			_, err := NewFirewallFromConfig(l, cs, cfg)
+			require.Error(t, err, "expected error for non-positive timeout duration")
+			assert.ErrorContains(t, err, "must be greater than 0")
+		})
+	}
+}
+
 func TestFirewall_AddRule(t *testing.T) {
 	ob := &bytes.Buffer{}
 	l := test.NewLoggerWithOutput(ob)