Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
seh opened this issue Jun 19, 2023 · 8 comments
Open
2 tasks done
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@seh
Copy link
Contributor

seh commented Jun 19, 2023

Eschewed features

  • This issue is not requesting templating, unstuctured edits, build-time side-effects from args or env vars, or any other eschewed feature.

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):

  • Root Only (default)
    The "resources" field entry (or files mentioned by other generators) may reference only files dominated by the kustomization root directory. Further, any symbolic links must point finally at target files that are dominated by the kustomization root directory as well.
  • None
    The "resources" field entry (or files mentioned by other generators) may reference any files with no restrictions.

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

  • I am interested in contributing this feature myself! 🎉
@seh seh added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 19, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 19, 2023
@k8s-ci-robot
Copy link
Contributor

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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@seh
Copy link
Contributor Author

seh commented Jun 21, 2023

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

  • Root Only Shallowly
  • Root Shallowly
  • Root Start
  • Root Referenced

@seh
Copy link
Contributor Author

seh commented Dec 26, 2023

Bazel removed its --experimental_use_sandboxfs command-line flag in commit bazelbuild/bazel@b6e2693, 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
Copy link
Contributor

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
Copy link

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2024
@seh
Copy link
Contributor Author

seh commented Apr 14, 2024

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2024
@mattcockrell
Copy link

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.

  • only honor the value defined in the initially called file
  • if undefined, default to the kustomization root directory
  • allow '--root-path' flag to override

@cyraid
Copy link

cyraid commented Jul 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
Development

No branches or pull requests

6 participants