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

[Proposal] Alternative (self-contained) embedded Tor backend #6

Open
karalabe opened this issue Jul 2, 2018 · 8 comments
Open

[Proposal] Alternative (self-contained) embedded Tor backend #6

karalabe opened this issue Jul 2, 2018 · 8 comments

Comments

@karalabe
Copy link
Contributor

karalabe commented Jul 2, 2018

Hey there!

I'm playing around with a hobby project in my spare time. For now I'm just building and evaluating the tech stack I'd like to build on top, and Tor came up as one of the components I'd like to pursue more seriously. At least in the initial experimentation phase.

I've looked around for Tor libraries in Go, and I think yours comes closest to a easily usable one, though I'll admit I haven't used it much yet. My primary concern was that by default you required the existence of a Tor process on the host machine. This imho limits the usage of your library to a very narrow group of people.

Before you object, I did see that you have an embeddable version of Tor that people can clone, build manually and then link against from this project. While it's a bit better, I don't think it expands your user base much, since the manual build step breaks all Go workflows. I.e. I cannot build my project on top of yours, because I cannot afford an extra build step (my primary target is mobile).

Left out in the cold, I started looking into how I could create a truly self-contained Go library out of Tor. It took an annoyingly long time because of various limitations of Go and CGO, but after a few weeks I ended up with a seemingly working solution (https://github.com/ipsn/go-libtor). Currently it builds on Linux amd64, 386, arm64 and arm (libc + musl); and Android amd64, 386, arm64 and arm. I don't think it would be too hard to add support for osx + windows, but those aren't really my priorities currently.

A nice design of the project is that it auto-updates every night via Travis cron jobs to the latest stable OpenSSL + Tor upstream projects, so hopefully it won't take constant effort to maintain. I'm sure there are a few rough edges in store but my question is whether you'd be interested in me submitting a PR to add direct support in your lib for my external lib?

API wise I made it fully compatible with your repo, and also written up process.Creator/Process wrapper based demo to highlight that it's fully cross compatible https://github.com/ipsn/go-libtor#usage. I can submit a PR to upstream the wrappers into your library if you'd be interested. Alternatively I can create the wrappers in my project too to have the same effect, but I thought yours might be a bit more appropriate coupling point. Tell me what you think.

Cheers
Peter

@cretz
Copy link
Owner

cretz commented Jul 2, 2018

Hi, thanks for your comment and showing me go-libtor. That's a very neat way to use CGO to avoid non-gcc/ld things like make.

I.e. I cannot build my project on top of yours, because I cannot afford an extra build step (my primary target is mobile).

The only difference between https://github.com/ipsn/go-libtor and https://github.com/cretz/tor-static is that the latter does ask you to build outside of CGO. I am unsure why CGO calling gcc is OK for you but using the ways recommended by the third party dependencies to build their libraries is not. The time to compile should be similar, again I just use the recommended build processes from the different projects (helps me support Windows). I am unsure why you cannot afford an extra build step when that's what CGO is doing. Building a static lib with one shell command or two seems inconsequential to me, but I don't understand your limitations.

Unfortunately with large third party dependencies, maintaining your own build scripts for their suite because of this one extra step can be a bit troublesome for maintenance. We in Go are a bit spoiled and a bit hobbled at the same time. We are spoiled because there are so many pure Go or Go+C projects that a simple go get does all building and we see that as normal. We are hobbled because Go's standard build chain doesn't allow custom build scripts for more complex builds whereas something like Cargo would make this no problem since I can have a build.rs script. But since I can't, I wrote https://github.com/cretz/tor-static/blob/master/build.go which makes it one step, but not 0. I'm afraid, due to the changing nature of these projects (wait until a version coming very soon of Tor that will require Rust in your build chain), stitching my own build system together to satisfy CGO is not worth saving the one extra step.

I'm sure there are a few rough edges in store but my question is whether you'd be interested in me submitting a PR to add direct support in your lib for my external lib?

I think the API separation that you have (w/ the process.Creator iface) is ideal. I am curious what you mean by "direct support" beyond this interface, but I can say that by default, I do not want to subject people to that large CGO build and that build suite instead of the libraries' own blessed ways. Again, Tor builds are going to change soon to require Rust and you'll have that extra step you didn't want anyways. I also don't think this repository is the best place for the entire set of Go files that will be constantly update as the third party libraries add and remove source files. However, if there is some other integration you mean that devs can easily opt in to, that kind of "direct support" I'd be happy to see.

Also, if https://github.com/ipsn/go-libtor is more than a PoC project, I'd love to mention it on the README. Then people can use your process.Creator impl in their code and it will build with a go get -u. Always happy to have more projects making it easier to use Tor for regular devs!

@karalabe
Copy link
Contributor Author

karalabe commented Jul 2, 2018

I am unsure why you cannot afford an extra build step when that's what CGO is doing. Building a static lib with one shell command or two seems inconsequential to me, but I don't understand your limitations.

I am creating Android libraries directly from Go code with gomobile, which generates a lot of ephemeral CGO files itself and build everything to 4 different architectures at the same time (amd64, arm64, 386 and arm), with different Android SDK versions and NDK features. I'm not saying it's impossible to pre-build Tor for the exact specific architectures, but I'm guessing you'd need to be very careful to match exactly the same flags, compilers, etc what gomobile is invoking under the hood. With a purely self-contained library I can just invoke gomobile bind and it automagically assembles everything correctly without me having to do the guesswork.

I am curious what you mean by "direct support" beyond this interface, but I can say that by default, I do not want to subject people to that large CGO build and that build suite instead of the libraries' own blessed ways.

I was just wondering whether you'd accept the Creator/Process interface implementations as a github.com/cretz/bine/libtor package, or whether I should keep these interfaces in my repo? I definitely don't want to upstream the constantly changing files, as that would make no sense. Just figured it's cleaner to have the go-libtor repo act az a purely static tor code and have the interfacing to bine elsewhere. But I can include the interfaces in my repo too, no problem.

Again, Tor builds are going to change soon to require Rust and you'll have that extra step you didn't want anyways.

That is truly something I'm afraid of :) No idea how I'd approach solving that.

Also, if https://github.com/ipsn/go-libtor is more than a PoC project, I'd love to mention it on the README.

I guess that will depend on whether or not I or someone else starts using it more seriously. I have an idea of where I'd like to use it, but my entire project is a spare time thing, so no guarantees. IPFS is also looking into using Tor in their own transports, though it's an experimental thing for them too. Either way, if people only use my lib for fast prototyping, it's goal is already fulfilled :)

As for mentioning in the README, I'll probably tweet out the repo, maybe write up a blog post about the "experience" for posterity. Feel free to link to if if you think it's valuable, but I'd first settle on the final "API" wrt the integration, before sending people in.

@cretz
Copy link
Owner

cretz commented Jul 2, 2018

With a purely self-contained library I can just invoke gomobile bind and it automagically assembles everything correctly without me having to do the guesswork.

Just so you know, all you have to do with the static libs in https://github.com/cretz/tor-static is compile them once, then they are referenced from there. There should be no guess work to call go run build.go but if there is special Android setup that we need to bake in to the build script for https://github.com/cretz/tor-static, we should do it because it will benefit every Android+Tor dev, not just Go ones. This is what https://github.com/iCepa/Tor.framework does w/ shell scripts to build the iOS third party libs statically. Another option probably for Android is that since these are static builds, they could be done in some CI and the .a files put somewhere for anyone to download. But no worries if none of this is anything you want to mess with.

That is truly something I'm afraid of :) No idea how I'd approach solving that.

You'll end up solving it by doing it outside of the incredibly limited CGO :-)

I was just wondering whether you'd accept the Creator/Process interface implementations as a github.com/cretz/bine/libtor package, or whether I should keep these interfaces in my repo?

I'd rather you keep them in a separate repo. Usually the interface is a better abstraction between these two things than this project having a tiny file referencing your project. Also, your project is not quite ready to support platforms like Windows. Finally, letting the dev choose when to include your project by explicitly importing a separate repo can save us from someone improperly running something like go get -u github.com/cretz/bine/... and it blowing up with that long CGO build.

As for mentioning in the README, I'll probably [...] before sending people in.

Word. I'll leave this issue open and just let me know when you're set and I'll happily link it and explain its benefit.

@karalabe
Copy link
Contributor Author

karalabe commented Jul 2, 2018

👍 Thanks for the feedback and the great work on the APIs :)

@karalabe
Copy link
Contributor Author

karalabe commented Jul 3, 2018

I've tried to do a simple integration my side, but there are some dependency issues caused by your package layout.

In order for me to support your interfaces, I need to implement these:

// Process is the interface implemented by Tor processes.
type Process interface {
	// Start starts the Tor process in the background and returns. It is
	// analagous to os/exec.Cmd.Start.
	Start() error
	// Wait waits for the Tor process to exit and returns error if it was not a
	// successful exit. It is analagous to os/exec.Cmd.Wait.
	Wait() error
}

// Creator is the interface for process creation.
type Creator interface {
	New(ctx context.Context, args ...string) (Process, error)
}

The first issue is that your Creator interface returns a Process, so my package can't be fully standalone, rather needs to reference your process package. This is the smaller issue.

The larger issue however is that you have a small helper method inside the process package, ControlPortFromFileContents, which references torutil, which explodes the dependency graph:

$ govendor list
 e  github.com/cretz/bine/process                                  
 e  github.com/cretz/bine/torutil                                  
 e  github.com/cretz/bine/torutil/ed25519                          
 e  github.com/cretz/bine/torutil/ed25519/internal/edwards25519    
 e  golang.org/x/crypto/ed25519                                    
 e  golang.org/x/crypto/ed25519/internal/edwards25519              
 e  golang.org/x/crypto/sha3

At this point I have a few suggestion on how to solve these issues:

  • The best would be to only have a single interface which doesn't return custom types (e.g. process.Process), so I could create an implementation that doesn't depend on your project at all.
  • The second best would be to move your interfaces into a separate package so anyone can implement backends without needing to pull in unrelated dependencies. I just need the interfaces, I don't really want to depend on implementation code in my repo.
  • The worse but workable solution would be to get rid of ControlPortFromFileContents and move it into whichever package actually needs it.

@cretz
Copy link
Owner

cretz commented Jul 3, 2018

I'm sorry, but I am afraid I disagree here that these are really even issues. First, those libs I reference will be referenced anyways by anyone using the bine project. I could just have tossed everything in one package and it would have been no problem either. The linker should keep the unused things out of the runtime binary.

That's not an "explosion" of your dependency graph (just wait until you use the actual library). If that kind of package list is a real issue, you're going to have a real hard time using IPFS, or even the Go stdlib for that matter. The way it is currently set up is quite reasonable and quite normal for a Go project. Also, again, using other parts of the this lib anyways brings in those dependencies too which are hardly "dependencies" in the traditional sense. In fact, that I don't use an popular error or log handling library makes this project quite lightweight in the traditional sense.

I'm afraid at this point if you believe that referencing my other packages in this way is harmful, I would suggest taking the MIT-licensed code and restructuring it. I believe in its current state it is quite lightweight, quite dependency free, and that my very few packages reference each other and the crypto libs is harmless.

@karalabe
Copy link
Contributor Author

karalabe commented Jul 3, 2018

That is fine. It was a suggestion, I can accept you decision.

@cretz
Copy link
Owner

cretz commented Jul 6, 2018

@karalabe - Heads up, code reorg coming in future version. See issue #7.

papr8ka pushed a commit to papr8ka/bine that referenced this issue Sep 18, 2022
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

No branches or pull requests

2 participants