Skip to content
This repository has been archived by the owner on Sep 29, 2024. It is now read-only.

Add xormask option #121

Closed
wants to merge 0 commits into from
Closed

Add xormask option #121

wants to merge 0 commits into from

Conversation

XMB5
Copy link
Contributor

@XMB5 XMB5 commented Sep 3, 2019

This PR adds the scramble xormask option from the openvpn xor patch. This option XORs all incoming and outgoing bytes (except for packet lengths when using TCP) with any byte. This does not add security; instead it is for bypassing firewalls. This implementation only supports one-byte xormasks, for example scramble xormask z. In this example, all incoming and outgoing bytes would be XORed with 0x7A, the ascii value of z.

The XORing happens in ControlPacket.m. The rest of the changes are for parsing the option from an ovpn config file.

Solves issue 38.

@XMB5
Copy link
Contributor Author

XMB5 commented Sep 3, 2019

I didn't test UDP because UDP wasn't working without any modifications

@@ -72,7 +75,7 @@ class NETCPLink: LinkInterface {
var newBuffer = buffer
newBuffer.append(contentsOf: data)
var until = 0
let packets = PacketStream.packets(fromStream: newBuffer, until: &until)
let packets = PacketStream.packets(fromStream: newBuffer, until: &until, xorMask: self!.xorMask)
Copy link
Member

@keeshux keeshux Sep 4, 2019

Choose a reason for hiding this comment

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

Unwrapped self here is very likely to raise unpredictable, obscure exceptions on disconnection. Better add a guard.

packetsToUse = packets!.map({ (packet) -> Data in
return Data(bytes: packet.map{$0 ^ self!.xorMask}, count: packet.count)
})
}
Copy link
Member

Choose a reason for hiding this comment

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

This code in dangerous. Replace _ in 61 and use it rather than self! for safety. Also, packets in the else block may be nil if xorMask != 0.

@keeshux
Copy link
Member

keeshux commented Sep 5, 2019

@XMB5 meanwhile could you send me an email to beta@passepartoutvpn.app from your main contact? Thank you!

@XMB5
Copy link
Contributor Author

XMB5 commented Sep 5, 2019

Sent it

@keeshux keeshux self-assigned this Sep 30, 2019
@keeshux keeshux added the enhancement New feature or request label Sep 30, 2019
@keeshux keeshux added this to the 2.2.0 milestone Oct 30, 2019
@keeshux keeshux removed this from the 2.2.0 milestone Nov 19, 2019
@keeshux
Copy link
Member

keeshux commented Nov 19, 2019

A follow-up: I think of postponing this because I'd like to evaluate pluggable transports in the (far) future. It's just that with a new API this code would need to be adjusted. I'll keep you updated along with my considerations.

@theNerdFromSiliconValley

Is there any update on this?

@keeshux
Copy link
Member

keeshux commented Feb 26, 2020

Is there any update on this?

Not until I have time to test it thoroughly. It's a deep change for a stable app in production, I can't mess up.

@cyberjoost
Copy link

cyberjoost commented Apr 6, 2020

Isn't tls-crypt, which is already supported, more effective or the same in effectiveness in bypassing firewalls? I have customers happily using OpenVPN with tls-crypt enabled to bypass the GFW, without xorpatch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants