Merge pull request #42245 from deads2k/rbac-06-namespace-leak

Automatic merge from submit-queue (batch tested with PRs 42126, 42130, 42232, 42245, 41932)

allow subject access review to non-existent namespace

A localsubjectaccessreview is a special kind of resource which can be created even when the namespace doesn't exist.  Since permissions can be granted at different scopes, you can reasonably check if someone *could* do something at a lower scope that isn't there yet.  In addition, the permission to do an access check is separate from the permission to list all namespaces, so we're leaking information.

@liggitt @kubernetes/sig-auth-pr-reviews

plugin/pkg/admission/namespace/lifecycle/BUILD

@@ -21,6 +21,7 @@ go_library(
         "//vendor:github.com/golang/glog",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/apimachinery/pkg/util/sets",
         "//vendor:k8s.io/apiserver/pkg/admission",
         "//vendor:k8s.io/apiserver/pkg/util/cache",
@@ -41,6 +42,7 @@ go_test(
         "//pkg/kubeapiserver/admission:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
+        "//vendor:k8s.io/apimachinery/pkg/runtime/schema",
         "//vendor:k8s.io/apimachinery/pkg/util/sets",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
         "//vendor:k8s.io/apiserver/pkg/admission",

plugin/pkg/admission/namespace/lifecycle/admission.go

@@ -25,6 +25,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
 	utilcache "k8s.io/apiserver/pkg/util/cache"
@@ -103,6 +104,11 @@ func (l *lifecycle) Admit(a admission.Attributes) error {
 		return nil
 	}
 
+	// always allow access review checks.  Returning status about the namespace would be leaking information
+	if isAccessReview(a) {
+		return nil
+	}
+
 	// we need to wait for our caches to warm
 	if !l.WaitForReady() {
 		return admission.NewForbidden(a, fmt.Errorf("not yet ready to handle request"))
@@ -206,3 +212,13 @@ func (l *lifecycle) Validate() error {
 	}
 	return nil
 }
+
+// accessReviewResources are resources which give a view into permissions in a namespace.  Users must be allowed to create these
+// resources because returning "not found" errors allows someone to search for the "people I'm going to fire in 2017" namespace.
+var accessReviewResources = map[schema.GroupResource]bool{
+	schema.GroupResource{Group: "authorization.k8s.io", Resource: "localsubjectaccessreviews"}: true,
+}
+
+func isAccessReview(a admission.Attributes) bool {
+	return accessReviewResources[a.GetResource().GroupResource()]
+}

plugin/pkg/admission/namespace/lifecycle/admission_test.go

@@ -23,6 +23,7 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/admission"
@@ -91,6 +92,24 @@ func newPod(namespace string) api.Pod {
 	}
 }
 
+func TestAccessReviewCheckOnMissingNamespace(t *testing.T) {
+	namespace := "test"
+	mockClient := newMockClientForTest(map[string]api.NamespacePhase{})
+	mockClient.AddReactor("get", "namespaces", func(action core.Action) (bool, runtime.Object, error) {
+		return true, nil, fmt.Errorf("nope, out of luck")
+	})
+	handler, informerFactory, err := newHandlerForTest(mockClient)
+	if err != nil {
+		t.Errorf("unexpected error initializing handler: %v", err)
+	}
+	informerFactory.Start(wait.NeverStop)
+
+	err = handler.Admit(admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "authorization.k8s.io", Version: "v1", Kind: "LocalSubjectAccesReview"}, namespace, "", schema.GroupVersionResource{Group: "authorization.k8s.io", Version: "v1", Resource: "localsubjectaccessreviews"}, "", admission.Create, nil))
+	if err != nil {
+		t.Error(err)
+	}
+}
+
 // TestAdmissionNamespaceDoesNotExist verifies pod is not admitted if namespace does not exist.
 func TestAdmissionNamespaceDoesNotExist(t *testing.T) {
 	namespace := "test"