From be4f2813b7cc13f682f2af5025d42813c8e7fbd3 Mon Sep 17 00:00:00 2001 From: Sebastian Florek Date: Thu, 24 Jan 2019 09:43:37 +0100 Subject: [PATCH] Fix in-cluster config (#3522) * Fix in-cluster config * Format files and remove obsolete changes * Add documentation * Add tests * Change error messages --- .../kubernetes-dashboard-arm-head.yaml | 17 ++++- .../recommended/kubernetes-dashboard-arm.yaml | 17 ++++- .../kubernetes-dashboard-head.yaml | 17 ++++- .../recommended/kubernetes-dashboard.yaml | 17 ++++- src/app/backend/auth/jwe/keyholder.go | 13 ++-- src/app/backend/client/api/common.go | 17 ++++- src/app/backend/client/api/types.go | 14 ++++ src/app/backend/client/csrf/manager.go | 68 +++++++++++++++++++ src/app/backend/client/csrf/manager_test.go | 62 +++++++++++++++++ src/app/backend/client/manager.go | 43 ++---------- src/app/backend/scaling/scale.go | 3 +- src/app/backend/sync/overwatch.go | 17 ++++- src/app/backend/sync/secret.go | 5 +- 13 files changed, 252 insertions(+), 58 deletions(-) create mode 100644 src/app/backend/client/csrf/manager.go create mode 100644 src/app/backend/client/csrf/manager_test.go diff --git a/aio/deploy/recommended/kubernetes-dashboard-arm-head.yaml b/aio/deploy/recommended/kubernetes-dashboard-arm-head.yaml index ebcc1d0c8..33fab1a0b 100755 --- a/aio/deploy/recommended/kubernetes-dashboard-arm-head.yaml +++ b/aio/deploy/recommended/kubernetes-dashboard-arm-head.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ------------------- Dashboard Secret ------------------- # +# ------------------- Dashboard Secrets ------------------- # apiVersion: v1 kind: Secret @@ -23,6 +23,19 @@ metadata: namespace: kube-system type: Opaque +--- + +apiVersion: v1 +kind: Secret +metadata: + labels: + k8s-app: kubernetes-dashboard-head + name: kubernetes-dashboard-csrf + namespace: kube-system +type: Opaque +data: + csrf: "" + --- # ------------------- Dashboard Service Account ------------------- # @@ -54,7 +67,7 @@ rules: # Allow Dashboard to get, update and delete Dashboard exclusive secrets. - apiGroups: [""] resources: ["secrets"] - resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs"] + resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs", "kubernetes-dashboard-csrf"] verbs: ["get", "update", "delete"] # Allow Dashboard to get and update 'kubernetes-dashboard-settings' config map. - apiGroups: [""] diff --git a/aio/deploy/recommended/kubernetes-dashboard-arm.yaml b/aio/deploy/recommended/kubernetes-dashboard-arm.yaml index 65cb1f17d..dcfd04fd4 100644 --- a/aio/deploy/recommended/kubernetes-dashboard-arm.yaml +++ b/aio/deploy/recommended/kubernetes-dashboard-arm.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ------------------- Dashboard Secret ------------------- # +# ------------------- Dashboard Secrets ------------------- # apiVersion: v1 kind: Secret @@ -23,6 +23,19 @@ metadata: namespace: kube-system type: Opaque +--- + +apiVersion: v1 +kind: Secret +metadata: + labels: + k8s-app: kubernetes-dashboard + name: kubernetes-dashboard-csrf + namespace: kube-system +type: Opaque +data: + csrf: "" + --- # ------------------- Dashboard Service Account ------------------- # @@ -54,7 +67,7 @@ rules: # Allow Dashboard to get, update and delete Dashboard exclusive secrets. - apiGroups: [""] resources: ["secrets"] - resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs"] + resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs", "kubernetes-dashboard-csrf"] verbs: ["get", "update", "delete"] # Allow Dashboard to get and update 'kubernetes-dashboard-settings' config map. - apiGroups: [""] diff --git a/aio/deploy/recommended/kubernetes-dashboard-head.yaml b/aio/deploy/recommended/kubernetes-dashboard-head.yaml index 99244e75f..6b455cda9 100755 --- a/aio/deploy/recommended/kubernetes-dashboard-head.yaml +++ b/aio/deploy/recommended/kubernetes-dashboard-head.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ------------------- Dashboard Secret ------------------- # +# ------------------- Dashboard Secrets ------------------- # apiVersion: v1 kind: Secret @@ -23,6 +23,19 @@ metadata: namespace: kube-system type: Opaque +--- + +apiVersion: v1 +kind: Secret +metadata: + labels: + k8s-app: kubernetes-dashboard-head + name: kubernetes-dashboard-csrf + namespace: kube-system +type: Opaque +data: + csrf: "" + --- # ------------------- Dashboard Service Account ------------------- # @@ -54,7 +67,7 @@ rules: # Allow Dashboard to get, update and delete Dashboard exclusive secrets. - apiGroups: [""] resources: ["secrets"] - resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs"] + resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs", "kubernetes-dashboard-csrf"] verbs: ["get", "update", "delete"] # Allow Dashboard to get and update 'kubernetes-dashboard-settings' config map. - apiGroups: [""] diff --git a/aio/deploy/recommended/kubernetes-dashboard.yaml b/aio/deploy/recommended/kubernetes-dashboard.yaml index ee6977bf3..1fa5b14f5 100644 --- a/aio/deploy/recommended/kubernetes-dashboard.yaml +++ b/aio/deploy/recommended/kubernetes-dashboard.yaml @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ------------------- Dashboard Secret ------------------- # +# ------------------- Dashboard Secrets ------------------- # apiVersion: v1 kind: Secret @@ -23,6 +23,19 @@ metadata: namespace: kube-system type: Opaque +--- + +apiVersion: v1 +kind: Secret +metadata: + labels: + k8s-app: kubernetes-dashboard + name: kubernetes-dashboard-csrf + namespace: kube-system +type: Opaque +data: + csrf: "" + --- # ------------------- Dashboard Service Account ------------------- # @@ -54,7 +67,7 @@ rules: # Allow Dashboard to get, update and delete Dashboard exclusive secrets. - apiGroups: [""] resources: ["secrets"] - resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs"] + resourceNames: ["kubernetes-dashboard-key-holder", "kubernetes-dashboard-certs", "kubernetes-dashboard-csrf"] verbs: ["get", "update", "delete"] # Allow Dashboard to get and update 'kubernetes-dashboard-settings' config map. - apiGroups: [""] diff --git a/src/app/backend/auth/jwe/keyholder.go b/src/app/backend/auth/jwe/keyholder.go index 0d0a0df78..21340a5c2 100644 --- a/src/app/backend/auth/jwe/keyholder.go +++ b/src/app/backend/auth/jwe/keyholder.go @@ -20,15 +20,16 @@ import ( "log" "sync" - "github.com/kubernetes/dashboard/src/app/backend/args" - authApi "github.com/kubernetes/dashboard/src/app/backend/auth/api" - syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" jose "gopkg.in/square/go-jose.v2" v1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" + + "github.com/kubernetes/dashboard/src/app/backend/args" + authApi "github.com/kubernetes/dashboard/src/app/backend/auth/api" + syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" ) // Entries held by resource used to synchronize encryption key data. @@ -104,11 +105,13 @@ func (self *rsaKeyHolder) update(obj runtime.Object) { } // Handler function executed by synchronizer used to store encryption key. It is called whenever watched object -// is gets deleted. It is then recreated based on local key. +// gets deleted. It is then recreated based on local key. func (self *rsaKeyHolder) recreate(obj runtime.Object) { secret := obj.(*v1.Secret) log.Printf("Synchronized secret %s has been deleted. Recreating.", secret.Name) - self.synchronizer.Create(self.getEncryptionKeyHolder()) + if err := self.synchronizer.Create(self.getEncryptionKeyHolder()); err != nil { + panic(err) + } } func (self *rsaKeyHolder) init() { diff --git a/src/app/backend/client/api/common.go b/src/app/backend/client/api/common.go index 9aea66024..83e72cebd 100644 --- a/src/app/backend/client/api/common.go +++ b/src/app/backend/client/api/common.go @@ -14,7 +14,11 @@ package api -import v1 "k8s.io/api/authorization/v1" +import ( + "crypto/rand" + + v1 "k8s.io/api/authorization/v1" +) // ToSelfSubjectAccessReview creates kubernetes API object based on provided data. func ToSelfSubjectAccessReview(namespace, name, resource, verb string) *v1.SelfSubjectAccessReview { @@ -29,3 +33,14 @@ func ToSelfSubjectAccessReview(namespace, name, resource, verb string) *v1.SelfS }, } } + +// GenerateCSRFKey generates random csrf key +func GenerateCSRFKey() string { + bytes := make([]byte, 256) + _, err := rand.Read(bytes) + if err != nil { + panic("could not generate csrf key") + } + + return string(bytes) +} diff --git a/src/app/backend/client/api/types.go b/src/app/backend/client/api/types.go index 3b0e371b7..9ec88e27f 100644 --- a/src/app/backend/client/api/types.go +++ b/src/app/backend/client/api/types.go @@ -25,6 +25,14 @@ import ( "k8s.io/client-go/tools/clientcmd/api" ) +const ( + // Resource information that are used as csrf token storage. Can be accessible by multiple dashboard replicas. + CsrfTokenSecretName = "kubernetes-dashboard-csrf" + + // Name of the data var that holds the csrf token inside the secret. + CsrfTokenSecretData = "csrf" +) + // ClientManager is responsible for initializing and creating clients to communicate with // kubernetes apiserver on demand. type ClientManager interface { @@ -51,3 +59,9 @@ type ResourceVerber interface { type CanIResponse struct { Allowed bool `json:"allowed"` } + +// CsrfTokenManager is responsible for generating, reading and updating token stored in a secret. +type CsrfTokenManager interface { + // Token returns current csrf token used for csrf signing. + Token() string +} diff --git a/src/app/backend/client/csrf/manager.go b/src/app/backend/client/csrf/manager.go new file mode 100644 index 000000000..a759737d6 --- /dev/null +++ b/src/app/backend/client/csrf/manager.go @@ -0,0 +1,68 @@ +// Copyright 2017 The Kubernetes Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package csrf + +import ( + "log" + + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + + "github.com/kubernetes/dashboard/src/app/backend/args" + "github.com/kubernetes/dashboard/src/app/backend/client/api" +) + +// Implements CsrfTokenManager interface. +type csrfTokenManager struct { + token string + client kubernetes.Interface +} + +func (self *csrfTokenManager) init() { + log.Printf("Initializing csrf token from %s secret", api.CsrfTokenSecretName) + tokenSecret, err := self.client.CoreV1(). + Secrets(args.Holder.GetNamespace()). + Get(api.CsrfTokenSecretName, v1.GetOptions{}) + + if err != nil { + panic(err) + } + + token := string(tokenSecret.Data[api.CsrfTokenSecretData]) + if len(token) == 0 { + log.Printf("Empty token. Generating and storing in a secret %s", api.CsrfTokenSecretName) + token = api.GenerateCSRFKey() + tokenSecret.StringData = map[string]string{api.CsrfTokenSecretData: token} + _, err := self.client.CoreV1().Secrets(args.Holder.GetNamespace()).Update(tokenSecret) + if err != nil { + panic(err) + } + } + + self.token = token +} + +// Token implements CsrfTokenManager interface. +func (self *csrfTokenManager) Token() string { + return self.token +} + +// NewCsrfTokenManager creates and initializes new instace of csrf token manager. +func NewCsrfTokenManager(client kubernetes.Interface) api.CsrfTokenManager { + manager := &csrfTokenManager{client: client} + manager.init() + + return manager +} diff --git a/src/app/backend/client/csrf/manager_test.go b/src/app/backend/client/csrf/manager_test.go new file mode 100644 index 000000000..c02d919a1 --- /dev/null +++ b/src/app/backend/client/csrf/manager_test.go @@ -0,0 +1,62 @@ +// Copyright 2017 The Kubernetes Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package csrf_test + +import ( + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + + "github.com/kubernetes/dashboard/src/app/backend/client/api" + "github.com/kubernetes/dashboard/src/app/backend/client/csrf" +) + +func TestCsrfTokenManager_Token(t *testing.T) { + cases := []struct { + info string + csrfSecret *v1.Secret + wantPanic bool + wantToken bool + }{ + {"should panic when secret does not exist", nil, true, false}, + {"should generate token when secret exists", + &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: api.CsrfTokenSecretName, + }, + }, false, true}, + } + + for _, c := range cases { + t.Run(c.info, func(t *testing.T) { + defer func() { + r := recover() + if (r != nil) != c.wantPanic { + t.Errorf("Recover = %v, wantPanic = %v", r, c.wantPanic) + } + }() + + client := fake.NewSimpleClientset(c.csrfSecret) + manager := csrf.NewCsrfTokenManager(client) + + if (len(manager.Token()) == 0) == c.wantToken { + t.Errorf("Expected token to exist: %v", c.wantToken) + } + }) + + } +} diff --git a/src/app/backend/client/manager.go b/src/app/backend/client/manager.go index 7ecdd59bf..69d16ac07 100644 --- a/src/app/backend/client/manager.go +++ b/src/app/backend/client/manager.go @@ -15,7 +15,6 @@ package client import ( - "crypto/rand" "errors" "log" "strings" @@ -31,6 +30,7 @@ import ( "github.com/kubernetes/dashboard/src/app/backend/args" authApi "github.com/kubernetes/dashboard/src/app/backend/auth/api" clientapi "github.com/kubernetes/dashboard/src/app/backend/client/api" + "github.com/kubernetes/dashboard/src/app/backend/client/csrf" kdErrors "github.com/kubernetes/dashboard/src/app/backend/errors" ) @@ -231,19 +231,6 @@ func (self *clientManager) buildConfigFromFlags(apiserverHost, kubeConfigPath st return nil, errors.New("could not create client config") } -// Based on rest config creates auth info structure. -func (self *clientManager) buildAuthInfoFromConfig(cfg *rest.Config) api.AuthInfo { - return api.AuthInfo{ - Token: cfg.BearerToken, - ClientCertificate: cfg.CertFile, - ClientKey: cfg.KeyFile, - ClientCertificateData: cfg.CertData, - ClientKeyData: cfg.KeyData, - Username: cfg.Username, - Password: cfg.Password, - } -} - // Based on auth info and rest config creates client cmd config. func (self *clientManager) buildCmdConfig(authInfo *api.AuthInfo, cfg *rest.Config) clientcmd.ClientConfig { cmdCfg := api.NewConfig() @@ -339,8 +326,8 @@ func (self *clientManager) secureConfig(req *restful.Request) (*rest.Config, err // Initializes client manager func (self *clientManager) init() { self.initInClusterConfig() - self.initCSRFKey() self.initInsecureClient() + self.initCSRFKey() } // Initializes in-cluster config if apiserverHost and kubeConfigPath were not provided. @@ -366,13 +353,13 @@ func (self *clientManager) initCSRFKey() { if self.inClusterConfig == nil { // Most likely running for a dev, so no replica issues, just generate a random key log.Println("Using random key for csrf signing") - self.generateCSRFKey() + self.csrfKey = clientapi.GenerateCSRFKey() return } // We run in a cluster, so we should use a signing key that is the same for potential replications - log.Println("Using service account token for csrf signing") - self.csrfKey = self.inClusterConfig.BearerToken + log.Println("Using secret token for csrf signing") + self.csrfKey = csrf.NewCsrfTokenManager(self.insecureClient).Token() } func (self *clientManager) initInsecureClient() { @@ -391,30 +378,10 @@ func (self *clientManager) initInsecureConfig() { panic(err) } - defaultAuthInfo := self.buildAuthInfoFromConfig(cfg) - authInfo := &defaultAuthInfo - - cmdConfig := self.buildCmdConfig(authInfo, cfg) - cfg, err = cmdConfig.ClientConfig() - if err != nil { - panic(err) - } - self.initConfig(cfg) self.insecureConfig = cfg } -// Generates random csrf key -func (self *clientManager) generateCSRFKey() { - bytes := make([]byte, 256) - _, err := rand.Read(bytes) - if err != nil { - panic("Fatal error. Could not generate csrf key") - } - - self.csrfKey = string(bytes) -} - // Returns true if in-cluster config is used func (self *clientManager) isRunningInCluster() bool { return self.inClusterConfig != nil diff --git a/src/app/backend/scaling/scale.go b/src/app/backend/scaling/scale.go index 54877a128..bc0ef4146 100644 --- a/src/app/backend/scaling/scale.go +++ b/src/app/backend/scaling/scale.go @@ -15,6 +15,8 @@ package scaling import ( + "strconv" + "k8s.io/client-go/discovery" "k8s.io/client-go/discovery/cached" "k8s.io/client-go/dynamic" @@ -23,7 +25,6 @@ import ( "k8s.io/client-go/restmapper" "k8s.io/client-go/scale" "k8s.io/client-go/scale/scheme/appsv1beta2" - "strconv" ) // ReplicaCounts provide the desired and actual number of replicas. diff --git a/src/app/backend/sync/overwatch.go b/src/app/backend/sync/overwatch.go index df3896913..522610b92 100644 --- a/src/app/backend/sync/overwatch.go +++ b/src/app/backend/sync/overwatch.go @@ -15,11 +15,13 @@ package sync import ( + "fmt" "log" "time" - syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" "k8s.io/apimachinery/pkg/util/wait" + + syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" ) // Overwatch is watching over every registered synchronizer. In case of error it will be logged and if RestartPolicy @@ -49,11 +51,15 @@ const ( NeverRestart RestartPolicy = "never" RestartDelay = 2 * time.Second + // We don't need to sync it with every instance. If a single instance synchronizer fails + // often, just force restart it. + MaxRestartCount = 15 ) type overwatch struct { - syncMap map[string]syncApi.Synchronizer - policyMap map[string]RestartPolicy + syncMap map[string]syncApi.Synchronizer + policyMap map[string]RestartPolicy + restartCount map[string]int registrationSignal chan string restartSignal chan string @@ -81,6 +87,10 @@ func (self *overwatch) monitorRestartEvents() { go wait.Forever(func() { select { case name := <-self.restartSignal: + if self.restartCount[name] > MaxRestartCount { + panic(fmt.Sprintf("synchronizer %s restart limit execeeded. Restarting pod.", name)) + } + log.Printf("Restarting synchronizer: %s.", name) synchronizer := self.syncMap[name] synchronizer.Start() @@ -112,6 +122,7 @@ func (self *overwatch) monitorSynchronizerStatus(synchronizer syncApi.Synchroniz // Wait a sec before restarting synchronizer in case it exited with error. time.Sleep(RestartDelay) self.broadcastRestartEvent(name) + self.restartCount[name]++ } close(stopCh) diff --git a/src/app/backend/sync/secret.go b/src/app/backend/sync/secret.go index fe3808d7e..20fa22459 100644 --- a/src/app/backend/sync/secret.go +++ b/src/app/backend/sync/secret.go @@ -22,14 +22,15 @@ import ( "sync" "time" - syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" - "github.com/kubernetes/dashboard/src/app/backend/sync/poll" v1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes" + + syncApi "github.com/kubernetes/dashboard/src/app/backend/sync/api" + "github.com/kubernetes/dashboard/src/app/backend/sync/poll" ) // Time interval between which secret should be resynchronized. -- GitLab