Restrict request handling to DEFAULT_ALLOWED_URL_SCHEMES (#1002)

* Raise `HttpProtocolException` if request line scheme do not match `DEFAULT_ALLOWED_URL_SCHEMES`

* ignore WPS329

* Fix tests

* Pin to 4.3.2

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Test coverage for exception handling

* type ignore

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: abhinavsingh

docs/requirements.in

@@ -1,6 +1,6 @@
 myst-parser[linkify] >= 0.15.2
 setuptools-scm >= 6.3.2
-Sphinx >= 4.3.0
+Sphinx == 4.3.2
 furo >= 2021.11.15
 sphinxcontrib-apidoc >= 0.3.0
 sphinxcontrib-towncrier >= 0.2.0a0

proxy/common/constants.py

@@ -94,6 +94,7 @@ def _env_threadless_compliant() -> bool:
 DEFAULT_EVENTS_QUEUE = None
 DEFAULT_ENABLE_STATIC_SERVER = False
 DEFAULT_ENABLE_WEB_SERVER = False
+DEFAULT_ALLOWED_URL_SCHEMES = [HTTP_PROTO, HTTPS_PROTO]
 DEFAULT_IPV4_HOSTNAME = ipaddress.IPv4Address('127.0.0.1')
 DEFAULT_IPV6_HOSTNAME = ipaddress.IPv6Address('::1')
 DEFAULT_KEY_FILE = None

proxy/core/event/dispatcher.py

@@ -61,7 +61,7 @@ def handle_event(self, ev: Dict[str, Any]) -> None:
             })
         elif ev['event_name'] == eventNames.UNSUBSCRIBE:
             # send ack
-            print('unsubscription request ack sent')
+            logger.info('unsubscription request ack sent')
             self.subscribers[ev['event_payload']['sub_id']].send({
                 'event_name': eventNames.UNSUBSCRIBED,
             })

proxy/http/handler.py

@@ -277,10 +277,14 @@ def _parse_first_request(self, data: memoryview) -> bool:
         # memoryview compliant
         try:
             self.request.parse(data.tobytes())
-        except Exception:
+        except HttpProtocolException as e:  # noqa: WPS329
+            self.work.queue(BAD_REQUEST_RESPONSE_PKT)
+            raise e
+        except Exception as e:
+            self.work.queue(BAD_REQUEST_RESPONSE_PKT)
             raise HttpProtocolException(
                 'Error when parsing request: %r' % data.tobytes(),
-            )
+            ) from e
         if not self.request.is_complete:
             return False
         # Discover which HTTP handler plugin is capable of

proxy/http/parser/parser.py

@@ -149,9 +149,11 @@ def del_headers(self, headers: List[bytes]) -> None:
         for key in headers:
             self.del_header(key.lower())
 
-    def set_url(self, url: bytes) -> None:
+    def set_url(self, url: bytes, allowed_url_schemes: Optional[List[bytes]] = None) -> None:
         """Given a request line, parses it and sets line attributes a.k.a. host, port, path."""
-        self._url = Url.from_bytes(url)
+        self._url = Url.from_bytes(
+            url, allowed_url_schemes=allowed_url_schemes,
+        )
         self._set_line_attributes()
 
     @property
@@ -204,7 +206,7 @@ def body_expected(self) -> bool:
         """Returns true if content or chunked response is expected."""
         return self._content_expected or self._is_chunked_encoded
 
-    def parse(self, raw: bytes) -> None:
+    def parse(self, raw: bytes, allowed_url_schemes: Optional[List[bytes]] = None) -> None:
         """Parses HTTP request out of raw bytes.
 
         Check for `HttpParser.state` after `parse` has successfully returned."""
@@ -217,7 +219,10 @@ def parse(self, raw: bytes) -> None:
             if self.state >= httpParserStates.HEADERS_COMPLETE:
                 more, raw = self._process_body(raw)
             elif self.state == httpParserStates.INITIALIZED:
-                more, raw = self._process_line(raw)
+                more, raw = self._process_line(
+                    raw,
+                    allowed_url_schemes=allowed_url_schemes,
+                )
             else:
                 more, raw = self._process_headers(raw)
             # When server sends a response line without any header or body e.g.
@@ -345,7 +350,11 @@ def _process_headers(self, raw: bytes) -> Tuple[bool, bytes]:
                 break
         return len(raw) > 0, raw
 
-    def _process_line(self, raw: bytes) -> Tuple[bool, bytes]:
+    def _process_line(
+            self,
+            raw: bytes,
+            allowed_url_schemes: Optional[List[bytes]] = None,
+    ) -> Tuple[bool, bytes]:
         while True:
             parts = raw.split(CRLF, 1)
             if len(parts) == 1:
@@ -363,7 +372,9 @@ def _process_line(self, raw: bytes) -> Tuple[bool, bytes]:
                     self.method = parts[0]
                     if self.method == httpMethods.CONNECT:
                         self._is_https_tunnel = True
-                    self.set_url(parts[1])
+                    self.set_url(
+                        parts[1], allowed_url_schemes=allowed_url_schemes,
+                    )
                     self.version = parts[2]
                     self.state = httpParserStates.LINE_RCVD
                     break

proxy/http/url.py

@@ -13,11 +13,13 @@
        http
        url
 """
-from typing import Optional, Tuple
+from typing import List, Optional, Tuple
 
-from ..common.constants import COLON, SLASH, AT
+from ..common.constants import COLON, DEFAULT_ALLOWED_URL_SCHEMES, SLASH, AT
 from ..common.utils import text_
 
+from .exception import HttpProtocolException
+
 
 class Url:
     """``urllib.urlparse`` doesn't work for proxy.py, so we wrote a simple URL.
@@ -59,7 +61,7 @@ def __str__(self) -> str:
         return url
 
     @classmethod
-    def from_bytes(cls, raw: bytes) -> 'Url':
+    def from_bytes(cls, raw: bytes, allowed_url_schemes: Optional[List[bytes]] = None) -> 'Url':
         """A URL within proxy.py core can have several styles,
         because proxy.py supports both proxy and web server use cases.
 
@@ -95,6 +97,10 @@ def from_bytes(cls, raw: bytes) -> 'Url':
             if len(parts) == 2:
                 scheme = parts[0]
                 rest = parts[1]
+                if scheme not in (allowed_url_schemes or DEFAULT_ALLOWED_URL_SCHEMES):
+                    raise HttpProtocolException(
+                        'Invalid scheme received in the request line %r' % raw,
+                    )
         else:
             rest = raw[len(SLASH + SLASH):]
         if scheme is not None or starts_with_double_slash:

tests/http/parser/test_http_parser.py

@@ -827,6 +827,7 @@ def test_parses_icap_protocol(self) -> None:
             b'I am posting this information.\r\n' +
             b'0\r\n' +
             b'\r\n',
+            allowed_url_schemes=[b'icap'],
         )
         self.assertEqual(self.parser.method, b'REQMOD')
         assert self.parser._url is not None

tests/http/test_protocol_handler.py

@@ -26,7 +26,7 @@
 from proxy.common.version import __version__
 from proxy.http.responses import (
     BAD_GATEWAY_RESPONSE_PKT, PROXY_AUTH_FAILED_RESPONSE_PKT,
-    PROXY_TUNNEL_ESTABLISHED_RESPONSE_PKT,
+    PROXY_TUNNEL_ESTABLISHED_RESPONSE_PKT, BAD_REQUEST_RESPONSE_PKT,
 )
 from proxy.common.constants import (
     CRLF, PLUGIN_HTTP_PROXY, PLUGIN_PROXY_AUTH, PLUGIN_WEB_SERVER,
@@ -114,6 +114,34 @@ async def test_proxy_authentication_failed(self) -> None:
             PROXY_AUTH_FAILED_RESPONSE_PKT,
         )
 
+    @pytest.mark.asyncio    # type: ignore[misc]
+    async def test_proxy_bails_out_for_unknown_schemes(self) -> None:
+        mock_selector_for_client_read(self)
+        self._conn.recv.return_value = CRLF.join([
+            b'REQMOD icap://icap-server.net/server?arg=87 ICAP/1.0',
+            b'Host: icap-server.net',
+            CRLF,
+        ])
+        await self.protocol_handler._run_once()
+        self.assertEqual(
+            self.protocol_handler.work.buffer[0],
+            BAD_REQUEST_RESPONSE_PKT,
+        )
+
+    @pytest.mark.asyncio    # type: ignore[misc]
+    async def test_proxy_bails_out_for_sip_request_lines(self) -> None:
+        mock_selector_for_client_read(self)
+        self._conn.recv.return_value = CRLF.join([
+            b'OPTIONS sip:nm SIP/2.0',
+            b'Accept: application/sdp',
+            CRLF,
+        ])
+        await self.protocol_handler._run_once()
+        self.assertEqual(
+            self.protocol_handler.work.buffer[0],
+            BAD_REQUEST_RESPONSE_PKT,
+        )
+
 
 class TestHttpProtocolHandler(Assertions):
 

tests/http/test_url.py

@@ -11,6 +11,7 @@
 import unittest
 
 from proxy.http import Url
+from proxy.http.exception import HttpProtocolException
 
 
 class TestUrl(unittest.TestCase):
@@ -145,10 +146,17 @@ def test_no_scheme_suffix(self) -> None:
         self.assertEqual(url.password, None)
 
     def test_any_scheme_suffix(self) -> None:
-        url = Url.from_bytes(b'icap://example-server.net/server?arg=87')
+        url = Url.from_bytes(
+            b'icap://example-server.net/server?arg=87',
+            allowed_url_schemes=[b'icap'],
+        )
         self.assertEqual(url.scheme, b'icap')
         self.assertEqual(url.hostname, b'example-server.net')
         self.assertEqual(url.port, None)
         self.assertEqual(url.remainder, b'/server?arg=87')
         self.assertEqual(url.username, None)
         self.assertEqual(url.password, None)
+
+    def test_assert_raises_for_unknown_schemes(self) -> None:
+        with self.assertRaises(HttpProtocolException):
+            Url.from_bytes(b'icap://example-server.net/server?arg=87')