[TT-16050]: Added key use in dynamic mtls behind insecure flag

[kofoworola]

Description

TT-16050

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-16050

Status	In Code Review
Summary	[Security] Dynamic mTLS is insecure due to option to use token for authentication

Generated at: 2026-01-30 12:48:18

[probelabs]

This PR introduces a security enhancement for dynamic mTLS by adding a new configuration flag, AllowUnsafeDynamicMTLSToken, which defaults to false. When disabled (the default secure setting), Tyk enforces the presence of a client certificate for APIs configured with dynamic mTLS. This mitigates a security vulnerability where a token could be used for authentication without a corresponding certificate, thereby strengthening the gateway's security posture.

Files Changed Analysis

• config/config.go: A new boolean field AllowUnsafeDynamicMTLSToken is added to SecurityConfig. This serves as a feature flag to control the mTLS authentication logic, with the default (false) enabling stricter security.
• gateway/mw_auth_key.go: The core authentication logic in ProcessRequest is significantly refactored. It now checks the AllowUnsafeDynamicMTLSToken flag for APIs using certificate-based authentication (UseCertificate: true). If the flag is false, the middleware requires a client certificate and rejects requests that provide only a token.
• gateway/handler_error.go: A new error message, MsgAuthCertRequired, is defined to inform clients when a certificate is required but not provided.
• gateway/mw_auth_key_test.go: The testing suite is significantly expanded with two new test functions, TestDynamicMTLSSecure and TestDynamicMTLSInsecure, to validate both the new secure-default behavior and the legacy insecure mode.
• cli/linter/schema.json: The new configuration option is added to the linter's schema to ensure it is recognized as a valid field.
• Other test files (cert_test.go, mw_auth_key_certificate_binding_test.go, mw_auth_key_mtls_combined_test.go) are updated to align with the new secure-by-default behavior, primarily by enabling the insecure flag for existing tests to pass without modification and updating expected status codes from 403 to 401.

Architecture & Impact Assessment

• What this PR accomplishes: This PR hardens the security of dynamic mTLS authentication. Previously, if an API was configured for dynamic mTLS, a request could be authenticated using just a token, even if no client certificate was presented. This change makes the presence of a client certificate mandatory by default, closing this security gap.

• Key technical changes introduced:

  • A new global configuration flag AllowUnsafeDynamicMTLSToken to toggle between strict (default) and legacy mTLS behavior.
  • Modified authentication flow in the AuthKey middleware to enforce certificate presence based on the new flag.
  • Introduction of a new error type (ErrAuthCertRequired) and updated HTTP status codes from 403 Forbidden to 401 Unauthorized for certain auth failures.

• Affected system components: The primary component affected is the Gateway Authentication Middleware (gateway/mw_auth_key.go). Any API definition that uses dynamic mTLS ("use_certificate": true) will be impacted by this change. This is a potential breaking change for clients that previously relied on token-based authentication for these specific APIs without presenting a certificate.

• Authentication Flow Diagram:

graph TD
    A[Request Received] --> B{API uses dynamic mTLS?};
    B -- No --> C[Proceed with other auth];
    B -- Yes --> D{Is `allow_unsafe_dynamic_mtls_token` true?};
    D --|No (Secure Default)|--> E{Client Certificate Present?};
    E -- Yes --> F[Validate Certificate & Proceed];
    E -- No --> G[Reject: 401 Unauthorized - Cert Required];
    D --|Yes (Insecure Mode)|--> H{Token OR Certificate Present?};
    H -- Yes --> I[Validate Credentials & Proceed];
    H -- No --> J[Reject Request];

Loading

Scope Discovery & Context Expansion

• The scope of this change is confined to the gateway's authentication layer, specifically affecting APIs configured with "use_certificate": true in their auth_configs section, as defined in apidef.AuthConfig (apidef/api_definitions.go).
• The impact is significant for these APIs, as client behavior may need to change to comply with the new default security posture. The global flag provides an escape hatch for operators who need to maintain backward compatibility during a transition period.

Metadata

• Review Effort: 4 / 5
• Primary Label: enhancement

---

Powered by Visor from Probelabs

Last updated: 2026-01-30T12:51:00.403Z | Triggered by: pr_updated | Commit: 8455940

💡 TIP: You can chat with Visor using /visor ask <your question>

[github-actions]

API Changes

--- prev.txt	2026-01-30 12:49:08.270714543 +0000
+++ current.txt	2026-01-30 12:48:58.059643516 +0000
@@ -7287,6 +7287,11 @@
 	// Specify public keys used for Certificate Pinning on global level.
 	PinnedPublicKeys map[string]string `json:"pinned_public_keys"`
 
+	// AllowUnsafeDynamicMTLSToken controls whether certificate presence is required for
+	// dynamic mTLS authentication. If set to false (default), requests with a token but
+	// no certificate will be rejected for APIs using dynamic mTLS.
+	AllowUnsafeDynamicMTLSToken bool `json:"allow_unsafe_dynamic_mtls_token"`
+
 	Certificates CertificatesConfig `json:"certificates"`
 
 	// CertificateExpiryMonitor configures the certificate expiry monitoring and notification feature
@@ -9070,6 +9075,7 @@
 const (
 	MsgAuthFieldMissing                        = "Authorization field missing"
 	MsgApiAccessDisallowed                     = "Access to this API has been disallowed"
+	MsgAuthCertRequired                        = "Client certificate required"
 	MsgBearerMailformed                        = "Bearer token malformed"
 	MsgKeyNotAuthorized                        = "Key not authorised"
 	MsgOauthClientRevoked                      = "Key not authorised. OAuth client access was revoked"
@@ -9104,8 +9110,9 @@
 	ErrAuthKeyNotFound               = "auth.key_not_found"
 	ErrAuthCertNotFound              = "auth.cert_not_found"
 	ErrAuthCertExpired               = "auth.cert_expired"
-	ErrAuthCertMismatch              = "auth.cert_mismatch"
 	ErrAuthKeyIsInvalid              = "auth.key_is_invalid"
+	ErrAuthCertRequired              = "auth.cert_required"
+	ErrAuthCertMismatch              = "auth.cert_mismatch"
 
 	MsgNonExistentKey      = "Attempted access with non-existent key."
 	MsgNonExistentCert     = "Attempted access with non-existent cert."

[probelabs]

✅ Security Check Passed

No security issues found – changes LGTM.

Architecture Issues (2)

Severity	Location	Issue
🟠 Error	gateway/mw_auth_key.go:158-171	The authentication logic for the insecure dynamic mTLS mode (`AllowUnsafeDynamicMTLSToken: true`) contains a flaw that alters the legacy behavior it is intended to preserve, and it also includes an unnecessary, redundant session lookup. Logic Flaw: If a request provides an invalid client certificate, the function rejects it at line 162 (ErrAuthCertMismatch) without checking for a valid authentication token that might also be present. The legacy behavior allowed a valid token to grant access, treating the certificate as a secondary factor. This change breaks compatibility for clients who might send a valid token alongside an expired or incorrect certificate. Redundancy: The call to k.CheckSessionAndIdentityForValidKey(key, r) at line 166 is redundant. The session is already checked for the token at the beginning of the function (line 132). This causes an unnecessary roundtrip to the session store, impacting performance. 💡 Suggestion Refactor the logic for the insecure mode to correctly replicate the legacy behavior and remove the redundant call. The logic should prioritize the token if present, and only use the certificate as a fallback or if no token is provided. A possible flow: Use the result of the initial session lookup (from line 132). If the token is valid, authentication should succeed. If the token is invalid or not present, then attempt to authenticate using the client certificate if one is provided. This ensures that a valid token is always honored, preserving the intended legacy behavior.
🟡 Warning	gateway/mw_auth_key.go:105-182	The `ProcessRequest` function has become overly complex and difficult to maintain due to the introduction of the new security flag. The deeply nested conditional logic for handling secure vs. insecure modes, combined with certificate vs. token authentication, significantly increases the cognitive load required to understand the control flow. 💡 Suggestion Refactor the `ProcessRequest` function to improve clarity and separation of concerns. Extract the complex logic for secure and insecure dynamic mTLS into separate, well-named private helper functions. For example: processSecureDynamicMTLS(r *http.Request) (user.SessionState, bool, error, int) processInsecureDynamicMTLS(r *http.Request, key string, initialSession user.SessionState, initialKeyExists bool) (user.SessionState, bool, error, int) This would transform the main ProcessRequest function into a clearer dispatcher, making the distinct logic for each authentication path easier to read, test, and maintain.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	gateway/mw_auth_key.go:101-108	The function `checkSessionWithCertFallback` performs two sequential lookups to the session store (`CheckSessionAndIdentityForValidKey`) if the first key is not found. For requests to dynamic mTLS endpoints, this can introduce an additional network round-trip on a critical authentication path, potentially increasing latency. 💡 Suggestion To optimize, consider using a batch lookup command (e.g., Redis MGET) if the session store driver supports it, allowing both potential keys to be checked in a single network call. This would reduce the latency impact of the fallback mechanism.

Quality Issues (1)

Severity	Location	Issue
🟠 Error	gateway/mw_auth_key.go:176-183	In insecure mode (`AllowUnsafeDynamicMTLSToken: true`), if an API is configured for dynamic mTLS, a request with no credentials (no token and no client certificate) is not rejected. The logic skips both credential checks and proceeds with an unauthenticated session, effectively bypassing authentication. The function should fail fast if no valid credentials are provided. 💡 Suggestion After checking for `certHash` and `key`, add a final check to ensure that a session was actually found. If no credentials were provided or if they were invalid, the request should be rejected with an `ErrAuthAuthorizationFieldMissing` error. The current implementation allows the function to proceed with a `false` value for `keyExists`, which leads to the authentication bypass.

---

Powered by Visor from Probelabs

Last updated: 2026-01-30T12:51:03.132Z | Triggered by: pr_updated | Commit: 8455940

💡 TIP: You can chat with Visor using /visor ask <your question>

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
97.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud