Support TLS termination in boardwalkd (#27)

* Initial TLS set-up; missing redirect to https

* automatically redirect reqs to server scheme://host:port

* reject invalid API requests

* update readme

* allow the server to be configured without a non-TLS port

* bump version

* Update comment in src/boardwalkd/server.py

README.md

@@ -465,8 +465,7 @@ __Authentication__: By default, `boardwalkd` uses anonymous authentication. It's
 important that an authentication method be configured. See
 `boardwalkd serve --help` for available options.
 
-__TLS__: `boardwalkd` doesn't yet support TLS directly. TLS should be terminated
-using a reverse proxy, or other means.
+__TLS__: `boardwalkd` supports TLS termination. See `boardwalkd serve --help`
 
 __No access to hosts__: `boardwalkd` is not involved in managing hosts; only
 CLI workers require direct access to hosts.

src/boardwalkd/cli.py

@@ -69,12 +69,19 @@ def cli():
     help=(
         "A valid python regex pattern to match accepted Host header values."
         " This prevents DNS rebinding attacks when the pattern is appropriately scoped"
+        " Requests reaching the server that don't match this pattern will return a 404"
     ),
     type=str,
     required=True,
 )
 @click.option(
-    "--port", help="The port number the server binds to", type=int, required=True
+    "--port",
+    help=(
+        "The non-TLS port number the server binds to. --port and/or"
+        " --tls-port must be configured"
+    ),
+    type=int,
+    default=None,
 )
 @click.option(
     "--slack-webhook-url",
@@ -91,9 +98,34 @@ def cli():
     type=str,
     default=None,
 )
+@click.option(
+    "--tls-crt",
+    help=("Path to TLS certificate chain file for use along with --tls-port"),
+    type=click.Path(exists=True, readable=True),
+    default=None,
+)
+@click.option(
+    "--tls-key",
+    help=("Path to TLS key file for use along with --tls-port"),
+    type=click.Path(exists=True, readable=True),
+    default=None,
+)
+@click.option(
+    "--tls-port",
+    help=(
+        "The TLS port number the server binds to. When configured, the --url"
+        " option must have an https:// scheme. When --tls-port is configured,"
+        " --tls-crt and --tls-key must also be supplied"
+    ),
+    type=int,
+    default=None,
+)
 @click.option(
     "--url",
-    help="The base URL where the server can be reached",
+    help=(
+        "The base URL where the server can be reached. UI Requests that do not"
+        " match the scheme or host:port of this URL will automatically be redirected"
+    ),
     type=str,
     required=True,
 )
@@ -102,17 +134,47 @@ def serve(
     auth_method: str,
     develop: bool,
     host_header_pattern: str,
-    port: int,
+    port: int | None,
     slack_error_webhook_url: str,
     slack_webhook_url: str,
+    tls_crt: str | None,
+    tls_key: str | None,
+    tls_port: int | None,
     url: str,
 ):
     """Runs the server"""
+    # Validate host_header_pattern
     try:
         host_header_regex = re.compile(host_header_pattern)
     except re.error:
         raise ClickException("Host pattern regex invalid")
 
+    # Validate any port is configured
+    if not (port or tls_port):
+        raise ClickException("One or both of --port or --tls-port must be configured")
+
+    # If there is no TLS port then reject setting a TLS key and cert
+    if (not tls_port) and (tls_crt or tls_key):
+        raise ClickException(
+            (
+                "--tls-crt and --tls-key should not be configured"
+                " unless --tls-port is also set"
+            )
+        )
+
+    # Validate TLS configuration (key and cert paths are already validated by click)
+    if tls_port is not None:
+        try:
+            assert tls_crt
+            assert tls_key
+        except AssertionError:
+            raise ClickException(
+                (
+                    "--tls-crt and --tls-key paths must be supplied when a"
+                    " --tls-port is configured"
+                )
+            )
+
     asyncio.run(
         run(
             auth_expire_days=auth_expire_days,
@@ -122,6 +184,9 @@ def serve(
             port_number=port,
             slack_error_webhook_url=slack_error_webhook_url,
             slack_webhook_url=slack_webhook_url,
+            tls_crt_path=tls_crt,
+            tls_key_path=tls_key,
+            tls_port_number=tls_port,
             url=url,
         )
     )

src/boardwalkd/protocol.py

@@ -173,6 +173,9 @@ def authenticated_request(
                     body=body,
                     auto_login_prompt=auto_login_prompt,
                 )
+            if e.code == 421:
+                # The server URL is probably incorrect
+                raise ConnectionRefusedError
             else:
                 raise e
 

src/boardwalkd/server.py

@@ -9,14 +9,15 @@
 import os
 import re
 import secrets
+import ssl
 import string
 from collections import deque
 from datetime import datetime, timedelta
 from distutils.util import strtobool
 from importlib.metadata import version as lib_version
 from pathlib import Path
 from typing import Any, Callable
-from urllib.parse import urljoin
+from urllib.parse import ParseResult, urljoin, urlparse
 
 import tornado.auth
 import tornado.escape
@@ -47,6 +48,16 @@
 class UIBaseHandler(tornado.web.RequestHandler):
     """Base request handler for UI paths"""
 
+    def prepare(self):
+        # If the request's scheme or host:port differs from the server's
+        # configured URL, then the request will be redirected to the configured
+        # server URL
+        req_url = urlparse(self.request.full_url())
+        svr_url: ParseResult = self.settings["url"]
+        if req_url.scheme != svr_url.scheme or req_url.netloc != svr_url.netloc:
+            req_url = req_url._replace(scheme=svr_url.scheme, netloc=svr_url.netloc)
+            return self.redirect(req_url.geturl())
+
     def get_current_user(self) -> bytes | None:
         """Required method for @tornado.web.authenticated to work"""
         return self.get_secure_cookie(
@@ -292,6 +303,14 @@ def get(self):
 class APIBaseHandler(tornado.web.RequestHandler):
     """Base request handler for API paths"""
 
+    def prepare(self):
+        # If the request's scheme or host:port differs from the server's
+        # configured URL, then the request will be rejected
+        req_url = urlparse(self.request.full_url())
+        svr_url: ParseResult = self.settings["url"]
+        if req_url.scheme != svr_url.scheme or req_url.netloc != svr_url.netloc:
+            return self.send_error(421)
+
     def check_xsrf_cookie(self):
         """We ignore this method on API requests"""
         pass
@@ -498,7 +517,7 @@ async def post(self, workspace: str):
                     workspace,
                     self.settings["slack_webhook_url"],
                     self.settings["slack_error_webhook_url"],
-                    self.settings["url"],
+                    self.settings["url"].geturl(),
                 )
 
         state.flush()
@@ -570,16 +589,16 @@ def log_request(handler: tornado.web.RequestHandler):
     )
 
 
-def make_server(
+def make_app(
     auth_expire_days: float,
     auth_method: str,
     develop: bool,
     host_header_pattern: re.Pattern[str],
     slack_error_webhook_url: str,
     slack_webhook_url: str,
     url: str,
-):
-    """Builds the tornado application server object"""
+) -> tornado.web.Application:
+    """Builds the tornado application object"""
     handlers: list[tornado.web.OutputTransform] = []
     settings = {
         "api_access_denied_url": urljoin(url, "/api/auth/denied"),
@@ -595,7 +614,7 @@ def make_server(
             "server_version": ui_method_server_version,
             "sort_events_by_date": ui_method_sort_events_by_date,
         },
-        "url": url,
+        "url": urlparse(url),
         "websocket_ping_interval": 10,
         "xsrf_cookies": True,
         "xsrf_cookie_kwargs": {"samesite": "Strict", "secure": True},
@@ -704,13 +723,16 @@ async def run(
     auth_method: str,
     develop: bool,
     host_header_pattern: re.Pattern[str],
-    port_number: int,
+    port_number: int | None,
+    tls_crt_path: str | None,
+    tls_key_path: str | None,
+    tls_port_number: int | None,
     slack_error_webhook_url: str,
     slack_webhook_url: str,
     url: str,
 ):
     """Starts the tornado server and IO loop"""
-    server = make_server(
+    app = make_app(
         auth_expire_days=auth_expire_days,
         auth_method=auth_method,
         develop=develop,
@@ -719,6 +741,23 @@ async def run(
         slack_webhook_url=slack_webhook_url,
         url=url,
     )
-    server.listen(port_number)
-    app_log.info(f"Server listening on port: {port_number}")
+
+    if port_number is not None:
+        app.listen(port_number)
+        # If port_number=0 a random open port will be selected and the log message
+        # will not be accurate
+        app_log.info(f"Server listening on non-TLS port: {port_number}")
+
+    if tls_port_number is not None:
+        if urlparse(url).scheme != "https":
+            raise ClickException(f"URL scheme must be HTTPS when TLS is enabled")
+
+        ssl_ctx = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
+        ssl_ctx.load_cert_chain(certfile=tls_crt_path, keyfile=tls_key_path)
+
+        app.listen(tls_port_number, ssl_options=ssl_ctx)
+        # If tls_port_number=0 a random open port will be selected and the log
+        # message will not be accurate
+        app_log.info(f"Server listening on TLS port: {tls_port_number}")
+
     await asyncio.Event().wait()