Skip to content

feat(x) explicit timeout in psiphon-fetch start #402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 1, 2025

Conversation

ohnorobo
Copy link
Contributor

@ohnorobo ohnorobo commented Mar 31, 2025

Currently the psiphon-fetch start defaults to psiphon's internal 5m start timeout, which seems too long. For any invalid but valid-enough (meaning containing any PropagationChannelId+SponsorId) config it hangs for a long time.

I'm not particularly married to the 30s

Current behavior

running

go run -C ./examples/fetch-psiphon -tags psiphon . -config minimal_config.json -v https://ipinfo.io

Where the content of minimal_config.json is

{ "PropagationChannelId":"ID1", "SponsorId":"ID2" }

hangs for 5 minutes before failing with the error

Could not start dialer: clientlib: tunnel establishment timeout error link

After this change

It fails in 30s

@ohnorobo ohnorobo requested a review from fortuna March 31, 2025 16:02
Copy link
Contributor

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename the PR to psiphon start (not dial)

@@ -80,7 +80,9 @@ func main() {

// Start the Psiphon dialer.
dialer := psiphon.GetSingletonDialer()
if err := dialer.Start(context.Background(), config); err != nil {
ctx, cancel := context.WithTimeout(context.Background(), 30 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a dial timeout. It's a start timeout.
It's not how Psiphon is intended to work. It's supposed to keep running in the background.

Though, it should fail quickly if the config is invalid. Can you file a bug on their repository?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

From trawling their code I don't think it's a bug. The 5m timeout is set deliberately here. I guess they set it so high to allow for really adverse network condition. But our fetch tool doesn't need to be so accommodating.

I didn't spend too long looking at the psiphon design, but I'm assuming they simply don't respond to made up IDs because they want to avoid an adversary being able to enumerate all the IDs.

You've definitely spend more time looking at the psiphon design than I have, do you know something that would contradict that?

@ohnorobo ohnorobo changed the title feat(x) explicit timeout in psiphon-fetch dial feat(x) explicit timeout in psiphon-fetch start Apr 1, 2025
@ohnorobo ohnorobo merged commit 7e2dd59 into main Apr 1, 2025
6 checks passed
@ohnorobo ohnorobo deleted the psiphon-fetch-timeout branch April 1, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants