Repository: caikit/caikit
Pull request #484 (2023-09-22 18:00:39)

HTTP With mTLS Inline
The uvicorn server uses python's SslContext which requires tls files be on disk and not passed by value.

Closes: https://github.com/caikit/caikit/issues/477

**What this PR does / why we need it**:

This PR updates the logic for booting the `uvicorn` server so that if the configured TLS values are passed as inline values, they will be temporarily written to a file so that they are loaded correctly. This is needed because [uvicorn uses `ssl.SslContext`](https://github.com/encode/uvicorn/blob/master/uvicorn/config.py#L110) to manage is TLS connection and that loads values from disk only.

**If applicable**:
- [X] this PR contains unit tests
- [X] this PR has been tested for backwards compatibility



Patch:
diff --git a/caikit/runtime/http_server/http_server.py b/caikit/runtime/http_server/http_server.py
index f3219de7..df2478d2 100644
--- a/caikit/runtime/http_server/http_server.py
+++ b/caikit/runtime/http_server/http_server.py
@@ -17,12 +17,16 @@ The server is responsible for binding caikit workloads to a consistent REST/SSE
 API based on the task definitions available at boot.
 """
 # Standard
+from contextlib import contextmanager
+from dataclasses import dataclass
 from functools import partial
 from typing import Any, Dict, Iterable, Optional, Type, get_args
 import asyncio
 import json
+import os
 import re
 import ssl
+import tempfile
 import threading
 import time
 
@@ -94,6 +98,14 @@ OPTIONAL_INPUTS_KEY = "parameters"
 # Endpoint to use for health checks
 HEALTH_ENDPOINT = "/health"
 
+# Small dataclass for consolidating TLS files
+@dataclass
+class _TlsFiles:
+    server_cert: Optional[str] = None
+    server_key: Optional[str] = None
+    client_cert: Optional[str] = None
+
+
 ## RuntimeHTTPServer ###########################################################
 
 
@@ -142,34 +154,41 @@ class RuntimeHTTPServer(RuntimeServerBase):
         )
 
         # Parse TLS configuration
-        tls_kwargs = {}
-        if (
-            self.tls_config
-            and self.tls_config.server.key
-            and self.tls_config.server.cert
-        ):
-            log.info("<RUN10001905I>", "Running with TLS")
-            tls_kwargs["ssl_keyfile"] = self.tls_config.server.key
-            tls_kwargs["ssl_certfile"] = self.tls_config.server.cert
-            if self.tls_config.client.cert:
-                log.info("<RUN10001809I>", "Running with mutual TLS")
-                tls_kwargs["ssl_ca_certs"] = self.tls_config.client.cert
-                tls_kwargs["ssl_cert_reqs"] = ssl.CERT_REQUIRED
-
-        # Start the server with a timeout_graceful_shutdown
-        # if not set in config, this is None and unvicorn accepts None or number of seconds
-        unvicorn_timeout_graceful_shutdown = (
-            get_config().runtime.http.server_shutdown_grace_period_seconds
-        )
-        config = uvicorn.Config(
-            self.app,
-            host="0.0.0.0",
-            port=self.port,
-            log_level=None,
-            log_config=None,
-            timeout_graceful_shutdown=unvicorn_timeout_graceful_shutdown,
-            **tls_kwargs,
-        )
+        # If any of the TLS values are not files, we assume that they're inline
+        # content. The python SslContext only takes files to load, so we use a
+        # temporary directory just long enough to load the config files.
+        with self._tls_files() as tls_files:
+            tls_kwargs = {}
+            if tls_files.server_key and tls_files.server_cert:
+                log.info("<RUN10001905I>", "Running with TLS")
+                tls_kwargs["ssl_keyfile"] = tls_files.server_key
+                tls_kwargs["ssl_certfile"] = tls_files.server_cert
+                if tls_files.client_cert:
+                    log.info("<RUN10001809I>", "Running with mutual TLS")
+                    tls_kwargs["ssl_ca_certs"] = tls_files.client_cert
+                    tls_kwargs["ssl_cert_reqs"] = ssl.CERT_REQUIRED
+            else:
+                log.info("<RUN10539515I>", "Running INSECURE")
+
+            # Start the server with a timeout_graceful_shutdown
+            # if not set in config, this is None and unvicorn accepts None or number of seconds
+            unvicorn_timeout_graceful_shutdown = (
+                get_config().runtime.http.server_shutdown_grace_period_seconds
+            )
+            config = uvicorn.Config(
+                self.app,
+                host="0.0.0.0",
+                port=self.port,
+                log_level=None,
+                log_config=None,
+                timeout_graceful_shutdown=unvicorn_timeout_graceful_shutdown,
+                **tls_kwargs,
+            )
+            # Make sure the config loads TLS files here so they can safely be
+            # deleted if they're ephemeral
+            config.load()
+
+        # Start the server with the loaded config
         self.server = uvicorn.Server(config=config)
 
         # Patch the exit handler to call this server's stop
@@ -532,6 +551,50 @@ class RuntimeHTTPServer(RuntimeServerBase):
         log.debug4("Server healthy")
         return "OK"
 
+    @contextmanager
+    def _tls_files(self) -> _TlsFiles:
+        """This contextmanager ensures that the tls config values are files on
+        disk since SslContext requires files
+
+        Returns:
+            tls_files (_TlsFiles): The set of configured TLS files
+        """
+        tls_cfg = self.tls_config or {}
+        tls_opts = {
+            "server_key": tls_cfg.get("server", {}).get("key", ""),
+            "server_cert": tls_cfg.get("server", {}).get("cert", ""),
+            "client_cert": tls_cfg.get("client", {}).get("cert", ""),
+        }
+        non_file_opts = {
+            key: val for key, val in tls_opts.items() if val and not os.path.isfile(val)
+        }
+        if not non_file_opts:
+            log.debug3("No TLS inline files configured")
+            yield _TlsFiles(**tls_opts)
+            return
+        # If any of the values are set and are not pointing to files, save them
+        # to a temporary directory
+        try:
+            with tempfile.TemporaryDirectory() as tls_dir:
+                updated_files = {
+                    key: val
+                    for key, val in tls_opts.items()
+                    if key not in non_file_opts and val
+                }
+                for fname, content in non_file_opts.items():
+                    temp_fname = os.path.join(tls_dir, fname)
+                    updated_files[fname] = temp_fname
+                    with open(temp_fname, "w", encoding="utf-8") as handle:
+                        handle.write(content)
+                yield _TlsFiles(**updated_files)
+        except OSError as err:
+            log.error(
+                "<RUN80977064E>",
+                "Cannot create temporary TLS files. Either pass config as file paths or run with write permissions.",
+                exc_info=True,
+            )
+            raise ValueError() from err
+
 
 def main(blocking: bool = True):
     server = RuntimeHTTPServer()


Test patch:
diff --git a/tests/runtime/http_server/test_http_server.py b/tests/runtime/http_server/test_http_server.py
index 5eff286e..b69bc73b 100644
--- a/tests/runtime/http_server/test_http_server.py
+++ b/tests/runtime/http_server/test_http_server.py
@@ -31,7 +31,7 @@ import requests
 import tls_test_tools
 
 # Local
-from caikit.core import MODEL_MANAGER
+from caikit.core import MODEL_MANAGER, DataObjectBase, dataobject
 from caikit.runtime import http_server
 from tests.conftest import temp_config
 from tests.runtime.conftest import (
@@ -56,40 +56,52 @@ def save_key_cert_pair(prefix, workdir, key=None, cert=None):
     return crtfile, keyfile
 
 
-@dataclass
-class KeyPair:
+@dataobject
+class KeyPair(DataObjectBase):
     cert: str
     key: str
 
 
-@dataclass
-class TLSConfig:
+@dataobject
+class TLSConfig(DataObjectBase):
     server: KeyPair
     client: KeyPair
 
 
 @contextmanager
 def generate_tls_configs(
-    port: int, tls: bool = False, mtls: bool = False, **http_config_overrides
+    port: int,
+    tls: bool = False,
+    mtls: bool = False,
+    inline: bool = False,
+    **http_config_overrides,
 ) -> Dict[str, Dict]:
     """Helper to generate tls configs"""
     with tempfile.TemporaryDirectory() as workdir:
         config_overrides = {}
         client_keyfile, client_certfile, ca_certfile = None, None, None
+        ca_cert, server_cert, server_key = None, None, None
         if mtls or tls:
             ca_key = tls_test_tools.generate_key()[0]
             ca_cert = tls_test_tools.generate_ca_cert(ca_key)
             ca_certfile, _ = save_key_cert_pair("ca", workdir, cert=ca_cert)
+            server_key, server_cert = tls_test_tools.generate_derived_key_cert_pair(
+                ca_key=ca_key
+            )
             server_certfile, server_keyfile = save_key_cert_pair(
-                "server",
-                workdir,
-                *tls_test_tools.generate_derived_key_cert_pair(ca_key=ca_key),
+                "server", workdir, server_key, server_cert
             )
 
-            tls_config = TLSConfig(
-                server=KeyPair(cert=server_certfile, key=server_keyfile),
-                client=KeyPair(cert="", key=""),
-            )
+            if inline:
+                tls_config = TLSConfig(
+                    server=KeyPair(cert=server_cert, key=server_key),
+                    client=KeyPair(cert=ca_cert if mtls else "", key=""),
+                )
+            else:
+                tls_config = TLSConfig(
+                    server=KeyPair(cert=server_certfile, key=server_keyfile),
+                    client=KeyPair(cert=ca_certfile if mtls else "", key=""),
+                )
             # need to save this ca_certfile in config_overrides so the tls tests below can access it from client side
             config_overrides["use_in_test"] = {"ca_cert": ca_certfile}
 
@@ -108,12 +120,11 @@ def generate_tls_configs(
                     workdir,
                     *tls_test_tools.generate_derived_key_cert_pair(ca_key=ca_key),
                 )
-                tls_config.client = KeyPair(cert=ca_certfile, key="")
                 # need to save the client cert and key in config_overrides so the mtls test below can access it
                 config_overrides["use_in_test"]["client_cert"] = client_certfile
                 config_overrides["use_in_test"]["client_key"] = client_keyfile
 
-            config_overrides["runtime"] = {"tls": tls_config}
+            config_overrides["runtime"] = {"tls": tls_config.to_dict()}
         config_overrides.setdefault("runtime", {})["http"] = {
             "server_shutdown_grace_period_seconds": 0.01,  # this is so the server is killed after 0.1 if no test is running
             "port": port,
@@ -186,6 +197,29 @@ def test_mutual_tls_server(open_port):
             resp.raise_for_status()
 
 
+def test_mutual_tls_server_inline(open_port):
+    """Test that mutual TLS works when the TLS content is passed by value rather
+    than with files
+    """
+    with generate_tls_configs(
+        open_port, tls=True, mtls=True, inline=True, http_config_overrides={}
+    ) as config_overrides:
+        with runtime_http_test_server(
+            open_port,
+            tls_config_override=config_overrides,
+        ) as http_server_with_mtls:
+            # start a non-blocking http server with mutual tls
+            resp = requests.get(
+                f"https://localhost:{http_server_with_mtls.port}/docs",
+                verify=config_overrides["use_in_test"]["ca_cert"],
+                cert=(
+                    config_overrides["use_in_test"]["client_cert"],
+                    config_overrides["use_in_test"]["client_key"],
+                ),
+            )
+            resp.raise_for_status()
+
+
 def test_mutual_tls_server_with_wrong_cert(open_port):
     with generate_tls_configs(
         open_port, tls=True, mtls=True, http_config_overrides={}