Skip to content

Changing the wildcard matching logic for client cert common names #175

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
qiluo-msft merged 3 commits into from
Jan 15, 2026
Merged

Changing the wildcard matching logic for client cert common names #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9f9d76
Modified the wildcard matching logic for client cert CN names.
mramezani95 Jan 14, 2026
8242338
Fixed indentation.
mramezani95 Jan 14, 2026
010d5dc
Multi-level subdomains are now accepted for wildcard matches. Added n…
mramezani95 Jan 15, 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
45 changes: 30 additions & 15 deletions go-server-server/go/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,41 @@ import (
)

func CommonNameMatch(r *http.Request) bool {
// During client cert authentication, after the certificate chain is validated by
// TLS, here we will further check if at least one of the common names in the end-entity certificate
// matches one of the trusted common names of the server config.
for _, peercert := range r.TLS.PeerCertificates {
commonName := peercert.Subject.CommonName
log.Printf("info: CommonName in the client cert: %s", commonName)
for _, name := range trustedCertCommonNames {
if strings.HasPrefix(name, "*") {
// During client cert authentication, after the certificate chain is validated by
// TLS, here we will further check if at least one of the common names in the end-entity certificate
// matches one of the trusted common names in the server config.

for _, name := range trustedCertCommonNames {
is_wildcard := false
domain := name
if strings.HasPrefix(name, "*.") {
if len(name) < 3 {
Copy link
Collaborator

@prsunny prsunny Jan 15, 2026

Choose a reason for hiding this comment

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

what is this check for?

Copy link
Collaborator

@prsunny prsunny Jan 15, 2026

Choose a reason for hiding this comment

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

can you add a comment above the check?

Copy link
Contributor Author

@mramezani95 mramezani95 Jan 15, 2026

Choose a reason for hiding this comment

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

This is to make sure that *. is not a valid trusted CN.

log.Printf("warning: Skipping invalid trusted common name %s", name)
continue;
}
is_wildcard = true
domain = name[1:] //strip "*" but keep the "." at the beginning
} else if strings.HasPrefix(name, "*") {
log.Printf("warning: Skipping invalid trusted common name %s", name)
continue;
}
for _, peercert := range r.TLS.PeerCertificates {
commonName := peercert.Subject.CommonName
Copy link

@qiluo-msft qiluo-msft Jan 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

commonName

nitpick: if commonName is empty string, return false early. Even we have a config with empty string in allowlist, we still need to return false. #WontFix

if is_wildcard {
// wildcard common name matching
domain := name[1:] //strip "*"
if strings.HasSuffix(commonName, domain) {
log.Printf("info: CommonName %s in the client cert matches trusted wildcard common name %s", commonName, name)
if len(commonName) > len(domain) && strings.HasSuffix(commonName, domain) {
log.Printf("info: Wildcard match between common name %s in the client cert and trusted common name %s", commonName, name)
return true;
}
} else {
if commonName == name {
log.Printf("info: Exact match with trusted common name %s", name)
return true;
}
} else if commonName == name {
return true;
}
}
}

log.Printf("error: Authentication Fail! None of the CommonNames in the client cert match any of the trusted common names")
return false;
log.Printf("error: Authentication Fail! None of the common names in the client cert match any of the trusted common names")
return false;
}
2 changes: 1 addition & 1 deletion supervisor/rest_api_test.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[program:rest-api]
command=/usr/sbin/go-server-server.test -test.coverprofile=/coverage.cov -systemtest=true -enablehttps=true -clientcert=/usr/sbin/cert/client/selfsigned.crt -servercert=/usr/sbin/cert/server/selfsigned.crt -serverkey=/usr/sbin/cert/server/selfsigned.key -localapitestdocker=true -clientcertcommonname=test.client.restapi.sonic,*.example.sonic,*test.sonic -loglevel trace
command=/usr/sbin/go-server-server.test -test.coverprofile=/coverage.cov -systemtest=true -enablehttps=true -clientcert=/usr/sbin/cert/client/selfsigned.crt -servercert=/usr/sbin/cert/server/selfsigned.crt -serverkey=/usr/sbin/cert/server/selfsigned.key -localapitestdocker=true -clientcertcommonname=test.client.restapi.sonic,*.example.sonic,*test.sonic,*. -loglevel trace
priority=1
autostart=false
autorestart=false
Expand Down
96 changes: 82 additions & 14 deletions test/test_restapi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,39 @@ def __exit__(self, exc_type, exc_value, traceback):

class TestClientCertAuth:

# Exact match tests for "test.client.restapi.sonic"
# Exact matching tests for "test.client.restapi.sonic"

def test_exact_match_success(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("test.client.restapi.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200

def test_exact_match_failure(self, setup_restapi_client):
def test_exact_match_failure_1(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("client.restapi.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

# Wildcard match tests for "*.example.sonic"
def test_exact_match_failure_2(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("test.client.restapi.com") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_exact_match_failure_3(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("sub.test.client.restapi.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_exact_match_failure_4(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("TEST.CLIENT.RESTAPI.SONIC") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

# Wildcard matching tests for "*.example.sonic"

def test_wildcard_match_success_1(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
Expand All @@ -85,7 +103,19 @@ def test_wildcard_match_success_1(self, setup_restapi_client):

def test_wildcard_match_success_2(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("another.test.example.sonic") as client_cert:
with ClientCert("a.example.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200

def test_wildcard_match_success_3(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("sub.test.example.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200

def test_wildcard_match_success_4(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("TEST.example.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200

Expand Down Expand Up @@ -119,38 +149,76 @@ def test_wildcard_match_failure_5(self, setup_restapi_client):
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

# Wildcard match tests for "*test.sonic"
def test_wildcard_match_failure_6(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert(".example.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_wildcard_match_success_a(self, setup_restapi_client):
def test_wildcard_match_failure_7(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("TEST.EXAMPLE.SONIC") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

# Matching tests for "*test.sonic" (invalid CN)

def test_invalid_match_failure_1(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("mytest.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200
assert r.status_code == 401

def test_wildcard_match_success_b(self, setup_restapi_client):
def test_invalid_match_failure_2(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("test.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200
assert r.status_code == 401

def test_wildcard_match_success_c(self, setup_restapi_client):
def test_invalid_match_failure_3(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("example.test.sonic") as client_cert:
with ClientCert("sub.test.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 200
assert r.status_code == 401

def test_wildcard_match_failure_a(self, setup_restapi_client):
def test_invalid_match_failure_4(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("est.sonic") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_wildcard_match_failure_b(self, setup_restapi_client):
def test_invalid_match_failure_5(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("test.sonico") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

# Corner cases

def test_empty_cn(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_missing_tld(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("test.example.") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_match_all(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("*") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401

def test_ends_with_dot(self, setup_restapi_client):
_, _, _, restapi_client = setup_restapi_client
with ClientCert("*.") as client_cert:
r = restapi_client.get_heartbeat(client_cert=(client_cert.cert, client_cert.key))
assert r.status_code == 401


class TestRestApiPositive:
"""Normal behaviour tests"""
Expand Down
Loading