Refactor random string stuff and seeding

Make sure we have a good random seed on the default RNG, that the
predictable RNG is clearly marked as such, that random strings are
actually the length requested, and that they contain a restricted set of
characters only.

cmd/syncthing/gui.go

@@ -353,7 +353,7 @@ func restPostConfig(m *model.Model, w http.ResponseWriter, r *http.Request) {
 		if curAcc := cfg.Options().URAccepted; newCfg.Options.URAccepted > curAcc {
 			// UR was enabled
 			newCfg.Options.URAccepted = usageReportVersion
-			newCfg.Options.URUniqueID = randomString(6)
+			newCfg.Options.URUniqueID = randomString(8)
 			err := sendUsageReport(m)
 			if err != nil {
 				l.Infoln("Usage report:", err)

cmd/syncthing/gui_csrf.go

@@ -17,8 +17,6 @@ package main
 
 import (
 	"bufio"
-	"crypto/rand"
-	"encoding/base64"
 	"fmt"
 	"net/http"
 	"os"
@@ -88,7 +86,7 @@ func validCsrfToken(token string) bool {
 }
 
 func newCsrfToken() string {
-	token := randomString(30)
+	token := randomString(32)
 
 	csrfMut.Lock()
 	csrfTokens = append(csrfTokens, token)
@@ -140,13 +138,3 @@ func loadCsrfTokens() {
 		csrfTokens = append(csrfTokens, s.Text())
 	}
 }
-
-func randomString(len int) string {
-	bs := make([]byte, len)
-	_, err := rand.Reader.Read(bs)
-	if err != nil {
-		l.Fatalln(err)
-	}
-
-	return base64.StdEncoding.EncodeToString(bs)
-}

cmd/syncthing/main.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"math/rand"
 	"net"
 	"net/http"
 	_ "net/http/pprof"
@@ -177,10 +176,6 @@ are mostly useful for developers. Use with care.
                available CPU cores.`
 )
 
-func init() {
-	rand.Seed(time.Now().UnixNano())
-}
-
 // Command line and environment options
 var (
 	reset             bool
@@ -383,6 +378,10 @@ func syncthingMain() {
 		}
 	}
 
+	// We reinitialize the predictable RNG with our device ID, to get a
+	// sequence that is always the same but unique to this syncthing instance.
+	predictableRandom.Seed(seedFromBytes(cert.Certificate[0]))
+
 	myID = protocol.NewDeviceID(cert.Certificate[0])
 	l.SetPrefix(fmt.Sprintf("[%s] ", myID.String()[:5]))
 
@@ -574,7 +573,7 @@ func syncthingMain() {
 		if opts.URUniqueID == "" {
 			// Previously the ID was generated from the node ID. We now need
 			// to generate a new one.
-			opts.URUniqueID = randomString(6)
+			opts.URUniqueID = randomString(8)
 			cfg.SetOptions(opts)
 			cfg.Save()
 		}
@@ -781,11 +780,8 @@ func setupExternalPort(igd *upnp.IGD, port int) int {
 		return 0
 	}
 
-	// We seed the random number generator with the node ID to get a
-	// repeatable sequence of random external ports.
-	rnd := rand.NewSource(certSeed(cert.Certificate[0]))
 	for i := 0; i < 10; i++ {
-		r := 1024 + int(rnd.Int63()%(65535-1024))
+		r := 1024 + predictableRandom.Intn(65535-1024)
 		err := igd.AddPortMapping(upnp.TCP, r, port, "syncthing", cfg.Options().UPnPLease*60)
 		if err == nil {
 			return r

cmd/syncthing/random.go

@@ -0,0 +1,67 @@
+// Copyright (C) 2014 The Syncthing Authors.
+//
+// This program is free software: you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation, either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package main
+
+import (
+	"crypto/md5"
+	cryptoRand "crypto/rand"
+	"encoding/binary"
+	mathRand "math/rand"
+)
+
+// randomCharset contains the characters that can make up a randomString().
+const randomCharset = "01234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-"
+
+// predictableRandom is an RNG that will always have the same sequence. It
+// will be seeded with the device ID during startup, so that the sequence is
+// predictable but varies between instances.
+var predictableRandom = mathRand.New(mathRand.NewSource(42))
+
+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.
+func randomString(l int) string {
+	bs := make([]byte, l)
+	for i := range bs {
+		bs[i] = randomCharset[mathRand.Intn(len(randomCharset))]
+	}
+	return string(bs)
+}
+
+// randomInt64 returns a strongly random int64, slowly
+func randomInt64() int64 {
+	var bs [8]byte
+	n, err := cryptoRand.Reader.Read(bs[:])
+	if n != 8 || err != nil {
+		panic("randomness failure")
+	}
+	return seedFromBytes(bs[:])
+}
+
+// seedFromBytes calculates a weak 64 bit hash from the given byte slice,
+// suitable for use a predictable random seed.
+func seedFromBytes(bs []byte) int64 {
+	h := md5.New()
+	h.Write(bs)
+	s := h.Sum(nil)
+	// The MD5 hash of the byte slice is 16 bytes long. We interpret it as two
+	// uint64s and XOR them together.
+	return int64(binary.BigEndian.Uint64(s[0:]) ^ binary.BigEndian.Uint64(s[8:]))
+}

cmd/syncthing/random_test.go

@@ -0,0 +1,80 @@
+// Copyright (C) 2014 The Syncthing Authors.
+//
+// This program is free software: you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation, either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package main
+
+import "testing"
+
+func TestPredictableRandom(t *testing.T) {
+	// predictable random sequence is predictable
+	e := 3440579354231278675
+	if v := predictableRandom.Int(); v != e {
+		t.Errorf("Unexpected random value %d != %d", v, e)
+	}
+}
+
+func TestSeedFromBytes(t *testing.T) {
+	// should always return the same seed for the same bytes
+	tcs := []struct {
+		bs []byte
+		v  int64
+	}{
+		{[]byte("hello world"), -3639725434188061933},
+		{[]byte("hello worlx"), -2539100776074091088},
+	}
+
+	for _, tc := range tcs {
+		if v := seedFromBytes(tc.bs); v != tc.v {
+			t.Errorf("Unexpected seed value %d != %d", v, tc.v)
+		}
+	}
+}
+
+func TestRandomString(t *testing.T) {
+	for _, l := range []int{0, 1, 2, 3, 4, 8, 42} {
+		s := randomString(l)
+		if len(s) != l {
+			t.Errorf("Incorrect length %d != %s", len(s), l)
+		}
+	}
+
+	strings := make([]string, 1000)
+	for i := range strings {
+		strings[i] = randomString(8)
+		for j := range strings {
+			if i == j {
+				continue
+			}
+			if strings[i] == strings[j] {
+				t.Errorf("Repeated random string %q", strings[i])
+			}
+		}
+	}
+}
+
+func TestRandomInt64(t *testing.T) {
+	ints := make([]int64, 1000)
+	for i := range ints {
+		ints[i] = randomInt64()
+		for j := range ints {
+			if i == j {
+				continue
+			}
+			if ints[i] == ints[j] {
+				t.Errorf("Repeated random int64 %d", ints[i])
+			}
+		}
+	}
+}

cmd/syncthing/tls.go

@@ -19,11 +19,9 @@ import (
 	"bufio"
 	"crypto/rand"
 	"crypto/rsa"
-	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
 	"crypto/x509/pkix"
-	"encoding/binary"
 	"encoding/pem"
 	"io"
 	"math/big"
@@ -45,13 +43,6 @@ func loadCert(dir string, prefix string) (tls.Certificate, error) {
 	return tls.LoadX509KeyPair(cf, kf)
 }
 
-func certSeed(bs []byte) int64 {
-	hf := sha256.New()
-	hf.Write(bs)
-	id := hf.Sum(nil)
-	return int64(binary.BigEndian.Uint64(id))
-}
-
 func newCertificate(dir string, prefix string) {
 	l.Infoln("Generating RSA key and certificate...")