mirrors/nebula
1
Fork 0
mirror of https://github.com/slackhq/nebula.git synced 2026-01-26 14:31:58 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions

remove sus conntrack change for ICMP

Browse source
This commit is contained in:
JackDoan 2026-01-23 13:35:57 -06:00
parent cf2b5455bf
commit f91346db5f
2 changed files with 4 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

22
firewall.go
View file

@ -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
}

17
firewall_test.go
View file

@ -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)
})
})