Merge commit from fork

Newly signed P256 based certificates will have their signature clamped to the low-s form.

Update CHANGELOG.md

CHANGELOG.md

@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [1.10.3] - 2026-02-06
+
+### Security
+
+- Fix an issue where blocklist bypass is possible when using curve P256 since the signature can have 2 valid representations.
+  Both fingerprint representations will be tested against the blocklist.
+  Any newly issued P256 based certificates will have their signature clamped to the low-s form.
+  Nebula will assert the low-s signature form when validating certificates in a future version. [GHSA-69x3-g4r3-p962](https://github.com/slackhq/nebula/security/advisories/GHSA-69x3-g4r3-p962)
+
+### Changed
+
+- Improve error reporting if nebula fails to start due to a tun device naming issue. (#1588)
+
 ## [1.10.2] - 2026-01-21
 
 ### Fixed
@@ -775,7 +788,8 @@ created.)
 
 - Initial public release.
 
-[Unreleased]: https://github.com/slackhq/nebula/compare/v1.10.2...HEAD
+[Unreleased]: https://github.com/slackhq/nebula/compare/v1.10.3...HEAD
+[1.10.3]: https://github.com/slackhq/nebula/releases/tag/v1.10.3
 [1.10.2]: https://github.com/slackhq/nebula/releases/tag/v1.10.2
 [1.10.1]: https://github.com/slackhq/nebula/releases/tag/v1.10.1
 [1.10.0]: https://github.com/slackhq/nebula/releases/tag/v1.10.0

cert/ca_pool.go

@@ -141,10 +141,23 @@ func (ncp *CAPool) VerifyCertificate(now time.Time, c Certificate) (*CachedCerti
 		return nil, err
 	}
 
+	// Pre nebula v1.10.3 could generate signatures in either high or low s form and validation
+	// of signatures allowed for either. Nebula v1.10.3 and beyond clamps signature generation to low-s form
+	// but validation still allows for either. Since a change in the signature bytes affects the fingerprint, we
+	// need to test both forms until such a time comes that we enforce low-s form on signature validation.
+	fp2, err := CalculateAlternateFingerprint(c)
+	if err != nil {
+		return nil, fmt.Errorf("could not calculate alternate fingerprint to verify: %w", err)
+	}
+	if fp2 != "" && ncp.IsBlocklisted(fp2) {
+		return nil, ErrBlockListed
+	}
+
 	cc := CachedCertificate{
 		Certificate:       c,
 		InvertedGroups:    make(map[string]struct{}),
 		Fingerprint:       fp,
+		fingerprint2:      fp2,
 		signerFingerprint: signer.Fingerprint,
 	}
 
@@ -158,6 +171,11 @@ func (ncp *CAPool) VerifyCertificate(now time.Time, c Certificate) (*CachedCerti
 // VerifyCachedCertificate is the same as VerifyCertificate other than it operates on a pre-verified structure and
 // is a cheaper operation to perform as a result.
 func (ncp *CAPool) VerifyCachedCertificate(now time.Time, c *CachedCertificate) error {
+	// Check any available alternate fingerprint forms for this certificate, re P256 high-s/low-s
+	if c.fingerprint2 != "" && ncp.IsBlocklisted(c.fingerprint2) {
+		return ErrBlockListed
+	}
+
 	_, err := ncp.verify(c.Certificate, now, c.Fingerprint, c.signerFingerprint)
 	return err
 }

cert/ca_pool_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/slackhq/nebula/cert/p256"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -170,6 +171,15 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
 	_, err = caPool.VerifyCertificate(time.Now(), c)
 	require.EqualError(t, err, "certificate is in the block list")
 
+	// Create a copy of the cert and swap to the alternate form for the signature
+	nc := c.Copy()
+	b, err := p256.Swap(c.Signature())
+	require.NoError(t, err)
+	require.NoError(t, nc.(*certificateV1).setSignature(b))
+
+	_, err = caPool.VerifyCertificate(time.Now(), nc)
+	require.EqualError(t, err, "certificate is in the block list")
+
 	caPool.ResetCertBlocklist()
 	_, err = caPool.VerifyCertificate(time.Now(), c)
 	require.NoError(t, err)
@@ -187,7 +197,7 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
 	require.NoError(t, err)
 
 	caPool = NewCAPool()
-	b, err := caPool.AddCAFromPEM(caPem)
+	b, err = caPool.AddCAFromPEM(caPem)
 	require.NoError(t, err)
 	assert.Empty(t, b)
 
@@ -196,7 +206,17 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
 	})
 
 	c, _, _, _ = NewTestCert(Version1, Curve_P256, ca, caKey, "test", time.Now(), time.Now().Add(5*time.Minute), nil, nil, []string{"test1"})
-	_, err = caPool.VerifyCertificate(time.Now(), c)
+	cc, err := caPool.VerifyCertificate(time.Now(), c)
+	require.NoError(t, err)
+
+	// Reset the blocklist and block the alternate form fingerprint
+	caPool.ResetCertBlocklist()
+	caPool.BlocklistFingerprint(cc.fingerprint2)
+	err = caPool.VerifyCachedCertificate(time.Now(), cc)
+	require.EqualError(t, err, "certificate is in the block list")
+
+	caPool.ResetCertBlocklist()
+	err = caPool.VerifyCachedCertificate(time.Now(), cc)
 	require.NoError(t, err)
 }
 
@@ -394,6 +414,15 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
 	_, err = caPool.VerifyCertificate(time.Now(), c)
 	require.EqualError(t, err, "certificate is in the block list")
 
+	// Create a copy of the cert and swap to the alternate form for the signature
+	nc := c.Copy()
+	b, err := p256.Swap(c.Signature())
+	require.NoError(t, err)
+	require.NoError(t, nc.(*certificateV2).setSignature(b))
+
+	_, err = caPool.VerifyCertificate(time.Now(), nc)
+	require.EqualError(t, err, "certificate is in the block list")
+
 	caPool.ResetCertBlocklist()
 	_, err = caPool.VerifyCertificate(time.Now(), c)
 	require.NoError(t, err)
@@ -411,7 +440,7 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
 	require.NoError(t, err)
 
 	caPool = NewCAPool()
-	b, err := caPool.AddCAFromPEM(caPem)
+	b, err = caPool.AddCAFromPEM(caPem)
 	require.NoError(t, err)
 	assert.Empty(t, b)
 
@@ -420,7 +449,17 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
 	})
 
 	c, _, _, _ = NewTestCert(Version2, Curve_P256, ca, caKey, "test", time.Now(), time.Now().Add(5*time.Minute), nil, nil, []string{"test1"})
-	_, err = caPool.VerifyCertificate(time.Now(), c)
+	cc, err := caPool.VerifyCertificate(time.Now(), c)
+	require.NoError(t, err)
+
+	// Reset the blocklist and block the alternate form fingerprint
+	caPool.ResetCertBlocklist()
+	caPool.BlocklistFingerprint(cc.fingerprint2)
+	err = caPool.VerifyCachedCertificate(time.Now(), cc)
+	require.EqualError(t, err, "certificate is in the block list")
+
+	caPool.ResetCertBlocklist()
+	err = caPool.VerifyCachedCertificate(time.Now(), cc)
 	require.NoError(t, err)
 }
 

cert/cert.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"net/netip"
 	"time"
+
+	"github.com/slackhq/nebula/cert/p256"
 )
 
 type Version uint8
@@ -110,6 +112,9 @@ type CachedCertificate struct {
 	InvertedGroups    map[string]struct{}
 	Fingerprint       string
 	signerFingerprint string
+
+	// A place to store a 2nd fingerprint if the certificate could have one, such as with P256
+	fingerprint2 string
 }
 
 func (cc *CachedCertificate) String() string {
@@ -152,3 +157,31 @@ func Recombine(v Version, rawCertBytes, publicKey []byte, curve Curve) (Certific
 
 	return c, nil
 }
+
+// CalculateAlternateFingerprint calculates a 2nd fingerprint representation for P256 certificates
+// CAPool blocklist testing through `VerifyCertificate` and `VerifyCachedCertificate` automatically performs this step.
+func CalculateAlternateFingerprint(c Certificate) (string, error) {
+	if c.Curve() != Curve_P256 {
+		return "", nil
+	}
+
+	nc := c.Copy()
+	b, err := p256.Swap(nc.Signature())
+	if err != nil {
+		return "", err
+	}
+
+	switch v := nc.(type) {
+	case *certificateV1:
+		err = v.setSignature(b)
+	case *certificateV2:
+		err = v.setSignature(b)
+	default:
+		return "", ErrUnknownVersion
+	}
+
+	if err != nil {
+		return "", err
+	}
+	return nc.Fingerprint()
+}

cert/p256/p256.go

@@ -0,0 +1,122 @@
+package p256
+
+import (
+	"crypto/elliptic"
+	"errors"
+	"math/big"
+
+	"filippo.io/bigmod"
+
+	"golang.org/x/crypto/cryptobyte"
+	"golang.org/x/crypto/cryptobyte/asn1"
+)
+
+var halfN = new(big.Int).Rsh(elliptic.P256().Params().N, 1)
+var nMod *bigmod.Modulus
+
+func init() {
+	n, err := bigmod.NewModulus(elliptic.P256().Params().N.Bytes())
+	if err != nil {
+		panic(err)
+	}
+	nMod = n
+}
+
+func IsNormalized(sig []byte) (bool, error) {
+	r, s, err := parseSignature(sig)
+	if err != nil {
+		return false, err
+	}
+	return checkLowS(r, s), nil
+}
+
+func checkLowS(_, s []byte) bool {
+	bigS := new(big.Int).SetBytes(s)
+	// Check if S <= (N/2), because we want to include the midpoint in the set of low-s
+	return bigS.Cmp(halfN) <= 0
+}
+
+func swap(r, s []byte) ([]byte, []byte, error) {
+	var err error
+	bigS, err := bigmod.NewNat().SetBytes(s, nMod)
+	if err != nil {
+		return nil, nil, err
+	}
+	sNormalized := nMod.Nat().Sub(bigS, nMod)
+
+	return r, sNormalized.Bytes(nMod), nil
+}
+
+func Normalize(sig []byte) ([]byte, error) {
+	r, s, err := parseSignature(sig)
+	if err != nil {
+		return nil, err
+	}
+
+	if checkLowS(r, s) {
+		return sig, nil
+	}
+
+	newR, newS, err := swap(r, s)
+	if err != nil {
+		return nil, err
+	}
+
+	return encodeSignature(newR, newS)
+}
+
+// Swap will change sig between its current form to the opposite high or low form.
+func Swap(sig []byte) ([]byte, error) {
+	r, s, err := parseSignature(sig)
+	if err != nil {
+		return nil, err
+	}
+
+	newR, newS, err := swap(r, s)
+	if err != nil {
+		return nil, err
+	}
+
+	return encodeSignature(newR, newS)
+}
+
+// parseSignature taken exactly from crypto/ecdsa/ecdsa.go
+func parseSignature(sig []byte) (r, s []byte, err error) {
+	var inner cryptobyte.String
+	input := cryptobyte.String(sig)
+	if !input.ReadASN1(&inner, asn1.SEQUENCE) ||
+		!input.Empty() ||
+		!inner.ReadASN1Integer(&r) ||
+		!inner.ReadASN1Integer(&s) ||
+		!inner.Empty() {
+		return nil, nil, errors.New("invalid ASN.1")
+	}
+	return r, s, nil
+}
+
+func encodeSignature(r, s []byte) ([]byte, error) {
+	var b cryptobyte.Builder
+	b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
+		addASN1IntBytes(b, r)
+		addASN1IntBytes(b, s)
+	})
+	return b.Bytes()
+}
+
+// addASN1IntBytes encodes in ASN.1 a positive integer represented as
+// a big-endian byte slice with zero or more leading zeroes.
+func addASN1IntBytes(b *cryptobyte.Builder, bytes []byte) {
+	for len(bytes) > 0 && bytes[0] == 0 {
+		bytes = bytes[1:]
+	}
+	if len(bytes) == 0 {
+		b.SetError(errors.New("invalid integer"))
+		return
+	}
+	b.AddASN1(asn1.INTEGER, func(c *cryptobyte.Builder) {
+		if bytes[0]&0x80 != 0 {
+			c.AddUint8(0)
+		}
+		c.AddBytes(bytes)
+	})
+}

cert/p256/p256_test.go

@@ -0,0 +1,28 @@
+package p256
+
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestFlipping(t *testing.T) {
+	priv, err1 := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err1)
+
+	out, err := ecdsa.SignASN1(rand.Reader, priv, []byte("big chungus"))
+	require.NoError(t, err)
+
+	r, s, err := parseSignature(out)
+	require.NoError(t, err)
+
+	r, s1, err := swap(r, s)
+	require.NoError(t, err)
+	r, s2, err := swap(r, s1)
+	require.NoError(t, err)
+	require.Equal(t, s, s2)
+	require.NotEqual(t, s, s1)
+}

cert/sign.go

@@ -9,6 +9,8 @@ import (
 	"fmt"
 	"net/netip"
 	"time"
+
+	"github.com/slackhq/nebula/cert/p256"
 )
 
 // TBSCertificate represents a certificate intended to be signed.
@@ -126,6 +128,13 @@ func (t *TBSCertificate) SignWith(signer Certificate, curve Curve, sp SignerLamb
 		return nil, err
 	}
 
+	if curve == Curve_P256 {
+		sig, err = p256.Normalize(sig)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	err = c.setSignature(sig)
 	if err != nil {
 		return nil, err

cert/sign_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/slackhq/nebula/cert/p256"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -89,3 +90,48 @@ func TestCertificateV1_SignP256(t *testing.T) {
 	require.NoError(t, err)
 	assert.NotNil(t, uc)
 }
+
+func TestCertificate_SignP256_AlwaysNormalized(t *testing.T) {
+	before := time.Now().Add(time.Second * -60).Round(time.Second)
+	after := time.Now().Add(time.Second * 60).Round(time.Second)
+	pubKey := []byte("01234567890abcedfghij1234567890ab1234567890abcedfghij1234567890ab")
+
+	tbs := TBSCertificate{
+		Version: Version1,
+		Name:    "testing",
+		Networks: []netip.Prefix{
+			mustParsePrefixUnmapped("10.1.1.1/24"),
+			mustParsePrefixUnmapped("10.1.1.2/16"),
+		},
+		UnsafeNetworks: []netip.Prefix{
+			mustParsePrefixUnmapped("9.1.1.2/24"),
+			mustParsePrefixUnmapped("9.1.1.3/16"),
+		},
+		Groups:    []string{"test-group1", "test-group2", "test-group3"},
+		NotBefore: before,
+		NotAfter:  after,
+		PublicKey: pubKey,
+		IsCA:      true,
+		Curve:     Curve_P256,
+	}
+
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	pub := elliptic.Marshal(elliptic.P256(), priv.PublicKey.X, priv.PublicKey.Y)
+	rawPriv := priv.D.FillBytes(make([]byte, 32))
+
+	for i := 0; i < 1000; i++ {
+		if i&1 == 1 {
+			tbs.Version = Version1
+		} else {
+			tbs.Version = Version2
+		}
+		c, err := tbs.Sign(nil, Curve_P256, rawPriv)
+		require.NoError(t, err)
+		assert.NotNil(t, c)
+		assert.True(t, c.CheckSignature(pub))
+		normie, err := p256.IsNormalized(c.Signature())
+		require.NoError(t, err)
+		assert.True(t, normie)
+	}
+}

cmd/nebula-service/main.go

@@ -78,16 +78,8 @@ func main() {
 	}
 
 	if !*configTest {
-		wait, err := ctrl.Start()
-		if err != nil {
-			util.LogWithContextIfNeeded("Error while running", err, l)
-			os.Exit(1)
-		}
-
-		go ctrl.ShutdownBlock()
-		wait()
-
-		l.Info("Goodbye")
+		ctrl.Start()
+		ctrl.ShutdownBlock()
 	}
 
 	os.Exit(0)

cmd/nebula/main.go

@@ -72,17 +72,9 @@ func main() {
 	}
 
 	if !*configTest {
-		wait, err := ctrl.Start()
-		if err != nil {
-			util.LogWithContextIfNeeded("Error while running", err, l)
-			os.Exit(1)
-		}
-
-		go ctrl.ShutdownBlock()
+		ctrl.Start()
 		notifyReady(l)
-		wait()
-
-		l.Info("Goodbye")
+		ctrl.ShutdownBlock()
 	}
 
 	os.Exit(0)

control.go

@@ -2,11 +2,9 @@ package nebula
 
 import (
 	"context"
-	"errors"
 	"net/netip"
 	"os"
 	"os/signal"
-	"sync"
 	"syscall"
 
 	"github.com/sirupsen/logrus"
@@ -15,16 +13,6 @@ import (
 	"github.com/slackhq/nebula/overlay"
 )
 
-type RunState int
-
-const (
-	Stopped  RunState = 0 // The control has yet to be started
-	Started  RunState = 1 // The control has been started
-	Stopping RunState = 2 // The control is stopping
-)
-
-var ErrAlreadyStarted = errors.New("nebula is already started")
-
 // Every interaction here needs to take extra care to copy memory and not return or use arguments "as is" when touching
 // core. This means copying IP objects, slices, de-referencing pointers and taking the actual value, etc
 
@@ -38,9 +26,6 @@ type controlHostLister interface {
 }
 
 type Control struct {
-	stateLock sync.Mutex
-	state     RunState
-
 	f                      *Interface
 	l                      *logrus.Logger
 	ctx                    context.Context
@@ -64,21 +49,10 @@ type ControlHostInfo struct {
 	CurrentRelaysThroughMe []netip.Addr     `json:"currentRelaysThroughMe"`
 }
 
-// Start actually runs nebula, this is a nonblocking call.
-// The returned function can be used to wait for nebula to fully stop.
-func (c *Control) Start() (func(), error) {
-	c.stateLock.Lock()
-	if c.state != Stopped {
-		c.stateLock.Unlock()
-		return nil, ErrAlreadyStarted
-	}
-
+// Start actually runs nebula, this is a nonblocking call. To block use Control.ShutdownBlock()
+func (c *Control) Start() {
 	// Activate the interface
-	err := c.f.activate()
-	if err != nil {
-		c.stateLock.Unlock()
-		return nil, err
-	}
+	c.f.activate()
 
 	// Call all the delayed funcs that waited patiently for the interface to be created.
 	if c.sshStart != nil {
@@ -98,33 +72,15 @@ func (c *Control) Start() (func(), error) {
 	}
 
 	// Start reading packets.
-	c.state = Started
-	c.stateLock.Unlock()
-	return c.f.run()
-}
-
-func (c *Control) State() RunState {
-	c.stateLock.Lock()
-	defer c.stateLock.Unlock()
-	return c.state
+	c.f.run()
 }
 
 func (c *Control) Context() context.Context {
 	return c.ctx
 }
 
-// Stop is a non-blocking call that signals nebula to close all tunnels and shut down
+// Stop signals nebula to shutdown and close all tunnels, returns after the shutdown is complete
 func (c *Control) Stop() {
-	c.stateLock.Lock()
-	if c.state != Started {
-		c.stateLock.Unlock()
-		// We are stopping or stopped already
-		return
-	}
-
-	c.state = Stopping
-	c.stateLock.Unlock()
-
 	// Stop the handshakeManager (and other services), to prevent new tunnels from
 	// being created while we're shutting them all down.
 	c.cancel()
@@ -133,7 +89,7 @@ func (c *Control) Stop() {
 	if err := c.f.Close(); err != nil {
 		c.l.WithError(err).Error("Close interface failed")
 	}
-	c.state = Stopped
+	c.l.Info("Goodbye")
 }
 
 // ShutdownBlock will listen for and block on term and interrupt signals, calling Control.Stop() once signalled

go.mod

@@ -4,6 +4,7 @@ go 1.25
 
 require (
 	dario.cat/mergo v1.0.2
+	filippo.io/bigmod v0.1.0
 	github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be
 	github.com/armon/go-radix v1.0.0
 	github.com/cyberdelia/go-metrics-graphite v0.0.0-20161219230853-39f87cc3b432

go.sum

@@ -1,6 +1,8 @@
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 dario.cat/mergo v1.0.2 h1:85+piFYR1tMbRrLcDwR18y4UKJ3aH1Tbzi24VRW1TK8=
 dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA=
+filippo.io/bigmod v0.1.0 h1:UNzDk7y9ADKST+axd9skUpBQeW7fG2KrTZyOE4uGQy8=
+filippo.io/bigmod v0.1.0/go.mod h1:OjOXDNlClLblvXdwgFFOQFJEocLhhtai8vGLy0JCZlI=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=

interface.go

@@ -6,8 +6,8 @@ import (
 	"fmt"
 	"io"
 	"net/netip"
+	"os"
 	"runtime"
-	"sync"
 	"sync/atomic"
 	"time"
 
@@ -88,7 +88,6 @@ type Interface struct {
 
 	writers []udp.Conn
 	readers []io.ReadWriteCloser
-	wg      sync.WaitGroup
 
 	metricHandshakes    metrics.Histogram
 	messageMetrics      *MessageMetrics
@@ -211,7 +210,7 @@ func NewInterface(ctx context.Context, c *InterfaceConfig) (*Interface, error) {
 // activate creates the interface on the host. After the interface is created, any
 // other services that want to bind listeners to its IP may do so successfully. However,
 // the interface isn't going to process anything until run() is called.
-func (f *Interface) activate() error {
+func (f *Interface) activate() {
 	// actually turn on tun dev
 
 	addr, err := f.outside.LocalAddr()
@@ -239,38 +238,33 @@ func (f *Interface) activate() error {
 		if i > 0 {
 			reader, err = f.inside.NewMultiQueueReader()
 			if err != nil {
-				return err
+				f.l.Fatal(err)
 			}
 		}
 		f.readers[i] = reader
 	}
 
-	if err = f.inside.Activate(); err != nil {
+	if err := f.inside.Activate(); err != nil {
 		f.inside.Close()
-		return err
+		f.l.Fatal(err)
 	}
-
-	return nil
 }
 
-func (f *Interface) run() (func(), error) {
+func (f *Interface) run() {
 	// Launch n queues to read packets from udp
 	for i := 0; i < f.routines; i++ {
-		f.wg.Add(1)
 		go f.listenOut(i)
 	}
 
 	// Launch n queues to read packets from tun dev
 	for i := 0; i < f.routines; i++ {
-		f.wg.Add(1)
 		go f.listenIn(f.readers[i], i)
 	}
-
-	return f.wg.Wait, nil
 }
 
 func (f *Interface) listenOut(i int) {
 	runtime.LockOSThread()
+
 	var li udp.Conn
 	if i > 0 {
 		li = f.writers[i]
@@ -285,21 +279,14 @@ func (f *Interface) listenOut(i int) {
 	fwPacket := &firewall.Packet{}
 	nb := make([]byte, 12, 12)
 
-	err := li.ListenOut(func(fromUdpAddr netip.AddrPort, payload []byte) {
+	li.ListenOut(func(fromUdpAddr netip.AddrPort, payload []byte) {
 		f.readOutsidePackets(ViaSender{UdpAddr: fromUdpAddr}, plaintext[:0], payload, h, fwPacket, lhh, nb, i, ctCache.Get(f.l))
 	})
-
-	if err != nil && !f.closed.Load() {
-		f.l.WithError(err).Error("Error while reading packet inbound packet, closing")
-		//TODO: Trigger Control to close
-	}
-
-	f.l.Debugf("underlay reader %v is done", i)
-	f.wg.Done()
 }
 
 func (f *Interface) listenIn(reader io.ReadWriteCloser, i int) {
 	runtime.LockOSThread()
+
 	packet := make([]byte, mtu)
 	out := make([]byte, mtu)
 	fwPacket := &firewall.Packet{}
@@ -310,18 +297,17 @@ func (f *Interface) listenIn(reader io.ReadWriteCloser, i int) {
 	for {
 		n, err := reader.Read(packet)
 		if err != nil {
-			if !f.closed.Load() {
-				f.l.WithError(err).Error("Error while reading outbound packet, closing")
-				//TODO: Trigger Control to close
+			if errors.Is(err, os.ErrClosed) && f.closed.Load() {
+				return
 			}
-			break
+
+			f.l.WithError(err).Error("Error while reading outbound packet")
+			// This only seems to happen when something fatal happens to the fd, so exit.
+			os.Exit(2)
 		}
 
 		f.consumeInsidePacket(packet[:n], fwPacket, nb, out, i, conntrackCache.Get(f.l))
 	}
-
-	f.l.Debugf("overlay reader %v is done", i)
-	f.wg.Done()
 }
 
 func (f *Interface) RegisterConfigChangeCallbacks(c *config.C) {
@@ -498,23 +484,21 @@ func (f *Interface) GetCertState() *CertState {
 func (f *Interface) Close() error {
 	f.closed.Store(true)
 
-	// Release the udp readers
 	for _, u := range f.writers {
 		err := u.Close()
 		if err != nil {
 			f.l.WithError(err).Error("Error while closing udp socket")
 		}
 	}
-
-	// Release the tun readers
 	for i, r := range f.readers {
 		if i == 0 {
-			continue // f.readers[0] is f.inside, which we want to save for last, since it closes other stuff too
+			continue // f.readers[0] is f.inside, which we want to save for last
 		}
 		if err := r.Close(); err != nil {
 			f.l.WithError(err).Error("Error while closing tun reader")
 		}
 	}
 
+	// Release the tun device
 	return f.inside.Close()
 }

main.go

@@ -292,15 +292,15 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg
 	}
 
 	return &Control{
-		f:                      ifce,
-		l:                      l,
-		ctx:                    ctx,
-		cancel:                 cancel,
-		sshStart:               sshStart,
-		statsStart:             statsStart,
-		dnsStart:               dnsStart,
-		lighthouseStart:        lightHouse.StartUpdateWorker,
-		connectionManagerStart: connManager.Start,
+		ifce,
+		l,
+		ctx,
+		cancel,
+		sshStart,
+		statsStart,
+		dnsStart,
+		lightHouse.StartUpdateWorker,
+		connManager.Start,
 	}, nil
 }
 

overlay/tun_linux.go

@@ -72,11 +72,6 @@ type ifreqQLEN struct {
 }
 
 func newTunFromFd(c *config.C, l *logrus.Logger, deviceFd int, vpnNetworks []netip.Prefix) (*tun, error) {
-	err := unix.SetNonblock(deviceFd, true)
-	if err != nil {
-		return nil, err
-	}
-
 	file := os.NewFile(uintptr(deviceFd), "/dev/net/tun")
 
 	t, err := newTunGeneric(c, l, file, vpnNetworks)
@@ -127,11 +122,6 @@ func newTun(c *config.C, l *logrus.Logger, vpnNetworks []netip.Prefix, multiqueu
 	}
 	name := strings.Trim(string(req.Name[:]), "\x00")
 
-	err = unix.SetNonblock(fd, true)
-	if err != nil {
-		return nil, err
-	}
-
 	file := os.NewFile(uintptr(fd), "/dev/net/tun")
 	t, err := newTunGeneric(c, l, file, vpnNetworks)
 	if err != nil {
@@ -155,12 +145,7 @@ func newTunGeneric(c *config.C, l *logrus.Logger, file *os.File, vpnNetworks []n
 		l:                         l,
 	}
 
-	err := unix.SetNonblock(t.fd, true)
-	if err != nil {
-		return nil, err
-	}
-
-	err = t.reload(c, true)
+	err := t.reload(c, true)
 	if err != nil {
 		return nil, err
 	}
@@ -266,11 +251,6 @@ func (t *tun) NewMultiQueueReader() (io.ReadWriteCloser, error) {
 		return nil, err
 	}
 
-	err = unix.SetNonblock(fd, true)
-	if err != nil {
-		return nil, err
-	}
-
 	file := os.NewFile(uintptr(fd), "/dev/net/tun")
 
 	return file, nil
@@ -281,6 +261,29 @@ func (t *tun) RoutesFor(ip netip.Addr) routing.Gateways {
 	return r
 }
 
+func (t *tun) Write(b []byte) (int, error) {
+	var nn int
+	maximum := len(b)
+
+	for {
+		n, err := unix.Write(t.fd, b[nn:maximum])
+		if n > 0 {
+			nn += n
+		}
+		if nn == len(b) {
+			return nn, err
+		}
+
+		if err != nil {
+			return nn, err
+		}
+
+		if n == 0 {
+			return nn, io.ErrUnexpectedEOF
+		}
+	}
+}
+
 func (t *tun) deviceBytes() (o [16]byte) {
 	for i, c := range t.Device {
 		o[i] = byte(c)
@@ -709,17 +712,11 @@ func (t *tun) Close() error {
 	}
 
 	if t.ReadWriteCloser != nil {
-		err := t.ReadWriteCloser.Close()
-		if err != nil {
-			t.l.WithField("error", err).Error("Failed to close read/write connection")
-		}
+		_ = t.ReadWriteCloser.Close()
 	}
 
 	if t.ioctlFd > 0 {
-		err := os.NewFile(t.ioctlFd, "ioctlFd").Close()
-		if err != nil {
-			t.l.WithField("error", err).Error("Failed to close ioctl fd")
-		}
+		_ = os.NewFile(t.ioctlFd, "ioctlFd").Close()
 		t.ioctlFd = 0
 	}
 

service/service.go

@@ -44,10 +44,7 @@ type Service struct {
 }
 
 func New(control *nebula.Control) (*Service, error) {
-	wait, err := control.Start()
-	if err != nil {
-		return nil, err
-	}
+	control.Start()
 
 	ctx := control.Context()
 	eg, ctx := errgroup.WithContext(ctx)
@@ -144,12 +141,6 @@ func New(control *nebula.Control) (*Service, error) {
 		}
 	})
 
-	// Add the nebula wait function to the group
-	eg.Go(func() error {
-		wait()
-		return nil
-	})
-
 	return &s, nil
 }
 

udp/conn.go

@@ -16,7 +16,7 @@ type EncReader func(
 type Conn interface {
 	Rebind() error
 	LocalAddr() (netip.AddrPort, error)
-	ListenOut(r EncReader) error
+	ListenOut(r EncReader)
 	WriteTo(b []byte, addr netip.AddrPort) error
 	ReloadConfig(c *config.C)
 	SupportsMultipleReaders() bool
@@ -31,8 +31,8 @@ func (NoopConn) Rebind() error {
 func (NoopConn) LocalAddr() (netip.AddrPort, error) {
 	return netip.AddrPort{}, nil
 }
-func (NoopConn) ListenOut(_ EncReader) error {
-	return nil
+func (NoopConn) ListenOut(_ EncReader) {
+	return
 }
 func (NoopConn) SupportsMultipleReaders() bool {
 	return false

udp/udp_darwin.go

@@ -165,7 +165,7 @@ func NewUDPStatsEmitter(udpConns []Conn) func() {
 	return func() {}
 }
 
-func (u *StdConn) ListenOut(r EncReader) error {
+func (u *StdConn) ListenOut(r EncReader) {
 	buffer := make([]byte, MTU)
 
 	for {
@@ -173,7 +173,8 @@ func (u *StdConn) ListenOut(r EncReader) error {
 		n, rua, err := u.ReadFromUDPAddrPort(buffer)
 		if err != nil {
 			if errors.Is(err, net.ErrClosed) {
-				return err
+				u.l.WithError(err).Debug("udp socket is closed, exiting read loop")
+				return
 			}
 
 			u.l.WithError(err).Error("unexpected udp socket receive error")

udp/udp_generic.go

@@ -10,9 +10,11 @@ package udp
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"net/netip"
+	"time"
 
 	"github.com/sirupsen/logrus"
 	"github.com/slackhq/nebula/config"
@@ -71,14 +73,25 @@ type rawMessage struct {
 	Len uint32
 }
 
-func (u *GenericConn) ListenOut(r EncReader) error {
+func (u *GenericConn) ListenOut(r EncReader) {
 	buffer := make([]byte, MTU)
 
+	var lastRecvErr time.Time
+
 	for {
 		// Just read one packet at a time
 		n, rua, err := u.ReadFromUDPAddrPort(buffer)
 		if err != nil {
-			return err
+			if errors.Is(err, net.ErrClosed) {
+				u.l.WithError(err).Debug("udp socket is closed, exiting read loop")
+				return
+			}
+			// Dampen unexpected message warns to once per minute
+			if lastRecvErr.IsZero() || time.Since(lastRecvErr) > time.Minute {
+				lastRecvErr = time.Now()
+				u.l.WithError(err).Warn("unexpected udp socket receive error")
+			}
+			continue
 		}
 
 		r(netip.AddrPortFrom(rua.Addr().Unmap(), rua.Port()), buffer[:n])

udp/udp_linux.go

@@ -9,7 +9,6 @@ import (
 	"net"
 	"net/netip"
 	"syscall"
-	"time"
 	"unsafe"
 
 	"github.com/rcrowley/go-metrics"
@@ -18,8 +17,6 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-var readTimeout = unix.NsecToTimeval(int64(3 * time.Second))
-
 type StdConn struct {
 	sysFd int
 	isV4  bool
@@ -27,6 +24,14 @@ type StdConn struct {
 	batch int
 }
 
+func maybeIPV4(ip net.IP) (net.IP, bool) {
+	ip4 := ip.To4()
+	if ip4 != nil {
+		return ip4, true
+	}
+	return ip, false
+}
+
 func NewListener(l *logrus.Logger, ip netip.Addr, port int, multi bool, batch int) (Conn, error) {
 	af := unix.AF_INET6
 	if ip.Is4() {
@@ -50,11 +55,6 @@ func NewListener(l *logrus.Logger, ip netip.Addr, port int, multi bool, batch in
 		}
 	}
 
-	// Set a read timeout
-	if err = unix.SetsockoptTimeval(fd, unix.SOL_SOCKET, unix.SO_RCVTIMEO, &readTimeout); err != nil {
-		return nil, fmt.Errorf("unable to set SO_RCVTIMEO: %s", err)
-	}
-
 	var sa unix.Sockaddr
 	if ip.Is4() {
 		sa4 := &unix.SockaddrInet4{Port: port}
@@ -122,7 +122,7 @@ func (u *StdConn) LocalAddr() (netip.AddrPort, error) {
 	}
 }
 
-func (u *StdConn) ListenOut(r EncReader) error {
+func (u *StdConn) ListenOut(r EncReader) {
 	var ip netip.Addr
 
 	msgs, buffers, names := u.PrepareRawMessages(u.batch)
@@ -134,7 +134,8 @@ func (u *StdConn) ListenOut(r EncReader) error {
 	for {
 		n, err := read(msgs)
 		if err != nil {
-			return err
+			u.l.WithError(err).Debug("udp socket is closed, exiting read loop")
+			return
 		}
 
 		for i := 0; i < n; i++ {
@@ -162,9 +163,6 @@ func (u *StdConn) ReadSingle(msgs []rawMessage) (int, error) {
 		)
 
 		if err != 0 {
-			if err == unix.EAGAIN || err == unix.EINTR || err == unix.EWOULDBLOCK {
-				continue
-			}
 			return 0, &net.OpError{Op: "recvmsg", Err: err}
 		}
 
@@ -184,14 +182,8 @@ func (u *StdConn) ReadMulti(msgs []rawMessage) (int, error) {
 			0,
 			0,
 		)
-		if err == unix.EAGAIN || err == unix.EINTR || err == unix.EWOULDBLOCK {
-			if int64(n) > 0 {
-				//ran out of time, but have some messages to return
-				return int(n), nil
-			} else {
-				continue
-			}
-		} else if err != 0 {
+
+		if err != 0 {
 			return 0, &net.OpError{Op: "recvmmsg", Err: err}
 		}
 

udp/udp_rio_windows.go

@@ -140,7 +140,7 @@ func (u *RIOConn) bind(l *logrus.Logger, sa windows.Sockaddr) error {
 	return nil
 }
 
-func (u *RIOConn) ListenOut(r EncReader) error {
+func (u *RIOConn) ListenOut(r EncReader) {
 	buffer := make([]byte, MTU)
 
 	var lastRecvErr time.Time
@@ -151,7 +151,8 @@ func (u *RIOConn) ListenOut(r EncReader) error {
 
 		if err != nil {
 			if errors.Is(err, net.ErrClosed) {
-				return err
+				u.l.WithError(err).Debug("udp socket is closed, exiting read loop")
+				return
 			}
 			// Dampen unexpected message warns to once per minute
 			if lastRecvErr.IsZero() || time.Since(lastRecvErr) > time.Minute {

udp/udp_tester.go

@@ -6,7 +6,6 @@ package udp
 import (
 	"io"
 	"net/netip"
-	"os"
 	"sync/atomic"
 
 	"github.com/sirupsen/logrus"
@@ -107,11 +106,11 @@ func (u *TesterConn) WriteTo(b []byte, addr netip.AddrPort) error {
 	return nil
 }
 
-func (u *TesterConn) ListenOut(r EncReader) error {
+func (u *TesterConn) ListenOut(r EncReader) {
 	for {
 		p, ok := <-u.RxPackets
 		if !ok {
-			return os.ErrClosed
+			return
 		}
 		r(p.From, p.Data)
 	}