Merge pull request #68267 from liggitt/node-label-update

Limit kubelets from updating their own labels when NodeRestriction is enabled

cmd/kubelet/app/options/BUILD

@@ -23,6 +23,7 @@ go_library(
         "//pkg/credentialprovider/azure:go_default_library",
         "//pkg/credentialprovider/gcp:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
         "//pkg/kubelet/apis/config:go_default_library",
         "//pkg/kubelet/apis/config/scheme:go_default_library",
         "//pkg/kubelet/apis/config/validation:go_default_library",
@@ -33,6 +34,7 @@ go_library(
         "//pkg/util/taints:go_default_library",
         "//pkg/version/verflag:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/flag:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/logs:go_default_library",

cmd/kubelet/app/options/options.go

@@ -27,11 +27,14 @@ import (
 	"github.com/spf13/pflag"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/apiserver/pkg/util/flag"
+	"k8s.io/klog"
 	"k8s.io/kubelet/config/v1beta1"
 	"k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/features"
+	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
 	kubeletscheme "k8s.io/kubernetes/pkg/kubelet/apis/config/scheme"
 	kubeletconfigvalidation "k8s.io/kubernetes/pkg/kubelet/apis/config/validation"
@@ -250,9 +253,40 @@ func ValidateKubeletFlags(f *KubeletFlags) error {
 	if f.NodeStatusMaxImages < -1 {
 		return fmt.Errorf("invalid configuration: NodeStatusMaxImages (--node-status-max-images) must be -1 or greater")
 	}
+
+	unknownLabels := sets.NewString()
+	for k := range f.NodeLabels {
+		if isKubernetesLabel(k) && !kubeletapis.IsKubeletLabel(k) {
+			unknownLabels.Insert(k)
+		}
+	}
+	if len(unknownLabels) > 0 {
+		// TODO(liggitt): in 1.15, return an error
+		klog.Warningf("unknown 'kubernetes.io' or 'k8s.io' labels specified with --node-labels: %v", unknownLabels.List())
+		klog.Warningf("in 1.15, --node-labels in the 'kubernetes.io' namespace must begin with an allowed prefix (%s) or be in the specifically allowed set (%s)", strings.Join(kubeletapis.KubeletLabelNamespaces(), ", "), strings.Join(kubeletapis.KubeletLabels(), ", "))
+	}
+
 	return nil
 }
 
+func isKubernetesLabel(key string) bool {
+	namespace := getLabelNamespace(key)
+	if namespace == "kubernetes.io" || strings.HasSuffix(namespace, ".kubernetes.io") {
+		return true
+	}
+	if namespace == "k8s.io" || strings.HasSuffix(namespace, ".k8s.io") {
+		return true
+	}
+	return false
+}
+
+func getLabelNamespace(key string) string {
+	if parts := strings.SplitN(key, "/", 2); len(parts) == 2 {
+		return parts[0]
+	}
+	return ""
+}
+
 // NewKubeletConfiguration will create a new KubeletConfiguration with default values
 func NewKubeletConfiguration() (*kubeletconfig.KubeletConfiguration, error) {
 	scheme, _, err := kubeletscheme.NewSchemeAndCodecs()
@@ -381,7 +415,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) {
 	fs.BoolVar(&f.ExperimentalCheckNodeCapabilitiesBeforeMount, "experimental-check-node-capabilities-before-mount", f.ExperimentalCheckNodeCapabilitiesBeforeMount, "[Experimental] if set true, the kubelet will check the underlying node for required components (binaries, etc.) before performing the mount")
 	fs.BoolVar(&f.ExperimentalNodeAllocatableIgnoreEvictionThreshold, "experimental-allocatable-ignore-eviction", f.ExperimentalNodeAllocatableIgnoreEvictionThreshold, "When set to 'true', Hard Eviction Thresholds will be ignored while calculating Node Allocatable. See https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/ for more details. [default=false]")
 	bindableNodeLabels := flag.ConfigurationMap(f.NodeLabels)
-	fs.Var(&bindableNodeLabels, "node-labels", "<Warning: Alpha feature> Labels to add when registering the node in the cluster.  Labels must be key=value pairs separated by ','.")
+	fs.Var(&bindableNodeLabels, "node-labels", fmt.Sprintf("<Warning: Alpha feature> Labels to add when registering the node in the cluster.  Labels must be key=value pairs separated by ','. Labels in the 'kubernetes.io' namespace must begin with an allowed prefix (%s) or be in the specifically allowed set (%s)", strings.Join(kubeletapis.KubeletLabelNamespaces(), ", "), strings.Join(kubeletapis.KubeletLabels(), ", ")))
 	fs.StringVar(&f.VolumePluginDir, "volume-plugin-dir", f.VolumePluginDir, "The full path of the directory in which to search for additional third party volume plugins")
 	fs.StringVar(&f.LockFilePath, "lock-file", f.LockFilePath, "<Warning: Alpha feature> The path to file for kubelet to use as a lock file.")
 	fs.BoolVar(&f.ExitOnLockContention, "exit-on-lock-contention", f.ExitOnLockContention, "Whether kubelet should exit upon lock-file contention.")

pkg/kubelet/apis/BUILD

@@ -13,7 +13,9 @@ go_library(
         "well_known_labels.go",
     ],
     importpath = "k8s.io/kubernetes/pkg/kubelet/apis",
-    deps = select({
+    deps = [
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+    ] + select({
         "@io_bazel_rules_go//go/platform:windows": [
             "//pkg/features:go_default_library",
             "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",

pkg/kubelet/apis/well_known_labels.go

@@ -16,6 +16,12 @@ limitations under the License.
 
 package apis
 
+import (
+	"strings"
+
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
 const (
 	LabelHostname           = "kubernetes.io/hostname"
 	LabelZoneFailureDomain  = "failure-domain.beta.kubernetes.io/zone"
@@ -26,8 +32,78 @@ const (
 
 	LabelOS   = "beta.kubernetes.io/os"
 	LabelArch = "beta.kubernetes.io/arch"
+
+	// GA versions of the legacy beta labels.
+	// TODO: update kubelet and controllers to set both beta and GA labels, then export these constants
+	labelZoneFailureDomainGA = "failure-domain.kubernetes.io/zone"
+	labelZoneRegionGA        = "failure-domain.kubernetes.io/region"
+	labelInstanceTypeGA      = "kubernetes.io/instance-type"
+	labelOSGA                = "kubernetes.io/os"
+	labelArchGA              = "kubernetes.io/arch"
+
+	// LabelNamespaceSuffixKubelet is an allowed label namespace suffix kubelets can self-set ([*.]kubelet.kubernetes.io/*)
+	LabelNamespaceSuffixKubelet = "kubelet.kubernetes.io"
+	// LabelNamespaceSuffixNode is an allowed label namespace suffix kubelets can self-set ([*.]node.kubernetes.io/*)
+	LabelNamespaceSuffixNode = "node.kubernetes.io"
+
+	// LabelNamespaceNodeRestriction is a forbidden label namespace that kubelets may not self-set when the NodeRestriction admission plugin is enabled
+	LabelNamespaceNodeRestriction = "node-restriction.kubernetes.io"
 )
 
 // When the --failure-domains scheduler flag is not specified,
 // DefaultFailureDomains defines the set of label keys used when TopologyKey is empty in PreferredDuringScheduling anti-affinity.
 var DefaultFailureDomains string = LabelHostname + "," + LabelZoneFailureDomain + "," + LabelZoneRegion
+
+var kubeletLabels = sets.NewString(
+	LabelHostname,
+	LabelZoneFailureDomain,
+	LabelZoneRegion,
+	LabelInstanceType,
+	LabelOS,
+	LabelArch,
+
+	labelZoneFailureDomainGA,
+	labelZoneRegionGA,
+	labelInstanceTypeGA,
+	labelOSGA,
+	labelArchGA,
+)
+
+var kubeletLabelNamespaces = sets.NewString(
+	LabelNamespaceSuffixKubelet,
+	LabelNamespaceSuffixNode,
+)
+
+// KubeletLabels returns the list of label keys kubelets are allowed to set on their own Node objects
+func KubeletLabels() []string {
+	return kubeletLabels.List()
+}
+
+// KubeletLabelNamespaces returns the list of label key namespaces kubelets are allowed to set on their own Node objects
+func KubeletLabelNamespaces() []string {
+	return kubeletLabelNamespaces.List()
+}
+
+// IsKubeletLabel returns true if the label key is one that kubelets are allowed to set on their own Node object.
+// This checks if the key is in the KubeletLabels() list, or has a namespace in the KubeletLabelNamespaces() list.
+func IsKubeletLabel(key string) bool {
+	if kubeletLabels.Has(key) {
+		return true
+	}
+
+	namespace := getLabelNamespace(key)
+	for allowedNamespace := range kubeletLabelNamespaces {
+		if namespace == allowedNamespace || strings.HasSuffix(namespace, "."+allowedNamespace) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func getLabelNamespace(key string) string {
+	if parts := strings.SplitN(key, "/", 2); len(parts) == 2 {
+		return parts[0]
+	}
+	return ""
+}

plugin/pkg/admission/noderestriction/BUILD

@@ -18,16 +18,19 @@ go_library(
         "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/informers:go_default_library",
         "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
         "//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
     ],
 )
 
@@ -42,10 +45,12 @@ go_test(
         "//pkg/apis/policy:go_default_library",
         "//pkg/auth/nodeidentifier:go_default_library",
         "//pkg/features:go_default_library",
+        "//pkg/kubelet/apis:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",

plugin/pkg/admission/noderestriction/admission.go

@@ -19,24 +19,28 @@ package noderestriction
 import (
 	"fmt"
 	"io"
+	"strings"
 
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
 	apiserveradmission "k8s.io/apiserver/pkg/admission/initializer"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/informers"
 	corev1lister "k8s.io/client-go/listers/core/v1"
 	csiv1alpha1 "k8s.io/csi-api/pkg/apis/csi/v1alpha1"
+	"k8s.io/klog"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
 	coordapi "k8s.io/kubernetes/pkg/apis/coordination"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/policy"
 	"k8s.io/kubernetes/pkg/auth/nodeidentifier"
 	"k8s.io/kubernetes/pkg/features"
+	kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
 )
 
 const (
@@ -330,6 +334,18 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error {
 			return admission.NewForbidden(a, fmt.Errorf("cannot create with non-nil configSource"))
 		}
 
+		// Don't allow a node to register with labels outside the allowed set.
+		// This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself.
+		modifiedLabels := getModifiedLabels(node.Labels, nil)
+		if forbiddenLabels := c.getForbiddenCreateLabels(modifiedLabels); len(forbiddenLabels) > 0 {
+			return admission.NewForbidden(a, fmt.Errorf("cannot set labels: %s", strings.Join(forbiddenLabels.List(), ", ")))
+		}
+		// check and warn if nodes set labels on create that would have been forbidden on update
+		// TODO(liggitt): in 1.17, expand getForbiddenCreateLabels to match getForbiddenUpdateLabels and drop this
+		if forbiddenUpdateLabels := c.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 {
+			klog.Warningf("node %q added disallowed labels on node creation: %s", nodeName, strings.Join(forbiddenUpdateLabels.List(), ", "))
+		}
+
 		// On create, get name from new object if unset in admission
 		if len(requestedName) == 0 {
 			requestedName = node.Name
@@ -353,19 +369,100 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error {
 		// We scope node access to things listed in the Node.Spec, so allowing this would allow a view escalation.
 		// We only do the check if the new node's configSource is non-nil; old kubelets might drop the field during a status update.
 		if node.Spec.ConfigSource != nil && !apiequality.Semantic.DeepEqual(node.Spec.ConfigSource, oldNode.Spec.ConfigSource) {
-			return admission.NewForbidden(a, fmt.Errorf("cannot update configSource to a new non-nil configSource"))
+			return admission.NewForbidden(a, fmt.Errorf("node %q cannot update configSource to a new non-nil configSource", nodeName))
 		}
 
 		// Don't allow a node to update its own taints. This would allow a node to remove or modify its
 		// taints in a way that would let it steer disallowed workloads to itself.
 		if !apiequality.Semantic.DeepEqual(node.Spec.Taints, oldNode.Spec.Taints) {
-			return admission.NewForbidden(a, fmt.Errorf("cannot modify taints"))
+			return admission.NewForbidden(a, fmt.Errorf("node %q cannot modify taints", nodeName))
+		}
+
+		// Don't allow a node to update labels outside the allowed set.
+		// This would allow a node to add or modify its labels in a way that would let it steer privileged workloads to itself.
+		modifiedLabels := getModifiedLabels(node.Labels, oldNode.Labels)
+		if forbiddenUpdateLabels := c.getForbiddenUpdateLabels(modifiedLabels); len(forbiddenUpdateLabels) > 0 {
+			return admission.NewForbidden(a, fmt.Errorf("cannot modify labels: %s", strings.Join(forbiddenUpdateLabels.List(), ", ")))
 		}
 	}
 
 	return nil
 }
 
+// getModifiedLabels returns the set of label keys that are different between the two maps
+func getModifiedLabels(a, b map[string]string) sets.String {
+	modified := sets.NewString()
+	for k, v1 := range a {
+		if v2, ok := b[k]; !ok || v1 != v2 {
+			modified.Insert(k)
+		}
+	}
+	for k, v1 := range b {
+		if v2, ok := a[k]; !ok || v1 != v2 {
+			modified.Insert(k)
+		}
+	}
+	return modified
+}
+
+func isKubernetesLabel(key string) bool {
+	namespace := getLabelNamespace(key)
+	if namespace == "kubernetes.io" || strings.HasSuffix(namespace, ".kubernetes.io") {
+		return true
+	}
+	if namespace == "k8s.io" || strings.HasSuffix(namespace, ".k8s.io") {
+		return true
+	}
+	return false
+}
+
+func getLabelNamespace(key string) string {
+	if parts := strings.SplitN(key, "/", 2); len(parts) == 2 {
+		return parts[0]
+	}
+	return ""
+}
+
+// getForbiddenCreateLabels returns the set of labels that may not be set by the node.
+// TODO(liggitt): in 1.17, expand to match getForbiddenUpdateLabels()
+func (c *nodePlugin) getForbiddenCreateLabels(modifiedLabels sets.String) sets.String {
+	if len(modifiedLabels) == 0 {
+		return nil
+	}
+
+	forbiddenLabels := sets.NewString()
+	for label := range modifiedLabels {
+		namespace := getLabelNamespace(label)
+		// forbid kubelets from setting node-restriction labels
+		if namespace == kubeletapis.LabelNamespaceNodeRestriction || strings.HasSuffix(namespace, "."+kubeletapis.LabelNamespaceNodeRestriction) {
+			forbiddenLabels.Insert(label)
+		}
+	}
+	return forbiddenLabels
+}
+
+// getForbiddenLabels returns the set of labels that may not be set by the node on update.
+func (c *nodePlugin) getForbiddenUpdateLabels(modifiedLabels sets.String) sets.String {
+	if len(modifiedLabels) == 0 {
+		return nil
+	}
+
+	forbiddenLabels := sets.NewString()
+	for label := range modifiedLabels {
+		namespace := getLabelNamespace(label)
+		// forbid kubelets from setting node-restriction labels
+		if namespace == kubeletapis.LabelNamespaceNodeRestriction || strings.HasSuffix(namespace, "."+kubeletapis.LabelNamespaceNodeRestriction) {
+			forbiddenLabels.Insert(label)
+		}
+		// forbid kubelets from setting unknown kubernetes.io and k8s.io labels on update
+		if isKubernetesLabel(label) && !kubeletapis.IsKubeletLabel(label) {
+			// TODO: defer to label policy once available
+			forbiddenLabels.Insert(label)
+		}
+	}
+	return forbiddenLabels
+}
+
 func (c *nodePlugin) admitServiceAccount(nodeName string, a admission.Attributes) error {
 	if a.GetOperation() != admission.Create {
 		return nil