Skip to content

Add TLS support for incoming MCP server connections #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dprince wants to merge 1 commit into
base: main
Choose a base branch
from
+26 −1
Open

Add TLS support for incoming MCP server connections #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions config.yaml.sample
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ openshift:
allow_write: false
insecure: false

# Incoming TLS settings for the MCP server
#tls:
# ssl_certfile: ./cert.pem
# ssl_keyfile: ./key.pem
# ssl_keyfile_password: null
# ssl_ca_certs: null

mcp_transport_security:
token: supersecret
enable_dns_rebinding_protection: false
Expand Down
10 changes: 10 additions & 0 deletions src/rhos_ls_mcps/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ def main():

# Pass string instead of an instance to support multiple workers.
# Pass the factory=True argument to use a function (create_app) instead of a variable.
ssl_kwargs = {}
if config.tls.ssl_certfile:
ssl_kwargs["ssl_certfile"] = config.tls.ssl_certfile
ssl_kwargs["ssl_keyfile"] = config.tls.ssl_keyfile
if config.tls.ssl_keyfile_password:
ssl_kwargs["ssl_keyfile_password"] = config.tls.ssl_keyfile_password
if config.tls.ssl_ca_certs:
ssl_kwargs["ssl_ca_certs"] = config.tls.ssl_ca_certs
Comment on lines +112 to +118

@coderabbitai coderabbitai Bot May 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In current Uvicorn docs, does setting ssl_ca_certsalone enforce client certificate verification, or mustssl_cert_reqsalso be set (e.g.,ssl.CERT_REQUIRED)?

💡 Result:

Setting ssl_ca_certs alone does not enforce client certificate verification in current Uvicorn docs; you must also set ssl_cert_reqs (e.g., ssl.CERT_REQUIRED). Uvicorn’s settings documentation describes --ssl-cert-reqs as “Whether client certificate is required” and states the default is ssl.CERT_NONE, i.e., no client cert requirement [1]. The same documentation lists --ssl-ca-certs as the CA certificates file, but it does not say that providing CA certs changes the “required/optional/none” behavior on its own [1]. The Uvicorn CLI options ultimately map to Python’s SSLContext verify mode; ssl_cert_reqs controls ctx.verify_mode, while ssl_ca_certs is used to load trust roots for verification when verification is enabled [2]. Therefore, for mutual TLS enforcement (clients must present valid certificates), set both ssl_ca_certs and ssl_cert_reqs=ssl.CERT_REQUIRED [1][2].

Citations:

🏁 Script executed:

# Check the file exists and read the relevant section
wc -l src/rhos_ls_mcps/main.py

Repository: openstack-lightspeed/rhos-mcps

Length of output: 103

🏁 Script executed:

# Read the TLS configuration section around lines 112-118
sed -n '100,130p' src/rhos_ls_mcps/main.py | cat -n

Repository: openstack-lightspeed/rhos-mcps

Length of output: 1496

🏁 Script executed:

# Check if ssl module is imported
grep -n "^import ssl\|^from ssl" src/rhos_ls_mcps/main.py

Repository: openstack-lightspeed/rhos-mcps

Length of output: 56

🏁 Script executed:

# Check if ssl_cert_reqs is set anywhere in the file
grep -n "ssl_cert_reqs" src/rhos_ls_mcps/main.py

Repository: openstack-lightspeed/rhos-mcps

Length of output: 56

mTLS is not actually enabled when only ssl_ca_certs is provided.

At line 118, CA certs are passed to the SSL configuration, but ssl_cert_reqs is never set. Without this, client certificates are not required or verified. According to Uvicorn's SSL documentation, ssl_cert_reqs controls whether client certificates are enforced (default is ssl.CERT_NONE); providing ssl_ca_certs alone does not change this behavior. Both ssl_ca_certs and ssl_cert_reqs=ssl.CERT_REQUIRED are required for mutual TLS enforcement.

🔧 Proposed fix 
+import ssl
...
     if config.tls.ssl_certfile:
         ssl_kwargs["ssl_certfile"] = config.tls.ssl_certfile
         ssl_kwargs["ssl_keyfile"] = config.tls.ssl_keyfile
         if config.tls.ssl_keyfile_password:
             ssl_kwargs["ssl_keyfile_password"] = config.tls.ssl_keyfile_password
         if config.tls.ssl_ca_certs:
             ssl_kwargs["ssl_ca_certs"] = config.tls.ssl_ca_certs
+            ssl_kwargs["ssl_cert_reqs"] = ssl.CERT_REQUIRED
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if config.tls.ssl_certfile:
ssl_kwargs["ssl_certfile"] = config.tls.ssl_certfile
ssl_kwargs["ssl_keyfile"] = config.tls.ssl_keyfile
if config.tls.ssl_keyfile_password:
ssl_kwargs["ssl_keyfile_password"] = config.tls.ssl_keyfile_password
if config.tls.ssl_ca_certs:
ssl_kwargs["ssl_ca_certs"] = config.tls.ssl_ca_certs
if config.tls.ssl_certfile:
ssl_kwargs["ssl_certfile"] = config.tls.ssl_certfile
ssl_kwargs["ssl_keyfile"] = config.tls.ssl_keyfile
if config.tls.ssl_keyfile_password:
ssl_kwargs["ssl_keyfile_password"] = config.tls.ssl_keyfile_password
if config.tls.ssl_ca_certs:
ssl_kwargs["ssl_ca_certs"] = config.tls.ssl_ca_certs
ssl_kwargs["ssl_cert_reqs"] = ssl.CERT_REQUIRED
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/rhos_ls_mcps/main.py` around lines 112 - 118, When
config.tls.ssl_ca_certs is provided we currently add it to ssl_kwargs but never
set ssl_cert_reqs, so mutual TLS is not enforced; update the block that builds
ssl_kwargs (the code that reads config.tls.ssl_certfile, ssl_keyfile,
ssl_keyfile_password, ssl_ca_certs) to also set ssl_kwargs["ssl_cert_reqs"] =
ssl.CERT_REQUIRED whenever config.tls.ssl_ca_certs is present (import ssl if not
already), ensuring that ssl_ca_certs + ssl_cert_reqs=ssl.CERT_REQUIRED are both
set to enable mTLS enforcement.

@lpiwowar lpiwowar May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dprince can you please resolve this comment please? You either accept the proposal by the AI or not in such a case you have to provide an explanation (and close the comment). We follow the rule that CodeRabbit comments need to be addressed first before human review.

@coderabbitai coderabbitai Bot May 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


uvicorn.run(
"rhos_ls_mcps.main:create_app",
host=config.ip,
Expand All @@ -118,6 +127,7 @@ def main():
access_log=False,
factory=True,
log_config=log_config,
**ssl_kwargs,
)


Expand Down
10 changes: 9 additions & 1 deletion src/rhos_ls_mcps/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,18 @@ class OpenShiftSettings(BaseSettings):
blocked_commands: list[str] = Field(default=oc_defaults.DEFAULT_BLOCKED_COMMANDS, description="Explicitly blocked commands")


class TLSSettings(BaseSettings):
ssl_certfile: Optional[str] = Field(default=None, description="Path to SSL certificate file for incoming TLS")
ssl_keyfile: Optional[str] = Field(default=None, description="Path to SSL private key file for incoming TLS")
ssl_keyfile_password: Optional[str] = Field(default=None, description="Password for encrypted SSL key file")
ssl_ca_certs: Optional[str] = Field(default=None, description="Path to CA certs file for client certificate verification (mutual TLS)")


class TransportSecuritySettings(BaseSettings):
token: Optional[str] = Field(default=os.environ.get("MCP_SECURITY_TOKEN"), description="Token to use for basic authentication (Env: MCP_SECURITY_TOKEN)")
enable_dns_rebinding_protection: bool = Field(default=False, description="Enable DNS rebinding protection")
allowed_hosts: list[str] = Field(default=["*:*"], description="Allowed hosts")
allowed_origins: list[str] = Field(default=["http://*:*"], description="Allowed origins")
allowed_origins: list[str] = Field(default=["http://*:*", "https://*:*"], description="Allowed origins")


class Settings(BaseSettings):
Expand All @@ -44,6 +51,7 @@ class Settings(BaseSettings):
openstack: OpenStackSettings = Field(default=OpenStackSettings(), description="OpenStack settings")
openshift: OpenShiftSettings = Field(default=OpenShiftSettings(), description="OpenShift settings")
mcp_transport_security: TransportSecuritySettings = Field(default=TransportSecuritySettings(), description="Transport security settings")
tls: TLSSettings = Field(default=TLSSettings(), description="Incoming TLS settings for the MCP server")


def load_config():
Expand Down
Loading