Skip to content
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

Fix unconditional Return in obfs4 transport #130

Closed
wants to merge 2 commits into from
Closed

Conversation

jmwample
Copy link
Member

@jmwample jmwample commented Jun 9, 2022

Fix for unconditional return in obfs4 WrapConnection - move checks on proper data length earlier in the function and continue through loop when the MAC mark that we are looking for is not found.

closes #128
closes #133

Changes

obfs4

  • move mark len check proper locations
    • too short check up front
    • not found, but not past max at end for retry
  • mark not found w/ one reg, continue to next instead of return
  • use tempdir instead of current dir for obfs4 state

PhantomSelector

  • update selection to set min and max to correct bounds
  • update min and max checks to be inclusive ensuring proper range
    • enables inclusion of short (/32, /31, /128, /127 etc.) subnets
  • change ReadVarint to Varint to prevent occasional overflow errors

Proof that the fixes works:

TestSuccessfulWrapMulti - checks that a connection can be established for an obfs4 session even when multiple obfs4 sessions map to the same phantom address.

TestPhantomSeededSelectionFuzz - ensures no errors occur when subnets of all sizes (/0-/32) are used as the phantom subnet for selecting phantom addresses many times over (10,000 selections).

TestPhantomsSeededSelectionV4Min - ensures that minimal subnets work (i.e. /31, /32, /128, etc.)

@jmwample
Copy link
Member Author

The fix(es) applied in this PR relates to #132 as we add a temporary fix that creates an ioutil.tempdir for each obfs4 session ensuring separation of state. However this is probably not a good long-term fix as file descriptor limitations might cause issues, also we probably don't need any of the files to begin with.

@jmwample
Copy link
Member Author

closing in favor of separated solutions: #135 and #136.

@jmwample jmwample closed this Jun 29, 2022
@jmwample jmwample deleted the obfs4-fix branch June 29, 2022 19:21
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.

Phantom Selector return nil Unconditional return in obfs4 Transport
1 participant