-
Notifications
You must be signed in to change notification settings - Fork 32
Change tor config before tor is launched #85
base: develop
Are you sure you want to change the base?
Change tor config before tor is launched #85
Conversation
FWIW, I've successfully run this branch with the following patch:
|
Are we sure that the options are being set when connecting to an existing Tor daemon? Setting Maybe the options are not being applied when calling |
If options aren't being set on I don't know if there's any cases where it ignores something in a SETCONF but continues anyway. |
I just tried on a running Tor with |
@meejah independently on whether the options are really set or an error is raised when connecting to tor via SOCKS port, talking today with @DonnchaC we thought that we should only support launching tor (and not connecting to), as that allows setting specific options and won't interfere with the running tor process. |
Okay, launching your own Tor sounds safest. In that case, you can set any options you want in a BTW .. the intent of the leading underscore there is that maybe if you need to do that, txtorcon should add features to |
I forgot to comment here that i realized we still need to support to connect to tor for the chutney tests. Unless we find a way launch tor in a chutney fashion, what it seems to me like rewriting chutney functionality from bwscanner, though @teor2345 will know better. |
In any case, tor configuration is changed in this PR in the way @meejah suggests before launching tor: ccddac6#diff-5d97531ed54d2f8c4d91ebce0fb91774R170 |
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.
Looks good, but see inline comments.
bwscanner/attacher.py
Outdated
tor = yield txtorcon.launch(reactor) | ||
# Launch tor with config, in order to don't get CONF_CHANGED when | ||
# updating options that can't be changed while tor is running. | ||
tor = yield txtorcon.launch(reactor, _tor_config=tor_config) | ||
else: |
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.
When launching tor like this (and passing in the config) you shouldn't have to assign the object nor do .save()
(as below).
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.
Just to clarify, .save()
is not being call when launching tor, in this part of the if
, it's call when connecting to tor
in the else
below, as you point out.
bwscanner/attacher.py
Outdated
# TODO: check whether CONF_CHANGED will happen here or not because | ||
# we get the state later | ||
tor.config = tor_config | ||
tor.config.save() |
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.
You shouldn't really assign to this in the Tor
object. Nor should you have to save()
(all config will have been passed in to tor on the command-line). If you do call .get_config()
anywhere else, it will return the same object (but, you should use the async method to retrieve the config in case it's not available yet).
BTW, do you know which config-options you're passing in that aren't provided by launch()
, or is it "maybe anything at all that Tor supports"..?
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.
The options we're passing are (https://github.com/juga0/bwscanner/blob/ccddac6f20ce7963e8fc2e8d9921f672da6be390/bwscanner/attacher.py#L146):
# Options for spawned or running Tor to load the correct descriptors.
tor_options = {
'LearnCircuitBuildTimeout': 0, # Disable adaptive circuit timeouts.
'CircuitBuildTimeout': circuit_build_timeout,
'UseEntryGuards': 0, # Disable UseEntryGuards to avoid PathBias warnings.
'UseMicroDescriptors': 0,
'FetchUselessDescriptors': 1,
'FetchDirInfoEarly': 1,
'FetchDirInfoExtraEarly': 1,
}
So what would be the correct way to force change the configuration (if any) when tor
is already running (by command line) and we use txtorcon.connect
?
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.
From http://timaq4ygg2iegci7.onion/guide.html#connecting-to-a-running-tor:
Similarly, the current configuration is available via .config. You can change the configuration
by updating attributes on this class but it won’t take effect until you call txtorcon.TorConfig.save().
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.
Hmm, that does read confusingly now that I re-read it. What I mean there is that you can "update things" by e.g. tor.config.UseEntryGuards = 1
and tor.config.save()
not by setting tor.config
directly. I'll try to make that docstring more-clear... :/
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.
This commit 353ebac#diff-5d97531ed54d2f8c4d91ebce0fb91774R180 should fix it.
I also moved TorConfig
setup inside of if launch_tor
353ebac#diff-5d97531ed54d2f8c4d91ebce0fb91774R163
ccddac6
to
416555a
Compare
to don't get error about options that can't be changed once it's launched.
353ebac
to
b9ad1a5
Compare
to don't get error about options that can't be changed once it's launched.
An alternative to #80, as commented in #83