-
-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
tor module: expose control socket #28938
Conversation
@offlinehacker, thanks for your PR! By analyzing the history of the files in this pull request, we identified @thoughtpolice, @Phreedom and @tnias to be potential reviewers. |
7e87c28
to
82cac2e
Compare
I would prefer it were hidden behind an `enable` option.
Also, the service now has two `preStart`s (probably from git-rebase).
|
fd0ce1e
to
b0bfae7
Compare
What do you sugest the option name to be as |
I mean one way would be |
Jaka Hudoklin <notifications@github.com> writes:
I mean one way would be `controlSocket.enable` and `controlSocket.path` options, as user would else need to know that `/var/run/tor/control` is where socket should be.
To be honest, I would just remove the `path` option. Too much work to
make it configurable with sandboxing. Just `controlSocket.enable` is
fine.
|
b0bfae7
to
91028fb
Compare
Ok, fixed, is this fine now? |
Yes, the new lines are fine, but the line
${optint "ControlPort" (toString cfg.controlPort)}
was recently reverted back to
${optint "ControlPort" cfg.controlPort}
in master (it was a bug).
Can you please rebase on top of `master` again? (Another reason to hate
GitHub with its overly strict "security model")
|
91028fb
to
ad842f3
Compare
I pushed an updated version into #37827. |
This exposes control socket of tor accessible to tor user and users in tor group
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)