Merge pull request kubernetes#125634 from ahmedtd/x509credentialID

Define credential IDs for X.509 certificates

Author: k8s-ci-robot

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,21 +617,35 @@ 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": {
 			Opts:  getDefaultVerifyOptions(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)
+		t.Run(k, func(t *testing.T) {
+			req, _ := http.NewRequest("GET", "/", nil)
+			if !testCase.Insecure {
+				req.TLS = &tls.ConnectionState{PeerCertificates: testCase.Certs}
+			}
 
-		resp, ok, err := a.AuthenticateRequest(req)
+			// this effectively tests the simple dynamic verify function.
+			a := New(testCase.Opts, testCase.User)
 
-		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
-		}
+			resp, ok, err := a.AuthenticateRequest(req)
 
-		if testCase.ExpectOK != ok {
-			t.Errorf("%s: Expected ok=%v, got %v", k, testCase.ExpectOK, ok)
-			continue
-		}
+			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 {
-			if testCase.ExpectUserName != resp.User.GetName() {
-				t.Errorf("%s: Expected user.name=%v, got %v", k, testCase.ExpectUserName, resp.User.GetName())
+			if testCase.ExpectOK != ok {
+				t.Fatalf("Expected ok=%v, got %v", testCase.ExpectOK, ok)
 			}
 
-			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)
+			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)
+				}
 			}
-		}
+		})
 	}
 }
 

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

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