Log Client Certificate Identity Information in Access Logs#145
Open
geigerj0 wants to merge 3 commits intocloudfoundry:mainfrom
Open
Log Client Certificate Identity Information in Access Logs#145geigerj0 wants to merge 3 commits intocloudfoundry:mainfrom
geigerj0 wants to merge 3 commits intocloudfoundry:mainfrom
Conversation
geigerj0
commented
May 5, 2026
Comment on lines
+30
to
+43
| if r.TLS != nil && len(r.TLS.PeerCertificates) > 0 { | ||
| indexLeafCertConnectionIsVerifiedAgainst := 0 // see also https://github.com/golang/go/blob/e929fb78e47dc191a402d34ca949d2e0c67e31b8/src/crypto/tls/common.go#L281-L282 | ||
| cert := r.TLS.PeerCertificates[indexLeafCertConnectionIsVerifiedAgainst] | ||
|
|
||
| lagerData["peer_cert_subject_common_name"] = cert.Subject.CommonName | ||
| lagerData["peer_cert_subject_organizational_unit"] = cert.Subject.OrganizationalUnit | ||
| lagerData["peer_cert_subject_organization"] = cert.Subject.Organization | ||
|
|
||
| lagerData["peer_cert_issuer_common_name"] = cert.Issuer.CommonName | ||
| lagerData["peer_cert_issuer_organizational_unit"] = cert.Issuer.OrganizationalUnit | ||
| lagerData["peer_cert_issuer_organization"] = cert.Issuer.Organization | ||
| } | ||
|
|
||
| return lagerData |
Author
There was a problem hiding this comment.
FYI: this causes the LogWrap-middleware to always log peer cert information. however, the related log messages are being emitted with log level DEBUG and thus will more or less never be visible because no one runs this with DEBUG productively
bbs/handlers/middleware/middleware.go
Lines 49 to 54 in e5644ae
geigerj0
commented
May 5, 2026
Comment on lines
+34
to
+40
| lagerData["peer_cert_subject_common_name"] = cert.Subject.CommonName | ||
| lagerData["peer_cert_subject_organizational_unit"] = cert.Subject.OrganizationalUnit | ||
| lagerData["peer_cert_subject_organization"] = cert.Subject.Organization | ||
|
|
||
| lagerData["peer_cert_issuer_common_name"] = cert.Issuer.CommonName | ||
| lagerData["peer_cert_issuer_organizational_unit"] = cert.Issuer.OrganizationalUnit | ||
| lagerData["peer_cert_issuer_organization"] = cert.Issuer.Organization |
Author
There was a problem hiding this comment.
we can definitely discuss these key names (peer_cert_*), maybe you have a better idea
geigerj0
commented
May 5, 2026
| When("request has TLS peer certificates", func() { | ||
| BeforeEach(func() { | ||
| accessLogger = lagertest.NewTestLogger("") | ||
| accessLogger.RegisterSink(lager.NewWriterSink(GinkgoWriter, lager.INFO)) // peer cert information should be logged at INFO level |
Author
There was a problem hiding this comment.
it is very important to test that the log messages appear also for INFO-logging because this is the level the access logger is being set up with:
Line 199 in e5644ae
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
BBS currently does not include TLS peer certificate information in its access logs, making it difficult to reliably identify authenticated clients. At the moment, only the client IP address is logged, while no certificate-based identity information about the caller is recorded.
bbs/handlers/middleware/middleware.go
Line 26 in e5644ae
Solution
This PR enhances the
LogWrapmiddleware to include TLS peer certificate information in request logs whenever such information is available.If an access logger is configured via
access_log_path, the TLS peer certificate details are logged at theINFOlevel and therefore written to the access log.Backward Compatibility
Breaking Change? No
Contact Me
Feel free to reach out directly via Slack: https://cloudfoundry.slack.com/team/U0578H2V37D