From 509ca9131b50c4d53e8a09737952755715343268 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Thu, 15 Jan 2026 23:56:51 +0000 Subject: [PATCH] Changing the wildcard matching logic for client cert common names Microsoft ADO ID: 36341347 PR #168 added support for wildcard matching between client cert CNs and trusted CNs. This PR changes the wildcard matching logic based on a discussion with @qiluo-msft and @prsunny. Changes: 1. Wildcard patterns starting with `*` but not `*.` (e.g., `*example.sonic`) are no longer valid. These patterns are skipped during the matching. 2. `*.` is not a valid trusted CN and will be skipped if present. 3. `*` must match one or more characters. For example, `*.example.sonic` will not match `.example.sonic`. **Note 1:** Comparison is always case sensitive, but `*` can match both upper and lower case characters. For example, `*.example.sonic` matches `TEST.example.sonic`, but it does not match `TEST.EXAMPLE.SONIC`. **Note 2:** Multi-level subdomain matching is allowed for wildcard patterns. For example, `*.example.sonic` matches `test.example.sonic`, `sub.test.example.sonic`, `one.sub.test.example.sonic`, and so on. --- go-server-server/go/auth.go | 45 ++++++++++------ supervisor/rest_api_test.conf | 2 +- test/test_restapi.py | 96 ++++++++++++++++++++++++++++++----- 3 files changed, 113 insertions(+), 30 deletions(-) diff --git a/go-server-server/go/auth.go b/go-server-server/go/auth.go index 502fde7..21c9e1b 100644 --- a/go-server-server/go/auth.go +++ b/go-server-server/go/auth.go @@ -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 { + 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 + 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; } \ No newline at end of file diff --git a/supervisor/rest_api_test.conf b/supervisor/rest_api_test.conf index 3afccb6..d056744 100644 --- a/supervisor/rest_api_test.conf +++ b/supervisor/rest_api_test.conf @@ -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 diff --git a/test/test_restapi.py b/test/test_restapi.py index 607b965..a23597d 100644 --- a/test/test_restapi.py +++ b/test/test_restapi.py @@ -61,7 +61,7 @@ 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 @@ -69,13 +69,31 @@ def test_exact_match_success(self, setup_restapi_client): 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 @@ -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 @@ -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"""