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

@@ -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
 })

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)
+				}
+			}
+		})
 	}
 }
 

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 {

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"
 )