-
Notifications
You must be signed in to change notification settings - Fork 409
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 an interface for Pluggable Transports #148
Conversation
These imports are not strictly necissary, so removing them has no adverse side effects. Including them may cause duplicate symbols when certain compile options are used, and when compiling with SWIG. Signed-off-by: Nick Beirne <[email protected]>
Add a Factory, Transport, and Connection interface to support Pluggable Transports. Factory: Generates a Transport object. OpenVPNClient has a virtual method that should be over-ridden by the implementer. Transport: Is able to "dial" an endpoint and can generate a Connection. Connection: Can be used to read and write data to the transport. May also be closed. Signed-off-by: Nick Beirne <[email protected]>
This commit exists to make reviewing this PR easier. This commit may be squashed with several of the following commits before merging, The PluggableTransports client and link are based on the TCP client and link. I am copying the files because they are largely the same code, and the next commit will contain the changes. I am not subclassing because The common TCP link object knows too much about asio sockets, and the PluggableTransports link does not rely on a system socket being available. This can possibly be solved by a refactor, but I think it's out of scope for this Pull Request. Signed-off-by: Nick Beirne <[email protected]>
Signed-off-by: Nick Beirne <[email protected]>
This is mostly renaming functions, changing the namespace, and renaming log functions. I am also editing logs to reflect PT and not TCP. During this rename I also needed to make ptlink stand alone, and not reliant on TCP link base. This are only two virtual functions which need to be implemented. I also needed to add pluggable transports errors to help accomidate the new transport. Signed-off-by: Nick Beirne <[email protected]>
Pluggable transports are never in raw mode Signed-off-by: Nick Beirne <[email protected]>
Signed-off-by: Nick Beirne <[email protected]>
Since we can't guarantee async operations exists for a given pluggable transport we now rely on asio's post operation. As a side-effect, errors are also no longer from asio, and instead are expected to be an ExceptionCode (but it falls back if an exception is thrown and its not that type). In async_send and async_recv we explicitly don't capture self. This is because the post happens in a different thread, and there would be a race condition between reference counted pointers to self. Instead we rely on the caller to maintain a strong reference to self for the duration of the operation. Signed-off-by: Nick Beirne <[email protected]>
Add a usePluggableTransports boolean to the config. This allows the implementer to choose between a pluggable transport and a TCP/UDP connection at run time rather than compile time. Signed-off-by: Nick Beirne <[email protected]>
Signed-off-by: Nick Beirne <[email protected]>
Signed-off-by: Nick Beirne <[email protected]>
Signed-off-by: Nick Beirne <[email protected]>
typedef RCPtr<Transport> Ptr; | ||
|
||
public: | ||
virtual PluggableTransports::Connection::Ptr dial(openvpn_io::ip::tcp::endpoint address) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory a listen
function should also be included here, to make a server. I can implement this functionality as well, but I didn't want to make this pull request larger.
virtual size_t send(const openvpn_io::const_buffer& buffer) = 0; | ||
virtual size_t receive(const openvpn_io::mutable_buffer& buffer) = 0; | ||
virtual void close() = 0; | ||
virtual int native_handle() = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
native_handle
is required to implement socket_protect
.
Hi @nbeirne I need help because I'm stucked. I have this code to initialize Obfs4Transport and setUsePluggableTransports. config.setUsePluggableTransports(true);
|
I can't help much with your specific obfs4 setup since they can vary so much, and it depends on the implementation. I used this implementation of obfs4, which does not require a local socks5 connection. I wrapped it in a c++ class and was able to use this version of openvpn3 to configure it as a transport. To integrate the transport follow these steps:
Does all this make sense? |
Any news on this? |
Hi, at the moment there is a larger ongoing discussion about whether it makes sense to integrate PT into OpenVPN directly or not. This said, @pokamest if you are aware of what is being used on the server side to communicate through interface, please let us know. As of now OpenVPN3 is a client only library, so we don't even know against what this code should be tested. |
Hi! |
That is correct. As long as the specific PT implementation on the client and server are protocol-compatible it should be fine. At TunnelBear I used OperatorFoundation's implementation of obfs4 in Go with this code and a light c++ interface to make I no longer work at TunnelBear but I'd like to advocate for this merge since it's incredibly useful to a fairly small number of people. |
Actually there are apps (notably "OpenVPN for Android") that run several processes in background. I always imagined that the PT daemon could be one of those. That would be an easy way to ship a PT daemon on Android along with OpenVPN. The PT component would be well suited for running outside OpenVPN, without the need to touch its codebase. What's wrong with that? As far as I understand this is already happening on the server side (even though nobody has seen such deployment yet), so the client can just do the same. |
One big blocker to merging this is the availibility of a setup to use this in. And with that I mean some how/tutorial how to setup both client and server side and client side. It is nice that Tunnelbear has some setup that works for them but if we as open source project should support this interface, we need something that everyone can run/use. |
I'm getting PT_SIZE_ERROR. I updated GO plugin to the new version of obfs4 (v2, v3), and the result was the same. I would appreciate if someone could give me some feedback. |
Problem was in Shapeshifter-obfs4-OpenVPN-Transport-Plugin-Cgo, after rewriting Obfs4_read It works perfectly |
Add transport class for https://github.com/outspace/Cloak plugin and OpenVPN#148
It's a client version of the Cloak plugin for [OpenVPN3 PT](OpenVPN/openvpn3#148) transport.
I want to close this due to almost two years of inactivity. I left TunnelBear shortly after opening this PR, but I will forward it to my former colleagues to see if they want to pick it up. @outspace is correct on the setup steps. The shapeshifter implementation had a bug on read, which was fixed in this commit. @schwabe If there is still interest in merging this, what needs to be done? Probably a rebase on the current development version and also documentation, but what else? |
AmneziaVPN still using this PR in the fork https://github.com/amnezia-vpn/openvpn3 |
I have not really looked at the code again if it is now coming with a proper setup to make this code actually useful without the properitary parts but I don't really see a movement here. While I think the idea is good it is very hard for us to accept code that we cannot test and maintain. |
That's fair. I think if I were to redo this today I'd name it differently and make it more generally useful - it's really just a way to replace the socket transport read/write code with a customizable implementation (in this case, a connection which encrypts data with Obfs4 before sending it off. In Amnesia's case a it looks like they were using Cloak). The default transport could even be the plain unix socket to support normal openvpn3 connections. Anyway, I'm going to close it now. Who knows, I might revisit this sometime in the future. |
This Pull Requests adds support for a "PluggableTransports" transport layer. It defines a public interface which makes it easy for an implementer to create a new pluggable transport. According to the pluggable transport spec we should provide a socket-like interface for ease of implementing the transport itself. I tried to mirror the asio functions as closely as possible, minus the async operations which are handled in the transport link.
Implementing and Using a New Pluggable Transport
To create a new transport the implementer must do a few things.
Transport
andClient
objects which implement your preferred obfuscation technique. Any configuration should be contained within theTransport
object.-DOPENVPN_PLUGGABLE_TRANSPORTS
and extendOpenVPNClient
to implement thenew_transport_factory
method.OpenVPNClient
with theconfig.usePluggableTransports
option.Why Not Use an External Transport
In theory it is possible to implement this same functionality with minimal changes to OpenVPN 3 itself, and to use an external transport. This approach would have a few notable downsides:
ptcli.hpp
andptlink.hpp
as an external transport. That's a lot of code just to get obfuscation support.More Details
The client and link objects are based on the TCP client and link. In this version of the code they handle interfacing with the rest of the OpenVPN code (socket_protect, packet alignment, conforming to the send/receive interface) and also queuing data for the synchronous 'send' calls. The actual 'send' call is provided by the implementer, and may be as simple as a opening a regular TCP socket. While this somewhat complicates the implementation internal to OpenVPN, it simplifies the code for the implementer.
In theory the client and link can subclass their TCP counterparts so that more code is shared. I consider the refactor this would involve to be out-of-scope for this pull request, at least at the moment.
Some Meta Business
I am submitting this pull request for review through Github for initial review. I will send a patch to the mailing list after I have a few eyes on it.
I structured this PR to be read commit-by-commit. It might be easier to digest if you look at the diff for each commit. If and when these changes are merged it would be possible to squash them into a single commit - but that's really up to the final reviewers.