From cafbe253a13118d789ee5eb82223d49dc6f2ecda Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Wed, 27 Apr 2022 14:20:40 +0300 Subject: [PATCH] Disallow remote bases usage in Kustomize overlays Add an optional flag for disabling remote bases. While the `--no-remote-bases` is set to `false` by default, Flux users are encouraged to enable it on production system for security and performance reasons. Using Kustomize remote bases means that kustomize-controller must clone the remote repositories on every reconciliation instead of using the source-controller artifacts cache. Allowing remote bases on multi-tenant clusters, means platform admins have no control over which repositories make up the desired state. Signed-off-by: Stefan Prodan --- controllers/kustomization_controller.go | 3 ++- controllers/kustomization_generator.go | 20 +++++++++++++++----- controllers/kustomization_generator_test.go | 13 ++++++++++--- docs/spec/v1beta2/kustomization.md | 4 ++++ main.go | 4 ++++ 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/controllers/kustomization_controller.go b/controllers/kustomization_controller.go index b6ca517d..8bfa2151 100644 --- a/controllers/kustomization_controller.go +++ b/controllers/kustomization_controller.go @@ -88,6 +88,7 @@ type KustomizationReconciler struct { ControllerName string statusManager string NoCrossNamespaceRefs bool + NoRemoteBases bool DefaultServiceAccount string KubeConfigOpts runtimeClient.KubeConfigOptions } @@ -655,7 +656,7 @@ func (r *KustomizationReconciler) build(ctx context.Context, workDir string, kus return nil, fmt.Errorf("error decrypting env sources: %w", err) } - m, err := secureBuildKustomization(workDir, dirPath) + m, err := secureBuildKustomization(workDir, dirPath, !r.NoRemoteBases) if err != nil { return nil, fmt.Errorf("kustomize build failed: %w", err) } diff --git a/controllers/kustomization_generator.go b/controllers/kustomization_generator.go index 72809876..daa43e0a 100644 --- a/controllers/kustomization_generator.go +++ b/controllers/kustomization_generator.go @@ -24,6 +24,7 @@ import ( "strings" "sync" + "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/krusty" "sigs.k8s.io/kustomize/api/provider" @@ -247,11 +248,20 @@ var kustomizeBuildMutex sync.Mutex // - load files from outside the kustomization dir path // (but not outside root) // - disable plugins except for the builtin ones -func secureBuildKustomization(root, dirPath string) (_ resmap.ResMap, err error) { - // Create secure FS for root - fs, err := securefs.MakeFsOnDiskSecureBuild(root) - if err != nil { - return nil, err +func secureBuildKustomization(root, dirPath string, allowRemoteBases bool) (_ resmap.ResMap, err error) { + var fs filesys.FileSystem + + // Create secure FS for root with or without remote base support + if allowRemoteBases { + fs, err = securefs.MakeFsOnDiskSecureBuild(root) + if err != nil { + return nil, err + } + } else { + fs, err = securefs.MakeFsOnDiskSecure(root) + if err != nil { + return nil, err + } } // Temporary workaround for concurrent map read and map write bug diff --git a/controllers/kustomization_generator_test.go b/controllers/kustomization_generator_test.go index 15054cda..da73ffb0 100644 --- a/controllers/kustomization_generator_test.go +++ b/controllers/kustomization_generator_test.go @@ -26,20 +26,27 @@ func Test_secureBuildKustomization(t *testing.T) { t.Run("remote build", func(t *testing.T) { g := NewWithT(t) - _, err := secureBuildKustomization("testdata/remote", "testdata/remote") + _, err := secureBuildKustomization("testdata/remote", "testdata/remote", true) g.Expect(err).ToNot(HaveOccurred()) }) + + t.Run("no remote build", func(t *testing.T) { + g := NewWithT(t) + + _, err := secureBuildKustomization("testdata/remote", "testdata/remote", false) + g.Expect(err).To(HaveOccurred()) + }) } func Test_secureBuildKustomization_panic(t *testing.T) { t.Run("build panic", func(t *testing.T) { g := NewWithT(t) - _, err := secureBuildKustomization("testdata/panic", "testdata/panic") + _, err := secureBuildKustomization("testdata/panic", "testdata/panic", false) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("recovered from kustomize build panic")) // Run again to ensure the lock is released - _, err = secureBuildKustomization("testdata/panic", "testdata/panic") + _, err = secureBuildKustomization("testdata/panic", "testdata/panic", false) g.Expect(err).To(HaveOccurred()) }) } diff --git a/docs/spec/v1beta2/kustomization.md b/docs/spec/v1beta2/kustomization.md index 2c7d459b..e19521f7 100644 --- a/docs/spec/v1beta2/kustomization.md +++ b/docs/spec/v1beta2/kustomization.md @@ -287,6 +287,10 @@ spec: path: "./deploy/production" ``` +For security and performance reasons, it is advised to disallow the usage of +[remote bases](https://github.com/kubernetes-sigs/kustomize/blob/a7f4db7fb41e17b2c826a524f545e6174b4dc6ac/examples/remoteBuild.md) +in Kustomize overlays. To enforce this setting, platform admins can use the `--no-remote-bases=true` flag. + ## Source reference The Kustomization `spec.sourceRef` is a reference to an object managed by diff --git a/main.go b/main.go index 2b125232..e8fdf8ef 100644 --- a/main.go +++ b/main.go @@ -77,6 +77,7 @@ func main() { rateLimiterOptions helper.RateLimiterOptions aclOptions acl.Options watchAllNamespaces bool + noRemoteBases bool httpRetry int defaultServiceAccount string ) @@ -88,6 +89,8 @@ func main() { flag.DurationVar(&requeueDependency, "requeue-dependency", 30*time.Second, "The interval at which failing dependencies are reevaluated.") flag.BoolVar(&watchAllNamespaces, "watch-all-namespaces", true, "Watch for custom resources in all namespaces, if set to false it will only watch the runtime namespace.") + flag.BoolVar(&noRemoteBases, "no-remote-bases", false, + "Disallow remote bases usage in Kustomize overlays. When this flag is enabled, all resources must refer to local files included in the source artifact.") flag.IntVar(&httpRetry, "http-retry", 9, "The maximum number of retries when failing to fetch artifacts over HTTP.") flag.StringVar(&defaultServiceAccount, "default-service-account", "", "Default service account used for impersonation.") clientOptions.BindFlags(flag.CommandLine) @@ -146,6 +149,7 @@ func main() { EventRecorder: eventRecorder, MetricsRecorder: metricsRecorder, NoCrossNamespaceRefs: aclOptions.NoCrossNamespaceRefs, + NoRemoteBases: noRemoteBases, KubeConfigOpts: kubeConfigOpts, StatusPoller: polling.NewStatusPoller(mgr.GetClient(), mgr.GetRESTMapper(), polling.Options{ CustomStatusReaders: []engine.StatusReader{jobStatusReader},