tor: Better validation for upstream bridges

- Convert line terminators in the form field so that it matches the
  current status information resulting in proper detection of change in
  value.

- Allow IPv6 addresses properly.  Allow "[ipv6]:port" format.

- Make specifying port optional.

- Allow spaces at the beginning and ending of the line to compensate for
  copy/paste errors.

- Allow empty lines between bridge specifications.

- Allow multiple spaces between the components.

- Raise error if upstream bridges are not provided when 'use upstream
  bridges' option is enabled.

- Write tests for upstream bridges validator.

plinth/modules/tor/forms.py

@@ -24,56 +24,64 @@ from django.core.exceptions import ValidationError
 from django.core.validators import validate_ipv46_address
 from django.forms import widgets
 from django.utils.translation import ugettext_lazy as _
+import re
 
 from plinth import cfg
 from plinth.utils import format_lazy
 
 
-BRIDGE_VALIDATION_ERROR_MESSAGE = _('Enter a valid bridge with this format: '
-                                    '[transport] IP:ORPort [fingerprint]')
-
-
 class TrimmedCharField(forms.CharField):
     """Trim the contents of a CharField"""
     def clean(self, value):
         """Clean and validate the field value"""
         if value:
             value = value.strip()
+            value = value.replace('\r\n', '\n')
 
         return super(TrimmedCharField, self).clean(value)
 
 
 def bridges_validator(bridges):
     """Validate upstream bridges entries."""
-    for bridge in bridges.split('\n'):
+    validation_error = ValidationError(
-        parts = bridge.split()
+        _('Enter a valid bridge with this format: '
+          '[transport] IP:ORPort [fingerprint]'), code='invalid')
+
+    bridges = [bridge.strip() for bridge in bridges.split('\n')]
+    bridges = [bridge for bridge in bridges if bridge]
+    if not bridges:
+        raise validation_error
+
+    for bridge in bridges:
+        parts = [part for part in bridge.split() if part]
 
         # IP:ORPort is required, transport and fingerprint are optional.
         # Transports may have additional options after the fingerprint.
-        if len(parts) < 1:
-            raise ValidationError(
-                BRIDGE_VALIDATION_ERROR_MESSAGE, code='invalid')
-
-        # May start with transport or IP:ORPort.
         try:
-            ip_info = parts[0].split(':')
+            ip_port_part = parts[0]
-            validate_ipv46_address(ip_info[0])
+            if re.match('[a-z_][a-z0-9_]*', parts[0]):
+                ip_port_part = parts[1]
+        except IndexError:
+            raise validation_error
+
+        match = re.match('\[([a-fA-F0-9:]+)\](?::([0-9]+))?', ip_port_part)
+        if match:
+            ip_address = match.group(1)
+            port = match.group(2)
+        else:
+            ip_parts = ip_port_part.rsplit(':', maxsplit=1)
+            ip_address = ip_parts[0]
+            port = ip_parts[1] if len(ip_parts) > 1 else None
+
+        try:
+            validate_ipv46_address(ip_address)
         except ValidationError:
-            try:
+            raise validation_error
-                ip_info = parts[1].split(':')
-                validate_ipv46_address(ip_info[0])
-            except (ValidationError, IndexError):
-                raise ValidationError(
-                    BRIDGE_VALIDATION_ERROR_MESSAGE, code='invalid')
 
-        try:
+        if port:
-            port = int(ip_info[1])
+            port = int(port)
-        except ValueError:
+            if port < 0 or port > 65535:
-            raise ValidationError(
+                raise validation_error
-                BRIDGE_VALIDATION_ERROR_MESSAGE, code='invalid')
-        if port < 0 or port > 65535:
-            raise ValidationError(
-                BRIDGE_VALIDATION_ERROR_MESSAGE, code='invalid')
 
 
 class TorForm(forms.Form):  # pylint: disable=W0232
@@ -131,3 +139,16 @@ class TorForm(forms.Form):  # pylint: disable=W0232
                     'network for installations and upgrades. This adds a '
                     'degree of privacy and security during software '
                     'downloads.'))
+
+    def clean(self):
+        """Validate the form for cross-field integrity."""
+        cleaned_data = super().clean()
+        use_upstream_bridges = cleaned_data.get('use_upstream_bridges')
+        upstream_bridges = cleaned_data.get('upstream_bridges')
+
+        if use_upstream_bridges and not upstream_bridges:
+            self.add_error('upstream_bridges', ValidationError(_(
+                'Specify at least one upstream bridge to use upstream '
+                'bridges.'), code='invalid'))
+
+        return cleaned_data

plinth/modules/tor/tests/test_tor.py

@@ -19,10 +19,12 @@
 Tests for Tor module.
 """
 
+from django.core.exceptions import ValidationError
 import os
 import unittest
 
 from plinth.modules.tor import utils
+from plinth.modules.tor import forms
 
 euid = os.geteuid()
 
@@ -44,3 +46,51 @@ class TestTor(unittest.TestCase):
         /etc/tor/torrc exists.
         """
         utils.get_status()
+
+
+class TestTorForm(unittest.TestCase):
+    """Test whether Tor configration form works."""
+    def test_bridge_validator(self):
+        """Test upstream bridges' form field validator."""
+        validator = forms.bridges_validator
+
+        # Just IP:port
+        validator('73.237.165.184:9001')
+        validator('73.237.165.184')
+        validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443')
+        validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]')
+
+        # With fingerprint
+        validator('73.237.165.184:9001 '
+                  '0D04F10F497E68D2AF32375BB763EC3458A908C8')
+
+        # With transport type
+        validator('obfs4 73.237.165.184:9001 '
+                  '0D04F10F497E68D2AF32375BB763EC3458A908C8')
+
+        # With transport type and extra options
+        validator('obfs4 10.1.1.1:30000 '
+                  '0123456789ABCDEF0123456789ABCDEF01234567 '
+                  'cert=A/b+1 iat-mode=0')
+
+        # Leading, trailing spaces and empty lines
+        validator('\n'
+                  '  \n'
+                  '73.237.165.184:9001 '
+                  '0D04F10F497E68D2AF32375BB763EC3458A908C8'
+                  '  \n'
+                  '73.237.165.184:9001 '
+                  '0D04F10F497E68D2AF32375BB763EC3458A908C8'
+                  '  \n'
+                  '\n')
+
+        # Invalid number for parts
+        self.assertRaises(ValidationError, validator, '  ')
+
+        # Invalid IP address/port
+        self.assertRaises(ValidationError, validator, '73.237.165.384:9001')
+        self.assertRaises(ValidationError, validator, '73.237.165.184:90001')
+        self.assertRaises(ValidationError, validator,
+                          '[a2001:db8:85a3:8d3:1319:8a2e:370:7348]:443')
+        self.assertRaises(ValidationError, validator,
+                          '[2001:db8:85a3:8d3:1319:8a2e:370:7348]:90443')