From f41db52560073d11837f05180bd10e497232fe52 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 13 Nov 2023 07:58:45 -0800 Subject: [PATCH 1/3] Bump the golang-x-dependencies group with 1 update (#1006) Bumps the golang-x-dependencies group with 1 update: [golang.org/x/sys](https://github.com/golang/sys). - [Commits](https://github.com/golang/sys/compare/v0.13.0...v0.14.0) --- updated-dependencies: - dependency-name: golang.org/x/sys dependency-type: direct:production update-type: version-update:semver-minor dependency-group: golang-x-dependencies ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 9c7b38fe9..4bef77c07 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( golang.org/x/crypto v0.14.0 golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 golang.org/x/net v0.17.0 - golang.org/x/sys v0.13.0 + golang.org/x/sys v0.14.0 golang.org/x/term v0.13.0 golang.zx2c4.com/wintun v0.0.0-20230126152724-0fa3db229ce2 golang.zx2c4.com/wireguard v0.0.0-20230325221338-052af4a8072b diff --git a/go.sum b/go.sum index 8564bd268..08f53789e 100644 --- a/go.sum +++ b/go.sum @@ -193,8 +193,8 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= -golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q= +golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= From 3356e03d855d4c9e2f36fe7636d1ebf781653bc4 Mon Sep 17 00:00:00 2001 From: Nate Brown Date: Mon, 13 Nov 2023 12:39:38 -0600 Subject: [PATCH 2/3] Default `pki.disconnect_invalid` to true and make it reloadable (#859) --- connection_manager.go | 2 +- connection_manager_test.go | 18 +++++++++--------- examples/config.yml | 2 +- interface.go | 16 +++++++++++++--- main.go | 2 +- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/connection_manager.go b/connection_manager.go index ce11f1966..a1897566a 100644 --- a/connection_manager.go +++ b/connection_manager.go @@ -432,7 +432,7 @@ func (n *connectionManager) isInvalidCertificate(now time.Time, hostinfo *HostIn return false } - if !n.intf.disconnectInvalid && err != cert.ErrBlockListed { + if !n.intf.disconnectInvalid.Load() && err != cert.ErrBlockListed { // Block listed certificates should always be disconnected return false } diff --git a/connection_manager_test.go b/connection_manager_test.go index e802904e1..5bc3f6f5c 100644 --- a/connection_manager_test.go +++ b/connection_manager_test.go @@ -253,18 +253,18 @@ func Test_NewConnectionManagerTest_DisconnectInvalid(t *testing.T) { lh := newTestLighthouse() ifce := &Interface{ - hostMap: hostMap, - inside: &test.NoopTun{}, - outside: &udp.NoopConn{}, - firewall: &Firewall{}, - lightHouse: lh, - handshakeManager: NewHandshakeManager(l, hostMap, lh, &udp.NoopConn{}, defaultHandshakeConfig), - l: l, - disconnectInvalid: true, - pki: &PKI{}, + hostMap: hostMap, + inside: &test.NoopTun{}, + outside: &udp.NoopConn{}, + firewall: &Firewall{}, + lightHouse: lh, + handshakeManager: NewHandshakeManager(l, hostMap, lh, &udp.NoopConn{}, defaultHandshakeConfig), + l: l, + pki: &PKI{}, } ifce.pki.cs.Store(cs) ifce.pki.caPool.Store(ncp) + ifce.disconnectInvalid.Store(true) // Create manager ctx, cancel := context.WithCancel(context.Background()) diff --git a/examples/config.yml b/examples/config.yml index 1cc94492f..c0ac0f6b2 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -11,7 +11,7 @@ pki: #blocklist: # - c99d4e650533b92061b09918e838a5a0a6aaee21eed1d12fd937682865936c72 # disconnect_invalid is a toggle to force a client to be disconnected if the certificate is expired or invalid. - #disconnect_invalid: false + #disconnect_invalid: true # The static host map defines a set of hosts with fixed IP addresses on the internet (or any network). # A host can have multiple fixed IP addresses defined here, and nebula will try each when establishing a tunnel. diff --git a/interface.go b/interface.go index fbf610a9b..a86b6f025 100644 --- a/interface.go +++ b/interface.go @@ -40,7 +40,6 @@ type InterfaceConfig struct { routines int MessageMetrics *MessageMetrics version string - disconnectInvalid bool relayManager *relayManager punchy *Punchy @@ -69,7 +68,7 @@ type Interface struct { dropLocalBroadcast bool dropMulticast bool routines int - disconnectInvalid bool + disconnectInvalid atomic.Bool closed atomic.Bool relayManager *relayManager @@ -176,7 +175,6 @@ func NewInterface(ctx context.Context, c *InterfaceConfig) (*Interface, error) { version: c.version, writers: make([]udp.Conn, c.routines), readers: make([]io.ReadWriteCloser, c.routines), - disconnectInvalid: c.disconnectInvalid, myVpnIp: myVpnIp, relayManager: c.relayManager, @@ -294,12 +292,24 @@ func (f *Interface) listenIn(reader io.ReadWriteCloser, i int) { func (f *Interface) RegisterConfigChangeCallbacks(c *config.C) { c.RegisterReloadCallback(f.reloadFirewall) c.RegisterReloadCallback(f.reloadSendRecvError) + c.RegisterReloadCallback(f.reloadDisconnectInvalid) c.RegisterReloadCallback(f.reloadMisc) + for _, udpConn := range f.writers { c.RegisterReloadCallback(udpConn.ReloadConfig) } } +func (f *Interface) reloadDisconnectInvalid(c *config.C) { + initial := c.InitialLoad() + if initial || c.HasChanged("pki.disconnect_invalid") { + f.disconnectInvalid.Store(c.GetBool("pki.disconnect_invalid", true)) + if !initial { + f.l.Infof("pki.disconnect_invalid changed to %v", f.disconnectInvalid.Load()) + } + } +} + func (f *Interface) reloadFirewall(c *config.C) { //TODO: need to trigger/detect if the certificate changed too if c.HasChanged("firewall") == false { diff --git a/main.go b/main.go index 08a32ff71..14696ac08 100644 --- a/main.go +++ b/main.go @@ -273,7 +273,6 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg routines: routines, MessageMetrics: messageMetrics, version: buildVersion, - disconnectInvalid: c.GetBool("pki.disconnect_invalid", false), relayManager: NewRelayManager(ctx, l, hostMap, c), punchy: punchy, @@ -303,6 +302,7 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg lightHouse.ifce = ifce ifce.RegisterConfigChangeCallbacks(c) + ifce.reloadDisconnectInvalid(c) ifce.reloadSendRecvError(c) handshakeManager.f = ifce From fe16ea566d273cfc368112bb6ffa286d76495aa9 Mon Sep 17 00:00:00 2001 From: Wade Simmons Date: Mon, 13 Nov 2023 10:43:51 -0800 Subject: [PATCH 3/3] firewall reject packets: cleanup error cases (#957) --- inside.go | 26 +++++++++++---- iputil/packet.go | 41 +++++++++++++++++++----- iputil/packet_test.go | 73 +++++++++++++++++++++++++++++++++++++++++++ outside.go | 4 ++- 4 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 iputil/packet_test.go diff --git a/inside.go b/inside.go index 9250b5e5a..ff9e80b6e 100644 --- a/inside.go +++ b/inside.go @@ -83,6 +83,10 @@ func (f *Interface) rejectInside(packet []byte, out []byte, q int) { } out = iputil.CreateRejectPacket(packet, out) + if len(out) == 0 { + return + } + _, err := f.readers[q].Write(out) if err != nil { f.l.WithError(err).Error("Failed to write to tun") @@ -94,12 +98,22 @@ func (f *Interface) rejectOutside(packet []byte, ci *ConnectionState, hostinfo * return } - // Use some out buffer space to build the packet before encryption - // Need 40 bytes for the reject packet (20 byte ipv4 header, 20 byte tcp rst packet) - // Leave 100 bytes for the encrypted packet (60 byte Nebula header, 40 byte reject packet) - out = out[:140] - outPacket := iputil.CreateRejectPacket(packet, out[100:]) - f.sendNoMetrics(header.Message, 0, ci, hostinfo, nil, outPacket, nb, out, q) + out = iputil.CreateRejectPacket(packet, out) + if len(out) == 0 { + return + } + + if len(out) > iputil.MaxRejectPacketSize { + if f.l.GetLevel() >= logrus.InfoLevel { + f.l. + WithField("packet", packet). + WithField("outPacket", out). + Info("rejectOutside: packet too big, not sending") + } + return + } + + f.sendNoMetrics(header.Message, 0, ci, hostinfo, nil, out, nb, packet, q) } func (f *Interface) Handshake(vpnIp iputil.VpnIp) { diff --git a/iputil/packet.go b/iputil/packet.go index 74ae37f09..b18e52447 100644 --- a/iputil/packet.go +++ b/iputil/packet.go @@ -6,8 +6,19 @@ import ( "golang.org/x/net/ipv4" ) +const ( + // Need 96 bytes for the largest reject packet: + // - 20 byte ipv4 header + // - 8 byte icmpv4 header + // - 68 byte body (60 byte max orig ipv4 header + 8 byte orig icmpv4 header) + MaxRejectPacketSize = ipv4.HeaderLen + 8 + 60 + 8 +) + func CreateRejectPacket(packet []byte, out []byte) []byte { - // TODO ipv4 only, need to fix when inside supports ipv6 + if len(packet) < ipv4.HeaderLen || int(packet[0]>>4) != ipv4.Version { + return nil + } + switch packet[9] { case 6: // tcp return ipv4CreateRejectTCPPacket(packet, out) @@ -19,20 +30,28 @@ func CreateRejectPacket(packet []byte, out []byte) []byte { func ipv4CreateRejectICMPPacket(packet []byte, out []byte) []byte { ihl := int(packet[0]&0x0f) << 2 - // ICMP reply includes header and first 8 bytes of the packet + if len(packet) < ihl { + // We need at least this many bytes for this to be a valid packet + return nil + } + + // ICMP reply includes original header and first 8 bytes of the packet packetLen := len(packet) if packetLen > ihl+8 { packetLen = ihl + 8 } outLen := ipv4.HeaderLen + 8 + packetLen + if outLen > cap(out) { + return nil + } - out = out[:(outLen)] + out = out[:outLen] ipHdr := out[0:ipv4.HeaderLen] - ipHdr[0] = ipv4.Version<<4 | (ipv4.HeaderLen >> 2) // version, ihl - ipHdr[1] = 0 // DSCP, ECN - binary.BigEndian.PutUint16(ipHdr[2:], uint16(ipv4.HeaderLen+8+packetLen)) // Total Length + ipHdr[0] = ipv4.Version<<4 | (ipv4.HeaderLen >> 2) // version, ihl + ipHdr[1] = 0 // DSCP, ECN + binary.BigEndian.PutUint16(ipHdr[2:], uint16(outLen)) // Total Length ipHdr[4] = 0 // id ipHdr[5] = 0 // . @@ -76,7 +95,15 @@ func ipv4CreateRejectTCPPacket(packet []byte, out []byte) []byte { ihl := int(packet[0]&0x0f) << 2 outLen := ipv4.HeaderLen + tcpLen - out = out[:(outLen)] + if len(packet) < ihl+tcpLen { + // We need at least this many bytes for this to be a valid packet + return nil + } + if outLen > cap(out) { + return nil + } + + out = out[:outLen] ipHdr := out[0:ipv4.HeaderLen] ipHdr[0] = ipv4.Version<<4 | (ipv4.HeaderLen >> 2) // version, ihl diff --git a/iputil/packet_test.go b/iputil/packet_test.go new file mode 100644 index 000000000..e1d0d95d8 --- /dev/null +++ b/iputil/packet_test.go @@ -0,0 +1,73 @@ +package iputil + +import ( + "net" + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/net/ipv4" +) + +func Test_CreateRejectPacket(t *testing.T) { + h := ipv4.Header{ + Len: 20, + Src: net.IPv4(10, 0, 0, 1), + Dst: net.IPv4(10, 0, 0, 2), + Protocol: 1, // ICMP + } + + b, err := h.Marshal() + if err != nil { + t.Fatalf("h.Marhshal: %v", err) + } + b = append(b, []byte{0, 3, 0, 4}...) + + expectedLen := ipv4.HeaderLen + 8 + h.Len + 4 + out := make([]byte, expectedLen) + rejectPacket := CreateRejectPacket(b, out) + assert.NotNil(t, rejectPacket) + assert.Len(t, rejectPacket, expectedLen) + + // ICMP with max header len + h = ipv4.Header{ + Len: 60, + Src: net.IPv4(10, 0, 0, 1), + Dst: net.IPv4(10, 0, 0, 2), + Protocol: 1, // ICMP + Options: make([]byte, 40), + } + + b, err = h.Marshal() + if err != nil { + t.Fatalf("h.Marhshal: %v", err) + } + b = append(b, []byte{0, 3, 0, 4, 0, 0, 0, 0}...) + + expectedLen = MaxRejectPacketSize + out = make([]byte, MaxRejectPacketSize) + rejectPacket = CreateRejectPacket(b, out) + assert.NotNil(t, rejectPacket) + assert.Len(t, rejectPacket, expectedLen) + + // TCP with max header len + h = ipv4.Header{ + Len: 60, + Src: net.IPv4(10, 0, 0, 1), + Dst: net.IPv4(10, 0, 0, 2), + Protocol: 6, // TCP + Options: make([]byte, 40), + } + + b, err = h.Marshal() + if err != nil { + t.Fatalf("h.Marhshal: %v", err) + } + b = append(b, []byte{0, 3, 0, 4}...) + b = append(b, make([]byte, 16)...) + + expectedLen = ipv4.HeaderLen + 20 + out = make([]byte, expectedLen) + rejectPacket = CreateRejectPacket(b, out) + assert.NotNil(t, rejectPacket) + assert.Len(t, rejectPacket, expectedLen) +} diff --git a/outside.go b/outside.go index 4139830b2..29189110c 100644 --- a/outside.go +++ b/outside.go @@ -406,7 +406,9 @@ func (f *Interface) decryptToTun(hostinfo *HostInfo, messageCounter uint64, out dropReason := f.firewall.Drop(out, *fwPacket, true, hostinfo, f.pki.GetCAPool(), localCache) if dropReason != nil { - f.rejectOutside(out, hostinfo.ConnectionState, hostinfo, nb, out, q) + // NOTE: We give `packet` as the `out` here since we already decrypted from it and we don't need it anymore + // This gives us a buffer to build the reject packet in + f.rejectOutside(out, hostinfo.ConnectionState, hostinfo, nb, packet, q) if f.l.Level >= logrus.DebugLevel { hostinfo.logger(f.l).WithField("fwPacket", fwPacket). WithField("reason", dropReason).