Skip to content

Adding client cert common name(s) to the auth failure log#188

Open
mramezani95 wants to merge 2 commits into
sonic-net:masterfrom
mramezani95:mramezani/restapi_log_client_cert_CNs
Open

Adding client cert common name(s) to the auth failure log#188
mramezani95 wants to merge 2 commits into
sonic-net:masterfrom
mramezani95:mramezani/restapi_log_client_cert_CNs

Conversation

@mramezani95
Copy link
Copy Markdown
Contributor

@mramezani95 mramezani95 commented May 29, 2026

In case of auth failure due to untrusted client cert CNs, restapi currently generates the following message:

Authentication Fail! None of the common names in the client cert match any of the trusted common names

This PR adds client cert CNs to this log. This is useful for troubleshooting purposes.
This change was tested using an untrusted client cert. The following logs were generated as expected:

INFO restapi#supervisord: restapi [  info ] 2026/05/29 02:36:32 request: GET /v1/state/heartbeat StateHeartbeatGet
INFO restapi#supervisord: restapi [ error ] 2026/05/29 02:36:32 Authentication Failed! None of the common names in the client cert chain matched any of the trusted common names. Client cert common names: ["client.restapi.sonic"]
INFO restapi#supervisord: restapi [  info ] 2026/05/29 02:36:32 request: return 401
INFO restapi#supervisord: restapi [ error ] 2026/05/29 02:36:32 Request ends with error message {"error":{"code":401,"message":"Authentication Fail with untrusted client cert"}}

Note: This change was tested on the 202511 branch since the master branch has an auth issue: sonic-net/sonic-fips#86

Signed-off-by: Mahdi Ramezani <mramezani@microsoft.com>
@mssonicbld
Copy link
Copy Markdown

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft
qiluo-msft previously approved these changes May 29, 2026
Comment thread go-server-server/go/auth.go Outdated
log.Printf("error: Authentication Fail! None of the common names in the client cert match any of the trusted common names")
commonNames := make([]string, 0)
for _, peercert := range r.TLS.PeerCertificates {
commonNames = append(commonNames, peercert.Subject.CommonName)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A peer presenting a CN like:

client.foo\n2026/05/29 02:36:32 INFO restapi#supervisord: restapi [ info ] Authentication succeeded

can forge a fake log entry in syslog, confuse SIEM line-parsers, or smuggle ANSI escapes into operator terminals tailing the log. This is CWE-117 / OWASP A09. The fix is one line — quote each CN before logging:

Suggested change
commonNames = append(commonNames, peercert.Subject.CommonName)
commonNames = append(commonNames, strconv.Quote(peercert.Subject.CommonName))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@qiluo-msft Fixed. Please review again.

Signed-off-by: Mahdi Ramezani <mramezani@microsoft.com>
@mssonicbld
Copy link
Copy Markdown

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants