From 6855577f055861f25e06f221ca8311bfd4661b7f Mon Sep 17 00:00:00 2001 From: juga0 Date: Thu, 6 Dec 2018 18:15:16 +0000 Subject: [PATCH 1/6] stem: refactor, move to a function parsing torrc launch_tor is also parsing three sources of torrc options. Move the `extra_lines` torrc option set by a user configuration file to a function so that it can be tested. Add docstring and remove comments. Add sucess test. --- sbws/util/stem.py | 77 ++++++++++++++++++------------------ tests/unit/util/test_stem.py | 13 ++++++ 2 files changed, 52 insertions(+), 38 deletions(-) create mode 100644 tests/unit/util/test_stem.py diff --git a/sbws/util/stem.py b/sbws/util/stem.py index c3fffd1f..bd8f7d02 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -117,44 +117,17 @@ def _init_controller_socket(socket): return c -def launch_tor(conf): - assert isinstance(conf, ConfigParser) - os.makedirs(conf.getpath('tor', 'datadir'), mode=0o700, exist_ok=True) - os.makedirs(conf.getpath('tor', 'log'), exist_ok=True) - os.makedirs(conf.getpath('tor', 'run_dpath'), mode=0o700, exist_ok=True) - # Bare minimum things, more or less - torrc = copy.deepcopy(TORRC_STARTING_POINT) - # Very important and/or common settings that we don't know until runtime - torrc.update({ - 'DataDirectory': conf.getpath('tor', 'datadir'), - 'PidFile': conf.getpath('tor', 'pid'), - 'ControlSocket': conf.getpath('tor', 'control_socket'), - 'Log': [ - 'NOTICE file {}'.format(os.path.join(conf.getpath('tor', 'log'), - 'notice.log')), - ], - # Things needed to make circuits fail a little faster. We get the - # circuit_timeout as a string instead of an int on purpose: stem only - # accepts strings. - 'LearnCircuitBuildTimeout': '0', - 'CircuitBuildTimeout': conf['general']['circuit_timeout'], - }) - # This block of code reads additional torrc lines from the user's - # config.ini so they can add arbitrary additional options. - # - # The user can't replace our options, only add to them. For example, - # there's no way to remove 'SocksPort auto' (if it is still in - # TORRC_STARTING_POINT). If you add a SocksPort in your config.ini, you'll - # open two socks ports. - # - # As an example, maybe the user hates their HDD and wants to fill it with - # debug logs, and wants to tell Tor to use only 1 CPU core. - # - # [tor] - # extra_lines = - # Log debug file /tmp/tor-debug.log - # NumCPUs 1 - for line in conf['tor']['extra_lines'].split('\n'): +def parse_user_torrc_config(torrc, torrc_text): + """Parse the user configuration torrc text call `extra_lines` + to a dictionary suitable to use with stem and update the existing torrc. + Example: + [tor] + extra_lines = + Log debug file /tmp/tor-debug.log + NumCPUs 1 + """ + + for line in torrc_text.split('\n'): # Remove leading and trailing whitespace, if any line = line.strip() # Ignore blank lines @@ -186,6 +159,34 @@ def launch_tor(conf): assert isinstance(existing_val, list) existing_val.append(value) torrc.update({key: existing_val}) + return torrc + + +def launch_tor(conf): + assert isinstance(conf, ConfigParser) + os.makedirs(conf.getpath('tor', 'datadir'), mode=0o700, exist_ok=True) + os.makedirs(conf.getpath('tor', 'log'), exist_ok=True) + os.makedirs(conf.getpath('tor', 'run_dpath'), mode=0o700, exist_ok=True) + # Bare minimum things, more or less + torrc = copy.deepcopy(TORRC_STARTING_POINT) + # Very important and/or common settings that we don't know until runtime + torrc.update({ + 'DataDirectory': conf.getpath('tor', 'datadir'), + 'PidFile': conf.getpath('tor', 'pid'), + 'ControlSocket': conf.getpath('tor', 'control_socket'), + 'Log': [ + 'NOTICE file {}'.format(os.path.join(conf.getpath('tor', 'log'), + 'notice.log')), + ], + # Things needed to make circuits fail a little faster. We get the + # circuit_timeout as a string instead of an int on purpose: stem only + # accepts strings. + 'LearnCircuitBuildTimeout': '0', + 'CircuitBuildTimeout': conf['general']['circuit_timeout'], + }) + + torrc = parse_user_torrc_config(torrc, conf['tor']['extra_lines']) + # Finally launch Tor try: stem.process.launch_tor_with_config( diff --git a/tests/unit/util/test_stem.py b/tests/unit/util/test_stem.py new file mode 100644 index 00000000..888f2db2 --- /dev/null +++ b/tests/unit/util/test_stem.py @@ -0,0 +1,13 @@ +"""Unit tests for stem.py""" + +from sbws.util.stem import parse_user_torrc_config + + +def test_parse_user_torrc_config_new_keyvalue_options_success(): + config_torrc_extra_lines = """ + Log debug file /tmp/tor-debug.log + NumCPUs 1 + """ + torrc_dict = parse_user_torrc_config({}, config_torrc_extra_lines) + assert torrc_dict == \ + {'Log': 'debug file /tmp/tor-debug.log', 'NumCPUs': '1'} From 1aa93cf84f2d08ba2b0da440aae91a9f0b409a72 Mon Sep 17 00:00:00 2001 From: juga0 Date: Thu, 6 Dec 2018 18:28:41 +0000 Subject: [PATCH 2/6] stem: stop merging existing torrc options. User won't be able set options that have been already set. Fixes bug #28738. Bugfix v0.1.1 --- sbws/util/stem.py | 15 ++------------- tests/unit/util/test_stem.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/sbws/util/stem.py b/sbws/util/stem.py index bd8f7d02..c3229736 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -145,20 +145,9 @@ def parse_user_torrc_config(torrc, torrc_text): # It's really easy to add to the torrc if the key doesn't exist if key not in torrc: torrc.update({key: value}) - # But if it does, we have to make a list of values. For example, say - # the user wants to add a SocksPort and we already have - # 'SocksPort auto' in the torrc. We'll go from - # torrc['SocksPort'] == 'auto' - # to - # torrc['SocksPort'] == ['auto', '9050'] else: - existing_val = torrc[key] - if isinstance(existing_val, str): - torrc.update({key: [existing_val, value]}) - else: - assert isinstance(existing_val, list) - existing_val.append(value) - torrc.update({key: existing_val}) + log.warn("The torrc option %s in `extra_lines` can not be set " + "because it was already set.", line) return torrc diff --git a/tests/unit/util/test_stem.py b/tests/unit/util/test_stem.py index 888f2db2..705ff554 100644 --- a/tests/unit/util/test_stem.py +++ b/tests/unit/util/test_stem.py @@ -11,3 +11,18 @@ def test_parse_user_torrc_config_new_keyvalue_options_success(): torrc_dict = parse_user_torrc_config({}, config_torrc_extra_lines) assert torrc_dict == \ {'Log': 'debug file /tmp/tor-debug.log', 'NumCPUs': '1'} + + +def test_parse_user_torrc_config_existing_keyvalue_options_fail(caplog): + torrc_dict = {'SocksPort': 'auto'} + config_torrc_extra_lines = """ + SocksPort 9050 + """ + torrc_dict_new = parse_user_torrc_config( + torrc_dict, config_torrc_extra_lines) + + # the new dictionary contains the existing option + assert torrc_dict_new == torrc_dict + + # but it does not contain the new option + assert config_torrc_extra_lines.strip() in caplog.records[-1].getMessage() From 1dd98cc2a85b1d1b4d24dc823f712b4c45988d16 Mon Sep 17 00:00:00 2001 From: juga0 Date: Thu, 6 Dec 2018 19:59:08 +0000 Subject: [PATCH 3/6] stem: remove info log without useful info --- sbws/util/stem.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sbws/util/stem.py b/sbws/util/stem.py index c3229736..23926c1a 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -140,8 +140,6 @@ def parse_user_torrc_config(torrc, torrc_text): fail_hard('All torrc lines must have 2 or more words. "%s" has ' 'fewer', line) key, value = kv - log.info('Adding "%s %s" to torrc with which we are launching Tor', - key, value) # It's really easy to add to the torrc if the key doesn't exist if key not in torrc: torrc.update({key: value}) From 56626a6183f25c377a1fd0974038920759043f0c Mon Sep 17 00:00:00 2001 From: juga0 Date: Fri, 21 Dec 2018 06:40:44 +0000 Subject: [PATCH 4/6] fixup! stem: remove info log without useful info --- sbws/util/stem.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sbws/util/stem.py b/sbws/util/stem.py index 23926c1a..b7d6acc4 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -140,6 +140,8 @@ def parse_user_torrc_config(torrc, torrc_text): fail_hard('All torrc lines must have 2 or more words. "%s" has ' 'fewer', line) key, value = kv + log.debug('Adding "%s %s" to torrc with which we are launching Tor', + key, value) # It's really easy to add to the torrc if the key doesn't exist if key not in torrc: torrc.update({key: value}) From 570269c55e332ac6b23833aff3d7bd8428ef4372 Mon Sep 17 00:00:00 2001 From: juga0 Date: Fri, 21 Dec 2018 07:09:18 +0000 Subject: [PATCH 5/6] fixup! stem: stop merging existing torrc options. --- sbws/util/stem.py | 25 ++++++++++++++++++------- tests/unit/util/test_stem.py | 10 ++++------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/sbws/util/stem.py b/sbws/util/stem.py index b7d6acc4..aa4fa69d 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -119,14 +119,15 @@ def _init_controller_socket(socket): def parse_user_torrc_config(torrc, torrc_text): """Parse the user configuration torrc text call `extra_lines` - to a dictionary suitable to use with stem and update the existing torrc. + to a dictionary suitable to use with stem and return a new torrc + dictionary that merges that dictionary with the existing torrc. Example: [tor] extra_lines = Log debug file /tmp/tor-debug.log NumCPUs 1 """ - + torrc_dict = torrc.copy() for line in torrc_text.split('\n'): # Remove leading and trailing whitespace, if any line = line.strip() @@ -144,11 +145,22 @@ def parse_user_torrc_config(torrc, torrc_text): key, value) # It's really easy to add to the torrc if the key doesn't exist if key not in torrc: - torrc.update({key: value}) + torrc_dict.update({key: value}) + # But if it does, we have to make a list of values. For example, say + # the user wants to add a SocksPort and we already have + # 'SocksPort auto' in the torrc. We'll go from + # torrc['SocksPort'] == 'auto' + # to + # torrc['SocksPort'] == ['auto', '9050'] else: - log.warn("The torrc option %s in `extra_lines` can not be set " - "because it was already set.", line) - return torrc + existing_val = torrc[key] + if isinstance(existing_val, str): + torrc_dict.update({key: [existing_val, value]}) + else: + assert isinstance(existing_val, list) + existing_val.append(value) + torrc_dict.update({key: existing_val}) + return torrc_dict def launch_tor(conf): @@ -175,7 +187,6 @@ def launch_tor(conf): }) torrc = parse_user_torrc_config(torrc, conf['tor']['extra_lines']) - # Finally launch Tor try: stem.process.launch_tor_with_config( diff --git a/tests/unit/util/test_stem.py b/tests/unit/util/test_stem.py index 705ff554..8edf789d 100644 --- a/tests/unit/util/test_stem.py +++ b/tests/unit/util/test_stem.py @@ -20,9 +20,7 @@ def test_parse_user_torrc_config_existing_keyvalue_options_fail(caplog): """ torrc_dict_new = parse_user_torrc_config( torrc_dict, config_torrc_extra_lines) - - # the new dictionary contains the existing option - assert torrc_dict_new == torrc_dict - - # but it does not contain the new option - assert config_torrc_extra_lines.strip() in caplog.records[-1].getMessage() + # the new dictionary contains the existing key option and a list with both + # the existing value and the new value + assert torrc_dict_new != torrc_dict + assert torrc_dict_new == {'SocksPort': ['auto', '9050']} From 1a71b47a5491c69d11753789a06b5b5c23ccd489 Mon Sep 17 00:00:00 2001 From: juga0 Date: Fri, 7 Dec 2018 09:37:20 +0000 Subject: [PATCH 6/6] stem: parse torrc options that are only a key User torrc options in `extra_lines` that are only one key and not a key value pair were not being parsed correctly. Add a test. Fixes bug #28715. Bugfix v0.1.1 --- sbws/util/stem.py | 16 ++++++++-------- tests/unit/util/test_stem.py | 8 ++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/sbws/util/stem.py b/sbws/util/stem.py index aa4fa69d..d8e5cf80 100644 --- a/sbws/util/stem.py +++ b/sbws/util/stem.py @@ -134,15 +134,13 @@ def parse_user_torrc_config(torrc, torrc_text): # Ignore blank lines if len(line) < 1: continue - # The way stem handles configuring Tor with a dictionary is the first - # word is a key and the remaining words are the value. + # Some torrc options are only a key, some are a key value pair. kv = line.split(None, 1) - if len(kv) < 2: - fail_hard('All torrc lines must have 2 or more words. "%s" has ' - 'fewer', line) - key, value = kv - log.debug('Adding "%s %s" to torrc with which we are launching Tor', - key, value) + if len(kv) > 1: + key, value = kv + else: + key = kv[0] + value = None # It's really easy to add to the torrc if the key doesn't exist if key not in torrc: torrc_dict.update({key: value}) @@ -160,6 +158,8 @@ def parse_user_torrc_config(torrc, torrc_text): assert isinstance(existing_val, list) existing_val.append(value) torrc_dict.update({key: existing_val}) + log.debug('Adding "%s %s" to torrc with which we are launching Tor', + key, value) return torrc_dict diff --git a/tests/unit/util/test_stem.py b/tests/unit/util/test_stem.py index 8edf789d..c2aafe98 100644 --- a/tests/unit/util/test_stem.py +++ b/tests/unit/util/test_stem.py @@ -24,3 +24,11 @@ def test_parse_user_torrc_config_existing_keyvalue_options_fail(caplog): # the existing value and the new value assert torrc_dict_new != torrc_dict assert torrc_dict_new == {'SocksPort': ['auto', '9050']} + + +def test_parse_user_torrc_config_new_key_option_success(): + config_torrc_extra_lines = """ + LongLivedPorts + """ + torrc_dict = parse_user_torrc_config({}, config_torrc_extra_lines) + assert torrc_dict == {'LongLivedPorts': None}