diff --git a/firewall.go b/firewall.go index 457210a6..dddc1a0e 100644 --- a/firewall.go +++ b/firewall.go @@ -654,19 +654,6 @@ func (f *Firewall) inConns(fp firewall.Packet, h *HostInfo, caPool *cert.CAPool, c, ok := conntrack.Conns[fp] - if !ok && fp.Protocol == firewall.ProtoICMP { - //todo this seems like it will also bite me - oldRemote := fp.RemotePort - oldLocal := fp.LocalPort - fp.RemotePort = 0 - fp.LocalPort = 0 - c, ok = conntrack.Conns[fp] - if ok { - fp.RemotePort = oldRemote - fp.LocalPort = oldLocal - } - } - if !ok { conntrack.Unlock() return nil @@ -769,15 +756,6 @@ func (f *Firewall) addConn(fp firewall.Packet, incoming bool) *conn { c.Expires = time.Now().Add(timeout) conntrack.Conns[fp] = c - //todo will this bite me somehow? - if fp.Protocol == firewall.ProtoICMP { - //not required for ICMPv6 because we don't decode or SNAT it - //create a duplicate conntrack entry with all the port information zeroed? - fp.RemotePort = 0 - fp.LocalPort = 0 - conntrack.Conns[fp] = c - } - conntrack.Unlock() return c } diff --git a/firewall_test.go b/firewall_test.go index 77e7cc69..4a98acfc 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -845,6 +845,7 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { fw := NewFirewall(l, time.Second, time.Minute, time.Hour, c.Certificate) require.NoError(t, fw.AddRule(true, firewall.ProtoAny, 0, 0, []string{"any"}, "", "", "", "", "")) t.Run("zero ports, allowed", func(t *testing.T) { + resetConntrack(fw) p := templ.Copy() p.LocalPort = 0 p.RemotePort = 0 @@ -858,6 +859,7 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { }) t.Run("nonzero ports, allowed", func(t *testing.T) { + resetConntrack(fw) p := templ.Copy() p.LocalPort = 0xabcd p.RemotePort = 0x1234 @@ -868,20 +870,9 @@ func TestFirewall_ICMPPortBehavior(t *testing.T) { require.NoError(t, fw.Drop(*p, nil, true, &h, cp, nil)) //now also allow outbound require.NoError(t, fw.Drop(*p, nil, false, &h, cp, nil)) - }) - - t.Run("nonzero ports, allowed", func(t *testing.T) { - p := templ.Copy() - p.LocalPort = 0xabcd - p.RemotePort = 0x1234 - // Drop outbound - assert.Equal(t, fw.Drop(*p, nil, false, &h, cp, nil), ErrNoMatchingRule) - // Allow inbound - resetConntrack(fw) - require.NoError(t, fw.Drop(*p, nil, true, &h, cp, nil)) - //now also allow outbound with a different ID + //different ID is blocked p.RemotePort++ - require.NoError(t, fw.Drop(*p, nil, false, &h, cp, nil)) + require.Equal(t, fw.Drop(*p, nil, false, &h, cp, nil), ErrNoMatchingRule) }) })