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

Add scramble xormask #170

Merged
merged 3 commits into from Jul 23, 2021
Merged

Add scramble xormask #170

merged 3 commits into from Jul 23, 2021

Conversation

keeshux
Copy link
Member

@keeshux keeshux commented May 8, 2020

Rebase of #121

Fixes #38

cc @XMB5

@keeshux keeshux added the enhancement New feature or request label May 8, 2020
@keeshux keeshux self-assigned this May 8, 2020
@keeshux keeshux force-pushed the add-scramble-xormask branch 4 times, most recently from b941a18 to 5a67fb4 Compare May 9, 2020 14:32
@keeshux keeshux added this to the 2.3.0 milestone Jun 9, 2020
@HosseyNJF
Copy link

Why this isn't being merged?

@x0r2d2
Copy link

x0r2d2 commented Oct 10, 2020

Why this isn't being merged?

+1
Hopefully, will be merged soon.

@keeshux keeshux removed this from the 3.1.0 milestone Dec 28, 2020
@makoni makoni mentioned this pull request Mar 8, 2021
@keeshux
Copy link
Member Author

keeshux commented Jul 21, 2021

@XMB5 I notice your additions in the copyright header. While I have no issue with crediting one's work (goes without saying), it's impossible to track authorship on a line by line basis. In fact the header is more of a Xcode placeholder.

Would you be okay with being credited in a different manner? E.g. in README, LICENSE or wherever else you're okay with. As is, I'm not sure about merging this as I don't want this to cause trouble in the future.

Feel free to contact me privately on me@davidederosa.com for more insight.

@keeshux keeshux force-pushed the add-scramble-xormask branch 2 times, most recently from d5c4a34 to 0eeec1f Compare July 21, 2021 16:15
@keeshux
Copy link
Member Author

keeshux commented Jul 21, 2021

Other than that, the "official" Tunnelblick patch takes a string as input. Do you confirm this PR only supports a single byte XOR mask?

@XMB5
Copy link
Contributor

XMB5 commented Jul 21, 2021

@keeshux I'm fine with being credited in any way, for example in the README, LICENSE, at the top of the file, or even only in git log (i.e. not written down in any file, only in git metadata).

And yes, this patch only supports a single byte mask--see ConfigurationParser.swift line 470:

optXorMask = Character($0[1]).asciiValue

Although the official patch supports multiple bytes, in practice, people often use only 1 byte because it is sufficient to evade firewalls (for example, ExpressVPN does this, although they use a custom option name instead of scramble xormask).

@keeshux
Copy link
Member Author

keeshux commented Jul 22, 2021

Nice! Please check out last 2 commits.

@keeshux keeshux added this to the 3.4.0 milestone Jul 22, 2021
@x0r2d2
Copy link

x0r2d2 commented Jul 22, 2021

@keeshux Am I right that you plan to merge it soon?

XMB5 and others added 2 commits July 22, 2021 10:55
Move XOR PR credits to README.
@XMB5
Copy link
Contributor

XMB5 commented Jul 23, 2021

@keeshux The new commits look good. My one comment is that maybe we should document this feature somewhere, perhaps in the README? Here's some text we could use:

--scramble xormask <character> -- XOR all incoming and outgoing bytes by the ascii value of this character

@keeshux keeshux merged commit e315734 into master Jul 23, 2021
@keeshux keeshux deleted the add-scramble-xormask branch July 23, 2021 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XOR Patch Feature
4 participants