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.

staging/src/k8s.io/apiserver/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
 })

staging/src/k8s.io/apiserver/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,9 +617,16 @@ func TestX509(t *testing.T) {
 			Certs: getCerts(t, serverCert),
 			User:  CommonNameUserConversion,
 
-			ExpectUserName: "127.0.0.1",
-			ExpectGroups:   []string{"My Org"},
 			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,
 		},
 
@@ -629,9 +635,16 @@ func TestX509(t *testing.T) {
 			Certs: getCerts(t, clientCNCert),
 			User:  CommonNameUserConversion,
 
-			ExpectUserName: "client_cn",
-			ExpectGroups:   []string{"My Org"},
 			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": {
@@ -641,9 +654,16 @@ 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,
+			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,
 		},
 
@@ -664,8 +684,12 @@ func TestX509(t *testing.T) {
 				return &authenticator.Response{User: &user.DefaultInfo{Name: "custom"}}, true, nil
 			}),
 
-			ExpectUserName: "custom",
 			ExpectOK: true,
+			ExpectResponse: &authenticator.Response{
+				User: &user.DefaultInfo{
+					Name: "custom",
+				},
+			},
 			ExpectErr: false,
 		},
 
@@ -697,8 +721,15 @@ func TestX509(t *testing.T) {
 			Certs: getCertsFromFile(t, "client-valid", "intermediate"),
 			User:  CommonNameUserConversion,
 
-			ExpectUserName: "My Client",
 			ExpectOK: true,
+			ExpectResponse: &authenticator.Response{
+				User: &user.DefaultInfo{
+					Name: "My Client",
+					Extra: map[string][]string{
+						user.CredentialIDKey: {"X509SHA256=794b0529fd1a72d55d52d98be9bab5b822d16f9ae86c4373fa7beee3cafe8582"},
+					},
+				},
+			},
 			ExpectErr: false,
 		},
 		"multi-level, expired": {
@@ -712,6 +743,7 @@ func TestX509(t *testing.T) {
 	}
 
 	for k, testCase := range testCases {
+		t.Run(k, func(t *testing.T) {
 			req, _ := http.NewRequest("GET", "/", nil)
 			if !testCase.Insecure {
 				req.TLS = &tls.ConnectionState{PeerCertificates: testCase.Certs}
@@ -723,31 +755,24 @@ func TestX509(t *testing.T) {
 			resp, ok, err := a.AuthenticateRequest(req)
 
 			if testCase.ExpectErr && err == nil {
-			t.Errorf("%s: Expected error, got none", k)
-			continue
+				t.Fatalf("Expected error, got none")
 			}
 			if !testCase.ExpectErr && err != nil {
-			t.Errorf("%s: Got unexpected error: %v", k, err)
-			continue
+				t.Fatalf("Got unexpected error: %v", err)
 			}
 
 			if testCase.ExpectOK != ok {
-			t.Errorf("%s: Expected ok=%v, got %v", k, testCase.ExpectOK, ok)
-			continue
+				t.Fatalf("Expected ok=%v, got %v", testCase.ExpectOK, ok)
 			}
 
 			if testCase.ExpectOK {
-			if testCase.ExpectUserName != resp.User.GetName() {
-				t.Errorf("%s: Expected user.name=%v, got %v", k, testCase.ExpectUserName, resp.User.GetName())
-			}
-
-			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)
+				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)
 				}
 			}
+		})
 	}
 }
 

staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go

@@ -36,9 +36,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.
@@ -156,7 +153,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 {

staging/src/k8s.io/apiserver/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"
 )

test/integration/auth/svcaccttoken_test.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount"
+	"k8s.io/apiserver/pkg/authentication/user"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -237,7 +238,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		info := doTokenReview(t, cs, treq, false)
 		// we are not testing the credential-id feature, so delete this value from the returned extra info map
 		if info.Extra != nil {
-			delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
+			delete(info.Extra, user.CredentialIDKey)
 		}
 		if len(info.Extra) > 0 {
 			t.Fatalf("expected Extra to be empty but got: %#v", info.Extra)
@@ -309,7 +310,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 
 		info := doTokenReview(t, cs, treq, false)
 		// we are not testing the credential-id feature, so delete this value from the returned extra info map
-		delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
+		delete(info.Extra, user.CredentialIDKey)
 		if len(info.Extra) != 2 {
 			t.Fatalf("expected Extra have length of 2 but was length %d: %#v", len(info.Extra), info.Extra)
 		}
@@ -405,7 +406,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 
 			info := doTokenReview(t, cs, treq, false)
 			// we are not testing the credential-id feature, so delete this value from the returned extra info map
-			delete(info.Extra, apiserverserviceaccount.CredentialIDKey)
+			delete(info.Extra, user.CredentialIDKey)
 			if len(info.Extra) != len(expectedExtraValues) {
 				t.Fatalf("expected Extra have length of %d but was length %d: %#v", len(expectedExtraValues), len(info.Extra), info.Extra)
 			}