Skip to content

Commit

Permalink
Use crypto/rand and pion randutil instead of math/rand
Browse files Browse the repository at this point in the history
math/rand isn't suitable for security-sensitive contexts, so replace it
with cryptographically secure random string generation.
  • Loading branch information
Daniel Kessler committed Oct 24, 2023
1 parent f15249d commit b4a2308
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
8 changes: 7 additions & 1 deletion internal/server/turn.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import (
"fmt"
"net"

"github.com/pion/randutil"
"github.com/pion/stun/v2"
"github.com/pion/turn/v3/internal/allocation"
"github.com/pion/turn/v3/internal/ipnet"
"github.com/pion/turn/v3/internal/proto"
)

const runesAlpha = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"

// See: https://tools.ietf.org/html/rfc5766#section-6.2
func handleAllocateRequest(r Request, m *stun.Message) error {
r.Log.Debugf("Received AllocateRequest from %s", r.SrcAddr.String())
Expand Down Expand Up @@ -106,7 +109,10 @@ func handleAllocateRequest(r Request, m *stun.Message) error {
return buildAndSendErr(r.Conn, r.SrcAddr, err, insufficientCapacityMsg...)
}
requestedPort = randomPort
reservationToken = randSeq(8)
reservationToken, err = randutil.GenerateCryptoRandomString(8, runesAlpha)
if err != nil {
return err
}

Check warning on line 115 in internal/server/turn.go

View check run for this annotation

Codecov / codecov/patch

internal/server/turn.go#L112-L115

Added lines #L112 - L115 were not covered by tests
}

// 7. At any point, the server MAY choose to reject the request with a
Expand Down
22 changes: 11 additions & 11 deletions internal/server/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ package server

import (
"crypto/md5" //nolint:gosec,gci
"crypto/rand"
"errors"
"fmt"
"io"
"math/rand"
"math/big"
"net"
"strconv"
"time"
Expand All @@ -22,22 +23,21 @@ const (
nonceLifetime = time.Hour // See: https://tools.ietf.org/html/rfc5766#section-4
)

func randSeq(n int) string {
letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
b := make([]rune, n)
for i := range b {
b[i] = letters[rand.Intn(len(letters))] //nolint:gosec
}
return string(b)
}

func buildNonce() (string, error) {
/* #nosec */
h := md5.New()
if _, err := io.WriteString(h, strconv.FormatInt(time.Now().Unix(), 10)); err != nil {
return "", fmt.Errorf("%w: %v", errFailedToGenerateNonce, err) //nolint:errorlint
}
if _, err := io.WriteString(h, strconv.FormatInt(rand.Int63(), 10)); err != nil { //nolint:gosec

maxInt63 := big.NewInt(1<<63 - 1)
maxInt63.Add(maxInt63, big.NewInt(1))
randInt63, err := rand.Int(rand.Reader, maxInt63)
if err != nil {
return "", fmt.Errorf("%w: %v", errFailedToGenerateNonce, err)

Check failure on line 37 in internal/server/util.go

View workflow job for this annotation

GitHub Actions / lint / Go

non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
}

Check warning on line 38 in internal/server/util.go

View check run for this annotation

Codecov / codecov/patch

internal/server/util.go#L37-L38

Added lines #L37 - L38 were not covered by tests

if _, err := io.WriteString(h, randInt63.String()); err != nil { //nolint:gosec
return "", fmt.Errorf("%w: %v", errFailedToGenerateNonce, err) //nolint:errorlint
}
return fmt.Sprintf("%x", h.Sum(nil)), nil
Expand Down

0 comments on commit b4a2308

Please sign in to comment.