Use server timestamp for speed test samples

psiphon/common/tactics/tactics.go

@@ -905,16 +905,17 @@ func (server *Server) handleSpeedTestRequest(
 		return
 	}
 
-	randomPadding, err := common.MakeSecureRandomPadding(
+	response, err := MakeSpeedTestResponse(
 		SPEED_TEST_PADDING_MIN_SIZE, SPEED_TEST_PADDING_MAX_SIZE)
 	if err != nil {
 		server.logger.WithContextFields(
-			common.LogFields{"error": err}).Warning("failed to generate response")
-		randomPadding = make([]byte, 0)
+			common.LogFields{"error": err}).Warning("failed to make response")
+		w.WriteHeader(http.StatusNotFound)
+		return
 	}
 
 	w.WriteHeader(http.StatusOK)
-	w.Write(randomPadding)
+	w.Write(response)
 }
 
 func (server *Server) handleTacticsRequest(
@@ -1155,17 +1156,17 @@ func FetchTactics(
 	if len(speedTestSamples) == 0 {
 
 		p := clientParameters.Get()
-		randomPadding, err := common.MakeSecureRandomPadding(
+		request, err := common.MakeSecureRandomPadding(
 			p.Int(parameters.SpeedTestPaddingMinBytes),
 			p.Int(parameters.SpeedTestPaddingMaxBytes))
 		if err != nil {
 			// TODO: log MakeSecureRandomPadding failure?
-			randomPadding = make([]byte, 0)
+			request = make([]byte, 0)
 		}
 
 		startTime := monotime.Now()
 
-		response, err := roundTripper(ctx, SPEED_TEST_END_POINT, randomPadding)
+		response, err := roundTripper(ctx, SPEED_TEST_END_POINT, request)
 
 		elaspedTime := monotime.Since(startTime)
 
@@ -1173,16 +1174,15 @@ func FetchTactics(
 			return nil, common.ContextError(err)
 		}
 
-		sample := SpeedTestSample{
-			Timestamp:        time.Now(), // *TODO* use server time
-			EndPointRegion:   endPointRegion,
-			EndPointProtocol: endPointProtocol,
-			RTTMilliseconds:  int(elaspedTime / time.Millisecond),
-			BytesUp:          len(randomPadding),
-			BytesDown:        len(response),
-		}
-
-		err = AddSpeedTestSample(clientParameters, storer, networkID, sample)
+		err = AddSpeedTestSample(
+			clientParameters,
+			storer,
+			networkID,
+			endPointRegion,
+			endPointProtocol,
+			elaspedTime,
+			request,
+			response)
 		if err != nil {
 			return nil, common.ContextError(err)
 		}
@@ -1257,6 +1257,37 @@ func FetchTactics(
 	return record, nil
 }
 
+// MakeSpeedTestResponse creates a speed test response prefixed
+// with a timestamp and followed by random padding. The timestamp
+// enables the client performing the speed test to record the
+// sample time with an accurate server clock; the random padding
+// is to frustrate fingerprinting.
+func MakeSpeedTestResponse(minPadding, maxPadding int) ([]byte, error) {
+
+	// MarshalBinary encoding (version 1) is 15 bytes:
+	// https://github.com/golang/go/blob/release-branch.go1.9/src/time/time.go#L1112
+
+	timestamp, err := time.Now().UTC().MarshalBinary()
+	if err == nil && len(timestamp) > 255 {
+		err = fmt.Errorf("unexpected marshaled time size: %d", len(timestamp))
+	}
+	if err != nil {
+		return nil, common.ContextError(err)
+	}
+
+	randomPadding, _ := common.MakeSecureRandomPadding(minPadding, maxPadding)
+	// On error, proceed without random padding.
+	// TODO: log error, even if proceeding?
+
+	response := make([]byte, 0, 1+len(timestamp)+len(randomPadding))
+
+	response = append(response, byte(len(timestamp)))
+	response = append(response, timestamp...)
+	response = append(response, randomPadding...)
+
+	return response, nil
+}
+
 // AddSpeedTestSample stores a new speed test sample. A maximum of
 // SpeedTestMaxSampleCount samples per network ID are stored, so once
 // that limit is reached, the oldest samples are removed to make room
@@ -1265,7 +1296,34 @@ func AddSpeedTestSample(
 	clientParameters *parameters.ClientParameters,
 	storer Storer,
 	networkID string,
-	sample SpeedTestSample) error {
+	endPointRegion string,
+	endPointProtocol string,
+	elaspedTime time.Duration,
+	request []byte,
+	response []byte) error {
+
+	if len(response) < 1 {
+		return common.ContextError(errors.New("unexpected empty response"))
+	}
+	timestampLength := int(response[0])
+	if len(response) < 1+timestampLength {
+		return common.ContextError(fmt.Errorf(
+			"unexpected response shorter than timestamp size %d", timestampLength))
+	}
+	var timestamp time.Time
+	err := timestamp.UnmarshalBinary(response[1 : 1+timestampLength])
+	if err != nil {
+		return common.ContextError(err)
+	}
+
+	sample := SpeedTestSample{
+		Timestamp:        timestamp,
+		EndPointRegion:   endPointRegion,
+		EndPointProtocol: endPointProtocol,
+		RTTMilliseconds:  int(elaspedTime / time.Millisecond),
+		BytesUp:          len(request),
+		BytesDown:        len(response),
+	}
 
 	maxCount := clientParameters.Get().Int(parameters.SpeedTestMaxSampleCount)
 	if maxCount == 0 {

psiphon/common/tactics/tactics_test.go

@@ -560,11 +560,20 @@ func TestTactics(t *testing.T) {
 
 	for i := 0; i < maxSamples*2; i++ {
 
-		sample := SpeedTestSample{
-			EndPointProtocol: differentEndPointProtocol,
+		response, err := MakeSpeedTestResponse(0, 0)
+		if err != nil {
+			t.Fatalf("MakeSpeedTestResponse failed: %s", err)
 		}
 
-		err = AddSpeedTestSample(clientParams, storer, networkID, sample)
+		err = AddSpeedTestSample(
+			clientParams,
+			storer,
+			networkID,
+			"",
+			differentEndPointProtocol,
+			100*time.Millisecond,
+			nil,
+			response)
 		if err != nil {
 			t.Fatalf("AddSpeedTestSample failed: %s", err)
 		}

psiphon/server/tunnelServer.go

@@ -40,6 +40,7 @@ import (
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/crypto/ssh"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/osl"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/protocol"
+	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/tactics"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/tun"
 	"github.com/marusama/semaphore"
 	cache "github.com/patrickmn/go-cache"
@@ -1234,14 +1235,11 @@ func (sshClient *sshClient) runTunnel(
 			var err error
 
 			if request.Type == "keepalive@openssh.com" {
-				// Random padding to frustrate fingerprinting.
-				responsePayload, err = common.MakeSecureRandomPadding(
+
+				// SSH keep alive round trips are used as speed test samples.
+				responsePayload, err = tactics.MakeSpeedTestResponse(
 					SSH_KEEP_ALIVE_PAYLOAD_MIN_BYTES, SSH_KEEP_ALIVE_PAYLOAD_MAX_BYTES)
-				if err != nil {
-					// Proceed without random padding.
-					responsePayload = make([]byte, 0)
-					err = nil
-				}
+
 			} else {
 
 				// All other requests are assumed to be API requests.

psiphon/tunnel.go

@@ -1426,14 +1426,14 @@ func (tunnel *Tunnel) sendSshKeepAlive(isFirstKeepAlive bool, timeout time.Durat
 	go func() {
 		// Random padding to frustrate fingerprinting.
 		p := tunnel.config.clientParameters.Get()
-		randomPadding, err := common.MakeSecureRandomPadding(
+		request, err := common.MakeSecureRandomPadding(
 			p.Int(parameters.SSHKeepAlivePaddingMinBytes),
 			p.Int(parameters.SSHKeepAlivePaddingMaxBytes))
 		p = nil
 		if err != nil {
 			NoticeAlert("MakeSecureRandomPadding failed: %s", common.ContextError(err))
 			// Proceed without random padding.
-			randomPadding = make([]byte, 0)
+			request = make([]byte, 0)
 		}
 
 		startTime := monotime.Now()
@@ -1441,7 +1441,7 @@ func (tunnel *Tunnel) sendSshKeepAlive(isFirstKeepAlive bool, timeout time.Durat
 		// Note: reading a reply is important for last-received-time tunnel
 		// duration calculation.
 		requestOk, response, err := tunnel.sshClient.SendRequest(
-			"keepalive@openssh.com", true, randomPadding)
+			"keepalive@openssh.com", true, request)
 
 		elaspedTime := monotime.Since(startTime)
 
@@ -1459,21 +1459,15 @@ func (tunnel *Tunnel) sendSshKeepAlive(isFirstKeepAlive bool, timeout time.Durat
 				tunnel.config.clientParameters.Get().WeightedCoinFlip(
 					parameters.SSHKeepAliveSpeedTestSampleProbability)) {
 
-			// TODO: refactor code in common with FetchTactics?
-			sample := tactics.SpeedTestSample{
-				Timestamp:        time.Now(), // *TODO* use server time
-				EndPointRegion:   tunnel.serverEntry.Region,
-				EndPointProtocol: tunnel.protocol,
-				RTTMilliseconds:  int(elaspedTime / time.Millisecond),
-				BytesUp:          len(randomPadding),
-				BytesDown:        len(response),
-			}
-
 			err = tactics.AddSpeedTestSample(
 				tunnel.config.clientParameters,
 				GetTacticsStorer(),
 				tunnel.config.NetworkIDGetter.GetNetworkID(),
-				sample)
+				tunnel.serverEntry.Region,
+				tunnel.protocol,
+				elaspedTime,
+				request,
+				response)
 			if err != nil {
 				NoticeAlert("AddSpeedTestSample failed: %s", common.ContextError(err))
 			}