Make sanity checking of bridges a reusable component in cli.Common, so we can reuse it for automatic bridge fetching in censorship circumvention

cli/onionshare_cli/common.py

@@ -28,6 +28,7 @@ import sys
 import threading
 import time
 import shutil
+import re
 from pkg_resources import resource_filename
 
 import colorama
@@ -432,6 +433,40 @@ class Common:
         r = random.SystemRandom()
         return "-".join(r.choice(wordlist) for _ in range(word_count))
 
+    def check_bridges_valid(self, bridges):
+        """
+        Does a regex check against a supplied list of bridges, to make sure they
+        are valid strings depending on the bridge type.
+        """
+        valid_bridges = []
+        self.log("Common", "check_bridges_valid", "Checking bridge syntax")
+        for bridge in bridges:
+            if bridge != "":
+                # Check the syntax of the custom bridge to make sure it looks legitimate
+                ipv4_pattern = re.compile(
+                    "(obfs4\s+)?(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]):([0-9]+)(\s+)([A-Z0-9]+)(.+)$"
+                )
+                ipv6_pattern = re.compile(
+                    "(obfs4\s+)?\[(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))\]:[0-9]+\s+[A-Z0-9]+(.+)$"
+                )
+                meek_lite_pattern = re.compile(
+                    "(meek_lite)(\s)+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+)(\s)+([0-9A-Z]+)(\s)+url=(.+)(\s)+front=(.+)"
+                )
+                snowflake_pattern = re.compile(
+                    "(snowflake)(\s)+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+)(\s)+([0-9A-Z]+)"
+                )
+                if (
+                    ipv4_pattern.match(bridge)
+                    or ipv6_pattern.match(bridge)
+                    or meek_lite_pattern.match(bridge)
+                    or snowflake_pattern.match(bridge)
+                ):
+                    valid_bridges.append(bridge)
+        if valid_bridges:
+            return valid_bridges
+        else:
+           return False
+
     @staticmethod
     def random_string(num_bytes, output_len=None):
         """

desktop/src/onionshare/tor_connection.py

@@ -301,11 +301,6 @@ class TorConnectionWidget(QtWidgets.QWidget):
                 # bridges, set that in our settings, as if the user had
                 # selected the built-in bridges for a specific PT themselves.
                 #
-                # @TODO should we fetch the built-in bridges from
-                # censorship_circumvention.request_builtin_bridges()?
-                #
-                # In fact, the bridge_string returned for a bridge type 'builtin'
-                # is in fact the same bridges we'd get from that other method anyway.
                 if bridge_source == "builtin":
                     self.settings.set("bridges_type", "built-in")
                     if bridge_type == "obfs4":
@@ -316,8 +311,10 @@ class TorConnectionWidget(QtWidgets.QWidget):
                         self.settings.set("bridges_builtin_pt", "meek-azure")
                 else:
                     self.settings.set("bridges_type", "custom")
-                    # @TODO do we want to to a sanity check on the bridges like custom ones?
+                    # Sanity check the bridges provided from the Tor API before saving
-                    self.settings.set("bridges_custom", "\n".join(bridge_strings))
+                    bridges_checked = self.common.check_bridges_valid(bridge_strings)
+                    if bridges_checked:
+                        self.settings.set("bridges_custom", "\n".join(bridges_checked))
 
                 self.common.log(
                     "TorConnectionWidget",

desktop/src/onionshare/tor_settings_tab.py

@@ -21,7 +21,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 from PySide2 import QtCore, QtWidgets, QtGui
 import sys
 import platform
-import re
 import os
 
 from onionshare_cli.meek import Meek
@@ -856,35 +855,10 @@ class TorSettingsTab(QtWidgets.QWidget):
             if self.bridge_custom_radio.isChecked():
                 settings.set("bridges_type", "custom")
 
-                new_bridges = []
                 bridges = self.bridge_custom_textbox.toPlainText().split("\n")
-                bridges_valid = False
+                bridges_valid = self.common.check_bridges_valid(bridges)
-                for bridge in bridges:
-                    if bridge != "":
-                        # Check the syntax of the custom bridge to make sure it looks legitimate
-                        ipv4_pattern = re.compile(
-                            "(obfs4\s+)?(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\.){3}([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5]):([0-9]+)(\s+)([A-Z0-9]+)(.+)$"
-                        )
-                        ipv6_pattern = re.compile(
-                            "(obfs4\s+)?\[(([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))\]:[0-9]+\s+[A-Z0-9]+(.+)$"
-                        )
-                        meek_lite_pattern = re.compile(
-                            "(meek_lite)(\s)+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+)(\s)+([0-9A-Z]+)(\s)+url=(.+)(\s)+front=(.+)"
-                        )
-                        snowflake_pattern = re.compile(
-                            "(snowflake)(\s)+([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:[0-9]+)(\s)+([0-9A-Z]+)"
-                        )
-                        if (
-                            ipv4_pattern.match(bridge)
-                            or ipv6_pattern.match(bridge)
-                            or meek_lite_pattern.match(bridge)
-                            or snowflake_pattern.match(bridge)
-                        ):
-                            new_bridges.append(bridge)
-                            bridges_valid = True
-
                 if bridges_valid:
-                    new_bridges = "\n".join(new_bridges) + "\n"
+                    new_bridges = "\n".join(bridges_valid) + "\n"
                     settings.set("bridges_custom", new_bridges)
                 else:
                     self.error_label.setText(