Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,14 @@ type SecurityConfig struct {

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

// AllowDynamicMTLSTokenAuth controls whether token-based authentication is allowed for APIs
// using dynamic client mTLS. When set to false (the default), requests to APIs with dynamic
// mTLS enabled must present a valid client certificate - token-only authentication will be
// rejected. This prevents bypassing mTLS by constructing tokens from publicly available
// certificate information. Set to true to restore the legacy behavior of allowing either
// certificate or token authentication.
AllowDynamicMTLSTokenAuth bool `json:"allow_dynamic_mtls_token_auth"`
}

type NewRelicConfig struct {
Expand Down
32 changes: 29 additions & 3 deletions gateway/mw_auth_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
ErrAuthCertNotFound = "auth.cert_not_found"
ErrAuthCertExpired = "auth.cert_expired"
ErrAuthKeyIsInvalid = "auth.key_is_invalid"
ErrAuthCertRequired = "auth.cert_required"

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."
MsgInvalidKey = "Attempted access with invalid key."
MsgCertificateRequired = "Client certificate required for this API."
)

func initAuthKeyErrors() {
Expand Down Expand Up @@ -59,6 +61,11 @@
Message: MsgCertificateExpired,
Code: http.StatusForbidden,
}

TykErrors[ErrAuthCertRequired] = config.TykError{
Message: MsgCertificateRequired,
Code: http.StatusForbidden,
}
}

// KeyExists will check if the key being used to access the API is in the request data,
Expand Down Expand Up @@ -100,12 +107,16 @@

key, authConfig := k.getAuthToken(k.getAuthType(), r)
var certHash string
// tokenFromHeader tracks whether the auth token came from request header/params (true)
// or was derived from a client certificate (false)
tokenFromHeader := false

keyExists := false
var session user.SessionState
updateSession := false
if key != "" {
key = stripBearer(key)
tokenFromHeader = true
} else if authConfig.UseCertificate && key == "" && r.TLS != nil && len(r.TLS.PeerCertificates) > 0 {
log.Debug("Trying to find key by client certificate")
certHash = k.Spec.OrgID + crypto.HexSHA256(r.TLS.PeerCertificates[0].Raw)
Expand All @@ -116,9 +127,24 @@
key = k.Gw.generateToken(k.Spec.OrgID, certHash)
} else {
k.Logger().Info("Attempted access with malformed header, no auth header found.")
return errorAndStatusCode(ErrAuthAuthorizationFieldMissing)
}

// When dynamic mTLS is enabled (UseCertificate=true), by default we require that
// authentication happens via a client certificate, not just a token. This prevents
// bypassing mTLS by constructing tokens from publicly available certificate info.
// The gateway config flag AllowDynamicMTLSTokenAuth can be set to true to restore
// the legacy behavior of allowing token-only auth.
if authConfig.UseCertificate && tokenFromHeader {

Check warning on line 138 in gateway/mw_auth_key.go

View check run for this annotation

probelabs / Visor: security

security Issue

The security fix that prevents an authentication bypass for dynamic mTLS APIs is not covered by automated tests. While the logic appears correct, the lack of tests makes it vulnerable to future regressions which could reintroduce the security flaw.
Raw output
Implement the test cases outlined in the pull request description to ensure the fix is robust and prevent regressions. This includes verifying that token-only requests are rejected by default, certificate-based requests succeed, the backward-compatibility flag works as expected, and non-mTLS APIs are unaffected.
if !k.Gw.GetConfig().Security.AllowDynamicMTLSTokenAuth {
// Token was provided but no client certificate was presented in the TLS handshake

Check failure on line 140 in gateway/mw_auth_key.go

View check run for this annotation

probelabs / Visor: architecture

logic Issue

The new security logic to prevent token-based authentication bypass for dynamic mTLS APIs is not covered by automated tests. For a security-sensitive change, it is critical to have tests that validate the new behavior (rejecting token-only auth), the backward-compatibility flag, and ensure no regressions for non-mTLS APIs or certificate-based mTLS auth.
Raw output
Add unit tests to `gateway/mw_auth_key_test.go` covering the scenarios outlined in the PR description's test plan:
1.  Request with token-only to dynamic mTLS API is rejected (default).
2.  Request with valid certificate to dynamic mTLS API succeeds (default).
3.  Request with token-only to dynamic mTLS API succeeds when `AllowDynamicMTLSTokenAuth` is true.
4.  Non-mTLS APIs are unaffected.
if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
k.Logger().Info("Token-based auth rejected for dynamic mTLS API - client certificate required")
return errorAndStatusCode(ErrAuthCertRequired)
}
}
}

Check warning on line 146 in gateway/mw_auth_key.go

View check run for this annotation

probelabs / Visor: quality

logic Issue

The new security-critical logic to reject token-based authentication for dynamic mTLS APIs is not covered by automated tests. The PR description includes a comprehensive test plan, but the corresponding tests have not been implemented. Without automated tests, there is a risk of future regressions and it's difficult to verify the correctness of all edge cases.
Raw output
Implement the test cases outlined in the PR description to ensure the new logic is fully validated. This should include tests for: 1. Rejection of token-only requests to mTLS APIs by default. 2. Successful authentication with a valid client certificate. 3. Successful token-only authentication when `AllowDynamicMTLSTokenAuth` is explicitly enabled for backward compatibility. 4. Verification that non-mTLS APIs are unaffected by this change.

session, keyExists = k.CheckSessionAndIdentityForValidKey(key, r)
key = session.KeyID
if !keyExists {
Expand Down
Loading