Send verification payload only when new payload is set, switched newClientVerificationPayload back to buffered chan, guard against multiple re-send failures, renamed controller.SetClientVerificationPayload to controller.SetClientVerificationPayloadForActiveTunnels to reflect new functionalitya

AndroidLibrary/psi/psi.go

@@ -124,7 +124,7 @@ func Stop() {
 	}
 }
 
-// This is a passthrough to Controller.SetClientVerificationPayload.
+// This is a passthrough to Controller.SetClientVerificationPayloadForActiveTunnels.
 // Note: should only be called after Start() and before Stop(); otherwise,
 // will silently take no action.
 func SetClientVerificationPayload(clientVerificationPayload string) {
@@ -133,6 +133,6 @@ func SetClientVerificationPayload(clientVerificationPayload string) {
 	defer controllerMutex.Unlock()
 
 	if controller != nil {
-		controller.SetClientVerificationPayload(clientVerificationPayload)
+		controller.SetClientVerificationPayloadForActiveTunnels(clientVerificationPayload)
 	}
 }

psiphon/config.go

@@ -68,6 +68,7 @@ const (
 	PSIPHON_API_CONNECTED_REQUEST_RETRY_PERIOD           = 5 * time.Second
 	PSIPHON_API_TUNNEL_STATS_MAX_COUNT                   = 100
 	PSIPHON_API_CLIENT_VERIFICATION_REQUEST_RETRY_PERIOD = 5 * time.Second
+	PSIPHON_API_CLIENT_VERIFICATION_REQUEST_MAX_RETRIES  = 10
 	FETCH_ROUTES_TIMEOUT_SECONDS                         = 60
 	DOWNLOAD_UPGRADE_TIMEOUT                             = 15 * time.Minute
 	DOWNLOAD_UPGRADE_RETRY_PERIOD_SECONDS                = 30

psiphon/controller.go

@@ -133,8 +133,8 @@ func NewController(config *Config) (controller *Controller, err error) {
 		signalFetchRemoteServerList: make(chan struct{}),
 		signalDownloadUpgrade:       make(chan string),
 		signalReportConnected:       make(chan struct{}),
-		// Buffer allows SetClientVerificationPayload to submit one new payload
-		// without blocking or dropping it.
+		// Buffer allows SetClientVerificationPayloadForActiveTunnels to submit one
+		// new payload without blocking or dropping it.
 		newClientVerificationPayload: make(chan string, 1),
 	}
 
@@ -249,11 +249,9 @@ func (controller *Controller) SignalComponentFailure() {
 	}
 }
 
-// SetClientVerificationPayload sets the client verification payload
-// that is to be sent in client verification requests to all established
-// tunnels. Calling this function both sets the payload to be used for
-// all future tunnels as wells as triggering requests with this payload
-// for all currently established tunneled.
+// SetClientVerificationPayloadForActiveTunnels sets the client verification
+// payload that is to be sent in client verification requests to all established
+// tunnels.
 //
 // Client verification is used to verify that the client is a
 // valid Psiphon client, which will determine how the server treats
@@ -264,10 +262,10 @@ func (controller *Controller) SignalComponentFailure() {
 // after tunnel-core starts, the payload cannot be simply specified in
 // the Config.
 //
-// SetClientVerificationPayload will not block enqueuing a new verification
+// SetClientVerificationPayloadForActiveTunnels will not block enqueuing a new verification
 // payload. One new payload can be enqueued, after which additional payloads
 // will be dropped if a payload is still enqueued.
-func (controller *Controller) SetClientVerificationPayload(clientVerificationPayload string) {
+func (controller *Controller) SetClientVerificationPayloadForActiveTunnels(clientVerificationPayload string) {
 	select {
 	case controller.newClientVerificationPayload <- clientVerificationPayload:
 	default:
@@ -589,8 +587,6 @@ loop:
 				break
 			}
 
-			establishedTunnel.SetClientVerificationPayload(clientVerificationPayload)
-
 			NoticeActiveTunnel(establishedTunnel.serverEntry.IpAddress, establishedTunnel.protocol)
 
 			if tunnelCount == 1 {

psiphon/tunnel.go

@@ -152,9 +152,11 @@ func EstablishTunnel(
 		shutdownOperateBroadcast: make(chan struct{}),
 		// A buffer allows at least one signal to be sent even when the receiver is
 		// not listening. Senders should not block.
-		signalPortForwardFailure:     make(chan struct{}, 1),
-		dialStats:                    dialStats,
-		newClientVerificationPayload: make(chan string),
+		signalPortForwardFailure: make(chan struct{}, 1),
+		dialStats:                dialStats,
+		// Buffer allows SetClientVerificationPayload to submit one new payload
+		// without blocking or dropping it.
+		newClientVerificationPayload: make(chan string, 1),
 	}
 
 	// Create a new Psiphon API server context for this tunnel. This includes
@@ -871,9 +873,11 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 
 		clientVerificationRequestSuccess := true
 		clientVerificationPayload := ""
+		failCount := 0
 		for {
 			// TODO: use reflect.SelectCase?
 			if clientVerificationRequestSuccess == true {
+				failCount = 0
 				select {
 				case clientVerificationPayload = <-tunnel.newClientVerificationPayload:
 				case <-signalStopClientVerificationRequests:
@@ -883,6 +887,12 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 				// If sendClientVerification failed to send the payload we
 				// will retry after a delay. Will use a new payload instead
 				// if that arrives in the meantime.
+				// If failures count is more than PSIPHON_API_CLIENT_VERIFICATION_REQUEST_MAX_RETRIES
+				// stop retrying for this tunnel.
+				failCount += 1
+				if failCount > PSIPHON_API_CLIENT_VERIFICATION_REQUEST_MAX_RETRIES {
+					return
+				}
 				timeout := time.After(PSIPHON_API_CLIENT_VERIFICATION_REQUEST_RETRY_PERIOD)
 				select {
 				case <-timeout: