Replace crypto/rand.Read() calls with random stream

psiphon/common/quic/obfuscator.go

@@ -21,6 +21,7 @@ package quic
 
 import (
 	"crypto/rand"
+	"crypto/rc4"
 	"crypto/sha256"
 	"fmt"
 	"io"
@@ -40,7 +41,7 @@ const (
 	MAX_QUIC_IPV6_PACKET_SIZE            = 1232
 	MAX_OBFUSCATED_QUIC_IPV4_PACKET_SIZE = 1372
 	MAX_OBFUSCATED_QUIC_IPV6_PACKET_SIZE = 1352
-	MAX_PADDING_SIZE                     = 64
+	MAX_PADDING                          = 64
 	NONCE_SIZE                           = 12
 )
 
@@ -65,6 +66,7 @@ type ObfuscatedPacketConn struct {
 	runWaitGroup   *sync.WaitGroup
 	stopBroadcast  chan struct{}
 	obfuscationKey [32]byte
+	randomStream   *rc4.Cipher
 	peerModesMutex sync.Mutex
 	peerModes      map[string]*peerMode
 }
@@ -98,6 +100,31 @@ func NewObfuscatedPacketConnPacketConn(
 		return nil, common.ContextError(err)
 	}
 
+	// Use a stream cipher to generate randomness for padding. This mitigates
+	// issues using a high volume (multiple per packet) of crypto/rand.Read
+	// calls, which use getrandom via a syscall; under high load with many
+	// clients, we observed very long syscall durations, perhaps due to lock
+	// contention. Using a userspace random stream avoids frequent syscall
+	// context switches as well as spinlock overhead.
+	//
+	// Note: use of RC4 is temporary. While RC4 isn't cryptographically
+	// secure, it is sufficient for obfuscation. At this time we aren't using
+	// github.com/Yawning/chacha20 for the random stream due to apparant
+	// "index out of range" bugs that occur after repeated XORKeyStream calls.
+	// RC4 performance will not be optimal as it no longer ships with
+	// optimized assembler implementations,
+	// https://github.com/golang/go/commit/30eda6715c6578de2086f03df36c4a8def838ec2.
+
+	var randomStreamKey [32]byte
+	_, err = rand.Read(randomStreamKey[:])
+	if err != nil {
+		return nil, common.ContextError(err)
+	}
+	packetConn.randomStream, err = rc4.NewCipher(randomStreamKey[:])
+	if err != nil {
+		return nil, common.ContextError(err)
+	}
+
 	if isServer {
 
 		packetConn.runWaitGroup = new(sync.WaitGroup)
@@ -231,7 +258,7 @@ func (conn *ObfuscatedPacketConn) ReadFrom(p []byte) (int, net.Addr, error) {
 			cipher.XORKeyStream(p[NONCE_SIZE:], p[NONCE_SIZE:])
 
 			paddingLen := int(p[NONCE_SIZE])
-			if paddingLen > MAX_PADDING_SIZE || paddingLen > n-(NONCE_SIZE+1) {
+			if paddingLen > MAX_PADDING || paddingLen > n-(NONCE_SIZE+1) {
 				return n, addr, newTemporaryNetError(common.ContextError(
 					fmt.Errorf("unexpected padding length: %d, %d", paddingLen, n)))
 			}
@@ -293,11 +320,17 @@ func (conn *ObfuscatedPacketConn) WriteTo(p []byte, addr net.Addr) (int, error)
 		buffer := b.buffer[:]
 		defer obfuscatorBufferPool.Put(b)
 
+		// Zero the buffer to ensure that randomStream.XORKeyStream calls set
+		// bytes to the key stream values. This loop should be compiler
+		// optimized:
+		// https://github.com/golang/go/commit/f03c9202c43e0abb130669852082117ca50aa9b1
+		for i := range buffer {
+			buffer[i] = 0
+		}
+
+		nonce := buffer[0:NONCE_SIZE]
 		for {
-			_, err := rand.Read(buffer[0:NONCE_SIZE])
-			if err != nil {
-				return 0, common.ContextError(err)
-			}
+			conn.randomStream.XORKeyStream(nonce, nonce)
 
 			// Don't use a random nonce that looks like QUIC, or the
 			// peer will not treat this packet as obfuscated.
@@ -309,33 +342,29 @@ func (conn *ObfuscatedPacketConn) WriteTo(p []byte, addr net.Addr) (int, error)
 		// Obfuscated QUIC padding results in packets that exceed the
 		// QUIC max packet size of 1280.
 
-		maxPaddingSize := maxObfuscatedPacketSize - (n + (NONCE_SIZE + 1))
-		if maxPaddingSize < 0 {
-			maxPaddingSize = 0
+		maxPaddingLen := maxObfuscatedPacketSize - (n + (NONCE_SIZE + 1))
+		if maxPaddingLen < 0 {
+			maxPaddingLen = 0
 		}
-		if maxPaddingSize > MAX_PADDING_SIZE {
-			maxPaddingSize = MAX_PADDING_SIZE
+		if maxPaddingLen > MAX_PADDING {
+			maxPaddingLen = MAX_PADDING
 		}
 
-		paddingLen, err := common.MakeSecureRandomRange(0, maxPaddingSize)
-		if err != nil {
-			return 0, common.ContextError(err)
-		}
+		paddingLen := 0 //maxPaddingLen //conn.getPaddingLen(maxPaddingLen)
 
 		buffer[NONCE_SIZE] = uint8(paddingLen)
-		_, err = rand.Read(buffer[(NONCE_SIZE + 1) : (NONCE_SIZE+1)+paddingLen])
-		if err != nil {
-			return 0, common.ContextError(err)
-		}
+		padding := buffer[(NONCE_SIZE + 1) : (NONCE_SIZE+1)+paddingLen]
+		conn.randomStream.XORKeyStream(padding, padding)
 
 		copy(buffer[(NONCE_SIZE+1)+paddingLen:], p)
 		dataLen := (NONCE_SIZE + 1) + paddingLen + n
 
-		cipher, err := chacha20.NewCipher(conn.obfuscationKey[:], buffer[0:NONCE_SIZE])
+		cipher, err := chacha20.NewCipher(conn.obfuscationKey[:], nonce)
 		if err != nil {
 			return 0, common.ContextError(err)
 		}
-		cipher.XORKeyStream(buffer[NONCE_SIZE:dataLen], buffer[NONCE_SIZE:dataLen])
+		packet := buffer[NONCE_SIZE:dataLen]
+		cipher.XORKeyStream(packet, packet)
 
 		p = buffer[:dataLen]
 	}
@@ -345,6 +374,33 @@ func (conn *ObfuscatedPacketConn) WriteTo(p []byte, addr net.Addr) (int, error)
 	return n, err
 }
 
+func (conn *ObfuscatedPacketConn) getPaddingLen(maxPadding int) int {
+
+	// Selects uniformly from [0, n], using the ObfuscatedPacketConn's
+	// random stream.
+
+	if maxPadding < 0 || maxPadding > 255 {
+		panic(fmt.Sprintf("unexpected max padding: %d", maxPadding))
+	}
+
+	if maxPadding == 0 {
+		return 0
+	}
+
+	maxRand := 255
+	upperBound := maxRand - (maxRand % (maxPadding + 1))
+
+	for {
+		var value [1]byte
+		conn.randomStream.XORKeyStream(value[:], value[:])
+
+		padding := int(value[0])
+		if padding <= upperBound {
+			return padding % (maxPadding + 1)
+		}
+	}
+}
+
 func isQUIC(buffer []byte) bool {
 
 	// As this function is called for every packet, it needs to be fast.

psiphon/common/quic/obfuscator_test.go

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2018, Psiphon Inc.
+ * All rights reserved.
+ *
+ * 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 quic
+
+import (
+	"testing"
+)
+
+func TestPaddingLen(t *testing.T) {
+
+	c, err := NewObfuscatedPacketConnPacketConn(nil, false, "key")
+	if err != nil {
+		t.Fatalf("NewObfuscatedPacketConnPacketConn failed: %s", err)
+	}
+
+	for max := 0; max <= MAX_PADDING; max++ {
+
+		counts := make(map[int]int)
+		repeats := 100000
+
+		for r := 0; r < repeats; r++ {
+			padding := c.getPaddingLen(max)
+			if padding < 0 || padding > max {
+				t.Fatalf("unexpected padding: max = %d, padding = %d", max, padding)
+			}
+			counts[padding] += 1
+		}
+
+		expected := repeats / (max + 1)
+
+		for i := 0; i <= max; i++ {
+			if counts[i] < (expected/10)*9 {
+				t.Logf("max = %d, counts = %+v", max, counts)
+				t.Fatalf("unexpected low count: max = %d, i = %d, count = %d", max, i, counts[i])
+			}
+		}
+	}
+}

psiphon/server/tunnelServer.go

@@ -1955,7 +1955,6 @@ func (sshClient *sshClient) logTunnel(additionalMetrics []LogFields) {
 	// sshClient.udpTrafficState.peakConcurrentDialingPortForwardCount isn't meaningful
 	logFields["peak_concurrent_port_forward_count_udp"] = sshClient.udpTrafficState.peakConcurrentPortForwardCount
 	logFields["total_port_forward_count_udp"] = sshClient.udpTrafficState.totalPortForwardCount
-	logFields["pre_handshake_random_stream_count"] = sshClient.preHandshakeRandomStreamMetrics.count
 
 	logFields["pre_handshake_random_stream_count"] = sshClient.preHandshakeRandomStreamMetrics.count
 	logFields["pre_handshake_random_stream_upstream_bytes"] = sshClient.preHandshakeRandomStreamMetrics.upstreamBytes