Skip to content

Add auth server TLS listener and SPIFFE middleware - #6843

Merged
jhrozek merged 1 commit into
mainfrom
spiffe-v2-03-tls-listener
Oct 21, 2026
Merged

jhrozek merged 1 commit into
mainfrom
spiffe-v2-03-tls-listener

Conversation

@jhrozek

@jhrozek jhrozek commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

SPIFFE X.509-SVID client authentication needs the client certificate
from a TLS handshake, but the MCP proxy port serves plain HTTP. Give
the embedded authorization server its own TLS listener on port 8443,
so the token endpoint can see client certificates without moving MCP
traffic to HTTPS.

The listener requests client certificates only when spiffe_x509 is
configured, sends the SPIFFE CAs as a hint, and reloads the serving
certificate when its files change, keeping the last good pair.
spiffeauth.Middleware extracts a claimed SPIFFE ID from the presented
peer certificate into the request context; it does not verify the
certificate. Malformed SPIFFE identities receive an OAuth JSON error.
Credential validation remains the responsibility of the client
authentication strategy.

EmbeddedAuthServer gains Start, and the proxy runner starts the
listener on the same host as the MCP port, rejecting a proxy or
diagnostics port that collides with it. Constructors reject configured
TLS listeners without a listener host so unsupported callers fail
explicitly. Graceful shutdown force-closes connections on timeout.

SPIFFE trust configuration remains rejected at startup; this change
does not yet enable SPIFFE client authentication.

Co-authored-by: Mecatl noreply@mecatl.dev

@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Oct 16, 2026
@ChrisJBurns

Copy link
Copy Markdown
Collaborator

Is there a perhaps a breaking change here due to the func signature changing? Nothing in codebase but any out of tree callers?

@jhrozek

jhrozek commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Good question. The change is source-compatible for anyone calling the constructors directly, because the new opts parameter is variadic. It would only break code that stores NewEmbeddedAuthServer or NewEmbeddedAuthServerWithStorage as a func value, or implements an interface or mock against the old signature.

I checked all in-repo callers (pkg/runner, pkg/vmcp/cli, the integration helpers) and the downstream consumers I know of. All of them call the constructors directly, so none breaks.

One behavior change to be aware of: the constructors now reject a configured TLS listener when no WithListenerHost is passed. That only applies to callers that set the new TLS listener config, which doesn't exist yet in any released version, so existing callers are unaffected.

@jhrozek
jhrozek force-pushed the spiffe-v2-03-tls-listener branch from e0bf21b to ab06841 Compare October 19, 2026 14:17
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Oct 19, 2026
@jhrozek
jhrozek force-pushed the spiffe-v2-03-tls-listener branch from ab06841 to 3a16520 Compare October 20, 2026 09:17
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Oct 20, 2026
@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.30159% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.77%. Comparing base (56fd4df) to head (84ad39b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/authserver/runner/embeddedauthserver.go 82.60% 12 Missing ⚠️
pkg/authserver/runner/tls_listener.go 90.98% 11 Missing ⚠️
pkg/runner/runner.go 55.55% 8 Missing ⚠️
pkg/authserver/spiffe/middleware.go 96.29% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6843    +/-   ##
========================================
  Coverage   79.76%   79.77%            
========================================
  Files         825      827     +2     
  Lines       84263    84498   +235     
========================================
+ Hits        67212    67405   +193     
- Misses      17046    17088    +42     
  Partials        5        5            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jhrozek
jhrozek force-pushed the spiffe-v2-03-tls-listener branch from 3a16520 to dd599c0 Compare October 20, 2026 18:17
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Oct 20, 2026
SPIFFE X.509-SVID client authentication needs the client certificate
from a TLS handshake, but the MCP proxy port serves plain HTTP. Give
the embedded authorization server its own TLS listener on port 8443,
so the token endpoint can see client certificates without moving MCP
traffic to HTTPS.

The listener requests client certificates only when spiffe_x509 is
configured, sends the SPIFFE CAs as a hint, and reloads the serving
certificate when its files change, keeping the last good pair.
spiffeauth.Middleware extracts a claimed SPIFFE ID from the presented
peer certificate into the request context; it does not verify the
certificate. Malformed SPIFFE identities receive an OAuth JSON error.
Credential validation remains the responsibility of the client
authentication strategy.

EmbeddedAuthServer gains Start, and the proxy runner starts the
listener on the same host as the MCP port, rejecting a proxy or
diagnostics port that collides with it. Constructors reject configured
TLS listeners without a listener host so unsupported callers fail
explicitly. Graceful shutdown force-closes connections on timeout.

SPIFFE trust configuration remains rejected at startup; this change
does not yet enable SPIFFE client authentication.

Co-authored-by: Mecatl <noreply@mecatl.dev>
@jhrozek
jhrozek force-pushed the spiffe-v2-03-tls-listener branch from 4a83fa2 to 84ad39b Compare October 20, 2026 19:44
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Oct 20, 2026
@jhrozek
jhrozek merged commit ad40e16 into main Oct 21, 2026
67 of 69 checks passed
@jhrozek
jhrozek deleted the spiffe-v2-03-tls-listener branch October 21, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants