September 2016 - tor-commits - lists.torproject.org

[ooni-probe/master] Check permissions on logfile as well
by art＠torproject.org 19 Sep '16

19 Sep '16

commit bfc0a51a0b63ceb1d07812f2a7edc6a63847ea90 Author: Arturo Filastò <arturo(a)filasto.net> Date: Tue Sep 6 17:09:18 2016 +0200 Check permissions on logfile as well * Handle the case where the measurement directory doesn't exist yet --- ooni/measurements.py | 2 ++ ooni/utils/log.py | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ooni/measurements.py b/ooni/measurements.py index fd722ee..294b182 100644 --- a/ooni/measurements.py +++ b/ooni/measurements.py @@ -128,6 +128,8 @@ def get_summary(measurement_id): def list_measurements(compute_size=False): measurements = [] measurement_path = FilePath(config.measurements_directory) + if not measurement_path.exists(): + return measurements for measurement_id in measurement_path.listdir(): try: measurements.append(get_measurement(measurement_id, compute_size)) diff --git a/ooni/utils/log.py b/ooni/utils/log.py index 67079e0..93e6b31 100644 --- a/ooni/utils/log.py +++ b/ooni/utils/log.py @@ -133,9 +133,10 @@ class OONILogger(object): logfile = os.path.expanduser(config.basic.logfile) log_folder = os.path.dirname(logfile) - if not os.access(log_folder, os.W_OK): - # If we don't have permissions to write to the log_folder, - # write to running dir. + if (not os.access(log_folder, os.W_OK) or + (os.path.exists(logfile) and not os.access(logfile, os.W_OK))): + # If we don't have permissions to write to the log_folder or + # logfile. log_folder = config.running_path logfile = os.path.join(log_folder, "ooniprobe.log")

1 0

[ooni-probe/master] Bugfix in updater
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 96c7b6103023f4968141eb707266bf2bfa167368 Author: Arturo Filastò <arturo(a)filasto.net> Date: Wed Sep 7 13:36:43 2016 +0200 Bugfix in updater --- data/updater.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/data/updater.py b/data/updater.py index 10086f5..65983aa 100755 --- a/data/updater.py +++ b/data/updater.py @@ -30,8 +30,7 @@ Description=lepidopter-update service [Service] Type=simple -Environment="UPDATER={0}" -ExecStart=$UPDATER --log-file /var/log/ooni/lepidopter-update.log update --watch +ExecStart={0} --log-file /var/log/ooni/lepidopter-update.log update --watch TimeoutStartSec=300 Restart=on-failure

1 0

[ooni-probe/master] Improve sanity checks on tor_data_dir
by art＠torproject.org 19 Sep '16

19 Sep '16

commit a731364e988798ebeb12e52eff1b1c11e18cdb2c Author: Arturo Filastò <arturo(a)filasto.net> Date: Wed Sep 7 16:49:19 2016 +0200 Improve sanity checks on tor_data_dir --- ooni/utils/onion.py | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/ooni/utils/onion.py b/ooni/utils/onion.py index 6e0d906..10f80a3 100644 --- a/ooni/utils/onion.py +++ b/ooni/utils/onion.py @@ -1,6 +1,8 @@ import os import re import pwd +import fcntl +import errno import string import StringIO import subprocess @@ -214,6 +216,25 @@ def get_client_transport(transport): raise UninstalledTransport +def is_tor_data_dir_usable(tor_data_dir): + """ + Checks if the Tor data dir specified is usable. This means that + it is not being locked and we have permissions to write to it. + """ + if not os.path.exists(tor_data_dir): + return True + + try: + fcntl.flock(open(os.path.join(tor_data_dir, 'lock'), 'w'), + fcntl.LOCK_EX | fcntl.LOCK_NB) + return True + except (IOError, OSError) as err: + if err.errno == errno.EACCES: + # Permission error + return False + elif err.errno == errno.EAGAIN: + # File locked + return False def get_tor_config(): tor_config = TorConfig() @@ -227,11 +248,22 @@ def get_tor_config(): if config.tor.data_dir: data_dir = os.path.expanduser(config.tor.data_dir) - - if not os.path.exists(data_dir): - log.debug("%s does not exist. Creating it." % data_dir) + # We only use the Tor data dir specified in the config file if + # 1. It is not locked (i.e. another process is using it) + # 2. We have write permissions to it + data_dir_usable = is_tor_data_dir_usable(data_dir) + try: os.makedirs(data_dir) - tor_config.DataDirectory = data_dir + log.debug("%s does not exist. Creating it." % data_dir) + except OSError as ose: + if ose.errno == errno.EEXIST: + pass + elif ose.errno == errno.EACCESS: + data_dir_usable = False + else: + raise + if data_dir_usable: + tor_config.DataDirectory = data_dir if config.tor.bridges: tor_config.UseBridges = 1

1 0

[ooni-probe/master] Convert last runtime into UTC
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 5d0f080b6d968172171c16cdd9a9ebd8cb887972 Author: Arturo Filastò <arturo(a)filasto.net> Date: Mon Sep 12 17:00:27 2016 +0200 Convert last runtime into UTC --- ooni/agent/scheduler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ooni/agent/scheduler.py b/ooni/agent/scheduler.py index a7713e0..0a6cc0f 100644 --- a/ooni/agent/scheduler.py +++ b/ooni/agent/scheduler.py @@ -16,6 +16,8 @@ from ooni.utils.files import human_size_to_bytes, directory_usage from ooni.deck.store import input_store, deck_store, DEFAULT_DECKS from ooni.settings import config from ooni.contrib import croniter +from ooni.contrib.dateutil.tz import tz + from ooni.geoip import probe_ip from ooni.measurements import list_measurements @@ -91,7 +93,8 @@ class ScheduledTask(object): return CANARY_DATE with self._last_run.open('r') as in_file: date_str = in_file.read() - return datetime.strptime(date_str, self._time_format) + return datetime.strptime(date_str, self._time_format).replace( + tzinfo=tz.tzutc()) def _update_last_run(self, last_run_time): with self._last_run.open('w') as out_file:

1 0

[ooni-probe/master] The updater needs to be updated
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 5f3e8e311013d76db1dc62b21f66f0f3ce371c44 Author: Arturo Filastò <arturo(a)filasto.net> Date: Tue Sep 13 13:00:11 2016 +0200 The updater needs to be updated --- data/updater.py | 6 +++--- ooni/ui/web/client/index.html | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/data/updater.py b/data/updater.py index f7225dd..4cacf3d 100755 --- a/data/updater.py +++ b/data/updater.py @@ -133,7 +133,7 @@ class InvalidPublicKey(Exception): pass -def verify_file(signature_path, signer_pk_path): +def verify_file(signature_path, file_path, signer_pk_path): tmp_dir = tempfile.mkdtemp() tmp_key = os.path.join(tmp_dir, "signing-key.gpg") @@ -148,7 +148,7 @@ def verify_file(signature_path, signer_pk_path): output = check_output(["gpg", "--batch", "--status-fd", "1", "--no-default-keyring", "--keyring", tmp_key, "--trust-model", "always", - "--verify", signature_path]) + "--verify", signature_path, file_path]) except CalledProcessError: raise InvalidSignature @@ -188,7 +188,7 @@ def perform_update(version, skip_verification=False): if skip_verification is not True: try: - verify_file(updater_sig_path, PUBLIC_KEY_PATH) + verify_file(updater_sig_path, updater_path, PUBLIC_KEY_PATH) except InvalidSignature: logging.error("Found an invalid signature. Bailing") raise UpdateFailed diff --git a/ooni/ui/web/client/index.html b/ooni/ui/web/client/index.html index ecb4cdb..357b6eb 100644 --- a/ooni/ui/web/client/index.html +++ b/ooni/ui/web/client/index.html @@ -13,5 +13,5 @@ <app> Loading... </app> - <script type="text/javascript" src="app.bundle.js?5432cc07ccbd2f5613be"></script></body> + <script type="text/javascript" src="app.bundle.js?43676d3982a3c11023d3"></script></body> </html>

1 0

[ooni-probe/master] Add more unittests for the scheduler
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 4865ade8dc39d924e6a14e00c9b5a9e741f50abc Author: Arturo Filastò <arturo(a)filasto.net> Date: Mon Sep 12 14:24:41 2016 +0200 Add more unittests for the scheduler * Make the logic for triggering scheduled deck tasks more robust --- ooni/agent/scheduler.py | 49 ++++++++++++++++++++++++++++++++++---------- ooni/tests/test_scheduler.py | 28 ++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/ooni/agent/scheduler.py b/ooni/agent/scheduler.py index 98b395b..4002369 100644 --- a/ooni/agent/scheduler.py +++ b/ooni/agent/scheduler.py @@ -1,6 +1,7 @@ import os import errno +from hashlib import md5 from datetime import datetime from twisted.application import service @@ -41,6 +42,10 @@ class FileSystemlockAndMutex(object): self._fs_lock.unlock() self._mutex.release() + @property + def locked(self): + return self._mutex.locked or self._fs_lock.locked + # We use this date to indicate that the scheduled task has never run. # Easter egg, try to see what is special about this date :)? CANARY_DATE = datetime(1957, 8, 4) @@ -68,6 +73,9 @@ class ScheduledTask(object): FilePath(scheduler_directory).child(self.identifier + ".lock").path ) + def cancel(self): + self._last_run_lock.release() + @property def should_run(self): current_time = datetime.utcnow() @@ -233,7 +241,8 @@ class RunDeck(ScheduledTask): def __init__(self, director, deck_id, schedule): self.deck_id = deck_id self.director = director - identifier = 'run-deck-' + deck_id + # We use as identifier also the schedule time + identifier = 'run-deck-' + deck_id + '-' + md5(schedule).hexdigest() super(RunDeck, self).__init__(schedule, identifier) @defer.inlineCallbacks @@ -316,21 +325,39 @@ class SchedulerService(service.MultiService): def schedule(self, task): self._scheduled_tasks.append(task) - def refresh_deck_list(self): - # Deletes all the RunDeck tasks and reschedules only the ones that - # are enabled. - for scheduled_task in self._scheduled_tasks[:]: - if isinstance(scheduled_task, RunDeck): - self._scheduled_tasks.remove(scheduled_task) + def unschedule(self, task): + # We first cancel the task so the run lock is deleted + task.cancel() + self._scheduled_tasks.remove(task) - if not config.is_initialized(): - # Disable scheduling measurements if we are not initialized. - return + def refresh_deck_list(self): + to_enable = [] for deck_id, deck in deck_store.list_enabled(): if deck.schedule is None: continue - self.schedule(RunDeck(self.director, deck_id, deck.schedule)) + to_enable.append((deck_id, deck.schedule)) + + # If we are not initialized we should not enable anything + if not config.is_initialized(): + to_enable = [] + + for scheduled_task in self._scheduled_tasks[:]: + if not isinstance(scheduled_task, RunDeck): + continue + + info = (scheduled_task.deck_id, scheduled_task.schedule) + if info in to_enable: + # If the task is already scheduled there is no need to + # enable it. + to_enable.remove(info) + else: + # If one of the tasks that is scheduled is no longer in the + # scheduled tasks. We should disable it. + self.unschedule(scheduled_task) + + for deck_id, schedule in to_enable: + self.schedule(RunDeck(self.director, deck_id, schedule)) def _task_did_not_run(self, failure, task): failure.trap(DidNotRun) diff --git a/ooni/tests/test_scheduler.py b/ooni/tests/test_scheduler.py index cb90a30..1350dde 100644 --- a/ooni/tests/test_scheduler.py +++ b/ooni/tests/test_scheduler.py @@ -1,11 +1,12 @@ import os import shutil +import random import tempfile from twisted.internet import defer from twisted.trial import unittest -from ooni.agent.scheduler import ScheduledTask, DidNotRun +from ooni.agent.scheduler import ScheduledTask, DidNotRun, FileSystemlockAndMutex class TestScheduler(unittest.TestCase): def test_scheduled_task(self): @@ -49,3 +50,28 @@ class TestScheduler(unittest.TestCase): self.assertEqual(dummy_st.should_run, False) shutil.rmtree(scheduler_directory) + + + @defer.inlineCallbacks + def test_filesystem_lock_and_mutex(self): + lock_dir = tempfile.mkdtemp() + lock_path = os.path.join(lock_dir, 'lock') + + lock = FileSystemlockAndMutex(lock_path) + + lock_count = 100 + unlock_count = 0 + dl = [] + for i in range(lock_count): + dl.append(lock.acquire()) + if random.choice([0, 1]) == 0: + unlock_count += 1 + lock.release() + + for i in range(lock_count - unlock_count): + lock.release() + + yield defer.DeferredList(dl) + self.assertFalse(lock.locked) + + shutil.rmtree(lock_dir)

1 0

[ooni-probe/master] Don't override the report_id
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 406c60567a16f6a795602f59c7ec5a1f3e5bcce2 Author: Arturo Filastò <arturo(a)filasto.net> Date: Tue Sep 13 17:16:10 2016 +0200 Don't override the report_id --- ooni/reporter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooni/reporter.py b/ooni/reporter.py index cfdce08..b149d49 100644 --- a/ooni/reporter.py +++ b/ooni/reporter.py @@ -608,7 +608,7 @@ class Report(object): if self.collector_client: self.oonib_reporter = OONIBReporter(self.test_details, self.collector_client) - self.test_details['report_id'] = yield self.open_oonib_reporter() + yield self.open_oonib_reporter() if not self.no_njson: self.njson_reporter = NJSONReporter(self.test_details,

1 0

[ooni-probe/master] Remove outdated comment
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 0e452b1c70607da8d389988bb72e85b6b229fe4b Author: Arturo Filastò <arturo(a)filasto.net> Date: Wed Sep 14 17:11:04 2016 +0200 Remove outdated comment --- ooni/resources.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ooni/resources.py b/ooni/resources.py index f97616c..ac0f967 100644 --- a/ooni/resources.py +++ b/ooni/resources.py @@ -5,11 +5,6 @@ from twisted.python.filepath import FilePath from twisted.internet import defer from twisted.web.client import downloadPage, getPage, HTTPClientFactory -# WARNING: this script is being run as part of the post install procedure. -# Be sure to not import either in this module or in the imported modules -# dependencies other than twisted. If you end up including something that is -# not twisted, then you will need to add it to the setup_requires in setup.py. - from ooni.utils import log, gunzip, rename from ooni.settings import config

1 0

[ooni-probe/master] Update updater script
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 1d5d892181bf06f21b7c8df8566eb0906212382b Author: Arturo Filastò <arturo(a)filasto.net> Date: Tue Sep 13 12:32:55 2016 +0200 Update updater script --- data/updater.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/data/updater.py b/data/updater.py index 65983aa..f7225dd 100755 --- a/data/updater.py +++ b/data/updater.py @@ -198,9 +198,12 @@ def perform_update(version, skip_verification=False): try: logging.info("Running install script") + if updater.__version__ != str(version): + logging.error("There is a version mismatch in the updater file. This could be a sign of a replay attack.") + raise UpdateFailed updater.run() except Exception: - logging.error("Failed to run the version update script for version {0}".format(version)) + logging.exception("Failed to run the version update script for version {0}".format(version)) raise UpdateFailed current_version_dir = os.path.dirname(CURRENT_VERSION_PATH) @@ -214,6 +217,8 @@ def perform_update(version, skip_verification=False): with open(CURRENT_VERSION_PATH, "w+") as out_file: out_file.write(str(version)) + logging.info("Updated to version {0}".format(version)) + def update_to_version(from_version, to_version, skip_verification=False): versions = range(from_version + 1, to_version + 1) for version in versions:

1 0

[ooni-probe/master] Address feedback from @bassosimone
by art＠torproject.org 19 Sep '16

19 Sep '16

commit 47b4587d22993343a26aef6eef3667fe959e005e Author: Arturo Filastò <arturo(a)filasto.net> Date: Wed Sep 14 17:03:21 2016 +0200 Address feedback from @bassosimone --- Vagrantfile | 5 ----- data/ooniprobe.conf.sample | 2 +- ooni/agent/scheduler.py | 24 +++++++++++++++++++----- ooni/deck/backend.py | 4 ++-- ooni/deck/deck.py | 2 +- ooni/director.py | 6 +++++- ooni/settings.py | 2 +- 7 files changed, 29 insertions(+), 16 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 8ff0bbd..71f7c7a 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -81,9 +81,4 @@ Vagrant.configure("2") do |config| end end - config.vm.define "testing" do |testing| - testing.vm.network "forwarded_port", guest: 8842, host: 8142 - testing.vm.synced_folder ".", "/data/ooni-probe" - end - end diff --git a/data/ooniprobe.conf.sample b/data/ooniprobe.conf.sample index f4c0812..ee9a2c9 100644 --- a/data/ooniprobe.conf.sample +++ b/data/ooniprobe.conf.sample @@ -3,7 +3,7 @@ # Keep in mind that indentation matters. basic: - # Where OONIProbe should be writing it's log file + # Where OONIProbe should be writing its log file logfile: /var/log/ooniprobe.log loglevel: WARNING privacy: diff --git a/ooni/agent/scheduler.py b/ooni/agent/scheduler.py index 2797fba..c86e3b6 100644 --- a/ooni/agent/scheduler.py +++ b/ooni/agent/scheduler.py @@ -24,9 +24,12 @@ from ooni.measurements import list_measurements class FileSystemlockAndMutex(object): """ This is a lock that is both a mutex lock and also on filesystem. - When you acquire it, it will first block on the mutex lock and then - once that is released it will attempt to acquire the lock on the - filesystem. + When you acquire it, it will first acquire the mutex lock and then + acquire the filesystem lock. The release order is inverted. + + This is to avoid concurrent usage within the same process. When using it + concurrently the mutex lock will block before the filesystem lock is + acquired. It's a way to support concurrent usage of the DeferredFilesystemLock without races. @@ -291,7 +294,7 @@ class SendHeartBeat(ScheduledTask): # XXX implement this pass -# Order mattters +# Order matters SYSTEM_TASKS = [ UpdateInputsAndResources ] @@ -303,7 +306,10 @@ def run_system_tasks(no_input_store=False): if no_input_store: log.debug("Not updating the inputs") - task_classes.remove(UpdateInputsAndResources) + try: + task_classes.remove(UpdateInputsAndResources) + except ValueError: + pass for task_class in task_classes: task = task_class() @@ -337,7 +343,15 @@ class SchedulerService(service.MultiService): self._scheduled_tasks.remove(task) def refresh_deck_list(self): + """ + This checks if there are some decks that have been enabled and + should be scheduled as periodic tasks to run on the next scheduler + cycle and if some have been disabled and should not be run. + It does so by listing the enabled decks and checking if the enabled + ones are already scheduled or if some of the scheduled ones are not + amongst the enabled decks. + """ to_enable = [] for deck_id, deck in deck_store.list_enabled(): if deck.schedule is None: diff --git a/ooni/deck/backend.py b/ooni/deck/backend.py index b2df9bc..a39492c 100644 --- a/ooni/deck/backend.py +++ b/ooni/deck/backend.py @@ -16,6 +16,8 @@ def sort_addresses_by_priority(priority_address, 'address': priority_address, 'type': backend_type } + # We prefer an onion collector to an https collector to a cloudfront + # collector to a plaintext collector address_priority = ['onion', 'https', 'cloudfront', 'http'] address_priority.remove(preferred_backend) address_priority.insert(0, preferred_backend) @@ -73,8 +75,6 @@ def get_reachable_test_helper(test_helper_name, test_helper_address, @defer.inlineCallbacks def get_reachable_collector(collector_address, collector_alternate, preferred_backend): - # We prefer onion collector to https collector to cloudfront - # collectors to plaintext collectors for collector_settings in sort_addresses_by_priority( collector_address, collector_alternate, diff --git a/ooni/deck/deck.py b/ooni/deck/deck.py index 3fc10ab..59946ab 100644 --- a/ooni/deck/deck.py +++ b/ooni/deck/deck.py @@ -25,7 +25,7 @@ def resolve_file_path(v, prepath=None): if v.startswith("$"): # This raises InputNotFound and we let it carry onto the caller return input_store.get(v[1:])["filepath"] - elif prepath is not None and (not os.path.isabs(v)): + if prepath is not None and (not os.path.isabs(v)): return FilePath(prepath).preauthChild(v).path return v diff --git a/ooni/director.py b/ooni/director.py index 1807e65..1e55bb6 100644 --- a/ooni/director.py +++ b/ooni/director.py @@ -112,7 +112,11 @@ class Director(object): def notify(self, event): for handler in self._subscribers: - handler(event) + try: + handler(event) + except Exception as exc: + log.err("Failed to run handler") + log.exception(exc) def _reset_director_state(self): self._director_state = 'not-running' diff --git a/ooni/settings.py b/ooni/settings.py index 14f135f..b4bb594 100644 --- a/ooni/settings.py +++ b/ooni/settings.py @@ -18,7 +18,7 @@ CONFIG_FILE_TEMPLATE = """\ # Keep in mind that indentation matters. basic: - # Where OONIProbe should be writing it's log file + # Where OONIProbe should be writing its log file # logfile: {logfile} # loglevel: WARNING # The maximum amount of data to store on disk. Once the quota is reached,

1 0