kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
11.03k stars 2.25k forks source link

Introduce new load restrictor that tolerates symlinks pointing out of tree #5216

Open seh opened 1 year ago

seh commented 1 year ago

Eschewed features

What would you like to have added?

At present, kustomize has two load restrictors or load restriction modes that govern which files a Kustomization may reference in its "resources" field (or files mentioned by other generators):

I propose that we introduce a third load restriction mode—called tentatively "Dominated Shallowly"—that sits between the two existing modes. Like "Root Only", it requires that all "resources" field entries (or files mentioned by other generators) must reference files dominated by the kustomization root directory, but—the crucial difference—any symbolic links may point finally at target files not dominated by the kustomization root directory. In other words, you have to dominate the links themselves, but the links can point anywhere.

Note that this load restriction mode does expose a kustomization author to a nefarious attack. It is possible that a kustomization's "configMapGenerator" field could mention the path to a symbolic link that points to a would-be innocuous regular file outside of the kustomization root, but then an attacker replaces that file with another symbolic link that points at a sensitive file such as /etc/passwd. As written, kustomize will traverse that chain of symbolic links and wind up reading the target /etc/passwd file, including its contents in the generated ConfigMap manifest. When running with the "Root Only" load restriction mode active, kustomize traps this transgression as a likely attack. In the proposed "Dominated Shallowly" mode, kustomize would tolerate this link path, assuming that the kustomization author is either in control of or trusts the safety of the referenced files.

Why is this needed?

Some tools like Bazel combine input files from many directories to present a "sandboxed" directory tree involving symbolic links that point to files originating from many other contributing directories (some static source files, and some generated files, sometimes also spread across several directories). The input files that kustomize sees could conform to the "Root Only" load restriction mode's expectations, if we first denormalize or flatten the symbolic links. Bazel offers sandboxing strategies that can accomplish this transformation either by copying files—normally avoided for efficiency—or by using alternative filesystems that can present a unified overlay of these disparate source files. Neither of these alternate sandboxing strategies are the default, and the latter isn't maintained today, effectively an abandoned experiment (successful as it may be).

Using kustomize within a harness like Bazel requires a difficult compromise: either disable kustomize's default "Root Only" load restriction mode, or use an uncommon Bazel sandboxing strategy, trading efficiency and hermetic safety to appease kustomize.

With a new load restriction mode, we could allow kustomize to consume input assembled by a harness like Bazel that still demands that input resource files be dominated, but only shallowly, asserting that the kustomization author is pointing (at least for the first hop) at the intended files.

Can you accomplish the motivating task without this feature, and if so, how?

It is possible to disable kustomize's load restriction entirely, or copy input files to assemble a fully dominated tree of input files.

What other solutions have you considered?

Mandate either use of alternative Bazel sandboxing strategies, or relaxing of kustomize's default load restriction mode. Users face either undue complexity or loss of safety. We could eliminate the former complexity while still retaining most of the latter safety.

Anything else we should know?

I first proposed this idea in the "kustomize" channel of the "Kubernetes" Slack workspace, and since then I've implemented this feature, stopping short of writing the related documentation.

If you're amenable to the idea, we still need to settle the name for the new load restriction mode. Again, my first proposed name is "Dominated Shallowly", but I recognize that that's quite a mouthful.

Feature ownership

k8s-ci-robot commented 1 year ago

This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
seh commented 1 year ago

More proposals for this new load restriction mode's name:

seh commented 10 months ago

Bazel removed its --experimental_use_sandboxfs command-line flag in commit bazelbuild/bazel@b6e2693f83a7ece37c902416de26a3807b541ceb, such that it's no longer viable to use sandboxfs to work around the "Root Only" load restrictor's intolerance for symbolic links pointing at files outside of the dominating kustomization root directory.

webwurst commented 9 months ago

Maybe alternatively there could be an argument to kustomize like --root-path /some/absolute/path which means: Any references that ends somewhere below that path is fine.

k8s-triage-robot commented 6 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

seh commented 6 months ago

/remove-lifecycle stale

mattcockrell commented 4 months ago

Maybe alternatively there could be an argument to kustomize like --root-path /some/absolute/path which means: Any references that ends somewhere below that path is fine.

If this could also be a value defined in the kustomization.yaml file, that would be convenient as well, with the following conditions.

cyraid commented 4 months ago

Maybe alternatively there could be an argument to kustomize like --root-path /some/absolute/path which means: Any references that ends somewhere below that path is fine.

I like the --root-path idea, as we're often dealing with a k8s or kubernetes folder which we trust, and doing things like kustomize build "<kubernetes>/base/release" | kubectl apply -f - referencing the files beneath it.

k8s-triage-robot commented 1 month ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

seh commented 1 month ago

/remove-lifecycle stale