lib/util: Add secure random numbers source (fixes #3178)

The math/rand package contains lots of convenient functions, for example
to get an integer in a specified range without running into issues
caused by just truncating a number from a different distribution and so
on. But it's insecure, and we use if for things that benefit from being
more secure like session IDs, CSRF tokens and API keys.

This implements a math/rand.Source that reads from crypto/rand.Reader,
this bridging the gap between them. It also updates our RandomString to
use the new source, thus giving us secure session IDs and CSRF tokens.

Some future work remains:

 - Fix API keys by making the generation in the UI use this code as well

 - Refactor out these things into an actual random package, and audit
   our use of randomness everywhere

I'll leave both of those for the future in order to not muddy the waters
on this diff...

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3180

lib/util/random.go

@@ -17,17 +17,27 @@ import (
 // randomCharset contains the characters that can make up a randomString().
 const randomCharset = "2345679abcdefghijkmnopqrstuvwxyzACDEFGHJKLMNPQRSTUVWXYZ"
 
+var (
+	// defaultSecureSource is a concurrency safe math/rand.Source with a
+	// cryptographically sound base.
+	defaltSecureSource = newSecureSource()
+
+	// defaultSecureRand is a math/rand.Rand based on the secure source.
+	defaultSecureRand = mathRand.New(defaltSecureSource)
+)
+
 func init() {
 	// The default RNG should be seeded with something good.
 	mathRand.Seed(RandomInt64())
 }
 
-// RandomString returns a string of random characters (taken from
-// randomCharset) of the specified length.
+// RandomString returns a strongly random string of characters (taken from
+// randomCharset) of the specified length. The returned string contains ~5.8
+// bits of entropy per character, due to the character set used.
 func RandomString(l int) string {
 	bs := make([]byte, l)
 	for i := range bs {
-		bs[i] = randomCharset[mathRand.Intn(len(randomCharset))]
+		bs[i] = randomCharset[defaultSecureRand.Intn(len(randomCharset))]
 	}
 	return string(bs)
 }

lib/util/securesource.go

@@ -0,0 +1,57 @@
+// Copyright (C) 2016 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package util
+
+import (
+	"bufio"
+	"crypto/rand"
+	"encoding/binary"
+	"io"
+	"sync"
+)
+
+// The secureSource is a math/rand.Source that reads bytes from
+// crypto/rand.Reader. It means we can use the convenience functions
+// provided by math/rand.Rand on top of a secure source of numbers. It is
+// concurrency safe for ease of use.
+type secureSource struct {
+	rd  io.Reader
+	mut sync.Mutex
+}
+
+func newSecureSource() *secureSource {
+	return &secureSource{
+		// Using buffering on top of the rand.Reader increases our
+		// performance by about 20%, even though it means we must use
+		// locking.
+		rd: bufio.NewReader(rand.Reader),
+	}
+}
+
+func (s *secureSource) Seed(int64) {
+	panic("SecureSource is not seedable")
+}
+
+func (s *secureSource) Int63() int64 {
+	var buf [8]byte
+
+	// Read eight bytes of entropy from the buffered, secure random number
+	// generator. The buffered reader isn't concurrency safe, so we lock
+	// around that.
+	s.mut.Lock()
+	_, err := io.ReadFull(s.rd, buf[:])
+	s.mut.Unlock()
+	if err != nil {
+		panic("randomness failure: " + err.Error())
+	}
+
+	// Grab those bytes as an uint64
+	v := binary.BigEndian.Uint64(buf[:])
+
+	// Mask of the high bit and return the resulting int63
+	return int64(v & (1<<63 - 1))
+}

lib/util/securesource_test.go

@@ -0,0 +1,96 @@
+// Copyright (C) 2016 The Syncthing Authors.
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this file,
+// You can obtain one at http://mozilla.org/MPL/2.0/.
+
+package util
+
+import "testing"
+
+func TestSecureSource(t *testing.T) {
+	// This is not a test to verify that the random numbers are secure,
+	// merely that the numbers look random at all and that we've haven't
+	// broken the implementation by masking off half the bits or always
+	// returning "4" (chosen by fair dice roll),
+
+	const nsamples = 10000
+
+	// Create a new source and sample values from it.
+	s := newSecureSource()
+	res0 := make([]int64, nsamples)
+	for i := range res0 {
+		res0[i] = s.Int63()
+	}
+
+	// Do it again
+	s = newSecureSource()
+	res1 := make([]int64, nsamples)
+	for i := range res1 {
+		res1[i] = s.Int63()
+	}
+
+	// There should (statistically speaking) be no repetition of the values,
+	// neither within the samples from a source nor between sources.
+	for _, v0 := range res0 {
+		for _, v1 := range res1 {
+			if v0 == v1 {
+				t.Errorf("Suspicious coincidence, %d repeated between res0/res1", v0)
+			}
+		}
+	}
+	for i, v0 := range res0 {
+		for _, v1 := range res0[i+1:] {
+			if v0 == v1 {
+				t.Errorf("Suspicious coincidence, %d repeated within res0", v0)
+			}
+		}
+	}
+	for i, v0 := range res1 {
+		for _, v1 := range res1[i+1:] {
+			if v0 == v1 {
+				t.Errorf("Suspicious coincidence, %d repeated within res1", v0)
+			}
+		}
+	}
+
+	// Count how many times each bit was set. On average each bit ought to
+	// be set in half of the samples, except the topmost bit which must
+	// never be set (int63). We raise an alarm if a single bit is set in
+	// fewer than 1/3 of the samples or more often than 2/3 of the samples.
+	var bits [64]int
+	for _, v := range res0 {
+		for i := range bits {
+			if v&1 == 1 {
+				bits[i]++
+			}
+			v >>= 1
+		}
+	}
+	for bit, count := range bits {
+		switch bit {
+		case 63:
+			// The topmost bit is never set
+			if count != 0 {
+				t.Errorf("The topmost bit was set %d times in %d samples (should be 0)", count, nsamples)
+			}
+		default:
+			if count < nsamples/3 {
+				t.Errorf("Bit %d was set only %d times out of %d", bit, count, nsamples)
+			}
+			if count > nsamples/3*2 {
+				t.Errorf("Bit %d was set fully %d times out of %d", bit, count, nsamples)
+			}
+		}
+	}
+}
+
+var sink int64
+
+func BenchmarkSecureSource(b *testing.B) {
+	s := newSecureSource()
+	for i := 0; i < b.N; i++ {
+		sink = s.Int63()
+	}
+	b.ReportAllocs()
+}