Fix session ID generation

Use crypto random according to
https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-26#section-5.2.1

go.mod

@@ -2,4 +2,7 @@ module github.com/pion/sdp/v2
 
 go 1.12
 
-require github.com/stretchr/testify v1.6.1
+require (
+	github.com/pion/randutil v0.1.0
+	github.com/stretchr/testify v1.6.1
+)

go.sum

@@ -1,5 +1,7 @@
 github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/pion/randutil v0.1.0 h1:CFG1UdESneORglEsnimhUjf33Rwjubwj6xfiOXBa3mA=
+github.com/pion/randutil v0.1.0/go.mod h1:XcJrSMMbbMRhASFVOlj/5hQial/Y8oH/HVo7TBZq+j8=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

jsep.go

@@ -50,12 +50,20 @@ var extMapURI = map[int]string{
 
 // NewJSEPSessionDescription creates a new SessionDescription with
 // some settings that are required by the JSEP spec.
-func NewJSEPSessionDescription(identity bool) *SessionDescription {
+//
+// Note: Since v2.4.0, session ID has been fixed to use crypto random according to
+//       JSEP spec, so that NewJSEPSessionDescription now returns error as a second
+//       return value.
+func NewJSEPSessionDescription(identity bool) (*SessionDescription, error) {
+	sid, err := newSessionID()
+	if err != nil {
+		return nil, err
+	}
 	d := &SessionDescription{
 		Version: 0,
 		Origin: Origin{
 			Username:       "-",
-			SessionID:      newSessionID(),
+			SessionID:      sid,
 			SessionVersion: uint64(time.Now().Unix()),
 			NetworkType:    "IN",
 			AddressType:    "IP4",
@@ -80,7 +88,7 @@ func NewJSEPSessionDescription(identity bool) *SessionDescription {
 		d.WithPropertyAttribute(AttrKeyIdentity)
 	}
 
-	return d
+	return d, nil
 }
 
 // WithPropertyAttribute adds a property attribute 'a=key' to the session description

util.go

@@ -5,12 +5,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"math/rand"
 	"sort"
 	"strconv"
 	"strings"
 
-	"time"
+	"github.com/pion/randutil"
 )
 
 const (
@@ -49,9 +48,13 @@ func (t ConnectionRole) String() string {
 	}
 }
 
-func newSessionID() uint64 {
-	r := rand.New(rand.NewSource(time.Now().UnixNano()))
-	return uint64(r.Uint32()*2) >> 2
+func newSessionID() (uint64, error) {
+	// https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-26#section-5.2.1
+	// Session ID is recommended to be constructed by generating a 64-bit
+	// quantity with the highest bit set to zero and the remaining 63-bits
+	// being cryptographically random.
+	id, err := randutil.CryptoUint64()
+	return id & (^(uint64(1) << 63)), err
 }
 
 // Codec represents a codec

util_test.go

@@ -142,3 +142,29 @@ func TestGetCodecForPayloadType(t *testing.T) {
 		}
 	}
 }
+
+func TestNewSessionID(t *testing.T) {
+	min := uint64(0x7FFFFFFFFFFFFFFF)
+	max := uint64(0)
+	for i := 0; i < 10000; i++ {
+		r, err := newSessionID()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if r > (1<<63)-1 {
+			t.Fatalf("Session ID must be less than 2**64-1, got %d", r)
+		}
+		if r < min {
+			min = r
+		}
+		if r > max {
+			max = r
+		}
+	}
+	if min > 0x1000000000000000 {
+		t.Error("Value around lower boundary was not generated")
+	}
+	if max < 0x7000000000000000 {
+		t.Error("Value around upper boundary was not generated")
+	}
+}