This was broken with the change to yaml.v3:
- https://github.com/slackhq/nebula/pull/1148
We forgot to update these references to `map[string]any`.
Without this fix, Nebula crashes with an error like this:
{"error":"config `lighthouse.calculated_remotes` has invalid type: map[string]interface {}","level":"error","msg":"Invalid lighthouse.calculated_remotes","time":"2025-07-29T15:50:06.479499Z"}
The `golang.org/x/crypto/ssh/terminal` was deprecated and moved to
`golang.org/x/term`. We already use the new package in
`cmd/nebula-cert`, so fix our remaining reference here.
See:
- https://github.com/golang/go/issues/31044
We switched to yaml.v3 with #1148, but missed this spot that was still
casting into `map[any]any` when yaml.v3 makes it `map[string]any`. Also
clean up a few more `interface{}` that were added as we changed them all
to `any` with #1148.
* upgrade to yaml.v3
The main nice fix here is that maps unmarshal into `map[string]any`
instead of `map[any]any`, so it cleans things up a bit.
* add config.AsBool
Since yaml.v3 doesn't automatically convert yes to bool now, for
backwards compat
* use type aliases for m
* more cleanup
* more cleanup
* more cleanup
* go mod cleanup
This doesn't change our go.mod, which still only requires go1.22 as a minimum. It only changes our builds to use go1.24 so we have the latest improvements.
Currently we just assume you are using the default Docker bridge network
config of `172.17.0.0/24`. This change works to try to detect if you are
using a different config, but still only works if you are using a `/24`
and aren't running any other containers. A future PR could make this
better by launching the lighthouse container first and then fetching
what the IP address is before continuing with the configuration.
* smoke-extra: cleanup ncat references
We can't run the `ncat` tests unless we can make sure to install it to
all of the vagrant boxes.
* more ncat
* more cleanup
* make boringcrypto: add checklinkname flag for go1.23
Starting with go1.23, we need to set -checklinkname=0 when building for
boringcrypto because we need to use go:linkname to access `newGCMTLS`.
Note that this does break builds when using a go version less than
go1.23.0. We can probably assume that someone using this Makefile and
manually building is using the latest release of Go though.
See:
- https://go.dev/doc/go1.23#linker
* build with go1.23
This doesn't change our go.mod, which still only requires go1.22 as
a minimum, only changes our builds to use go1.23 so we have the latest
improvements.
* fix `make test-boringcrypto` as well
* also fix boringcrypto e2e test
If the host OS is already big endian, we were swapping bytes when we
shouldn't have. Use the Go helper to make sure we do the endianness
correctly
Fixes: #1189
A local index collision happens when two tunnels attempt to use the same
random int32 index ID. This is a rare chance, and we have code to deal
with it, but we have a panic because we return the wrong thing in this
case. This change should fix the panic.
macos-11 was deprecated and removed:
> The macos-11 label has been deprecated and will no longer be available after 28 June 2024.
We can just use macos-latest instead.
Clean up the messageCounter checks added in #1154. Instead of checking that
messageCounter is still at 2, just initialize it to 2 and only increment for
non-handshake messages. Handshake packets will always be packets 1 and 2.
We had a rare deadlock in GetOrHandshake because we kept the hostmap
lock when we do the call to StartHandshake. StartHandshake can block
while sending to the lighthouse query worker channel, and that worker
needs to be able to grab the hostmap lock to do its work. Other calls
for StartHandshake don't hold the hostmap lock so we should be able to
drop it here.
This lock was originally added with: https://github.com/slackhq/nebula/pull/954
* WIP smoke test freebsd
* fix bitrot
We now test that the firewall blocks inbound on host3 from host2
* WIP ipv6 test
* cleanup
* rename to make clear
* fix filename
* restore
* no sudo docker
* WIP
* WIP
* WIP
* WIP
* extra smoke tests
* WIP
* WIP
* add over improvements made in smoke.sh
* more tests
* use generic/freebsd14
* cleanup from test
* smoke test openbsd-amd64
* add netbsd-amd64
* try to fix vagrant
* update to go1.21
Since the first minor version update has already been released, we can
probably feel comfortable updating to go1.21. This version now enforces
that the go version on the system is compatible with the version
specified in go.mod, so we can remove the old logic around checking the
minimum version in the Makefile.
- https://go.dev/doc/go1.21#tools
> To improve forwards compatibility, Go 1.21 now reads the go line in a go.work or go.mod file as a strict minimum requirement: go 1.21.0 means that the workspace or module cannot be used with Go 1.20 or with Go 1.21rc1. This allows projects that depend on fixes made in later versions of Go to ensure that they are not used with earlier versions. It also gives better error reporting for projects that make use of new Go features: when the problem is that a newer Go version is needed, that problem is reported clearly, instead of attempting to build the code and printing errors about unresolved imports or syntax errors.
* update to go1.22
* bump gvisor
* fix merge conflicts
* use latest gvisor `go` branch
Need to use the latest commit on the `go` branch, see:
- https://github.com/google/gvisor?tab=readme-ov-file#using-go-get
* mod tidy
* more fixes
* give smoketest more time
Is this why it is failing?
* also a little more sleep here
---------
Co-authored-by: Jack Doan <me@jackdoan.com>
* avoid deadlock in lighthouse queryWorker
If the lighthouse queryWorker tries to grab to call StartHandshake on
a lighthouse vpnIp, we can deadlock on the handshake_manager lock. This
change drops the handshake_manager lock before we send on the lighthouse
queryChan (which could block), and also avoids sending to the channel if
this is a lighthouse IP itself.
* need to hold lock during cacheCb
This used to work correctly because when the multiple routines work was
first added in #382, but an important part to discover the listen port
before opening the other listeners on the same socket was lost in this
PR: #653.
This change should fix the regression and allow multiple routines to
work correctly when listen.port is set to `0`.
Thanks to @rawdigits for tracking down and discovering this regression.
Ensure that we don't break the build for mobile by doing a `go build`
for all of the non-main modules in the repo. Should hopefully catch
issues like #1035 sooner.
* cache cert verification
CheckSignature and Verify are expensive methods, and certificates are
static. Cache the results.
* use atomics
* make sure public key bytes match
* add VerifyWithCache and ResetCache
* cleanup
* use VerifyWithCache
* doc
* document P256 and BoringCrypto
Some basic descriptions of P256 and BoringCrypto added to the bottom of
README.md so that their prupose is not a mystery.
* typo
* add test for GOEXPERIMENT=boringcrypto
* fix NebulaCertificate.Sign
Set the PublicKey field in a more compatible way for the tests. The
current method grabs the public key from the certificate, but the
correct thing to do is to derive it from the private key. Either way
doesn't really matter as I don't think the Sign method actually even
uses the PublicKey field.
* assert boring
* cleanup tests
* Support NIST curve P256
This change adds support for NIST curve P256. When you use `nebula-cert ca`
or `nebula-cert keygen`, you can specify `-curve P256` to enable it. The
curve to use is based on the curve defined in your CA certificate.
Internally, we use ECDSA P256 to sign certificates, and ECDH P256 to do
Noise handshakes. P256 is not supported natively in Noise Protocol, so
we define `DHP256` in the `noiseutil` package to implement support for
it.
You cannot have a mixed network of Curve25519 and P256 certificates,
since the Noise protocol will only attempt to parse using the Curve
defined in the host's certificate.
* verify the curves match in VerifyPrivateKey
This would have failed anyways once we tried to actually use the bytes
in the private key, but its better to detect the issue up front with
a better error message.
* add cert.Curve argument to Sign method
* fix mismerge
* use crypto/ecdh
This is the preferred method for doing ECDH functions now, and also has
a boringcrypto specific codepath.
* remove other ecdh uses of crypto/elliptic
use crypto/ecdh instead
This adds a few build targets to compile with `GOEXPERIMENT=boringcrypto`:
- `bin-boringcrypto`
- `release-boringcrypto`
It also adds a field to the intial start up log indicating if
boringcrypto is enabled in the binary.
* build with go1.20
This has been out for a bit and is up to go1.20.4. We have been using
go1.20 for the Slack builds and have seen no issues.
* need the quotes
* use go install
* add calculated_remotes
This setting allows us to "guess" what the remote might be for a host
while we wait for the lighthouse response. For networks that hard
designed with in mind, it can help speed up handshake performance, as well as
improve resiliency in the case that all lighthouses are down.
Example:
lighthouse:
# ...
calculated_remotes:
# For any Nebula IPs in 10.0.10.0/24, this will apply the mask and add
# the calculated IP as an initial remote (while we wait for the response
# from the lighthouse). Both CIDRs must have the same mask size.
# For example, Nebula IP 10.0.10.123 will have a calculated remote of
# 192.168.1.123
10.0.10.0/24:
- mask: 192.168.1.0/24
port: 4242
* figure out what is up with this test
* add test
* better logic for sending handshakes
Keep track of the last light of hosts we sent handshakes to. Only log
handshake sent messages if the list has changed.
Remove the test Test_NewHandshakeManagerTrigger because it is faulty and
makes no sense. It relys on the fact that no handshake packets actually
get sent, but with these changes we would send packets now (which it
should!)
* use atomic.Pointer
* cleanup to make it clearer
* fix typo in example
* firewall: add option to send REJECT replies
This change allows you to configure the firewall to send REJECT packets
when a packet is denied.
firewall:
# Action to take when a packet is not allowed by the firewall rules.
# Can be one of:
# `drop` (default): silently drop the packet.
# `reject`: send a reject reply.
# - For TCP, this will be a RST "Connection Reset" packet.
# - For other protocols, this will be an ICMP port unreachable packet.
outbound_action: drop
inbound_action: drop
These packets are only sent to established tunnels, and only on the
overlay network (currently IPv4 only).
$ ping -c1 192.168.100.3
PING 192.168.100.3 (192.168.100.3) 56(84) bytes of data.
From 192.168.100.3 icmp_seq=2 Destination Port Unreachable
--- 192.168.100.3 ping statistics ---
2 packets transmitted, 0 received, +1 errors, 100% packet loss, time 31ms
$ nc -nzv 192.168.100.3 22
(UNKNOWN) [192.168.100.3] 22 (?) : Connection refused
This change also modifies the smoke test to capture tcpdump pcaps from
both the inside and outside to inspect what is going on over the wire.
It also now does TCP and UDP packet tests using the Nmap version of
ncat.
* calculate seq and ack the same was as the kernel
The logic a bit confusing, so we copy it straight from how the kernel
does iptables `--reject-with tcp-reset`:
- https://github.com/torvalds/linux/blob/v5.19/net/ipv4/netfilter/nf_reject_ipv4.c#L193-L221
* cleanup
With Nebula 1.4.0, if you create an unsafe_route that has a collision with an existing route on the system, the unsafe_route will be silently dropped (and the existing system route remains).
With Nebula 1.5.0, this same situation will cause Nebula to fail to start with an error (EEXIST).
This change restores the Nebula 1.4.0 behavior (but with a WARN log as well).
With the previous implementation, we check if route.MTU is greater than zero,
but it will always be because we set it to the default MTU in
parseUnsafeRoutes. This change leaves it as zero in parseUnsafeRoutes so
it can be examined later.
* don't set ConnectionState to nil
We might have packets processing in another thread, so we can't safely
just set this to nil. Since we removed it from the hostmaps, the next
packets to process should start the handshake over again.
I believe this comment is outdated or incorrect, since the next
handshake will start over with a new HostInfo, I don't think there is
any way a counter reuse could happen:
> We must null the connectionstate or a counter reuse may happen
Here is a panic we saw that I think is related:
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x93a037]
goroutine 59 [running, locked to thread]:
github.com/slackhq/nebula.(*Firewall).Drop(...)
github.com/slackhq/nebula/firewall.go:380
github.com/slackhq/nebula.(*Interface).consumeInsidePacket(...)
github.com/slackhq/nebula/inside.go:59
github.com/slackhq/nebula.(*Interface).listenIn(...)
github.com/slackhq/nebula/interface.go:233
created by github.com/slackhq/nebula.(*Interface).run
github.com/slackhq/nebula/interface.go:191
* use closeTunnel
When running in `-test` mode, `tun` is set to nil. So we should move the
defer into the `!configTest` if block.
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x54855c]
goroutine 1 [running]:
github.com/slackhq/nebula.Main.func3(0x4000135e80, {0x0, 0x0})
github.com/slackhq/nebula/main.go:176 +0x2c
github.com/slackhq/nebula.Main(0x400022e060, 0x1, {0x76faa0, 0x5}, 0x4000230000, 0x0)
github.com/slackhq/nebula/main.go:316 +0x2414
main.main()
github.com/slackhq/nebula/cmd/nebula/main.go:54 +0x540
Without these comments, gofmt 1.16.9 will complain. Since we otherwise
still support building with go1.16, lets add the comments to make it
easier to compile and gofmt.
Related: #588
We have a few small race conditions with creating the HostInfo.ConnectionState
since we add the host info to the pendingHostMap before we set this
field. We can make everything a lot easier if we just add an "init"
function so that we can set this field in the hostinfo before we add it
to the hostmap.
This allows you to configure remote allow lists specific to different
subnets of the inside CIDR. Example:
remote_allow_ranges:
10.42.42.0/24:
192.168.0.0/16: true
This would only allow hosts with a VPN IP in the 10.42.42.0/24 range to
have private IPs (and thus don't connect over public IPs).
The PR also refactors AllowList into RemoteAllowList and LocalAllowList to make it clearer which methods are allowed on which allow list.
If we receive a handshake packet for a tunnel that has already been
completed, check to see if the new remote is preferred. If so, update to
the preferred remote and send a test packet to influence the other side
to do the same.
* Add more metrics
This change adds the following counter metrics:
Metrics to track packets dropped at the firewall:
firewall.dropped.local_ip
firewall.dropped.remote_ip
firewall.dropped.no_rule
Metrics to track handshakes attempts that have been initiated and ones
that have timed out (ones that have completed are tracked by the
existing "handshakes" histogram).
handshake_manager.initiated
handshake_manager.timed_out
Metrics to track when cached_packets are dropped because we run out of
buffer space, and how many are sent once the handshake completes.
hostinfo.cached_packets.dropped
hostinfo.cached_packets.sent
This change also notes how many cached packets we have when we log the
final "Handshake received" message for either stage1 for stage2.
* separate incoming/outgoing metrics
* remove "allowed" firewall metrics
We don't need this on the hotpath, they aren't worh it.
* don't need pointers here
This check was accidentally typo'd in #396 from `%` to `&`. Restore the
correct functionality here (we want to do the check every "PromoteEvery"
count packets).
This is how Prometheus recommends you do it, and how they do it
themselves in their client. This makes it easy to see which versions you
have deployed in your fleet, and query over it too.
The change for #401 incorrectly called HostInfo.ForcePromoteBest in
stage2, when we really we want to pick the remote that we received the
response from.
There are some subtle race conditions with the previous handshake_ix implementation, mostly around collisions with localIndexId. This change refactors it so that we have a "commit" phase during the handshake where we grab the lock for the hostmap and ensure that we have a unique local index before storing it. We also now avoid using the pending hostmap at all for receiving stage1 packets, since we have everything we need to just store the completed handshake.
Co-authored-by: Nate Brown <nbrown.us@gmail.com>
Co-authored-by: Ryan Huber <rhuber@gmail.com>
Co-authored-by: forfuncsake <drussell@slack-corp.com>
We missed this race with #396 (and I think this is also the crash in
issue #226). We need to lock a little higher in the getOrHandshake
method, before we reset hostinfo.ConnectionInfo. Previously, two
routines could enter this section and confuse the handshake process.
This could result in the other side sending a recv_error that also has
a race with setting hostinfo.ConnectionInfo back to nil. So we make sure
to grab the lock in handleRecvError as well.
Neither of these code paths are in the hot path (handling packets
between two hosts over an active tunnel) so there should be no
performance concerns.
This change fixes all of the known data races that `make smoke-docker-race` finds, except for one.
Most of these races are around the handshake phase for a hostinfo, so we add a RWLock to the hostinfo and Lock during each of the handshake stages.
Some of the other races are around consistently using `atomic` around the `messageCounter` field. To make this harder to mess up, I have renamed the field to `atomicMessageCounter` (I also removed the unnecessary extra pointer deference as we can just point directly to the struct field).
The last remaining data race is around reading `ConnectionInfo.ready`, which is a boolean that is only written to once when the handshake has finished. Due to it being in the hot path for packets and the rare case that this could actually be an issue, holding off on fixing that one for now.
here is the results of `make smoke-docker-race`:
before:
lighthouse1: Found 2 data race(s)
host2: Found 36 data race(s)
host3: Found 17 data race(s)
host4: Found 31 data race(s)
after:
host2: Found 1 data race(s)
host4: Found 1 data race(s)
Fixes: #147Fixes: #226Fixes: #283Fixes: #316
Previously, every packet we see gets a lock on the conntrack table and updates it. When running with multiple routines, this can cause heavy lock contention and limit our ability for the threads to run independently. This change caches reads from the conntrack table for a very short period of time to reduce this lock contention. This cache will currently default to disabled unless you are running with multiple routines, in which case the default cache delay will be 1 second. This means that entries in the conntrack table may be up to 1 second out of date and remain in a routine local cache for up to 1 second longer than the global table.
Instead of calling time.Now() for every packet, this cache system relies on a tick thread that updates the current cache "version" each tick. Every packet we check if the cache version is out of date, and reset the cache if so.
We are currently seeing some cases where we are not deleting entries
correctly from the pending hostmap. I believe this is a case of
an inbound timer tick firing and deleting the Hosts map entry for
a newer handshake attempt than intended, thus leaving the old Indexes
entry orphaned. This change adds some extra checking when deleteing from
the Indexes and Hosts maps to ensure we clean everything up correctly.
This makes it easier to use the docker container smoke test that
GitHub actions runs. There is also `make smoke-docker-race` that runs the
smoke test with `-race` enabled.
This change allows a server running with `tun.disabled: true` (usually
a lighthouse) to still reply to ICMP EchoRequest packets. This allows
you to "ping" the lighthouse Nebula IP as a quick check to make sure the
tunnel is up, even when running with tun.disabled.
This is still gated by allowing `icmp` packets in the inbound firewall
rules.
This change is for Linux only.
Previously, when running with multiple tun.routines, we would only have one file descriptor. This change instead sets IFF_MULTI_QUEUE and opens a file descriptor for each routine. This allows us to process with multiple threads while preventing out of order packet reception issues.
To attempt to distribute the flows across the queues, we try to write to the tun/UDP queue that corresponds with the one we read from. So if we read a packet from tun queue "2", we will write the outgoing encrypted packet to UDP queue "2". Because of the nature of how multi queue works with flows, a given host tunnel will be sticky to a given routine (so if you try to performance benchmark by only using one tunnel between two hosts, you are only going to be using a max of one thread for each direction).
Because this system works much better when we can correlate flows between the tun and udp routines, we are deprecating the undocumented "tun.routines" and "listen.routines" parameters and introducing a new "routines" parameter that sets the value for both. If you use the old undocumented parameters, the max of the values will be used and a warning logged.
Co-authored-by: Nate Brown <nbrown.us@gmail.com>
The change introduced by #320 incorrectly re-uses the output buffer for
sending punchBack packets. Since we are currently spawning a new
goroutine for each send here, we need to allocate a new buffer each
time. We can come back and optimize this in the future, but for now we
should fix the regression.
This change adds an index based on HostInfo.remoteIndexId. This allows
us to use HostMap.QueryReverseIndex without having to loop over all
entries in the map (this can be a bottleneck under high traffic
lighthouses).
Without this patch, a high traffic lighthouse server receiving recv_error
packets and lots of handshakes, cpu pprof trace can look like this:
flat flat% sum% cum cum%
2000ms 32.26% 32.26% 3040ms 49.03% github.com/slackhq/nebula.(*HostMap).QueryReverseIndex
870ms 14.03% 46.29% 1060ms 17.10% runtime.mapiternext
Which shows 50% of total cpu time is being spent in QueryReverseIndex.
We noticed that the number of memory allocations LightHouse.HandleRequest creates for each call can seriously impact performance for high traffic lighthouses. This PR introduces a benchmark in the first commit and then optimizes memory usage by creating a LightHouseHandler struct. This struct allows us to re-use memory between each lighthouse request (one instance per UDP listener go-routine).
Packet 1 is always a stage 1 handshake and packet 2 is always stage 2.
Normal packets don't start flowing until the message counter is 3 or
higher.
Currently we only receive either packet 1 or 2 depending on if
we are the initiator or responder for the handshake, so we end up
marking one of these as "lost". We should mark these packets as "seen"
when we are the one sending them, since we don't expect to see them from
the other side.
Currently, if a packet arrives on the tun device with a destination that
is not a routable Nebula IP, `queryUnsafeRoute` converts that IP to
0.0.0.0 and we store that packet and try to look up that IP with the
lighthouse. This doesn't make any sense to do, if we get a packet that
is unroutable we should just drop it.
Note, we have a few configurable options like `drop_local_broadcast`
and `drop_multicast` which do this for a few specific types, but since
no packets like this will send correctly I think we should just drop
anything that is unroutable.
We are currently triggering a fast handshake for static hosts right
inside HandshakeManager.AddVpnIP, but this can actually trigger before
we have generated the handshake packet to use. Instead, we should be
triggering right after we call ixHandshakeStage0 in getOrHandshake
(which generates the handshake packet)
Wade Simmons