Project

General

Profile

Feature #11402

onioncircuits: add TOR_CONTROL_* environment variables

Added by dawuud over 3 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
Start date:
05/08/2016
Due date:
% Done:

100%

Feature Branch:
onioncircuits:feature/11402-control-env-vars
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Onion Circuits

Description

We'd like onioncircuits to play nice with the Subgraph tor control port filtering daemon:
https://github.com/subgraph/roflcoptor

To do this we need onioncircuits to connect to a specified unix domain socket in which
our filtering daemon is listening. Here's the simple four line patch:

https://github.com/david415/onioncircuits/tree/add_control_port_env.0
https://github.com/david415/onioncircuits/commit/f9bc497857b469f3641b38480dda630c9535406d


Related issues

Blocked by Tails - Feature #11443: Upgrade Onion Circuits to 0.4 Resolved 05/18/2016

History

#1 Updated by intrigeri over 3 years ago

  • Status changed from New to In Progress
  • Assignee set to anonym
  • Target version set to Tails_2.4
  • QA Check set to Ready for QA
  • Type of work changed from Debian to Code
  • Affected tool set to Onion Circuits

We'd like onioncircuits to play nice with the Subgraph tor control port filtering daemon: https://github.com/subgraph/roflcoptor

Cool!

To do this we need onioncircuits to connect to a specified unix domain socket in which our filtering daemon is listening. Here's the simple four line patch:

Thank you. anonym, can you please code-review this?

(Once all pending Onion Circuit patches are out, I can make an upstream release, and then if time allows it can flow into Tails 2.4 via sid→testing→jessie-backports, even if I don't manage to do #11361. I'll upload a backport of 0.3-1 so it clears the backports NEW queue, and then next time it'll be accepted automatically.)

#2 Updated by anonym over 3 years ago

  • Assignee changed from anonym to dawuud
  • % Done changed from 0 to 30
  • QA Check changed from Ready for QA to Info Needed
  • Feature Branch set to onioncircuits:feature/11402-control-env-vars

Thanks for your contribution!

I have a few issues with this patch:

  1. TOR_CONTROL_PORT is a poor name since it in fact is used for the path to the socket control file, given that we pass it to stem.connection.connect_socket_file(). TOR_CONTROL_SOCKET is commonly used in the environment for that purpose and is IMHO a better name.
  1. As a general rule I think it is good to avoid overriding library defaults, and how you handle the path keyword argument violates that. I.e. when the env var is not set, we should do what onioncircuits do without your patch (calling connect_socket_file() without path set).

While looking at this I also noticed that connect_socket_file() is deprecated in favor of connect(), which has the added benefit that we also easily can support using a control port. So instead of forcing such code ~maintenance work on you I have pushed a branch called feature/11402-control-env-vars -- please let us know (on this ticket) if it suites your needs (and please reassign the ticket to me then)!

#3 Updated by dawuud over 3 years ago

Yes that's perfect. Thanks!

#4 Updated by intrigeri over 3 years ago

  • Assignee changed from dawuud to anonym

#5 Updated by anonym over 3 years ago

  • Subject changed from onioncircuits: add TOR_CONTROL_PORT environment variable to onioncircuits: add TOR_CONTROL_* environment variables
  • Assignee changed from anonym to alant
  • % Done changed from 30 to 50
  • QA Check changed from Info Needed to Ready for QA

Let's make this happen, then! alant, what do you think?

#6 Updated by alant over 3 years ago

  • Status changed from In Progress to Fix committed
  • QA Check changed from Ready for QA to Pass

Merged, thanks

#7 Updated by anonym over 3 years ago

  • Assignee deleted (alant)
  • % Done changed from 50 to 100

#8 Updated by intrigeri over 3 years ago

#9 Updated by anonym over 3 years ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF