From 6e64f08f08076ce6b84977d662a9415ec8daf21c Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 21 Dec 2017 09:22:53 +1100 Subject: [PATCH 1/6] #493, #500 - detect if the Tor connection (bundled, TorBrowser or otherwise) has been lost while the app is open. Stop a running share if so --- onionshare/onion.py | 6 +++++ onionshare_gui/onionshare_gui.py | 17 ++++++++++++++ onionshare_gui/settings_dialog.py | 39 +++++++++++++++++-------------- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/onionshare/onion.py b/onionshare/onion.py index 2f79719b..adfb4dbf 100644 --- a/onionshare/onion.py +++ b/onionshare/onion.py @@ -375,6 +375,12 @@ class Onion(object): # ephemeral stealth onion services are not supported self.supports_stealth = False + def is_authenticated(self): + """ + Returns True if the Tor connection is still working, or False otherwise. + """ + return self.c.is_authenticated() + def start_onion_service(self, port): """ Start a onion service on port 80, pointing to the given port, and diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index 3ed30db7..f6ebf6be 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -219,6 +219,11 @@ class OnionShareGui(QtWidgets.QMainWindow): def reload_settings(): common.log('OnionShareGui', 'open_settings', 'settings have changed, reloading') self.settings.load() + # We might've stopped the main requests timer if a Tor connection failed. + # If we've reloaded settings, we probably succeeded in obtaining a new + # connection. If so, restart the timer. + if not self.timer.isActive(): + self.timer.start() d = SettingsDialog(self.onion, self.qtapp, self.config) d.settings_saved.connect(reload_settings) @@ -389,6 +394,18 @@ class OnionShareGui(QtWidgets.QMainWindow): """ self.update() + # Have we lost connection to Tor somehow? + try: + # Tor Browser may not even have been open when we started OnionShare, + # in which case onion.is_authenticated() throws a NoneType error + self.onion + if not self.onion.is_authenticated(): + self.timer.stop() + self.start_server_error(strings._('error_tor_protocol_error')) + self._tor_connection_canceled() + except: + pass + # scroll to the bottom of the dl progress bar log pane # if a new download has been added if self.new_download: diff --git a/onionshare_gui/settings_dialog.py b/onionshare_gui/settings_dialog.py index df806a06..7ead3f1b 100644 --- a/onionshare_gui/settings_dialog.py +++ b/onionshare_gui/settings_dialog.py @@ -459,26 +459,31 @@ class SettingsDialog(QtWidgets.QDialog): # If Tor isn't connected, or if Tor settings have changed, Reinitialize # the Onion object reboot_onion = False - if self.onion.connected_to_tor: - def changed(s1, s2, keys): - """ - Compare the Settings objects s1 and s2 and return true if any values - have changed for the given keys. - """ - for key in keys: - if s1.get(key) != s2.get(key): - return True - return False + try: + self.onion + if self.onion.is_authenticated(): + def changed(s1, s2, keys): + """ + Compare the Settings objects s1 and s2 and return true if any values + have changed for the given keys. + """ + for key in keys: + if s1.get(key) != s2.get(key): + return True + return False - if changed(settings, self.old_settings, [ - 'connection_type', 'control_port_address', - 'control_port_port', 'socks_address', 'socks_port', - 'socket_file_path', 'auth_type', 'auth_password']): + if changed(settings, self.old_settings, [ + 'connection_type', 'control_port_address', + 'control_port_port', 'socks_address', 'socks_port', + 'socket_file_path', 'auth_type', 'auth_password']): + reboot_onion = True + + else: + # Tor isn't connected, so try connecting reboot_onion = True - - else: - # Tor isn't connected, so try connecting + except: + # We definitely aren't connected, as the onion object had no attribute is_authenticated() reboot_onion = True # Do we need to reinitialize Tor? From 2b36938d5321afaca4daed4cbbcb6d8e095cd953 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 21 Dec 2017 12:15:17 +1100 Subject: [PATCH 2/6] Ensure we don't consider the Tor connection 'lost' if we're still in the connection dialog process (wait for that thread to finish). Also various implementations of onion.is_authenticated() for a more robust check than onion.connected_to_tor, which seemed to help some corner cases --- onionshare/onion.py | 7 +++- onionshare_gui/onionshare_gui.py | 24 +++++-------- onionshare_gui/settings_dialog.py | 45 +++++++++++-------------- onionshare_gui/tor_connection_dialog.py | 14 ++++---- 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/onionshare/onion.py b/onionshare/onion.py index adfb4dbf..364bcb89 100644 --- a/onionshare/onion.py +++ b/onionshare/onion.py @@ -375,11 +375,16 @@ class Onion(object): # ephemeral stealth onion services are not supported self.supports_stealth = False + def is_authenticated(self): """ Returns True if the Tor connection is still working, or False otherwise. """ - return self.c.is_authenticated() + if self.c: + return self.c.is_authenticated() + else: + return False + def start_onion_service(self, port): """ diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index f6ebf6be..b7783738 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -142,10 +142,10 @@ class OnionShareGui(QtWidgets.QMainWindow): self.set_server_active(False) # Start the "Connecting to Tor" dialog, which calls onion.connect() - tor_con = TorConnectionDialog(self.qtapp, self.settings, self.onion) - tor_con.canceled.connect(self._tor_connection_canceled) - tor_con.open_settings.connect(self._tor_connection_open_settings) - tor_con.start() + self.tor_con = TorConnectionDialog(self.qtapp, self.settings, self.onion) + self.tor_con.canceled.connect(self._tor_connection_canceled) + self.tor_con.open_settings.connect(self._tor_connection_open_settings) + self.tor_con.start() # After connecting to Tor, check for updates self.check_for_updates() @@ -394,17 +394,11 @@ class OnionShareGui(QtWidgets.QMainWindow): """ self.update() - # Have we lost connection to Tor somehow? - try: - # Tor Browser may not even have been open when we started OnionShare, - # in which case onion.is_authenticated() throws a NoneType error - self.onion - if not self.onion.is_authenticated(): - self.timer.stop() - self.start_server_error(strings._('error_tor_protocol_error')) - self._tor_connection_canceled() - except: - pass + # Has the Tor Connection dialog finished, but we've since lost connection to Tor somehow? + if not self.onion.is_authenticated() and self.tor_con.t.isFinished(): + self.timer.stop() + self.start_server_error(strings._('error_tor_protocol_error')) + self._tor_connection_canceled() # scroll to the bottom of the dl progress bar log pane # if a new download has been added diff --git a/onionshare_gui/settings_dialog.py b/onionshare_gui/settings_dialog.py index 7ead3f1b..15d8f634 100644 --- a/onionshare_gui/settings_dialog.py +++ b/onionshare_gui/settings_dialog.py @@ -459,31 +459,26 @@ class SettingsDialog(QtWidgets.QDialog): # If Tor isn't connected, or if Tor settings have changed, Reinitialize # the Onion object reboot_onion = False - try: - self.onion - if self.onion.is_authenticated(): - def changed(s1, s2, keys): - """ - Compare the Settings objects s1 and s2 and return true if any values - have changed for the given keys. - """ - for key in keys: - if s1.get(key) != s2.get(key): - return True - return False + if self.onion.is_authenticated(): + def changed(s1, s2, keys): + """ + Compare the Settings objects s1 and s2 and return true if any values + have changed for the given keys. + """ + for key in keys: + if s1.get(key) != s2.get(key): + return True + return False - if changed(settings, self.old_settings, [ - 'connection_type', 'control_port_address', - 'control_port_port', 'socks_address', 'socks_port', - 'socket_file_path', 'auth_type', 'auth_password']): + if changed(settings, self.old_settings, [ + 'connection_type', 'control_port_address', + 'control_port_port', 'socks_address', 'socks_port', + 'socket_file_path', 'auth_type', 'auth_password']): - reboot_onion = True - - else: - # Tor isn't connected, so try connecting reboot_onion = True - except: - # We definitely aren't connected, as the onion object had no attribute is_authenticated() + + else: + # Tor isn't connected, so try connecting reboot_onion = True # Do we need to reinitialize Tor? @@ -497,7 +492,7 @@ class SettingsDialog(QtWidgets.QDialog): common.log('SettingsDialog', 'save_clicked', 'Onion done rebooting, connected to Tor: {}'.format(self.onion.connected_to_tor)) - if self.onion.connected_to_tor and not tor_con.wasCanceled(): + if self.onion.is_authenticated() and not tor_con.wasCanceled(): self.settings_saved.emit() self.close() @@ -510,7 +505,7 @@ class SettingsDialog(QtWidgets.QDialog): Cancel button clicked. """ common.log('SettingsDialog', 'cancel_clicked') - if not self.onion.connected_to_tor: + if not self.onion.is_authenticated(): Alert(strings._('gui_tor_connection_canceled', True), QtWidgets.QMessageBox.Warning) sys.exit() else: @@ -565,7 +560,7 @@ class SettingsDialog(QtWidgets.QDialog): common.log('SettingsDialog', 'closeEvent') # On close, if Tor isn't connected, then quit OnionShare altogether - if not self.onion.connected_to_tor: + if not self.onion.is_authenticated(): common.log('SettingsDialog', 'closeEvent', 'Closing while not connected to Tor') # Wait 1ms for the event loop to finish, then quit diff --git a/onionshare_gui/tor_connection_dialog.py b/onionshare_gui/tor_connection_dialog.py index fa4c7860..1785f391 100644 --- a/onionshare_gui/tor_connection_dialog.py +++ b/onionshare_gui/tor_connection_dialog.py @@ -57,12 +57,12 @@ class TorConnectionDialog(QtWidgets.QProgressDialog): def start(self): common.log('TorConnectionDialog', 'start') - t = TorConnectionThread(self, self.settings, self.onion) - t.tor_status_update.connect(self._tor_status_update) - t.connected_to_tor.connect(self._connected_to_tor) - t.canceled_connecting_to_tor.connect(self._canceled_connecting_to_tor) - t.error_connecting_to_tor.connect(self._error_connecting_to_tor) - t.start() + self.t = TorConnectionThread(self, self.settings, self.onion) + self.t.tor_status_update.connect(self._tor_status_update) + self.t.connected_to_tor.connect(self._connected_to_tor) + self.t.canceled_connecting_to_tor.connect(self._canceled_connecting_to_tor) + self.t.error_connecting_to_tor.connect(self._error_connecting_to_tor) + self.t.start() # The main thread needs to remain active, and checkign for Qt events, # until the thread is finished. Otherwise it won't be able to handle @@ -126,7 +126,7 @@ class TorConnectionThread(QtCore.QThread): # Connect to the Onion try: self.onion.connect(self.settings, False, self._tor_status_update) - if self.onion.connected_to_tor: + if self.onion.is_authenticated(): self.connected_to_tor.emit() else: self.canceled_connecting_to_tor.emit() From c3c50c095516149475e8d141b66c98361eccddb9 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Thu, 21 Dec 2017 14:08:13 +1100 Subject: [PATCH 3/6] Fixes for the edge case where Tor connection dialog is canceled, but the Tor process remains open in the background, which was causing onion.is_authenticated() to True. Remove excessive alerts --- onionshare/onion.py | 2 +- onionshare_gui/onionshare_gui.py | 14 +++++++++----- onionshare_gui/settings_dialog.py | 2 ++ onionshare_gui/tor_connection_dialog.py | 3 ++- share/locale/en.json | 2 +- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/onionshare/onion.py b/onionshare/onion.py index 364bcb89..6b50a00d 100644 --- a/onionshare/onion.py +++ b/onionshare/onion.py @@ -380,7 +380,7 @@ class Onion(object): """ Returns True if the Tor connection is still working, or False otherwise. """ - if self.c: + if self.c is not None: return self.c.is_authenticated() else: return False diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index b7783738..a5efa743 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -176,6 +176,7 @@ class OnionShareGui(QtWidgets.QMainWindow): quit, or open settings. """ common.log('OnionShareGui', '_tor_connection_canceled') + self.timer.stop() def ask(): a = Alert(strings._('gui_tor_connection_ask', True), QtWidgets.QMessageBox.Question, buttons=QtWidgets.QMessageBox.NoButton, autostart=False) @@ -394,11 +395,14 @@ class OnionShareGui(QtWidgets.QMainWindow): """ self.update() - # Has the Tor Connection dialog finished, but we've since lost connection to Tor somehow? - if not self.onion.is_authenticated() and self.tor_con.t.isFinished(): - self.timer.stop() - self.start_server_error(strings._('error_tor_protocol_error')) - self._tor_connection_canceled() + # Have we lost connection to Tor somehow? + if self.tor_con.t.isFinished(): + if not self.onion.is_authenticated(): + self.timer.stop() + if self.server_status.status != self.server_status.STATUS_STOPPED: + self.server_status.stop_server() + self.status_bar.clearMessage() + self._tor_connection_canceled() # scroll to the bottom of the dl progress bar log pane # if a new download has been added diff --git a/onionshare_gui/settings_dialog.py b/onionshare_gui/settings_dialog.py index 15d8f634..b4c7363a 100644 --- a/onionshare_gui/settings_dialog.py +++ b/onionshare_gui/settings_dialog.py @@ -460,6 +460,7 @@ class SettingsDialog(QtWidgets.QDialog): # the Onion object reboot_onion = False if self.onion.is_authenticated(): + common.log('SettingsDialog', 'save_clicked', 'Connected to Tor') def changed(s1, s2, keys): """ Compare the Settings objects s1 and s2 and return true if any values @@ -478,6 +479,7 @@ class SettingsDialog(QtWidgets.QDialog): reboot_onion = True else: + common.log('SettingsDialog', 'save_clicked', 'Not connected to Tor') # Tor isn't connected, so try connecting reboot_onion = True diff --git a/onionshare_gui/tor_connection_dialog.py b/onionshare_gui/tor_connection_dialog.py index 1785f391..ab797171 100644 --- a/onionshare_gui/tor_connection_dialog.py +++ b/onionshare_gui/tor_connection_dialog.py @@ -86,6 +86,7 @@ class TorConnectionDialog(QtWidgets.QProgressDialog): def _canceled_connecting_to_tor(self): common.log('TorConnectionDialog', '_canceled_connecting_to_tor') self.active = False + self.onion.cleanup() # Cancel connecting to Tor QtCore.QTimer.singleShot(1, self.cancel) @@ -126,7 +127,7 @@ class TorConnectionThread(QtCore.QThread): # Connect to the Onion try: self.onion.connect(self.settings, False, self._tor_status_update) - if self.onion.is_authenticated(): + if self.onion.connected_to_tor: self.connected_to_tor.emit() else: self.canceled_connecting_to_tor.emit() diff --git a/share/locale/en.json b/share/locale/en.json index 0756843e..344736fc 100644 --- a/share/locale/en.json +++ b/share/locale/en.json @@ -114,7 +114,7 @@ "update_error_check_error": "Error checking for updates: Maybe you're not connected to Tor, or maybe the OnionShare website is down.", "update_error_invalid_latest_version": "Error checking for updates: The OnionShare website responded saying the latest version is '{}', but that doesn't appear to be a valid version string.", "update_not_available": "You are running the latest version of OnionShare.", - "gui_tor_connection_ask": "Would you like to open OnionShare settings to troubleshoot connecting to Tor?", + "gui_tor_connection_ask": "Tor isn't running.\n\nWould you like to open OnionShare settings to troubleshoot connecting to Tor?", "gui_tor_connection_ask_open_settings": "Open Settings", "gui_tor_connection_ask_quit": "Quit", "gui_tor_connection_error_settings": "Try adjusting how OnionShare connects to the Tor network in Settings.", From 5faecdb3c6b9389111a329037c2d7428bb9f8482 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 23 Dec 2017 16:43:58 +1100 Subject: [PATCH 4/6] Don't show alert dialogs when Tor disconnects - just display in statusbar and stop any active share. --- onionshare_gui/onionshare_gui.py | 10 +++++----- share/locale/en.json | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index a5efa743..7c44b769 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -176,7 +176,6 @@ class OnionShareGui(QtWidgets.QMainWindow): quit, or open settings. """ common.log('OnionShareGui', '_tor_connection_canceled') - self.timer.stop() def ask(): a = Alert(strings._('gui_tor_connection_ask', True), QtWidgets.QMessageBox.Question, buttons=QtWidgets.QMessageBox.NoButton, autostart=False) @@ -223,8 +222,10 @@ class OnionShareGui(QtWidgets.QMainWindow): # We might've stopped the main requests timer if a Tor connection failed. # If we've reloaded settings, we probably succeeded in obtaining a new # connection. If so, restart the timer. - if not self.timer.isActive(): - self.timer.start() + if self.onion.is_authenticated(): + if not self.timer.isActive(): + self.timer.start() + self.status_bar.clearMessage() d = SettingsDialog(self.onion, self.qtapp, self.config) d.settings_saved.connect(reload_settings) @@ -401,8 +402,7 @@ class OnionShareGui(QtWidgets.QMainWindow): self.timer.stop() if self.server_status.status != self.server_status.STATUS_STOPPED: self.server_status.stop_server() - self.status_bar.clearMessage() - self._tor_connection_canceled() + self.status_bar.showMessage(strings._('gui_tor_connection_lost', True)) # scroll to the bottom of the dl progress bar log pane # if a new download has been added diff --git a/share/locale/en.json b/share/locale/en.json index 344736fc..abd734c9 100644 --- a/share/locale/en.json +++ b/share/locale/en.json @@ -114,11 +114,12 @@ "update_error_check_error": "Error checking for updates: Maybe you're not connected to Tor, or maybe the OnionShare website is down.", "update_error_invalid_latest_version": "Error checking for updates: The OnionShare website responded saying the latest version is '{}', but that doesn't appear to be a valid version string.", "update_not_available": "You are running the latest version of OnionShare.", - "gui_tor_connection_ask": "Tor isn't running.\n\nWould you like to open OnionShare settings to troubleshoot connecting to Tor?", + "gui_tor_connection_ask": "Would you like to open OnionShare settings to troubleshoot connecting to Tor?", "gui_tor_connection_ask_open_settings": "Open Settings", "gui_tor_connection_ask_quit": "Quit", "gui_tor_connection_error_settings": "Try adjusting how OnionShare connects to the Tor network in Settings.", "gui_tor_connection_canceled": "OnionShare cannot connect to Tor.\n\nMake sure you're connected to the internet, then re-open OnionShare to configure the Tor connection.", + "gui_tor_connection_lost": "Disconnected from Tor.", "gui_server_started_after_timeout": "The server started after your chosen auto-timeout.\nPlease start a new share.", "gui_server_timeout_expired": "The chosen timeout has already expired.\nPlease update the timeout and then you may start sharing.", "share_via_onionshare": "Share via OnionShare" From 0834580f02f5a0f9ae01505c5cf73ed1001ec5ac Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sun, 24 Dec 2017 14:07:20 +1100 Subject: [PATCH 5/6] Disable the 'Start Sharing' button if the connection to Tor has been lost. Re-enable it if we've subsequently reconnected to Tor via Settings dialog --- onionshare_gui/onionshare_gui.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index 7c44b769..4fbebf9c 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -225,7 +225,11 @@ class OnionShareGui(QtWidgets.QMainWindow): if self.onion.is_authenticated(): if not self.timer.isActive(): self.timer.start() - self.status_bar.clearMessage() + # If there were some files listed for sharing, we should be ok to + # re-enable the 'Start Sharing' button now. + if self.server_status.file_selection.get_num_files() > 0: + self.server_status.server_button.setEnabled(True) + self.status_bar.clearMessage() d = SettingsDialog(self.onion, self.qtapp, self.config) d.settings_saved.connect(reload_settings) @@ -402,6 +406,7 @@ class OnionShareGui(QtWidgets.QMainWindow): self.timer.stop() if self.server_status.status != self.server_status.STATUS_STOPPED: self.server_status.stop_server() + self.server_status.server_button.setEnabled(False) self.status_bar.showMessage(strings._('gui_tor_connection_lost', True)) # scroll to the bottom of the dl progress bar log pane From 47e0ef4f83a23ccb8ab8793c1c572c6dd36b1c4f Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Wed, 27 Dec 2017 09:29:45 +1100 Subject: [PATCH 6/6] Add systray notification when Tor connection is lost, thanks @Baccount --- onionshare_gui/onionshare_gui.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/onionshare_gui/onionshare_gui.py b/onionshare_gui/onionshare_gui.py index 4fbebf9c..b720a04b 100644 --- a/onionshare_gui/onionshare_gui.py +++ b/onionshare_gui/onionshare_gui.py @@ -408,6 +408,8 @@ class OnionShareGui(QtWidgets.QMainWindow): self.server_status.stop_server() self.server_status.server_button.setEnabled(False) self.status_bar.showMessage(strings._('gui_tor_connection_lost', True)) + if self.systemTray.supportsMessages() and self.settings.get('systray_notifications'): + self.systemTray.showMessage(strings._('gui_tor_connection_lost', True), strings._('gui_tor_connection_error_settings', True)) # scroll to the bottom of the dl progress bar log pane # if a new download has been added