From 72a449fe98ade6bbbb643e7b0b3c38c046ac6f97 Mon Sep 17 00:00:00 2001 From: Taahir Ahmed Date: Fri, 21 Jun 2024 16:21:35 -0700 Subject: [PATCH] Define credential IDs for X.509 certificates This commit expands the existing credential ID concept to cover X.509 certificates. We use the certificate's signature as the credential ID, since this safe and unique. Kubernetes-commit: 2ad2bd8907d979f709cd924af7986be71c31ce12 --- pkg/authentication/request/x509/x509.go | 8 ++ pkg/authentication/request/x509/x509_test.go | 133 +++++++++++-------- pkg/authentication/serviceaccount/util.go | 5 +- pkg/authentication/user/user.go | 6 +- 4 files changed, 93 insertions(+), 59 deletions(-) diff --git a/pkg/authentication/request/x509/x509.go b/pkg/authentication/request/x509/x509.go index d67c53547..bdc1b1790 100644 --- a/pkg/authentication/request/x509/x509.go +++ b/pkg/authentication/request/x509/x509.go @@ -17,6 +17,7 @@ limitations under the License. package x509 import ( + "crypto/sha256" "crypto/x509" "crypto/x509/pkix" "encoding/hex" @@ -276,10 +277,17 @@ var CommonNameUserConversion = UserConversionFunc(func(chain []*x509.Certificate if len(chain[0].Subject.CommonName) == 0 { return nil, false, nil } + + fp := sha256.Sum256(chain[0].Raw) + id := "X509SHA256=" + hex.EncodeToString(fp[:]) + return &authenticator.Response{ User: &user.DefaultInfo{ Name: chain[0].Subject.CommonName, Groups: chain[0].Subject.Organization, + Extra: map[string][]string{ + user.CredentialIDKey: {id}, + }, }, }, true, nil }) diff --git a/pkg/authentication/request/x509/x509_test.go b/pkg/authentication/request/x509/x509_test.go index efb6f2aca..31a781122 100644 --- a/pkg/authentication/request/x509/x509_test.go +++ b/pkg/authentication/request/x509/x509_test.go @@ -23,11 +23,11 @@ import ( "errors" "io/ioutil" "net/http" - "reflect" "sort" "testing" "time" + "github.com/google/go-cmp/cmp" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" @@ -581,9 +581,8 @@ func TestX509(t *testing.T) { Opts x509.VerifyOptions User UserConversion - ExpectUserName string - ExpectGroups []string ExpectOK bool + ExpectResponse *authenticator.Response ExpectErr bool }{ "non-tls": { @@ -618,10 +617,17 @@ func TestX509(t *testing.T) { Certs: getCerts(t, serverCert), User: CommonNameUserConversion, - ExpectUserName: "127.0.0.1", - ExpectGroups: []string{"My Org"}, - ExpectOK: true, - ExpectErr: false, + ExpectOK: true, + ExpectResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "127.0.0.1", + Groups: []string{"My Org"}, + Extra: map[string][]string{ + user.CredentialIDKey: {"X509SHA256=92209d1e0dd36a018f244f5e1b88e2d47b049e9cfcd4b7c87c65875866872230"}, + }, + }, + }, + ExpectErr: false, }, "common name": { @@ -629,10 +635,17 @@ func TestX509(t *testing.T) { Certs: getCerts(t, clientCNCert), User: CommonNameUserConversion, - ExpectUserName: "client_cn", - ExpectGroups: []string{"My Org"}, - ExpectOK: true, - ExpectErr: false, + ExpectOK: true, + ExpectResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "client_cn", + Groups: []string{"My Org"}, + Extra: map[string][]string{ + user.CredentialIDKey: {"X509SHA256=dd0a6a295055fa94455c522b0d54ef0499186f454a7cf978b8b346dc35b254f7"}, + }, + }, + }, + ExpectErr: false, }, "ca with multiple organizations": { Opts: x509.VerifyOptions{ @@ -641,10 +654,17 @@ func TestX509(t *testing.T) { Certs: getCerts(t, caWithGroups), User: CommonNameUserConversion, - ExpectUserName: "ROOT CA WITH GROUPS", - ExpectGroups: []string{"My Org", "My Org 1", "My Org 2"}, - ExpectOK: true, - ExpectErr: false, + ExpectOK: true, + ExpectResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "ROOT CA WITH GROUPS", + Groups: []string{"My Org", "My Org 1", "My Org 2"}, + Extra: map[string][]string{ + user.CredentialIDKey: {"X509SHA256=6f337bb6576b6f942bd5ac5256f621e352aa7b34d971bda9b8f8981f51bba456"}, + }, + }, + }, + ExpectErr: false, }, "custom conversion error": { @@ -664,9 +684,13 @@ func TestX509(t *testing.T) { return &authenticator.Response{User: &user.DefaultInfo{Name: "custom"}}, true, nil }), - ExpectUserName: "custom", - ExpectOK: true, - ExpectErr: false, + ExpectOK: true, + ExpectResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "custom", + }, + }, + ExpectErr: false, }, "future cert": { @@ -697,9 +721,16 @@ func TestX509(t *testing.T) { Certs: getCertsFromFile(t, "client-valid", "intermediate"), User: CommonNameUserConversion, - ExpectUserName: "My Client", - ExpectOK: true, - ExpectErr: false, + ExpectOK: true, + ExpectResponse: &authenticator.Response{ + User: &user.DefaultInfo{ + Name: "My Client", + Extra: map[string][]string{ + user.CredentialIDKey: {"X509SHA256=794b0529fd1a72d55d52d98be9bab5b822d16f9ae86c4373fa7beee3cafe8582"}, + }, + }, + }, + ExpectErr: false, }, "multi-level, expired": { Opts: multilevelOpts, @@ -712,42 +743,36 @@ func TestX509(t *testing.T) { } for k, testCase := range testCases { - req, _ := http.NewRequest("GET", "/", nil) - if !testCase.Insecure { - req.TLS = &tls.ConnectionState{PeerCertificates: testCase.Certs} - } - - // this effectively tests the simple dynamic verify function. - a := New(testCase.Opts, testCase.User) - - resp, ok, err := a.AuthenticateRequest(req) - - if testCase.ExpectErr && err == nil { - t.Errorf("%s: Expected error, got none", k) - continue - } - if !testCase.ExpectErr && err != nil { - t.Errorf("%s: Got unexpected error: %v", k, err) - continue - } - - if testCase.ExpectOK != ok { - t.Errorf("%s: Expected ok=%v, got %v", k, testCase.ExpectOK, ok) - continue - } - - if testCase.ExpectOK { - if testCase.ExpectUserName != resp.User.GetName() { - t.Errorf("%s: Expected user.name=%v, got %v", k, testCase.ExpectUserName, resp.User.GetName()) + t.Run(k, func(t *testing.T) { + req, _ := http.NewRequest("GET", "/", nil) + if !testCase.Insecure { + req.TLS = &tls.ConnectionState{PeerCertificates: testCase.Certs} } - groups := resp.User.GetGroups() - sort.Strings(testCase.ExpectGroups) - sort.Strings(groups) - if !reflect.DeepEqual(testCase.ExpectGroups, groups) { - t.Errorf("%s: Expected user.groups=%v, got %v", k, testCase.ExpectGroups, groups) + // this effectively tests the simple dynamic verify function. + a := New(testCase.Opts, testCase.User) + + resp, ok, err := a.AuthenticateRequest(req) + + if testCase.ExpectErr && err == nil { + t.Fatalf("Expected error, got none") } - } + if !testCase.ExpectErr && err != nil { + t.Fatalf("Got unexpected error: %v", err) + } + + if testCase.ExpectOK != ok { + t.Fatalf("Expected ok=%v, got %v", testCase.ExpectOK, ok) + } + + if testCase.ExpectOK { + sort.Strings(testCase.ExpectResponse.User.GetGroups()) + sort.Strings(resp.User.GetGroups()) + if diff := cmp.Diff(testCase.ExpectResponse, resp); diff != "" { + t.Errorf("Bad response; diff (-want +got)\n%s", diff) + } + } + }) } } diff --git a/pkg/authentication/serviceaccount/util.go b/pkg/authentication/serviceaccount/util.go index 53769d950..949b60922 100644 --- a/pkg/authentication/serviceaccount/util.go +++ b/pkg/authentication/serviceaccount/util.go @@ -30,9 +30,6 @@ const ( ServiceAccountUsernameSeparator = ":" ServiceAccountGroupPrefix = "system:serviceaccounts:" AllServiceAccountsGroup = "system:serviceaccounts" - // CredentialIDKey is the key used in a user's "extra" to specify the unique - // identifier for this identity document). - CredentialIDKey = "authentication.kubernetes.io/credential-id" // IssuedCredentialIDAuditAnnotationKey is the annotation key used in the audit event that is persisted to the // '/token' endpoint for service accounts. // This annotation indicates the generated credential identifier for the service account token being issued. @@ -150,7 +147,7 @@ func (sa *ServiceAccountInfo) UserInfo() user.Info { if info.Extra == nil { info.Extra = make(map[string][]string) } - info.Extra[CredentialIDKey] = []string{sa.CredentialID} + info.Extra[user.CredentialIDKey] = []string{sa.CredentialID} } if sa.NodeName != "" { if info.Extra == nil { diff --git a/pkg/authentication/user/user.go b/pkg/authentication/user/user.go index 4d6ec0980..1af6f2b27 100644 --- a/pkg/authentication/user/user.go +++ b/pkg/authentication/user/user.go @@ -66,8 +66,8 @@ func (i *DefaultInfo) GetExtra() map[string][]string { return i.Extra } -// well-known user and group names const ( + // well-known user and group names SystemPrivilegedGroup = "system:masters" NodesGroup = "system:nodes" MonitoringGroup = "system:monitoring" @@ -81,4 +81,8 @@ const ( KubeProxy = "system:kube-proxy" KubeControllerManager = "system:kube-controller-manager" KubeScheduler = "system:kube-scheduler" + + // CredentialIDKey is the key used in a user's "extra" to specify the unique + // identifier for this identity document). + CredentialIDKey = "authentication.kubernetes.io/credential-id" )