Repository: pion/turn
Pull request #119 (2020-01-20 16:33:42)

Implement nonces server side
There are no tests yet that explictly confirm
that the client can handle expired nonces

Relates to #111


codecov-io: # [Codecov](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=h1) Report
> Merging [#119](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=desc) into [master](https://codecov.io/gh/pion/turn/commit/2f14d21c8408f5f174cad6bce9a16fe40280cd48?src=pr&el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `30.3%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/turn/pull/119/graphs/tree.svg?width=650&token=1UomjQ7lwC&height=150&src=pr)](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #119      +/-   ##
==========================================
+ Coverage   65.78%   65.97%   +0.19%     
==========================================
  Files          36       36              
  Lines        1806     1825      +19     
==========================================
+ Hits         1188     1204      +16     
+ Misses        491      487       -4     
- Partials      127      134       +7
```


| [Impacted Files](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/server/server.go](https://codecov.io/gh/pion/turn/pull/119/diff?src=pr&el=tree#diff-aW50ZXJuYWwvc2VydmVyL3NlcnZlci5nbw==) | `64% <ø> (ø)` | :arrow_up: |
| [server.go](https://codecov.io/gh/pion/turn/pull/119/diff?src=pr&el=tree#diff-c2VydmVyLmdv) | `53.01% <100%> (+1.16%)` | :arrow_up: |
| [internal/server/turn.go](https://codecov.io/gh/pion/turn/pull/119/diff?src=pr&el=tree#diff-aW50ZXJuYWwvc2VydmVyL3R1cm4uZ28=) | `58.17% <18.75%> (-2.64%)` | :arrow_down: |
| [internal/server/util.go](https://codecov.io/gh/pion/turn/pull/119/diff?src=pr&el=tree#diff-aW50ZXJuYWwvc2VydmVyL3V0aWwuZ28=) | `51.47% <33.33%> (-1.87%)` | :arrow_down: |
| [client.go](https://codecov.io/gh/pion/turn/pull/119/diff?src=pr&el=tree#diff-Y2xpZW50Lmdv) | `69.75% <0%> (+3.91%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=footer). Last update [2f14d21...847edb3](https://codecov.io/gh/pion/turn/pull/119?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/client.go b/client.go
index fe9cc8c..c2fa678 100644
--- a/client.go
+++ b/client.go
@@ -69,7 +69,12 @@ type Client struct {
 
 // NewClient returns a new Client instance. listeningAddress is the address and port to listen on, default "0.0.0.0:0"
 func NewClient(config *ClientConfig) (*Client, error) {
-	log := config.LoggerFactory.NewLogger("turnc")
+	loggerFactory := config.LoggerFactory
+	if loggerFactory == nil {
+		loggerFactory = logging.NewDefaultLoggerFactory()
+	}
+
+	log := loggerFactory.NewLogger("turnc")
 
 	if config.Conn == nil {
 		return nil, fmt.Errorf("conn cannot not be nil")
diff --git a/internal/client/conn.go b/internal/client/conn.go
index c609d5c..5b5dcdb 100644
--- a/internal/client/conn.go
+++ b/internal/client/conn.go
@@ -17,6 +17,7 @@ import (
 const (
 	maxReadQueueSize    = 1024
 	permRefreshInterval = 120 * time.Second
+	maxRetryAttempts    = 3
 )
 
 const (
@@ -157,6 +158,7 @@ func (c *UDPConn) ReadFrom(p []byte) (n int, addr net.Addr, err error) {
 // see SetDeadline and SetWriteDeadline.
 // On packet-oriented connections, write timeouts are rare.
 func (c *UDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
+	var err error
 	_, ok := addr.(*net.UDPAddr)
 	if !ok {
 		return 0, fmt.Errorf("addr is not a net.UDPAddr")
@@ -176,26 +178,31 @@ func (c *UDPConn) WriteTo(p []byte, addr net.Addr) (int, error) {
 	// all the data transmission. This is done assuming that the request
 	// will be mostly likely successful and we can tolerate some loss of
 	// UDP packet (or reorder), inorder to minimize the latency in most cases.
-	err := func() error {
+	createPermission := func() error {
 		perm.mutex.Lock()
 		defer perm.mutex.Unlock()
 
 		if perm.state() == permStateIdle {
 			// punch a hole! (this would block a bit..)
-			if err := c.createPermissions(addr); err != nil {
+			if err = c.createPermissions(addr); err != nil {
 				c.permMap.delete(addr)
 				return err
 			}
 			perm.setState(permStatePermitted)
 		}
 		return nil
-	}()
+	}
+
+	for i := 0; i < maxRetryAttempts; i++ {
+		if err = createPermission(); err != errTryAgain {
+			break
+		}
+	}
 	if err != nil {
 		return 0, err
 	}
 
 	// bind channel
-
 	b, ok := c.bindingMgr.findByAddr(addr)
 	if !ok {
 		b = c.bindingMgr.create(addr)
@@ -386,7 +393,7 @@ func (c *UDPConn) createPermissions(addrs ...net.Addr) error {
 		var code stun.ErrorCodeAttribute
 		if err = code.GetFrom(res); err == nil {
 			if code.Code == stun.CodeStaleNonce {
-				c.setNonceFromMsg(msg)
+				c.setNonceFromMsg(res)
 				return errTryAgain
 			}
 			err = fmt.Errorf("%s (error %s)", res.Type, code)
@@ -555,7 +562,7 @@ func (c *UDPConn) onRefreshTimers(id int) {
 		lifetime := c.lifetime()
 		// limit the max retries on errTryAgain to 3
 		// when stale nonce returns, sencond retry should succeed
-		for i := 0; i < 3; i++ {
+		for i := 0; i < maxRetryAttempts; i++ {
 			err = c.refreshAllocation(lifetime, false)
 			if err != errTryAgain {
 				break
@@ -566,7 +573,7 @@ func (c *UDPConn) onRefreshTimers(id int) {
 		}
 	case timerIDRefreshPerms:
 		var err error
-		for i := 0; i < 3; i++ {
+		for i := 0; i < maxRetryAttempts; i++ {
 			err = c.refreshPermissions()
 			if err != errTryAgain {
 				break
diff --git a/internal/server/server.go b/internal/server/server.go
index c20f558..8e1c043 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -1,9 +1,10 @@
-//Package server implements the private API for implement a TURN server
+//Package server implements the private API to implement a TURN server
 package server
 
 import (
 	"fmt"
 	"net"
+	"sync"
 	"time"
 
 	"github.com/pion/logging"
@@ -14,18 +15,20 @@ import (
 
 // Request contains all the state needed to process a single incoming datagram
 type Request struct {
+	// Current Request State
 	Conn    net.PacketConn
 	SrcAddr net.Addr
 	Buff    []byte
 
-	// Shared across all Requests
+	// Server State
+	AllocationManager *allocation.Manager
+	Nonces            *sync.Map
+
+	// User Configuration
+	AuthHandler        func(username string, realm string, srcAddr net.Addr) (key []byte, ok bool)
 	Log                logging.LeveledLogger
-	AllocationManager  *allocation.Manager
 	Realm              string
 	ChannelBindTimeout time.Duration
-
-	// User Callbacks
-	AuthHandler func(username string, realm string, srcAddr net.Addr) (key []byte, ok bool)
 }
 
 // HandleRequest processes the give Request
diff --git a/internal/server/turn.go b/internal/server/turn.go
index f7b66b3..9be7b12 100644
--- a/internal/server/turn.go
+++ b/internal/server/turn.go
@@ -19,17 +19,8 @@ func handleAllocateRequest(r Request, m *stun.Message) error {
 	//    mechanism of [https://tools.ietf.org/html/rfc5389#section-10.2.2]
 	//    unless the client and server agree to use another mechanism through
 	//    some procedure outside the scope of this document.
-	messageIntegrity, rtrnMsg, err := authenticateRequest(r, m, stun.MethodAllocate)
-
-	// In some cases authenticateRequest may wish to send a message back to the user
-	if rtrnMsg != nil {
-		if sendErr := buildAndSend(r.Conn, r.SrcAddr, rtrnMsg...); sendErr != nil {
-			return sendErr
-		}
-	}
-
-	// If we send an message or had on error return
-	if rtrnMsg != nil || err != nil {
+	messageIntegrity, hasAuth, err := authenticateRequest(r, m, stun.MethodAllocate)
+	if !hasAuth {
 		return err
 	}
 
@@ -177,8 +168,8 @@ func handleAllocateRequest(r Request, m *stun.Message) error {
 func handleRefreshRequest(r Request, m *stun.Message) error {
 	r.Log.Debugf("received RefreshRequest from %s", r.SrcAddr.String())
 
-	messageIntegrity, _, err := authenticateRequest(r, m, stun.MethodRefresh)
-	if err != nil {
+	messageIntegrity, hasAuth, err := authenticateRequest(r, m, stun.MethodRefresh)
+	if !hasAuth {
 		return err
 	}
 
@@ -220,8 +211,8 @@ func handleCreatePermissionRequest(r Request, m *stun.Message) error {
 		return fmt.Errorf("no allocation found for %v:%v", r.SrcAddr, r.Conn.LocalAddr())
 	}
 
-	messageIntegrity, _, err := authenticateRequest(r, m, stun.MethodChannelBind)
-	if err != nil {
+	messageIntegrity, hasAuth, err := authenticateRequest(r, m, stun.MethodCreatePermission)
+	if !hasAuth {
 		return err
 	}
 
@@ -303,9 +294,9 @@ func handleChannelBindRequest(r Request, m *stun.Message) error {
 
 	badRequestMsg := buildMsg(m.TransactionID, stun.NewType(stun.MethodChannelBind, stun.ClassErrorResponse), &stun.ErrorCodeAttribute{Code: stun.CodeBadRequest})
 
-	messageIntegrity, _, err := authenticateRequest(r, m, stun.MethodChannelBind)
-	if err != nil {
-		return buildAndSendErr(r.Conn, r.SrcAddr, err, badRequestMsg...)
+	messageIntegrity, hasAuth, err := authenticateRequest(r, m, stun.MethodChannelBind)
+	if !hasAuth {
+		return err
 	}
 
 	var channel proto.ChannelNumber
diff --git a/internal/server/util.go b/internal/server/util.go
index 3cef092..60e93de 100644
--- a/internal/server/util.go
+++ b/internal/server/util.go
@@ -17,6 +17,8 @@ import (
 
 const (
 	maximumAllocationLifetime = time.Hour // https://tools.ietf.org/html/rfc5766#section-6.2 defines 3600 seconds recommendation
+	nonceLifetime             = time.Hour // https://tools.ietf.org/html/rfc5766#section-4
+
 )
 
 func randSeq(n int) string {
@@ -62,19 +64,28 @@ func buildMsg(transactionID [stun.TransactionIDSize]byte, msgType stun.MessageTy
 	return append([]stun.Setter{&stun.Message{TransactionID: transactionID}, msgType}, additional...)
 }
 
-func authenticateRequest(r Request, m *stun.Message, callingMethod stun.Method) (stun.MessageIntegrity, []stun.Setter, error) {
-	if !m.Contains(stun.AttrMessageIntegrity) {
+func authenticateRequest(r Request, m *stun.Message, callingMethod stun.Method) (stun.MessageIntegrity, bool, error) {
+	respondWithNonce := func(responseCode stun.ErrorCode) (stun.MessageIntegrity, bool, error) {
 		nonce, err := buildNonce()
 		if err != nil {
-			return stun.MessageIntegrity{}, nil, err
+			return nil, false, err
+		}
+
+		// Nonce has already been taken
+		if _, keyCollision := r.Nonces.LoadOrStore(nonce, time.Now()); keyCollision {
+			return nil, false, fmt.Errorf("duplicated Nonce generated, discarding request")
 		}
 
-		return nil, buildMsg(m.TransactionID,
+		return nil, false, buildAndSend(r.Conn, r.SrcAddr, buildMsg(m.TransactionID,
 			stun.NewType(callingMethod, stun.ClassErrorResponse),
-			&stun.ErrorCodeAttribute{Code: stun.CodeUnauthorized},
+			&stun.ErrorCodeAttribute{Code: responseCode},
 			stun.NewNonce(nonce),
 			stun.NewRealm(r.Realm),
-		), nil
+		)...)
+	}
+
+	if !m.Contains(stun.AttrMessageIntegrity) {
+		return respondWithNonce(stun.CodeUnauthorized)
 	}
 
 	nonceAttr := &stun.Nonce{}
@@ -82,28 +93,33 @@ func authenticateRequest(r Request, m *stun.Message, callingMethod stun.Method)
 	realmAttr := &stun.Realm{}
 	badRequestMsg := buildMsg(m.TransactionID, stun.NewType(callingMethod, stun.ClassErrorResponse), &stun.ErrorCodeAttribute{Code: stun.CodeBadRequest})
 
-	if err := realmAttr.GetFrom(m); err != nil {
-		return nil, badRequestMsg, err
+	if err := nonceAttr.GetFrom(m); err != nil {
+		return nil, false, buildAndSendErr(r.Conn, r.SrcAddr, err, badRequestMsg...)
 	}
 
-	if err := nonceAttr.GetFrom(m); err != nil {
-		return nil, badRequestMsg, err
+	// Assert Nonce exists and is not expired
+	nonceCreationTime, ok := r.Nonces.Load(string(*nonceAttr))
+	if !ok || time.Since(nonceCreationTime.(time.Time)) >= nonceLifetime {
+		r.Nonces.Delete(nonceAttr)
+		return respondWithNonce(stun.CodeStaleNonce)
 	}
 
-	if err := usernameAttr.GetFrom(m); err != nil {
-		return nil, badRequestMsg, err
+	if err := realmAttr.GetFrom(m); err != nil {
+		return nil, false, buildAndSendErr(r.Conn, r.SrcAddr, err, badRequestMsg...)
+	} else if err := usernameAttr.GetFrom(m); err != nil {
+		return nil, false, buildAndSendErr(r.Conn, r.SrcAddr, err, badRequestMsg...)
 	}
 
-	ourKey, ok := r.AuthHandler(usernameAttr.String(), usernameAttr.String(), r.SrcAddr)
+	ourKey, ok := r.AuthHandler(usernameAttr.String(), realmAttr.String(), r.SrcAddr)
 	if !ok {
-		return nil, badRequestMsg, fmt.Errorf("no user exists for %s", usernameAttr.String())
+		return nil, false, buildAndSendErr(r.Conn, r.SrcAddr, fmt.Errorf("no user exists for %s", usernameAttr.String()), badRequestMsg...)
 	}
 
 	if err := stun.MessageIntegrity(ourKey).Check(m); err != nil {
-		return nil, badRequestMsg, err
+		return nil, false, buildAndSendErr(r.Conn, r.SrcAddr, err, badRequestMsg...)
 	}
 
-	return stun.MessageIntegrity(ourKey), nil, nil
+	return stun.MessageIntegrity(ourKey), true, nil
 }
 
 func allocationLifeTime(m *stun.Message) time.Duration {
diff --git a/server.go b/server.go
index 54710ef..a3458d9 100644
--- a/server.go
+++ b/server.go
@@ -4,6 +4,7 @@ package turn
 import (
 	"fmt"
 	"net"
+	"sync"
 	"time"
 
 	"github.com/pion/logging"
@@ -22,6 +23,7 @@ type Server struct {
 	authHandler        AuthHandler
 	realm              string
 	channelBindTimeout time.Duration
+	nonces             *sync.Map
 
 	packetConnConfigs []PacketConnConfig
 	listenerConfigs   []ListenerConfig
@@ -45,6 +47,7 @@ func NewServer(config ServerConfig) (*Server, error) {
 		channelBindTimeout: config.ChannelBindTimeout,
 		packetConnConfigs:  config.PacketConnConfigs,
 		listenerConfigs:    config.ListenerConfigs,
+		nonces:             &sync.Map{},
 	}
 
 	if s.channelBindTimeout == 0 {
@@ -150,6 +153,7 @@ func (s *Server) readLoop(p net.PacketConn, allocationManager *allocation.Manage
 			Realm:              s.realm,
 			AllocationManager:  allocationManager,
 			ChannelBindTimeout: s.channelBindTimeout,
+			Nonces:             s.nonces,
 		}); err != nil {
 			s.log.Errorf("error when handling datagram: %v", err)
 		}


Test patch:
diff --git a/client_test.go b/client_test.go
index cfafd1e..678a378 100644
--- a/client_test.go
+++ b/client_test.go
@@ -14,43 +14,31 @@ import (
 
 func createListeningTestClient(t *testing.T, loggerFactory logging.LoggerFactory) (*Client, net.PacketConn, bool) {
 	conn, err := net.ListenPacket("udp4", "0.0.0.0:0")
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
+	assert.NoError(t, err)
+
 	c, err := NewClient(&ClientConfig{
 		Conn:          conn,
 		Software:      "TEST SOFTWARE",
 		LoggerFactory: loggerFactory,
 	})
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
-	err = c.Listen()
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
+	assert.NoError(t, err)
+	assert.NoError(t, c.Listen())
 
 	return c, conn, true
 }
 
 func createListeningTestClientWithSTUNServ(t *testing.T, loggerFactory logging.LoggerFactory) (*Client, net.PacketConn, bool) {
 	conn, err := net.ListenPacket("udp4", "0.0.0.0:0")
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
+	assert.NoError(t, err)
+
 	c, err := NewClient(&ClientConfig{
 		STUNServerAddr: "stun1.l.google.com:19302",
 		Conn:           conn,
 		Net:            vnet.NewNet(nil),
 		LoggerFactory:  loggerFactory,
 	})
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
-	err = c.Listen()
-	if !assert.NoError(t, err, "should succeed") {
-		return nil, nil, false
-	}
+	assert.NoError(t, err)
+	assert.NoError(t, c.Listen())
 
 	return c, conn, true
 }
@@ -86,9 +74,7 @@ func TestClientWithSTUN(t *testing.T) {
 		var err1 error
 
 		to, err := net.ResolveUDPAddr("udp4", "stun1.l.google.com:19302")
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		// stun1.l.google.com:19302, more at https://gist.github.com/zziuni/3741933#file-stuns-L5
 		go func() {
@@ -128,9 +114,7 @@ func TestClientWithSTUN(t *testing.T) {
 		defer c.Close()
 
 		to, err := net.ResolveUDPAddr("udp4", "127.0.0.1:9")
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		c.rto = 10 * time.Millisecond // force short timeout
 
@@ -139,3 +123,63 @@ func TestClientWithSTUN(t *testing.T) {
 		assert.NoError(t, pc.Close())
 	})
 }
+
+// Create an allocation, and then delete all nonces
+// The subsequent Write on the allocation will cause a CreatePermission
+// which will be forced to handle a stale nonce response
+func TestClientNonceExpiration(t *testing.T) {
+	// lim := test.TimeOut(time.Second * 30)
+	// defer lim.Stop()
+
+	// report := test.CheckRoutines(t)
+	// defer report()
+
+	udpListener, err := net.ListenPacket("udp4", "0.0.0.0:3478")
+	assert.NoError(t, err)
+
+	server, err := NewServer(ServerConfig{
+		AuthHandler: func(username, realm string, srcAddr net.Addr) (key []byte, ok bool) {
+			return GenerateAuthKey(username, realm, "pass"), true
+		},
+		PacketConnConfigs: []PacketConnConfig{
+			{
+				PacketConn: udpListener,
+				RelayAddressGenerator: &RelayAddressGeneratorStatic{
+					RelayAddress: net.ParseIP("127.0.0.1"),
+					Address:      "0.0.0.0",
+				},
+			},
+		},
+		Realm: "pion.ly",
+	})
+	assert.NoError(t, err)
+
+	conn, err := net.ListenPacket("udp4", "0.0.0.0:0")
+	assert.NoError(t, err)
+
+	client, err := NewClient(&ClientConfig{
+		Conn:           conn,
+		STUNServerAddr: "127.0.0.1:3478",
+		TURNServerAddr: "127.0.0.1:3478",
+		Username:       "foo",
+		Password:       "pass",
+	})
+	assert.NoError(t, err)
+	assert.NoError(t, client.Listen())
+
+	allocation, err := client.Allocate()
+	assert.NoError(t, err)
+
+	server.nonces.Range(func(key, value interface{}) bool {
+		server.nonces.Delete(key)
+		return true
+	})
+
+	_, err = allocation.WriteTo([]byte{0x00}, &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 8080})
+	assert.NoError(t, err)
+
+	// Shutdown
+	assert.NoError(t, allocation.Close())
+	assert.NoError(t, conn.Close())
+	assert.NoError(t, server.Close())
+}
diff --git a/internal/server/turn_test.go b/internal/server/turn_test.go
index c442dbe..2c0f04d 100644
--- a/internal/server/turn_test.go
+++ b/internal/server/turn_test.go
@@ -2,6 +2,7 @@ package server
 
 import (
 	"net"
+	"sync"
 	"testing"
 	"time"
 
@@ -76,6 +77,7 @@ func TestAllocationLifeTime(t *testing.T) {
 		staticKey := []byte("ABC")
 		r := Request{
 			AllocationManager: allocationManager,
+			Nonces:            &sync.Map{},
 			Conn:              l,
 			SrcAddr:           &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 5000},
 			Log:               logger,
@@ -83,6 +85,7 @@ func TestAllocationLifeTime(t *testing.T) {
 				return staticKey, true
 			},
 		}
+		r.Nonces.Store(string(staticKey), time.Now())
 
 		fiveTuple := &allocation.FiveTuple{SrcAddr: r.SrcAddr, DstAddr: r.Conn.LocalAddr(), Protocol: allocation.UDP}
 
diff --git a/server_test.go b/server_test.go
index d77b72d..f7b821d 100644
--- a/server_test.go
+++ b/server_test.go
@@ -22,7 +22,6 @@ func TestServer(t *testing.T) {
 	defer report()
 
 	loggerFactory := logging.NewDefaultLoggerFactory()
-	log := loggerFactory.NewLogger("test")
 
 	credMap := map[string][]byte{
 		"user": GenerateAuthKey("user", "pion.ly", "pass"),
@@ -30,9 +29,7 @@ func TestServer(t *testing.T) {
 
 	t.Run("simple", func(t *testing.T) {
 		udpListener, err := net.ListenPacket("udp4", "0.0.0.0:3478")
-		if err != nil {
-			t.Fatalf("Failed to create TURN server listener: %s", err)
-		}
+		assert.NoError(t, err)
 
 		server, err := NewServer(ServerConfig{
 			AuthHandler: func(username, realm string, srcAddr net.Addr) (key []byte, ok bool) {
@@ -50,7 +47,6 @@ func TestServer(t *testing.T) {
 					},
 				},
 			},
-
 			Realm:         "pion.ly",
 			LoggerFactory: loggerFactory,
 		})
@@ -58,15 +54,9 @@ func TestServer(t *testing.T) {
 
 		assert.Equal(t, proto.DefaultLifetime, server.channelBindTimeout, "should match")
 
-		// make sure the server is listening before running
-		// the client.
-		time.Sleep(100 * time.Microsecond)
-
-		log.Debug("creating a client.")
 		conn, err := net.ListenPacket("udp4", "0.0.0.0:0")
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
+
 		client, err := NewClient(&ClientConfig{
 			Conn:          conn,
 			LoggerFactory: loggerFactory,
@@ -223,9 +213,7 @@ func TestServerVNet(t *testing.T) {
 
 	t.Run("SendBindingRequest", func(t *testing.T) {
 		v, err := buildVNet()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 		defer func() {
 			assert.NoError(t, v.Close())
 		}()
@@ -249,9 +237,7 @@ func TestServerVNet(t *testing.T) {
 
 		log.Debug("sending a binding request.")
 		reflAddr, err := client.SendBindingRequest()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 		log.Debugf("mapped-address: %v", reflAddr.String())
 		udpAddr := reflAddr.(*net.UDPAddr)
 
@@ -262,14 +248,10 @@ func TestServerVNet(t *testing.T) {
 
 	t.Run("Echo via relay", func(t *testing.T) {
 		v, err := buildVNet()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		lconn, err := v.netL0.ListenPacket("udp4", "0.0.0.0:0")
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		log.Debug("creating a client.")
 		client, err := NewClient(&ClientConfig{
@@ -281,26 +263,18 @@ func TestServerVNet(t *testing.T) {
 			Net:            v.netL0,
 			LoggerFactory:  loggerFactory,
 		})
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
-		err = client.Listen()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+
+		assert.NoError(t, err)
+		assert.NoError(t, client.Listen())
 
 		log.Debug("sending a binding request.")
 		conn, err := client.Allocate()
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		log.Debugf("laddr: %s", conn.LocalAddr().String())
 
 		echoConn, err := v.net1.ListenPacket("udp4", "1.2.3.5:5678")
-		if !assert.NoError(t, err, "should succeed") {
-			return
-		}
+		assert.NoError(t, err)
 
 		go func() {
 			buf := make([]byte, 1500)
@@ -316,9 +290,7 @@ func TestServerVNet(t *testing.T) {
 
 				// echo the data
 				_, err2 = echoConn.WriteTo(buf[:n], from)
-				if !assert.NoError(t, err2, "should succeed") {
-					break
-				}
+				assert.NoError(t, err2)
 			}
 		}()
 
@@ -327,12 +299,10 @@ func TestServerVNet(t *testing.T) {
 		for i := 0; i < 10; i++ {
 			log.Debug("sending \"Hello\"..")
 			_, err = conn.WriteTo([]byte("Hello"), echoConn.LocalAddr())
-			if !assert.NoError(t, err, "should succeed") {
-				return
-			}
+			assert.NoError(t, err)
 
 			_, from, err2 := conn.ReadFrom(buf)
-			assert.NoError(t, err2, "should succeed")
+			assert.NoError(t, err2)
 
 			// verify the message was received from the relay address
 			assert.Equal(t, echoConn.LocalAddr().String(), from.String(), "should match")