Skip to content

Conversation

@andrei-tyk
Copy link
Contributor

@andrei-tyk andrei-tyk commented Nov 20, 2025

Description

This PR introduces certificate-token binding for static mTLS authentication, providing protection against token theft and misuse in mTLS environments. When enabled via the new Security.EnableCertificateBinding configuration option, access tokens become linked to the client certificate used during authentication, and subsequent requests must present the same certificate or be rejected with a 403 Forbidden error. The implementation adds a new MtlsStaticCertificateBindings field to SessionState to store bound certificate hashes, refactors the AuthKey middleware's certificate validation logic into modular functions that handle both the new binding mode and legacy dynamic mTLS behavior for backward compatibility, and extends CertificateCheckMW to validate certificates against token bindings when they're not in the static whitelist. Comprehensive tests cover success cases with correct certificates, rejection of mismatched or missing certificates, and verification that binding is only enforced when both globally enabled and the session has bindings configured.

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-15640
Status In Code Review
Summary Gateway: [AuthToken] Enforce Token–Certificate binding

Generated at: 2025-11-26 10:18:52

@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2025

🎯 Recommended Merge Targets

Based on JIRA ticket TT-15640: Feature Request: [AuthToken] Enforce Token–Certificate binding

Fix Version: Tyk 5.11.0

⚠️ Warning: Expected release branches not found in repository

Required:

  • master - No matching release branches found. Fix will be included in future releases.

📋 Workflow

  1. Merge this PR to master first
@github-actions
Copy link
Contributor

github-actions bot commented Nov 20, 2025

API Changes

--- prev.txt	2025-11-26 10:19:12.414011167 +0000 +++ current.txt	2025-11-26 10:19:02.484038497 +0000 @@ -7157,6 +7157,13 @@	// CertificateExpiryMonitor configures the certificate expiry monitoring and notification feature	CertificateExpiryMonitor CertificateExpiryMonitorConfig `json:"certificate_expiry_monitor"` + +// DisableCertificateTokenBinding enables certificate-token binding for static mTLS authentication. +// When enabled, access tokens will be linked (bound) to one or more client certificates during creation or update. +// Any subsequent request with that token must present one of the bound certificates, otherwise the request will be rejected. +// This provides protection against token theft and misuse in mTLS environments. +// Environment variable: TYK_GW_SECURITY_ENABLECERTIFICATETOKENBINDING +DisableCertificateTokenBinding bool `json:"disable_certificate_token_binding"` } type ServiceConfig struct { @@ -8956,11 +8963,13 @@	ErrAuthKeyNotFound = "auth.key_not_found"	ErrAuthCertNotFound = "auth.cert_not_found"	ErrAuthCertExpired = "auth.cert_expired" +ErrAuthCertMismatch = "auth.cert_mismatch"	ErrAuthKeyIsInvalid = "auth.key_is_invalid" -MsgNonExistentKey = "Attempted access with non-existent key." -MsgNonExistentCert = "Attempted access with non-existent cert." -MsgInvalidKey = "Attempted access with invalid key." +MsgNonExistentKey = "Attempted access with non-existent key." +MsgNonExistentCert = "Attempted access with non-existent cert." +MsgCertificateMismatch = "Attempted access with incorrect certificate." +MsgInvalidKey = "Attempted access with invalid key." ) const (	KID = "kid" @@ -14823,6 +14832,7 @@	OauthClientID string `json:"oauth_client_id" msg:"oauth_client_id"`	OauthKeys map[string]string `json:"oauth_keys" msg:"oauth_keys"`	Certificate string `json:"certificate" msg:"certificate"` +MtlsStaticCertificateBindings []string `json:"mtls_static_certificate_bindings" msg:"mtls_static_certificate_bindings"`	BasicAuthData BasicAuthData `json:"basic_auth_data" msg:"basic_auth_data"`	JWTData JWTData `json:"jwt_data" msg:"jwt_data"`	HMACEnabled bool `json:"hmac_enabled" msg:"hmac_enabled"`
@probelabs
Copy link

probelabs bot commented Nov 21, 2025

This PR introduces Token-Certificate Binding for mTLS, a security feature that cryptographically links an authentication token to a specific client certificate. When enabled, the gateway enforces that any request using a bound token must also present the corresponding client certificate. This mitigates the risk of a compromised token being used without the associated certificate, as requests with a missing or mismatched certificate are rejected.

Files Changed Analysis

The changes are centered around implementing the token-binding logic in the gateway's authentication middleware.

  • Configuration (config/config.go, cli/linter/schema.json): A new global configuration flag, DisableCertificateTokenBinding, is added to SecurityConfig to enable or disable this feature gateway-wide. The API definition schema is also updated.
  • Session State (user/session.go): The SessionState object is extended with a new field, MtlsStaticCertificateBindings []string, to store the SHA256 hashes of client certificates bound to a token.
  • Core Logic (gateway/mw_auth_key.go): The AuthKey middleware contains the primary enforcement logic. It has been refactored to validate the presented certificate against the bindings stored in the session. The implementation maintains backward compatibility by handling the legacy dynamic mTLS behavior when the new binding feature is not in use.
  • Whitelist Middleware (gateway/mw_certificate_check.go): The certificate whitelist middleware is enhanced with a fallback mechanism. If a presented certificate is not found in the static whitelist, the middleware now attempts to validate it by checking if it is bound to the token provided in the request.
  • Testing (gateway/mw_auth_key_*_test.go): Two comprehensive test suites have been added (mw_auth_key_certificate_binding_test.go, mw_auth_key_mtls_combined_test.go) to cover various scenarios, including success cases with correct certificates, rejection of mismatched certificates, and interactions with existing static mTLS features.

Architecture & Impact Assessment

  • What this PR accomplishes: It enhances security in mTLS environments by preventing token misuse. A stolen token is rendered useless without the corresponding client certificate and its private key.

  • Key technical changes introduced:

    1. Global Configuration: A new gateway-wide flag controls the feature.
    2. Session State Extension: The SessionState now stores an array of certificate hashes, allowing multiple certificates to be bound to a single token.
    3. Refactored Authentication Logic: The AuthKey middleware is the primary enforcer, comparing the presented certificate against hashes in the session.
    4. Modified Middleware Interaction: The CertificateCheckMW is updated to allow certificates that are not in the static whitelist if they are validated via a token binding. This introduces a new dependency between the whitelist and auth-token middlewares.
  • Affected system components:

    • Gateway Authentication Middleware (mw_auth_key, mw_certificate_check)
    • Configuration Loading (config.go)
    • Session Management & Storage (user/session.go)

Request Flow with Certificate Binding

sequenceDiagram participant Client participant Tyk Gateway participant CertificateCheckMW participant AuthKeyMW participant SessionStore Client->>Tyk Gateway: Request with mTLS Cert + Auth Token Tyk Gateway->>CertificateCheckMW: Process Request activate CertificateCheckMW CertificateCheckMW->>CertificateCheckMW: 1. Is Cert in static whitelist? alt No (Not in whitelist) CertificateCheckMW->>SessionStore: 2. (New) Get Session for Token SessionStore-->>CertificateCheckMW: Session Data CertificateCheckMW->>CertificateCheckMW: 3. Compare presented Cert with bound Certs in Session alt Match CertificateCheckMW-->>Tyk Gateway: Validation OK else Mismatch deactivate CertificateCheckMW CertificateCheckMW-->>Client: 403 Forbidden (Certificate not allowed) end else Yes (In whitelist) CertificateCheckMW-->>Tyk Gateway: Validation OK end deactivate CertificateCheckMW Tyk Gateway->>AuthKeyMW: Process Request activate AuthKeyMW AuthKeyMW->>SessionStore: 4. Get Session from Token SessionStore-->>AuthKeyMW: Session Data alt Feature Enabled AND Session has Bindings AuthKeyMW->>AuthKeyMW: 5. (New) Compare presented Cert with bound Certs in Session alt Match AuthKeyMW-->>Tyk Gateway: Authentication OK else Mismatch deactivate AuthKeyMW AuthKeyMW-->>Client: 403 Forbidden (Certificate Mismatch) end else Legacy mTLS or No Bindings AuthKeyMW->>AuthKeyMW: Perform legacy validation AuthKeyMW-->>Tyk Gateway: Authentication OK end deactivate AuthKeyMW Tyk Gateway->>Upstream API: Proxy Request 
Loading

Scope Discovery & Context Expansion

  • Operational Impact: This PR implements the enforcement of the binding. To use this feature, administrators must update their key provisioning workflows. The client certificate's SHA256 hash (prefixed with the organization ID) must be calculated and included in the new mtls_static_certificate_bindings field of the session object when creating or updating a token. This impacts any tooling used for key management, such as the Tyk Dashboard or custom scripts using the Gateway API.

  • Architectural Considerations: The implementation introduces a significant architectural change where the CertificateCheckMW now performs token and session validation, a responsibility that previously belonged solely to AuthKeyMW. This creates a duplication of logic (e.g., fetching the session from a data store) in the request lifecycle, which has performance and maintenance implications. This tightens the coupling between the two middlewares and could be a point of concern for future refactoring.

Metadata
  • Review Effort: 4 / 5
  • Primary Label: feature

Powered by Visor from Probelabs

Last updated: 2025-11-26T10:25:18.060Z | Triggered by: pr_updated | Commit: 6a65caf

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

@probelabs
Copy link

probelabs bot commented Nov 21, 2025

Security Issues (4)

Severity Location Issue
🟠 Error gateway/mw_auth_key.go:107-111
The comparison of the client certificate hash against the bound certificate hashes is performed using the standard `==` operator, which is not constant-time. This could potentially expose the application to a timing attack, where an attacker could measure response time variations to incrementally guess a valid certificate hash.
💡 SuggestionReplace the direct string comparison with a constant-time comparison function to mitigate timing attacks. Use `crypto/subtle.ConstantTimeCompare` for all security-sensitive comparisons.
🔧 Suggested Fix
 certMatched := false for _, boundCert := range session.MtlsStaticCertificateBindings { if subtle.ConstantTimeCompare([]byte(certHash), []byte(boundCert)) == 1 { certMatched = true break } }
🟠 Error gateway/mw_certificate_check.go:145
The comparison of the client certificate hash against the session's certificate is performed using the standard `==` operator, which is not constant-time. This is vulnerable to timing attacks. A similar vulnerability exists on line 154 inside the loop checking `MtlsStaticCertificateBindings`.
💡 SuggestionReplace the direct string comparison with `crypto/subtle.ConstantTimeCompare` to ensure the comparison time is independent of the input, mitigating timing attacks.
🔧 Suggested Fix
if authConfig.UseCertificate && subtle.ConstantTimeCompare([]byte(session.Certificate), []byte(certHash)) == 1 {
🟢 Info gateway/mw_certificate_check.go:125-164
The `validateCertificateFromToken` function duplicates logic from the `AuthKey` middleware, such as retrieving the auth token and loading the session from the key store. This tightens the coupling between `CertificateCheckMW` and `AuthKeyMW` and introduces a risk of future inconsistencies if the session retrieval logic is modified in one place but not the other.
💡 SuggestionConsider refactoring to avoid duplicating the session lookup. For example, the session, once retrieved by `AuthKeyMW`, could be stored in the request context for subsequent middlewares like `CertificateCheckMW` to consume. This would centralize the session loading logic and improve maintainability.
🟡 Warning config/config.go:695
The configuration property is named `DisableCertificateTokenBinding`, while the corresponding environment variable mentioned in the comment is `TYK_GW_SECURITY_ENABLECERTIFICATETOKENBINDING`. This inverted naming is highly confusing and increases the risk of misconfiguration, where an operator might unintentionally disable this important security feature.
💡 SuggestionAlign the naming of the configuration property and the environment variable to avoid confusion. For example, rename the struct field to `EnableCertificateTokenBinding` (and update its `json` tag) and adjust the logic where it's used (e.g., `if k.Gw.GetConfig().Security.EnableCertificateTokenBinding`). This makes the configuration explicit and less error-prone.

Architecture Issues (3)

Severity Location Issue
🟠 Error gateway/mw_certificate_check.go:124-172
The `CertificateCheckMW` middleware is modified to validate certificates against token bindings if they are not found in the static whitelist. This change violates the Single Responsibility Principle by introducing authentication logic (token parsing, session lookup) into a middleware designed for static certificate validation. This creates tight coupling between `CertificateCheckMW` and the authentication system, duplicates session lookup logic, and introduces performance overhead (session store access) early in the request lifecycle.
💡 SuggestionThe responsibility for validating token-certificate bindings should reside solely within the authentication middleware (`AuthKeyMW`). Remove the token validation logic from `CertificateCheckMW`. If the goal is to allow a certificate if it's either in the whitelist OR bound to a token, this logic should be orchestrated within `AuthKeyMW`. `CertificateCheckMW` should not perform session lookups itself.
🟡 Warning config/config.go:696
The configuration option `DisableCertificateTokenBinding` uses a double negative, which makes its purpose less intuitive and the code harder to read. A name like `EnableCertificateTokenBinding` would be clearer.
💡 SuggestionRename the configuration option to `EnableCertificateTokenBinding` and adjust its usage in `gateway/mw_auth_key.go`. The default value should be `false` to maintain backward compatibility.
🟡 Warning user/session.go:268
A new field `MtlsStaticCertificateBindings` has been added to the `SessionState` struct. This approach has a wide impact, affecting the core session object which is stored and handled by various components. An alternative design would be to leverage key metadata, which would be less invasive to the core `SessionState` structure and could simplify integration with tools like the Dashboard and Portal that already work with key metadata.
💡 SuggestionConsider using key metadata to store certificate bindings instead of adding a new field to `SessionState`. This could reduce the surface area of the change and align better with existing extension mechanisms.

Performance Issues (1)

Severity Location Issue
🟡 Warning gateway/mw_certificate_check.go:138
The `validateCertificateFromToken` function in `CertificateCheckMW` fetches the user session from the session store. The `AuthKey` middleware, which runs later in the request lifecycle, fetches the exact same session again. This results in two separate lookups for the same session data for requests that fall into this validation path (i.e., when a certificate is not in the static whitelist but might be bound to a token). This redundant operation adds unnecessary latency, especially when using a remote session store like Redis.
💡 SuggestionTo avoid the redundant lookup, the session should be fetched only once. The session state can be stored in the request context (`context.WithValue`) after the first lookup in `CertificateCheckMW`. The `AuthKey` middleware can then attempt to retrieve the session from the context first before falling back to fetching it from the session store. This pattern is often referred to as "request-scoped caching".

Quality Issues (3)

Severity Location Issue
🟠 Error config/config.go:700
The configuration option `DisableCertificateTokenBinding` has a confusing name, comment, and associated environment variable. The name uses negative logic (`Disable...`), while the comment and environment variable (`...ENABLE...`) use positive logic. This inconsistency makes the configuration difficult to understand and prone to misconfiguration. The default boolean value of `false` means the feature is enabled by default, which could be a surprising change for users.
💡 SuggestionRename the field to `EnableCertificateTokenBinding` and update the comment accordingly to make the logic straightforward (`true` enables, `false` disables) and align it with the environment variable name. If backward compatibility of the JSON field name is a concern, at a minimum, correct the comment to accurately describe the field's behavior and reconsider if this feature should be enabled by default.
🟠 Error gateway/mw_certificate_check.go:125-179
The `validateCertificateFromToken` function in `CertificateCheckMW` fetches the session object from the session store. This same session object is fetched again later in the request lifecycle by the `AuthKey` middleware. This duplicated data fetching is inefficient, adds unnecessary latency to requests, and creates tight coupling between the two middlewares.
💡 SuggestionTo avoid duplicate work, fetch the session only once. The session can be fetched in the first middleware that needs it (`CertificateCheckMW`) and then stored in the request's context (`r.Context()`). Subsequent middlewares, like `AuthKeyMW`, can then retrieve the session from the context instead of making another call to the session store.
🟡 Warning gateway/mw_auth_key_mtls_combined_test.go:96
The test case name "Success: allowlisted cert with matching token binding" is ambiguous. It tests the legacy dynamic mTLS binding (using `session.Certificate`) rather than the new static certificate binding feature (which uses `session.MtlsStaticCertificateBindings`). While the test is valid for ensuring backward compatibility, its name could be misinterpreted.
💡 SuggestionRename the test case to more accurately reflect what it's testing, for example: "Success: allowlisted cert with legacy dynamic mTLS binding".

Powered by Visor from Probelabs

Last updated: 2025-11-26T10:25:21.472Z | Triggered by: pr_updated | Commit: 6a65caf

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

Copy link
Contributor

@andyo-tyk andyo-tyk left a comment

Choose a reason for hiding this comment

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

Observations and suggestions on docs content

config/config.go Outdated
// CertificateExpiryMonitor configures the certificate expiry monitoring and notification feature
CertificateExpiryMonitor CertificateExpiryMonitorConfig `json:"certificate_expiry_monitor"`

// EnableCertificateBinding enables certificate-token binding for static mTLS authentication.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// EnableCertificateBinding enables certificate-token binding for static mTLS authentication.
// Used to enable certificate-token binding for static mTLS authentication.

The configuration name is enable_certificate_token_binding / TYK_GW_SECURITY_ENABLECERTIFICATETOKENBINDING - so should we use plain text rather than concatenating the name (EnableCertificateTokenBinding), given that this is targeting the docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a required standard for godocs

Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we have this for all the other configs that are generated from godocs and appear here? https://tyk.io/docs/tyk-oss-gateway/configuration

@buger
Copy link
Member

buger commented Nov 25, 2025

See my comment on tyk-analytics repo. But basically why not to leverage Key metadata with some reserved keywords for this? It will significantly simplify dependencies and overall surface of those changes, including portal (which already writes a bunch of metadata fields now).
Also do not see too much value from having it as config option on gateway. This metadata/field is new feature, it can be used only if it was attached on portal side. So simply checking if this field is set or not is the flag by itself.

andrei-tyk and others added 6 commits November 25, 2025 17:17
Co-authored-by: andyo-tyk <99968932+andyo-tyk@users.noreply.github.com>
Co-authored-by: andyo-tyk <99968932+andyo-tyk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants