From 5c09099084ba0983d62ea3bb71209c40bdb9c71c Mon Sep 17 00:00:00 2001 From: Ta-Ching Chen Date: Mon, 24 Feb 2020 20:02:33 +0800 Subject: [PATCH] Avoid exposing sensitive data to client (#1543) This PR changes the behavior of controller API which wrongly exposes sensitive data to the client. Now, the API only returns success if secret/configmap exists; otherwise, an error will be returned. --- pkg/controller/api.go | 4 +-- pkg/controller/client/v1/fake/fake_misc.go | 11 +++--- pkg/controller/client/v1/misc.go | 41 +++++----------------- pkg/controller/configmapApi.go | 13 ++----- pkg/controller/secretApi.go | 13 ++----- pkg/fission-cli/cmd/function/create.go | 4 +-- pkg/fission-cli/cmd/function/update.go | 4 +-- pkg/fission-cli/cmd/spec/spec.go | 4 +-- 8 files changed, 27 insertions(+), 67 deletions(-) diff --git a/pkg/controller/api.go b/pkg/controller/api.go index 4a7884c0..13c95173 100644 --- a/pkg/controller/api.go +++ b/pkg/controller/api.go @@ -239,8 +239,8 @@ func (api *API) GetHandler() http.Handler { r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiUpdate).Methods("PUT") r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiDelete).Methods("DELETE") - r.HandleFunc("/v2/secrets/{secret}", api.SecretGet).Methods("GET") - r.HandleFunc("/v2/configmaps/{configmap}", api.ConfigMapGet).Methods("GET") + r.HandleFunc("/v2/secrets/{secret}", api.SecretExists).Methods("GET") + r.HandleFunc("/v2/configmaps/{configmap}", api.ConfigMapExists).Methods("GET") r.HandleFunc("/v2/canaryconfigs", api.CanaryConfigApiCreate).Methods("POST") r.HandleFunc("/v2/canaryconfigs/{canaryConfig}", api.CanaryConfigApiGet).Methods("GET") diff --git a/pkg/controller/client/v1/fake/fake_misc.go b/pkg/controller/client/v1/fake/fake_misc.go index 6ac04859..e780bf24 100644 --- a/pkg/controller/client/v1/fake/fake_misc.go +++ b/pkg/controller/client/v1/fake/fake_misc.go @@ -17,11 +17,10 @@ limitations under the License. package fake import ( - v1 "github.com/fission/fission/pkg/controller/client/v1" "io" - apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "github.com/fission/fission/pkg/controller/client/v1" "github.com/fission/fission/pkg/info" ) @@ -34,12 +33,12 @@ func newMiscClient(c *v1.V1) v1.MiscInterface { return &FakeMisc{} } -func (c *FakeMisc) SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) { - return nil, nil +func (c *FakeMisc) SecretExists(m *metav1.ObjectMeta) error { + return nil } -func (c *FakeMisc) ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) { - return nil, nil +func (c *FakeMisc) ConfigMapExists(m *metav1.ObjectMeta) error { + return nil } func (c *FakeMisc) GetSvcURL(label string) (string, error) { diff --git a/pkg/controller/client/v1/misc.go b/pkg/controller/client/v1/misc.go index 3d7b1fef..9e8c71ba 100644 --- a/pkg/controller/client/v1/misc.go +++ b/pkg/controller/client/v1/misc.go @@ -24,7 +24,6 @@ import ( "net/http" "github.com/pkg/errors" - apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/fission/fission/pkg/controller/client/rest" @@ -39,8 +38,8 @@ type ( } MiscInterface interface { - SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) - ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) + SecretExists(m *metav1.ObjectMeta) error + ConfigMapExists(m *metav1.ObjectMeta) error GetSvcURL(label string) (string, error) ServerInfo() (*info.ServerInfo, error) PodLogs(m *metav1.ObjectMeta) (io.ReadCloser, int, error) @@ -55,52 +54,28 @@ func newMiscClient(c *V1) MiscInterface { return &Misc{client: c.restClient} } -func (c *Misc) SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) { +func (c *Misc) SecretExists(m *metav1.ObjectMeta) error { relativeUrl := fmt.Sprintf("secrets/%v", m.Name) relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace) resp, err := c.client.Get(relativeUrl) if err != nil { - return nil, err + return err } defer resp.Body.Close() - - body, err := handleResponse(resp) - if err != nil { - return nil, err - } - - var secret apiv1.Secret - err = json.Unmarshal(body, &secret) - if err != nil { - return nil, err - } - - return &secret, nil + return nil } -func (c *Misc) ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) { +func (c *Misc) ConfigMapExists(m *metav1.ObjectMeta) error { relativeUrl := fmt.Sprintf("configmaps/%v", m.Name) relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace) resp, err := c.client.Get(relativeUrl) if err != nil { - return nil, err + return err } defer resp.Body.Close() - - body, err := handleResponse(resp) - if err != nil { - return nil, err - } - - var configMap apiv1.ConfigMap - err = json.Unmarshal(body, &configMap) - if err != nil { - return nil, err - } - - return &configMap, nil + return nil } func (c *Misc) GetSvcURL(label string) (string, error) { diff --git a/pkg/controller/configmapApi.go b/pkg/controller/configmapApi.go index 7896eb06..94068f22 100644 --- a/pkg/controller/configmapApi.go +++ b/pkg/controller/configmapApi.go @@ -17,7 +17,6 @@ limitations under the License. package controller import ( - "encoding/json" "net/http" "github.com/gorilla/mux" @@ -25,7 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (a *API) ConfigMapGet(w http.ResponseWriter, r *http.Request) { +func (a *API) ConfigMapExists(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) name := vars["configmap"] ns := a.extractQueryParamFromRequest(r, "namespace") @@ -33,17 +32,11 @@ func (a *API) ConfigMapGet(w http.ResponseWriter, r *http.Request) { ns = metav1.NamespaceDefault } - configMap, err := a.kubernetesClient.CoreV1().ConfigMaps(ns).Get(name, metav1.GetOptions{}) + _, err := a.kubernetesClient.CoreV1().ConfigMaps(ns).Get(name, metav1.GetOptions{}) if err != nil { a.logger.Error("error getting config map", zap.Error(err), zap.String("config_map_name", name), zap.String("namespace", ns)) a.respondWithError(w, err) return } - - resp, err := json.Marshal(configMap) - if err != nil { - a.respondWithError(w, err) - return - } - a.respondWithSuccess(w, resp) + a.respondWithSuccess(w, nil) } diff --git a/pkg/controller/secretApi.go b/pkg/controller/secretApi.go index 7a2cb91f..4a3c68eb 100644 --- a/pkg/controller/secretApi.go +++ b/pkg/controller/secretApi.go @@ -17,7 +17,6 @@ limitations under the License. package controller import ( - "encoding/json" "net/http" "github.com/gorilla/mux" @@ -25,7 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) { +func (a *API) SecretExists(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) name := vars["secret"] ns := a.extractQueryParamFromRequest(r, "namespace") @@ -33,7 +32,7 @@ func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) { ns = metav1.NamespaceDefault } - secret, err := a.kubernetesClient.CoreV1().Secrets(ns).Get(name, metav1.GetOptions{}) + _, err := a.kubernetesClient.CoreV1().Secrets(ns).Get(name, metav1.GetOptions{}) if err != nil { a.logger.Error("error getting secret", zap.Error(err), @@ -42,11 +41,5 @@ func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) { a.respondWithError(w, err) return } - - resp, err := json.Marshal(secret) - if err != nil { - a.respondWithError(w, err) - return - } - a.respondWithSuccess(w, resp) + a.respondWithSuccess(w, nil) } diff --git a/pkg/fission-cli/cmd/function/create.go b/pkg/fission-cli/cmd/function/create.go index 32661775..ec3da96f 100644 --- a/pkg/fission-cli/cmd/function/create.go +++ b/pkg/fission-cli/cmd/function/create.go @@ -222,7 +222,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error { // check the referenced secret is in the same ns as the function, if not give a warning. if !toSpec { // TODO: workaround in order not to block users from creating function spec, remove it. for _, secretName := range secretNames { - _, err := opts.Client().V1().Misc().SecretGet(&metav1.ObjectMeta{ + err := opts.Client().V1().Misc().SecretExists(&metav1.ObjectMeta{ Namespace: fnNamespace, Name: secretName, }) @@ -248,7 +248,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error { // check the referenced cfgmap is in the same ns as the function, if not give a warning. if !toSpec { for _, cfgMapName := range cfgMapNames { - _, err := opts.Client().V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ + err := opts.Client().V1().Misc().ConfigMapExists(&metav1.ObjectMeta{ Namespace: fnNamespace, Name: cfgMapName, }) diff --git a/pkg/fission-cli/cmd/function/update.go b/pkg/fission-cli/cmd/function/update.go index a560d160..44dd5c3c 100644 --- a/pkg/fission-cli/cmd/function/update.go +++ b/pkg/fission-cli/cmd/function/update.go @@ -87,7 +87,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error { // check that the referenced secret is in the same ns as the function, if not give a warning. for _, secretName := range secretNames { - _, err := opts.Client().V1().Misc().SecretGet(&metav1.ObjectMeta{ + err := opts.Client().V1().Misc().SecretExists(&metav1.ObjectMeta{ Namespace: fnNamespace, Name: secretName, }) @@ -111,7 +111,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error { // check that the referenced cfgmap is in the same ns as the function, if not give a warning. for _, cfgMapName := range cfgMapNames { - _, err := opts.Client().V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ + err := opts.Client().V1().Misc().ConfigMapExists(&metav1.ObjectMeta{ Namespace: fnNamespace, Name: cfgMapName, }) diff --git a/pkg/fission-cli/cmd/spec/spec.go b/pkg/fission-cli/cmd/spec/spec.go index bcdec8e3..62854e87 100644 --- a/pkg/fission-cli/cmd/spec/spec.go +++ b/pkg/fission-cli/cmd/spec/spec.go @@ -400,7 +400,7 @@ func (fr *FissionResources) Validate(input cli.Input) ([]string, error) { return warnings, err } for _, cm := range f.Spec.ConfigMaps { - _, err := client.V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ + err := client.V1().Misc().ConfigMapExists(&metav1.ObjectMeta{ Name: cm.Name, Namespace: cm.Namespace, }) @@ -410,7 +410,7 @@ func (fr *FissionResources) Validate(input cli.Input) ([]string, error) { } for _, s := range f.Spec.Secrets { - _, err := client.V1().Misc().SecretGet(&metav1.ObjectMeta{ + err := client.V1().Misc().SecretExists(&metav1.ObjectMeta{ Name: s.Name, Namespace: s.Namespace, })