Skip to content

Commit

Permalink
Bug fixes
Browse files Browse the repository at this point in the history
- Always use concurrency-safe `constraints` in serverEntriesReporter.

- Don't try to parse empty SteeringIP in handshake response; log when an
  unexpected steering IP is received; change info notices to warnings.

- Add TargetAPIEncoding workaround to new clientlib test cases.
  • Loading branch information
rod-hynes committed Jun 14, 2024
1 parent 1150f12 commit 7929fd6
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 22 deletions.
30 changes: 30 additions & 0 deletions ClientLibrary/clientlib/clientlib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,21 @@ func TestMultipleStartTunnel(t *testing.T) {
t.Skipf("error loading configuration file: %s", err)
}

var config map[string]interface{}
err = json.Unmarshal(configJSON, &config)
if err != nil {
t.Fatalf("json.Unmarshal failed: %v", err)
}

// Use the legacy encoding to both exercise that case, and facilitate a
// gradual network upgrade to new encoding support.
config["TargetAPIEncoding"] = protocol.PSIPHON_API_ENCODING_JSON

configJSON, err = json.Marshal(config)
if err != nil {
t.Fatalf("json.Marshal failed: %v", err)
}

testDataDirName, err := os.MkdirTemp("", "psiphon-clientlib-test")
if err != nil {
t.Fatalf("ioutil.TempDir failed: %v", err)
Expand Down Expand Up @@ -331,6 +346,21 @@ func TestPsiphonTunnel_Dial(t *testing.T) {
t.Skipf("error loading configuration file: %s", err)
}

var config map[string]interface{}
err = json.Unmarshal(configJSON, &config)
if err != nil {
t.Fatalf("json.Unmarshal failed: %v", err)
}

// Use the legacy encoding to both exercise that case, and facilitate a
// gradual network upgrade to new encoding support.
config["TargetAPIEncoding"] = protocol.PSIPHON_API_ENCODING_JSON

configJSON, err = json.Marshal(config)
if err != nil {
t.Fatalf("json.Marshal failed: %v", err)
}

testDataDirName, err := os.MkdirTemp("", "psiphon-clientlib-test")
if err != nil {
t.Fatalf("ioutil.TempDir failed: %v", err)
Expand Down
2 changes: 1 addition & 1 deletion psiphon/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ loop:

NoticeCandidateServers(
controller.config.EgressRegion,
controller.protocolSelectionConstraints,
constraints,
response.initialCandidates,
response.candidates,
duration)
Expand Down
48 changes: 27 additions & 21 deletions psiphon/serverApi.go
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ func (serverContext *ServerContext) doHandshakeRequest(ignoreStatsRegexps bool)
err := serverContext.tunnel.config.SetParameters(
tacticsRecord.Tag, true, tacticsRecord.Tactics.Parameters)
if err != nil {
NoticeInfo("apply handshake tactics failed: %s", err)
NoticeWarning("apply handshake tactics failed: %s", err)
}
// The error will be due to invalid tactics values
// from the server. When SetParameters fails, all
Expand All @@ -418,28 +418,34 @@ func (serverContext *ServerContext) doHandshakeRequest(ignoreStatsRegexps bool)
}
}

if serverContext.tunnel.dialParams.steeringIPCacheKey != "" {
if handshakeResponse.SteeringIP != "" {

if serverContext.tunnel.dialParams.steeringIPCacheKey == "" {
NoticeWarning("unexpected steering IP")

// Cache any received steering IP, which will also extend the TTL for
// an existing entry.
//
// As typical tunnel duration is short and dialing can be challenging,
// this established tunnel is retained and the steering IP will be
// used on any subsequent dial to the same fronting provider,
// assuming the TTL has not expired.
//
// Note: to avoid TTL expiry for long-lived tunnels, the TTL could be
// set or extended at the end of the tunnel lifetime; however that
// may result in unintended steering.

IP := net.ParseIP(handshakeResponse.SteeringIP)
if IP != nil && !common.IsBogon(IP) {
serverContext.tunnel.dialParams.steeringIPCache.Set(
serverContext.tunnel.dialParams.steeringIPCacheKey,
handshakeResponse.SteeringIP,
lrucache.DefaultExpiration)
} else {
NoticeInfo("ignoring invalid steering IP")

// Cache any received steering IP, which will also extend the TTL for
// an existing entry.
//
// As typical tunnel duration is short and dialing can be challenging,
// this established tunnel is retained and the steering IP will be
// used on any subsequent dial to the same fronting provider,
// assuming the TTL has not expired.
//
// Note: to avoid TTL expiry for long-lived tunnels, the TTL could be
// set or extended at the end of the tunnel lifetime; however that
// may result in unintended steering.

IP := net.ParseIP(handshakeResponse.SteeringIP)
if IP != nil && !common.IsBogon(IP) {
serverContext.tunnel.dialParams.steeringIPCache.Set(
serverContext.tunnel.dialParams.steeringIPCacheKey,
handshakeResponse.SteeringIP,
lrucache.DefaultExpiration)
} else {
NoticeWarning("ignoring invalid steering IP")
}
}
}

Expand Down

0 comments on commit 7929fd6

Please sign in to comment.