Merge pull request #14227 from liggitt/node-authz

Automatic merge from submit-queue (batch tested with PRs 16224, 14227)

Node admission/authorization 

Enables the node authorizer and NodeRestriction admission plugins

Author: openshift-merge-robot

pkg/cmd/server/bootstrappolicy/dead.go

@@ -9,6 +9,8 @@ import (
 
 var (
 	deadClusterRoles = []rbac.ClusterRole{}
+
+	deadClusterRoleBindings = []rbac.ClusterRoleBinding{}
 )
 
 func addDeadClusterRole(name string) {
@@ -25,12 +27,33 @@ func addDeadClusterRole(name string) {
 	)
 }
 
+func addDeadClusterRoleBinding(name, roleName string) {
+	for _, existing := range deadClusterRoleBindings {
+		if name == existing.Name {
+			glog.Fatalf("%q was already registered", name)
+		}
+	}
+
+	deadClusterRoleBindings = append(deadClusterRoleBindings,
+		rbac.ClusterRoleBinding{
+			ObjectMeta: metav1.ObjectMeta{Name: name},
+			RoleRef:    rbac.RoleRef{APIGroup: rbac.GroupName, Kind: "ClusterRole", Name: roleName},
+		},
+	)
+}
+
 // GetDeadClusterRoles returns cluster roles which should no longer have any permissions.
 // These are enumerated so that a reconcile that tightens permissions will properly.
 func GetDeadClusterRoles() []rbac.ClusterRole {
 	return deadClusterRoles
 }
 
+// GetDeadClusterRoleBindings returns cluster role bindings which should no longer have any subjects.
+// These are enumerated so that a reconcile that tightens permissions will properly remove them.
+func GetDeadClusterRoleBindings() []rbac.ClusterRoleBinding {
+	return deadClusterRoleBindings
+}
+
 func init() {
 	// these were replaced by kube controller roles
 	addDeadClusterRole("system:replication-controller")
@@ -50,4 +73,7 @@ func init() {
 	addDeadClusterRole("system:build-controller")
 	addDeadClusterRole("system:deploymentconfig-controller")
 	addDeadClusterRole("system:deployment-controller")
+
+	// this was replaced by the node authorizer
+	addDeadClusterRoleBinding("system:nodes", "system:node")
 }

pkg/cmd/server/bootstrappolicy/policy.go

@@ -696,13 +696,13 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
 			Rules: []rbac.PolicyRule{
 				// Needed to check API access.  These creates are non-mutating
 				rbac.NewRule("create").Groups(kAuthnGroup).Resources("tokenreviews").RuleOrDie(),
-				rbac.NewRule("create").Groups(authzGroup, legacyAuthzGroup).Resources("subjectaccessreviews", "localsubjectaccessreviews").RuleOrDie(),
 				rbac.NewRule("create").Groups(kAuthzGroup).Resources("subjectaccessreviews", "localsubjectaccessreviews").RuleOrDie(),
 				// Needed to build serviceLister, to populate env vars for services
 				rbac.NewRule(read...).Groups(kapiGroup).Resources("services").RuleOrDie(),
 				// Nodes can register themselves
-				// TODO: restrict to creating a node with the same name they announce
+				// Use the NodeRestriction admission plugin to limit a node to creating/updating its own API object.
 				rbac.NewRule("create", "get", "list", "watch").Groups(kapiGroup).Resources("nodes").RuleOrDie(),
+				rbac.NewRule("update", "patch", "delete").Groups(kapiGroup).Resources("nodes").RuleOrDie(),
 				// TODO: restrict to the bound node once supported
 				rbac.NewRule("update", "patch").Groups(kapiGroup).Resources("nodes/status").RuleOrDie(),
 
@@ -731,7 +731,7 @@ func GetOpenshiftBootstrapClusterRoles() []rbac.ClusterRole {
 				// Needed for glusterfs volumes
 				rbac.NewRule("get").Groups(kapiGroup).Resources("endpoints").RuleOrDie(),
 				// Nodes are allowed to request CSRs (specifically, request serving certs)
-				rbac.NewRule("get", "create").Groups(certificates.GroupName).Resources("certificatesigningrequests").RuleOrDie(),
+				rbac.NewRule("get", "create", "list", "watch").Groups(certificates.GroupName).Resources("certificatesigningrequests").RuleOrDie(),
 			},
 		},
 
@@ -966,9 +966,6 @@ func GetOpenshiftBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
 		newOriginClusterBinding(StatusCheckerRoleBindingName, StatusCheckerRoleName).
 			Groups(AuthenticatedGroup, UnauthenticatedGroup).
 			BindingOrDie(),
-		newOriginClusterBinding(NodeRoleBindingName, NodeRoleName).
-			Groups(NodesGroup).
-			BindingOrDie(),
 		newOriginClusterBinding(NodeProxierRoleBindingName, NodeProxierRoleName).
 			// Allow node identities to run node proxies
 			Groups(NodesGroup).
@@ -1009,6 +1006,10 @@ func GetOpenshiftBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
 
 func GetBootstrapClusterRoleBindings() []rbac.ClusterRoleBinding {
 	openshiftClusterRoleBindings := GetOpenshiftBootstrapClusterRoleBindings()
+	// dead cluster roles need to be checked for conflicts (in case something new comes up)
+	// so add them to this list.
+	openshiftClusterRoleBindings = append(openshiftClusterRoleBindings, GetDeadClusterRoleBindings()...)
+
 	kubeClusterRoleBindings := bootstrappolicy.ClusterRoleBindings()
 	kubeControllerClusterRoleBindings := bootstrappolicy.ControllerRoleBindings()
 	openshiftControllerClusterRoleBindings := ControllerRoleBindings()

pkg/cmd/server/bootstrappolicy/policy_test.go

@@ -14,6 +14,7 @@ import (
 	"k8s.io/kubernetes/pkg/apis/rbac"
 	"k8s.io/kubernetes/pkg/apis/rbac/v1beta1"
 	rulevalidation "k8s.io/kubernetes/pkg/registry/rbac/validation"
+	kbootstrappolicy "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
 
 	"github.com/openshift/origin/pkg/api/v1"
 	"github.com/openshift/origin/pkg/cmd/server/bootstrappolicy"
@@ -109,6 +110,7 @@ func TestCovers(t *testing.T) {
 	var clusterAdmin *rbac.ClusterRole
 	var storageAdmin *rbac.ClusterRole
 	var imageBuilder *rbac.ClusterRole
+	var nodeRole *rbac.ClusterRole
 
 	for i := range allRoles {
 		role := allRoles[i]
@@ -135,6 +137,8 @@ func TestCovers(t *testing.T) {
 			storageAdmin = &role
 		case bootstrappolicy.ImageBuilderRoleName:
 			imageBuilder = &role
+		case bootstrappolicy.NodeRoleName:
+			nodeRole = &role
 		}
 	}
 
@@ -176,4 +180,19 @@ func TestCovers(t *testing.T) {
 	if covers, miss := rulevalidation.Covers(systemMaster.Rules, clusterAdmin.Rules); !covers {
 		t.Errorf("failed to cover: %#v", miss)
 	}
+
+	// Make sure our node role covers upstream node rules
+	if covers, miss := rulevalidation.Covers(nodeRole.Rules, kbootstrappolicy.NodeRules()); !covers {
+		t.Errorf("upstream node role has extra permissions:")
+		for _, r := range miss {
+			t.Logf("\t%s", r.CompactString())
+		}
+	}
+	// Make sure our node role doesn't have any extra permissions
+	if covers, miss := rulevalidation.Covers(kbootstrappolicy.NodeRules(), nodeRole.Rules); !covers {
+		t.Errorf("openshift node role has extra permissions:")
+		for _, r := range miss {
+			t.Logf("\t%s", r.CompactString())
+		}
+	}
 }

pkg/cmd/server/origin/admission/register.go

@@ -28,6 +28,7 @@ import (
 	securityadmission "github.com/openshift/origin/pkg/security/admission"
 	serviceadmit "github.com/openshift/origin/pkg/service/admission"
 
+	"k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
 	storageclassdefaultadmission "k8s.io/kubernetes/plugin/pkg/admission/storageclass/setdefault"
 
 	imagepolicyapi "github.com/openshift/origin/pkg/image/admission/imagepolicy/api"
@@ -83,6 +84,7 @@ var (
 		serviceadmit.RestrictedEndpointsPluginName,
 		"LimitRanger",
 		"ServiceAccount",
+		noderestriction.PluginName,
 		"SecurityContextConstraint",
 		"SCCExecRestrictions",
 		"PersistentVolumeLabel",
@@ -111,7 +113,6 @@ var (
 		// these are new, reassess post-rebase
 		"Initializers",
 		"GenericAdmissionWebhook",
-		"NodeRestriction",
 		"PodTolerationRestriction",
 	)
 )

pkg/cmd/server/origin/master_config.go

@@ -43,20 +43,25 @@ import (
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	kapi "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
 	kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	kinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions"
 	kinternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
+	coreinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/core/internalversion"
 	rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion"
 	rbaclisters "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion"
 	sacontroller "k8s.io/kubernetes/pkg/controller/serviceaccount"
 	kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
 	rbacregistryvalidation "k8s.io/kubernetes/pkg/registry/rbac/validation"
 	"k8s.io/kubernetes/pkg/serviceaccount"
+	noderestriction "k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
 	saadmit "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
 	storageclassdefaultadmission "k8s.io/kubernetes/plugin/pkg/admission/storageclass/setdefault"
+	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/node"
 	rbacauthorizer "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac"
+	kbootstrappolicy "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
 
 	"github.com/openshift/origin/pkg/auth/authenticator/request/paramtoken"
 	authnregistry "github.com/openshift/origin/pkg/auth/oauth/registry"
@@ -233,6 +238,8 @@ func BuildMasterConfig(options configapi.MasterConfig, informers InformerAccess)
 		kubeAuthorizer,
 		kubeSubjectLocator,
 		informers.GetInternalKubeInformers().Rbac().InternalVersion().ClusterRoles().Lister(),
+		informers.GetInternalKubeInformers().Core().InternalVersion().Pods(),
+		informers.GetInternalKubeInformers().Core().InternalVersion().PersistentVolumes(),
 		options.ProjectConfig.ProjectRequestMessage,
 	)
 
@@ -404,6 +411,7 @@ var (
 		"PodPreset",
 		"LimitRanger",
 		"ServiceAccount",
+		noderestriction.PluginName,
 		"SecurityContextConstraint",
 		storageclassdefaultadmission.PluginName,
 		"AlwaysPullImages",
@@ -415,7 +423,6 @@ var (
 		"DefaultTolerationSeconds",
 		"Initializers",
 		"GenericAdmissionWebhook",
-		"NodeRestriction",
 		"PodTolerationRestriction",
 		// NOTE: ResourceQuota and ClusterResourceQuota must be the last 2 plugins.
 		// DO NOT ADD ANY PLUGINS AFTER THIS LINE!
@@ -448,6 +455,7 @@ var (
 		"PodPreset",
 		"LimitRanger",
 		"ServiceAccount",
+		noderestriction.PluginName,
 		"SecurityContextConstraint",
 		storageclassdefaultadmission.PluginName,
 		"AlwaysPullImages",
@@ -459,7 +467,6 @@ var (
 		"DefaultTolerationSeconds",
 		"Initializers",
 		"GenericAdmissionWebhook",
-		"NodeRestriction",
 		"PodTolerationRestriction",
 		// NOTE: ResourceQuota and ClusterResourceQuota must be the last 2 plugins.
 		// DO NOT ADD ANY PLUGINS AFTER THIS LINE!
@@ -786,14 +793,27 @@ func buildKubeAuth(r rbacinformers.Interface) (kauthorizer.Authorizer, rbacregis
 	return kubeAuthorizer, ruleResolver, kubeSubjectLocator
 }
 
-func newAuthorizer(kubeAuthorizer kauthorizer.Authorizer, kubeSubjectLocator rbacauthorizer.SubjectLocator, clusterRoleGetter rbaclisters.ClusterRoleLister, projectRequestDenyMessage string) (kauthorizer.Authorizer, authorizer.SubjectLocator) {
+func newAuthorizer(
+	kubeAuthorizer kauthorizer.Authorizer,
+	kubeSubjectLocator rbacauthorizer.SubjectLocator,
+	clusterRoleGetter rbaclisters.ClusterRoleLister,
+	podInformer coreinformers.PodInformer,
+	pvInformer coreinformers.PersistentVolumeInformer,
+	projectRequestDenyMessage string,
+) (kauthorizer.Authorizer, authorizer.SubjectLocator) {
 	messageMaker := authorizer.NewForbiddenMessageResolver(projectRequestDenyMessage)
 	roleBasedAuthorizer := authorizer.NewAuthorizer(kubeAuthorizer, messageMaker)
 	subjectLocator := authorizer.NewSubjectLocator(kubeSubjectLocator)
+
 	scopeLimitedAuthorizer := scope.NewAuthorizer(roleBasedAuthorizer, clusterRoleGetter, messageMaker)
 
+	graph := node.NewGraph()
+	node.AddGraphEventHandlers(graph, podInformer, pvInformer)
+	nodeAuthorizer := node.NewAuthorizer(graph, nodeidentifier.NewDefaultNodeIdentifier(), kbootstrappolicy.NodeRules())
+
 	authorizer := authorizerunion.New(
 		authorizerfactory.NewPrivilegedGroups(user.SystemPrivilegedGroup), // authorizes system:masters to do anything, just like upstream
+		nodeAuthorizer,
 		scopeLimitedAuthorizer)
 
 	return authorizer, subjectLocator