Skip to content

[TT-1981] Client TLS certificate is required" error should be logged at info level #7958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shults merged 8 commits into from
Apr 17, 2026
+0 −8
Merged

[TT-1981] Client TLS certificate is required" error should be logged at info level #7958

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
211ee9e
remove log
shults Apr 1, 2026
65e92e9
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
shults Apr 9, 2026
e70117a
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
shults Apr 9, 2026
1a33663
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
radkrawczyk Apr 14, 2026
95fc991
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
radkrawczyk Apr 15, 2026
efa2cef
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
radkrawczyk Apr 15, 2026
9596b2a
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
shults Apr 17, 2026
51e85af
Merge branch 'master' into TT-1981-client-tls-certificate-is-required…
shults Apr 17, 2026
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: 0 additions & 8 deletions gateway/mw_certificate_check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@
"net/http"
"time"

"github.com/sirupsen/logrus"

"github.com/TykTechnologies/tyk/certs"
"github.com/TykTechnologies/tyk/internal/certcheck"
"github.com/TykTechnologies/tyk/internal/crypto"
"github.com/TykTechnologies/tyk/pkg/errpack"
"github.com/TykTechnologies/tyk/storage"
)

Expand Down Expand Up @@ -114,21 +111,16 @@
certIDs := append(m.Spec.ClientCertificates, m.Spec.GlobalConfig.Security.Certificates.API...)
apiCerts := m.Gw.CertificateManager.List(certIDs, certs.CertificatePublic)
if err := crypto.ValidateRequestCerts(r, apiCerts); err != nil {
log.
WithField("api_id", m.Spec.APIID).
WithField("api_name", m.Spec.Name).
WithField("mw", m.Name()).
Log(errpack.LogLevel(err, logrus.WarnLevel), "Certificate validation failed: ", err)
m.batchCertificatesExpirationCheck(apiCerts)
return err, http.StatusForbidden
Comment thread
edsonmichaque marked this conversation as resolved.
}

m.batchCertificatesExpirationCheck(apiCerts)
}

return nil, http.StatusOK
}

Check warning on line 123 in gateway/mw_certificate_check.go

View check run for this annotation

probelabs / Visor: security

security Issue 

Removing the explicit WARN-level log for a failed client certificate validation reduces the visibility of a significant security event. While the failure is logged as part of the generic access log at the INFO level, this change makes it harder to distinguish between routine client errors and potential systematic attacks (e.g., scanning for misconfigurations) without parsing all access logs. Security-sensitive failures like certificate validation should ideally be logged at a higher severity level (WARN or ERROR) to enable effective monitoring and alerting.
Raw output 
Consider retaining a specific, higher-severity log for this security-relevant event. If the goal is to reduce verbosity for common client errors, this could be made configurable. Alternatively, ensure that the centralized error logging can tag or elevate logs based on the error type, so that certificate validation failures can still trigger alerts.

Check warning on line 123 in gateway/mw_certificate_check.go

View check run for this annotation

probelabs / Visor: quality

undefined Issue 

Removing the explicit `WARN`-level log for a client certificate validation failure reduces the visibility of a security-relevant event. While the error is handled and likely logged as part of the standard access log at `INFO` level, demoting this specific failure from `WARN` to `INFO` can make it harder for operators to detect and debug mTLS configuration issues or potential security probing. The original log also contained specific context (`api_id`, `api_name`, `mw`) that might be lost in the generic access log.
Raw output 
Consider retaining the log but changing its level to `INFO` or `DEBUG` within this middleware if the goal is to reduce noise at the `WARN` level. Alternatively, ensure that the upstream error handler logs all the same contextual information (`api_id`, `api_name`, middleware name) that is being removed here, and provide a clear justification in the PR description for why changing the log level from `WARN` to `INFO` is appropriate for this type of security event.
// batchCertificatesExpirationCheck batches certificates for expiry checking using the configured BackgroundBatcher.
func (m *CertificateCheckMW) batchCertificatesExpirationCheck(certificates []*tls.Certificate) {
log.
Expand Down
Loading