Skip to content

ASI-07: Add mTLS and HMAC request signing for agent↔gateway communication#4679

Merged
lpcox merged 4 commits into
mainfrom
copilot/asi-07-add-mtls-message-signing
Apr 28, 2026
Merged

ASI-07: Add mTLS and HMAC request signing for agent↔gateway communication#4679
lpcox merged 4 commits into
mainfrom
copilot/asi-07-add-mtls-message-signing

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

Per OWASP Agentic Top 10 ASI-07, the MCP gateway communicated over plain HTTP with only an API key — no transport encryption, no message signing, no replay protection. This adds opt-in mTLS and HMAC-SHA256 request signing.

Phase 1: mTLS

New flags/env vars: --tls-cert / MCP_GATEWAY_TLS_CERT, --tls-key / MCP_GATEWAY_TLS_KEY, --tls-ca / MCP_GATEWAY_CA_CERT.

  • internal/server/gateway_tls.goLoadGatewayTLS(cert, key, ca): loads PEM files, sets tls.RequireAndVerifyClientCert when ca is non-empty (mTLS)
  • internal/cmd/root.go — wraps the TCP listener with TLS post-bind; plain HTTP remains default (no certs = no change in behaviour)
  • Partial TLS flag validation: --tls-cert and --tls-key must both be provided together; --tls-ca requires cert+key to also be set — prevents silent plaintext fallback from incomplete config
  • writeGatewayConfig now receives a tlsEnabled flag so emitted server URLs use https:// when TLS is active

Phase 2: HMAC Request Signing + Replay Protection

New flag/env var: --hmac-secret / MCP_GATEWAY_HMAC_SECRET.

  • internal/server/hmac.gohmacMiddleware injected into wrapWithMiddleware (both routed and unified modes); API key auth runs before HMAC so unauthenticated requests are rejected before paying the body-read cost
  • Validates three headers per request:
    • X-MCP-Timestamp — must be within ±30 s of server clock
    • X-MCP-Nonce — must not have been seen before (in-process cache, 60 s TTL)
    • X-MCP-SignatureHMAC-SHA256(secret, "timestamp\nnonce\npath\nhex(sha256(body))")
  • Nonce is recorded only after successful signature verification — prevents DoS cache poisoning via requests with invalid signatures
  • A read-only seenNonce pre-check provides fast replay rejection before the body-read step
  • HMAC applies to /mcp handlers only; common endpoints (/health, /close) are not HMAC-protected
# Enable one-way TLS
awmg --tls-cert server.crt --tls-key server.key --config config.toml --routed

# Enable mTLS (client cert required)
awmg --tls-cert server.crt --tls-key server.key --tls-ca ca.crt ...

# Enable HMAC signing (can combine with TLS)
awmg --hmac-secret $(openssl rand -hex 32) ...

Both features are opt-in and backward compatible — all flags default to empty/disabled, so omitting them leaves plain-HTTP + API-key behaviour completely unchanged.

Test coverage

  • gateway_tls_test.go: server-only TLS, mTLS config, live mTLS handshake (proper CA + client cert with ExtKeyUsageClientAuth), error paths
  • hmac_test.go: valid signature, missing headers, stale/future timestamps, replay detection, wrong secret, nonce cache concurrency, nonce cache DoS prevention (TestHMACMiddleware_InvalidSigDoesNotPoisonNonceCache), seenNonce read-only pre-check (TestNonceCache_SeenNonce)
  • stdout_config_test.go: TestWriteGatewayConfig_TLSScheme verifies http:// vs https:// URL emission based on TLS state

Copilot AI and others added 2 commits April 27, 2026 18:10
…munication

- Add TLS server support: --tls-cert, --tls-key, --tls-ca flags and
  MCP_GATEWAY_TLS_CERT/MCP_GATEWAY_TLS_KEY/MCP_GATEWAY_CA_CERT env vars.
  Gateway starts plain HTTP if no certs are configured (backward compatible).
- Add mTLS client cert verification when --tls-ca / MCP_GATEWAY_CA_CERT is set.
- Add HMAC-SHA256 request signing middleware: --hmac-secret flag and
  MCP_GATEWAY_HMAC_SECRET env var. Each request must carry
  X-MCP-Timestamp, X-MCP-Nonce, and X-MCP-Signature headers.
  Enforces 30-second timestamp window and per-nonce replay protection.
- Middleware is backward compatible: disabled when no secret is configured.
- Comprehensive tests for TLS loading, mTLS handshake, HMAC validation,
  replay protection, and all rejection paths.

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/be0d13b4-c2b0-49cd-9f9b-0f0680c26634

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Add mTLS and message signing for agent↔gateway communication ASI-07: Add mTLS and HMAC request signing for agent↔gateway communication Apr 27, 2026
Copilot AI requested a review from lpcox April 27, 2026 18:14
@lpcox lpcox marked this pull request as ready for review April 27, 2026 18:40
Copilot AI review requested due to automatic review settings April 27, 2026 18:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds opt-in transport- and message-level protections for agent↔gateway traffic (ASI-07), introducing TLS (including mTLS) at the listener level and HMAC request signing with replay protection in the HTTP middleware stack.

Changes:

  • Add CLI/env flags for TLS cert/key/CA and an HMAC secret.
  • Add TLS config loader and tests covering one-way TLS + mTLS handshake/error paths.
  • Add HMAC middleware with timestamp/nonce/signature validation and extensive unit tests; plumb it into routed + unified server creation.
Show a summary per file
File Description
internal/cmd/root.go Switches server startup to httpServer.Serve(listener) and optionally wraps listener with TLS; passes HMAC secret into server constructors.
internal/cmd/flags_tls.go Adds TLS/HMAC CLI flags with env var defaults.
internal/server/gateway_tls.go Implements LoadGatewayTLS to load server cert/key and optionally require/verify client certs using a CA.
internal/server/gateway_tls_test.go Tests TLS config loading + live mTLS handshake and error paths.
internal/server/hmac.go Adds HMAC request-signing middleware, nonce cache, and helper for conditional enablement.
internal/server/hmac_test.go Adds unit tests for HMAC validation scenarios and nonce cache concurrency.
internal/server/http_helpers.go Extends wrapWithMiddleware to include HMAC in the middleware stack.
internal/server/transport.go Extends unified server creation signature to accept HMAC secret and wires it into middleware wrapping.
internal/server/routed.go Extends routed server creation signature to accept HMAC secret and wires it into middleware wrapping.
internal/server/*_test.go (multiple) Updates existing tests to match new function signatures (adds hmacSecret argument).

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 18/18 changed files
  • Comments generated: 7

Comment thread internal/cmd/root.go Outdated
Comment on lines +411 to +425
if tlsCertPath != "" && tlsKeyPath != "" {
tlsCfg, err = server.LoadGatewayTLS(tlsCertPath, tlsKeyPath, tlsCAPath)
if err != nil {
_ = listener.Close()
return fmt.Errorf("failed to configure TLS: %w", err)
}
listener = tls.NewListener(listener, tlsCfg)
scheme := "https"
if tlsCAPath != "" {
scheme = "https (mTLS)"
}
logger.StartupInfo("TLS enabled: cert=%s, key=%s, ca=%s — listening on %s://%s", tlsCertPath, tlsKeyPath, tlsCAPath, scheme, listenAddr)
} else {
logger.StartupInfo("TLS not configured — listening on http://%s (set --tls-cert/--tls-key to enable)", listenAddr)
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

TLS enablement is treated as “on” only when both --tls-cert and --tls-key are set; if a user accidentally provides only one (or sets --tls-ca without cert/key), the server silently falls back to plain HTTP. This is risky because it can lead to unintended plaintext deployments. Consider returning an error when any TLS-related flag is set without the complete required set (cert+key, and optional CA).

Copilot uses AI. Check for mistakes.
Comment thread internal/cmd/root.go
Comment on lines +404 to +432
// Build net.Listener — optionally wrapping with TLS (ASI-07 Phase 1).
// Plain HTTP is still used when no TLS certificate is configured (backward compatible).
listener, err := net.Listen("tcp", listenAddr)
if err != nil {
return fmt.Errorf("failed to listen on %s: %w", listenAddr, err)
}
var tlsCfg *tls.Config
if tlsCertPath != "" && tlsKeyPath != "" {
tlsCfg, err = server.LoadGatewayTLS(tlsCertPath, tlsKeyPath, tlsCAPath)
if err != nil {
_ = listener.Close()
return fmt.Errorf("failed to configure TLS: %w", err)
}
listener = tls.NewListener(listener, tlsCfg)
scheme := "https"
if tlsCAPath != "" {
scheme = "https (mTLS)"
}
logger.StartupInfo("TLS enabled: cert=%s, key=%s, ca=%s — listening on %s://%s", tlsCertPath, tlsKeyPath, tlsCAPath, scheme, listenAddr)
} else {
logger.StartupInfo("TLS not configured — listening on http://%s (set --tls-cert/--tls-key to enable)", listenAddr)
}
if hmacSecret != "" {
logger.StartupInfo("HMAC request signing enabled (ASI-07)")
}

// Start HTTP server in background
go func() {
if err := httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed {
if err := httpServer.Serve(listener); err != nil && err != http.ErrServerClosed {
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

When TLS is enabled via --tls-cert/--tls-key, the server will speak HTTPS on the listener, but writeGatewayConfigToStdout still emits http://... URLs. This will cause clients that consume the emitted config to attempt plaintext connections and fail. Consider threading the effective scheme (http/https) into the config writer (or deriving it from the TLS flags) so the emitted URLs match the actual listener.

Copilot uses AI. Check for mistakes.
Comment thread internal/server/transport.go Outdated
// If apiKey is provided, all requests except /health require authentication (spec 7.1)
func CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey string) *http.Server {
logTransport.Printf("Creating HTTP server for MCP: addr=%s, auth_enabled=%v", addr, apiKey != "")
// If hmacSecret is provided, each request must carry a valid HMAC-SHA256 signature (ASI-07)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The doc comment says “each request must carry a valid HMAC… signature”, but the HMAC middleware is only applied via wrapWithMiddleware to the /mcp handlers; the common endpoints registered by registerCommonEndpoints (notably /close) are not HMAC-protected. Either tighten the implementation so HMAC is enforced on sensitive common endpoints too, or narrow the wording to “each MCP request” to avoid a misleading security guarantee.

Suggested change
// If hmacSecret is provided, each request must carry a valid HMAC-SHA256 signature (ASI-07)
// If hmacSecret is provided, each MCP request must carry a valid HMAC-SHA256 signature (ASI-07)

Copilot uses AI. Check for mistakes.
Comment thread internal/server/routed.go Outdated
// Multiple routes from the same Authorization header share a session
// If apiKey is provided, all requests except /health require authentication (spec 7.1)
func CreateHTTPServerForRoutedMode(addr string, unifiedServer *UnifiedServer, apiKey string) *http.Server {
// If hmacSecret is provided, each request must carry a valid HMAC-SHA256 signature (ASI-07)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

This comment promises “each request must carry a valid HMAC… signature”, but HMAC is only applied to the /mcp/<server> handlers via wrapWithMiddleware; endpoints like /health and /close are registered separately and are not HMAC-protected. Either apply HMAC to the common endpoints as well (at least /close), or narrow the wording to avoid overstating the protection.

Suggested change
// If hmacSecret is provided, each request must carry a valid HMAC-SHA256 signature (ASI-07)
// If hmacSecret is provided, routed /mcp/<server> requests must carry a valid
// HMAC-SHA256 signature (ASI-07); common endpoints are registered separately.

Copilot uses AI. Check for mistakes.
Comment thread internal/server/http_helpers.go Outdated
Comment on lines 254 to 262
// Apply auth middleware if API key is configured (spec 7.1)
authedHandler := applyAuthIfConfigured(apiKey, shutdownHandler.ServeHTTP)

// Apply HMAC signature verification if secret is configured (ASI-07)
hmacHandler := applyHMACIfConfigured(hmacSecret, authedHandler)

// Wrap with OTEL tracing span (outermost, so it covers auth + shutdown + logging)
tracingHandler := WithOTELTracing(authedHandler, logTag)
tracingHandler := WithOTELTracing(hmacHandler, logTag)

Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

wrapWithMiddleware currently wraps the authed handler with applyHMACIfConfigured, which means HMAC validation runs before API key auth. This contradicts the comment (“auth → HMAC”) and the PR description (“after API key auth”), and it also makes unauthenticated requests pay the HMAC/body-read cost. Swap the wrapping order so that API key auth executes before HMAC validation (i.e., have auth middleware wrap the HMAC-wrapped handler).

See below for a potential fix:

	// Apply HMAC signature verification if secret is configured (ASI-07)
	hmacHandler := applyHMACIfConfigured(hmacSecret, shutdownHandler.ServeHTTP)

	// Apply auth middleware if API key is configured (spec 7.1)
	authedHandler := applyAuthIfConfigured(apiKey, hmacHandler)

	// Wrap with OTEL tracing span (outermost, so it covers auth + HMAC + shutdown + logging)
	tracingHandler := WithOTELTracing(authedHandler, logTag)

Copilot uses AI. Check for mistakes.
Comment thread internal/server/hmac.go Outdated
Comment on lines +123 to +128
// Validate nonce uniqueness (replay protection)
if !cache.checkAndSet(nonce) {
logHMAC.Printf("HMAC rejected: replay detected nonce=%s, remote=%s", nonce, r.RemoteAddr)
rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "HMAC nonce already used (replay detected)", "auth", "hmac_validation_failed", "replay_detected")
return
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The nonce is recorded (checkAndSet) before the signature is verified. This lets any caller (even with an invalid signature) consume nonces and grow the cache, creating an easy DoS vector and potentially causing legitimate requests to be rejected if they reuse a nonce. Only mark the nonce as “seen” after the HMAC signature has been validated successfully (and consider rejecting obviously malformed nonces before any caching).

Copilot uses AI. Check for mistakes.
Comment thread internal/cmd/root.go Outdated
Comment on lines +419 to +422
if tlsCAPath != "" {
scheme = "https (mTLS)"
}
logger.StartupInfo("TLS enabled: cert=%s, key=%s, ca=%s — listening on %s://%s", tlsCertPath, tlsKeyPath, tlsCAPath, scheme, listenAddr)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The startup log builds scheme as "https (mTLS)" and then formats it as %s://%s, which produces an invalid URL-like string (https (mTLS)://...). Keep the scheme as https for the URL and append an “mTLS enabled” note separately so logs remain parseable/copy-pastable.

Suggested change
if tlsCAPath != "" {
scheme = "https (mTLS)"
}
logger.StartupInfo("TLS enabled: cert=%s, key=%s, ca=%s — listening on %s://%s", tlsCertPath, tlsKeyPath, tlsCAPath, scheme, listenAddr)
tlsModeNote := ""
if tlsCAPath != "" {
tlsModeNote = " (mTLS enabled)"
}
logger.StartupInfo("TLS enabled: cert=%s, key=%s, ca=%s — listening on %s://%s%s", tlsCertPath, tlsKeyPath, tlsCAPath, scheme, listenAddr, tlsModeNote)

Copilot uses AI. Check for mistakes.
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 27, 2026

@copilot address this review feedback #4679 (review)

- Fix nonce DoS: record nonce only after successful signature verification
  (invalid-sig requests can no longer poison the replay-protection cache);
  add read-only seenNonce fast-reject before body read
- Fix middleware ordering: API key auth now runs before HMAC so unauthenticated
  requests are rejected cheaply without paying body-read overhead
- Fix log message: https (mTLS):// → https:// with separate mTLS note
- Add partial-TLS config validation: error when only one of --tls-cert/--tls-key
  is provided, or --tls-ca is set without cert+key
- Thread tlsEnabled into writeGatewayConfig so emitted URLs use https:// when TLS
  is active
- Narrow doc comments on CreateHTTPServerForMCP/ForRoutedMode to accurately state
  that HMAC applies to /mcp handlers, not common endpoints like /health and /close
- Add tests: TestHMACMiddleware_InvalidSigDoesNotPoisonNonceCache,
  TestNonceCache_SeenNonce, TestWriteGatewayConfig_TLSScheme

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/07fc0aad-bc5a-4216-9f18-ed549f648a83

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 27, 2026

@copilot address this review feedback #4679 (review)

Addressed all 7 items from the review in commit 8f56e9f:

  1. Nonce DoScheckAndSet is now called only after successful HMAC signature verification. A read-only seenNonce pre-check handles fast replay rejection before body read, so invalid-signature requests cannot pollute the cache.
  2. Middleware ordering — Auth (applyAuthIfConfigured) now wraps HMAC (applyHMACIfConfigured), so unauthenticated requests are rejected before paying the body-read cost.
  3. Log messagehttps (mTLS)://…https://… (with a separate (mTLS enabled) note appended).
  4. Partial TLS config validation--tls-cert and --tls-key must both be provided together; --tls-ca requires both cert+key to be set. Returns an error for incomplete configurations.
  5. writeGatewayConfig TLS scheme — A tlsEnabled bool parameter is threaded through; emitted server URLs now use https:// when TLS is configured.
  6. Doc comments in transport.go / routed.go — Narrowed to clarify that HMAC applies to /mcp handlers only, not common endpoints like /health and /close.
  7. New testsTestHMACMiddleware_InvalidSigDoesNotPoisonNonceCache, TestNonceCache_SeenNonce, TestWriteGatewayConfig_TLSScheme.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build1994641822/b513/launcher.test /tmp/go-build1994641822/b513/launcher.test -test.testlogfile=/tmp/go-build1994641822/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o kl1s/G0ioUdQerie0acXWkl1s -trimpath x_amd64/vet -p go.opentelemetrydocker-cli-plugin-metadata -lang=go1.25 x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1994641822/b495/config.test /tmp/go-build1994641822/b495/config.test -test.testlogfile=/tmp/go-build1994641822/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s -W 2Fu7_w7IV .cfg 64/pkg/tool/linux_amd64/vet . --gdwarf2 --64 64/pkg/tool/linux_amd64/vet 5798�� om/grpc-ecosystem/grpc-gateway/v2@v2.28.0/runtim-errorsas .cfg 64/pkg/tool/linux_amd64/vet --gdwarf-5 .io/otel/propaga/tmp/go-build1427442246/b418/vet.cfg -o 64/pkg/tool/linux_amd64/vet (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build1994641822/b513/launcher.test /tmp/go-build1994641822/b513/launcher.test -test.testlogfile=/tmp/go-build1994641822/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o kl1s/G0ioUdQerie0acXWkl1s -trimpath x_amd64/vet -p go.opentelemetrydocker-cli-plugin-metadata -lang=go1.25 x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build1994641822/b513/launcher.test /tmp/go-build1994641822/b513/launcher.test -test.testlogfile=/tmp/go-build1994641822/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o kl1s/G0ioUdQerie0acXWkl1s -trimpath x_amd64/vet -p go.opentelemetrydocker-cli-plugin-metadata -lang=go1.25 x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build1994641822/b522/mcp.test /tmp/go-build1994641822/b522/mcp.test -test.testlogfile=/tmp/go-build1994641822/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Apr 28, 2026

@copilot make sure that these options are off by default to remain backward compatible

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 28, 2026

@copilot make sure that these options are off by default to remain backward compatible

All four security options are already off by default and the implementation is fully backward compatible:

  • --tls-cert, --tls-key, --tls-ca all default to "" (also via env vars MCP_GATEWAY_TLS_CERT, MCP_GATEWAY_TLS_KEY, MCP_GATEWAY_CA_CERT). TLS is only activated when both cert+key are non-empty — when neither is set the server starts a plain HTTP listener and logs "TLS not configured — listening on http://...".
  • --hmac-secret defaults to "" (env var MCP_GATEWAY_HMAC_SECRET). applyHMACIfConfigured returns the handler unchanged when the secret is empty, so HMAC validation is never invoked.

No flags set = identical behaviour to before this PR.

@lpcox lpcox merged commit ce1e8c8 into main Apr 28, 2026
17 checks passed
@lpcox lpcox deleted the copilot/asi-07-add-mtls-message-signing branch April 28, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ASI-07: Add mTLS and message signing for agent↔gateway communication

3 participants