Update kuberc KEP with credential plugin allowlist

Currently, the kubeconfig may specify arbitrary binaries to run as
client-go credential plugins. Due to the fact that kubeconfig files are
often generated, and because they contain a lot of noise, there is
significant friction in manually inspecting the kubeconfig for
suspicious binaries after it is generated.

To encourage secure behavior, we want to introduce an allowlist to the
kuberc, which will describe the conditions under which a binary plugin
may be run. Currently the only condition is the name (absolute path or
basename of a binary found in `PATH`).

* Revise based on feedback
  - Describe behavior when allowlist is `nil`
  - Describe behavior when allowlist is empty
  - Describe future plans for field additions
* Add SIG Auth as participating SIG
* Update TOC for KEP 3104
* Correct `os.LookPath` to `exec.LookPath`
* Add note about when the allowlist will not apply
* Avoid confusion between nil list and empty list
    In the credential plugin allowlist, consider nil and empty lists an
    error. Jordan Liggitt has pointed out that treating them differently is
    generally avoided in APIs, as it can confuse users.
    In order to make the user's intention more explicit, an additional
    field, `credentialPluginPolicy` has been added. Its value must be one of
    `EnableAll | DisableAll | Allowlist`. `EnableAll` and `DisableAll` are
    self-explanatory and do not require an allowlist. `Allowlist` will use
    the allowlist as defined in the KEP.
* Note that this change considers the following cases to be errors:
  - a policy of `EnableAll` with an allowlist provided
  - a policy of `DisableAll` with an allowlist provided
  - a policy of `Allowlist` with no allowlist provided (i.e. a `nil`
  - allowlist`
  - a policy of `Allowlist` with an explicitly empty allowlist provided
* Clarify v1beta1 and kubectl v1.35 only (allowlist)
* Update kep.yaml
  - Add sig-api-machinery as participating SIG
  - Add enj as reviewer
  - Update latest milestone to v1.35
* Allowlist: rename `name` field to `command`
* Clarify meaning of "the operation will fail"
* Specify appropriate api group and version
* Move detail to new section under `Design Details`
* Add table showing `command` comparison

Signed-off-by: Peter Engelbert <[email protected]>

Author: pmengelbert

keps/sig-cli/3104-introduce-kuberc/README.md

@@ -88,6 +88,7 @@ tags, and then generate with `hack/update-toc.sh`.
     - [Story 2](#story-2)
     - [Story 3](#story-3)
     - [Story 4](#story-4)
+    - [Story 5](#story-5)
   - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional)
     - [Open Questions](#open-questions)
   - [Risks and Mitigations](#risks-and-mitigations)
@@ -104,6 +105,7 @@ tags, and then generate with `hack/update-toc.sh`.
     - [option](#option-1)
     - [prependarg](#prependarg)
     - [appendarg](#appendarg)
+  - [Allowlist Design Details](#allowlist-design-details)
   - [Test Plan](#test-plan)
       - [Prerequisite testing updates](#prerequisite-testing-updates)
       - [Unit tests](#unit-tests)
@@ -273,6 +275,10 @@ As a user I would like to be able to opt out of deprecation warnings.
 
 https://github.com/kubernetes/kubectl/issues/1317
 
+#### Story 5
+
+As a user I would like to be able to prevent the execution of untrusted binaries by the client-go credential plugin system.
+
 ### Notes/Constraints/Caveats (Optional)
 
 <!--
@@ -331,6 +337,11 @@ fields (`apiVersion`, `kind`):
 
 * `aliases` Allows users to declare their own command aliases, including options and values.
 * `defaults` Enables users to set default options to be applied to commands.
+* `credentialPluginPolicy` (available in **kubectl.config.k8s.io/v1beta1**)
+  Allows users to deny all, alow all, or allow some client-go credential plugins.
+* `credentialPluginAllowlist` (available in **kubectl.config.k8s.io/v1beta1**)
+  Enables users to specify criteria for trusting binaries to be executed by
+  the client-go credential plugin system.
 
 `aliases` will not be permitted to override built-in commands but will take
 precedence over plugins (builtins -> aliases -> plugins). Any additional options
@@ -343,6 +354,7 @@ intended behavior and realizing that targeting options effectively addresses the
 use cases. During command execution, a merge will be occur, with inline overrides
 taking precedence over the defaults.
 
+
 ```
 apiVersion: kubectl.config.k8s.io/v1beta1
 kind: Preference
@@ -368,6 +380,10 @@ defaults:
       - name: interactive
         default: "true"
 
+credentialPluginPolicy: Allowlist
+credentialPluginAllowlist:
+    - command: cloudplatform-credential-helper
+    - command: custom-credential-script
 ```
 
 ### Kubectl Kuberc Management Command (kubectl kuberc)
@@ -441,12 +457,68 @@ This gives us the opportunity to standardize kuberc files.
 
 #### prependarg
 
-`--prependarg` is an arbitrary list of strings that accepts anything in a string array format. 
+`--prependarg` is an arbitrary list of strings that accepts anything in a string array format.
 
 #### appendarg
 
 `--appendarg` is an arbitrary list of strings that accepts anything in string array format.
 
+### Allowlist Design Details
+
+`credentialPluginAllowlist` allows the end-user to provide an array of objects
+describing required conditions for executing a credential plugin binary. The
+overall result of a check against the allowlist will be the logical OR of the
+individual checks against the allowlist's entries. Each allowlist entry MUST
+have at least one nonempty field describing the conditions required for the
+plugin's execution. If multiple fields are specified within an entry, the
+binary in question must meet all of the required conditions in that entry in
+order to be executed (i.e. they are combined with logical AND).
+
+Each element in the allowlist is a set of criteria; if the binary in question
+meets all of the criteria in at least one **set** of criteria, the plugin will
+be allowed to execute.  If no criteria set succeeds after comparing the binary
+to all sets of criteria, the operation will be immediately aborted and an error
+returned.
+
+At the outset, the entry object will have only one field, `command`. The path of
+the binary specified in the kubeconfig will be compared against that named in
+the `command` field. This field may contain a basename, or the full path of a
+plugin. To ensure an exact match, `exec.LookPath` will be called on both the
+`command` field and the binary named in the kubeconfig. The resulting absolute
+paths must match. The following table illustrates this:
+
+
+| Scenario | `PATH=` | Allowlist `command` | `exec.LookPath(allowlist.Command)` | kubeconfig `command` | `exec.LookPath(execConfig.Command)` | success? |
+|----------|---------|---------------------|------------------------------------|----------------------|-------------------------------------|----------|
+| kubeconfig lists full path; `my-binary` is in both `/usr/local/bin` and `/usr/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | my-binary | /usr/local/bin/my-binary | /usr/bin/my-binary | /usr/bin/my-binary | false |
+| kubeconfig lists full path; `my-binary` is only in `/usr/local/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | my-binary | /usr/local/bin/my-binary | /usr/bin/my-binary | /usr/bin/my-binary | false |
+| kubeconfig lists full path; `my-binary` is only in `/usr/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | my-binary | /usr/bin/my-binary | /usr/bin/my-binary | /usr/bin/my-binary | true |
+| kubeconfig lists full path; `my-binary` is only in `/usr/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | /usr/bin/my-binary | /usr/bin/my-binary | /usr/bin/my-binary | /usr/bin/my-binary | true |
+| kuberc lists full path; `my-binary` is only in `/usr/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | /usr/bin/my-binary | /usr/bin/my-binary | my-binary | /usr/bin/my-binary | true |
+| kuberc lists full path; `my-binary` is in `/usr/local/bin` | `PATH=/usr/local/bin:/usr/bin:<...>`  | /usr/bin/my-binary | /usr/bin/my-binary | my-binary | /usr/local/bin/my-binary | false |
+| neither lists full path; `my-binary` is in `/usr/bin`; equivalent to basename match | `PATH=/usr/local/bin:/usr/bin:<...>`  | my-binary | /usr/bin/my-binary | my-binary | /usr/bin/my-binary | true |
+
+If `credentialPluginPolicy` is set to `Allowlist`, but a
+`credentialPluginAllowlist` is not provided, it will be considered an
+configuration error. Rather than guess at what the user intended, the operation
+will be aborted just before the `exec` call. An error describing the
+misconfiguration will be returned. This is because the allowlist is a security
+control, and it is likely the user has made a mistake. Since the output may be
+long, it would be easy for a security warning to be lost at the beginning of
+the output. An explicitly empty allowlist (i.e. `credentialPluginAllowlist: []`),
+in combination with `credentialPluginPolicy: Allowlist` will be considered an
+error for the same reason. The user should instead use `credentialPluginPolicy:
+DisableAll` in this case.
+
+Commands that don't create a client, such as `kubectl config view` will not be
+affected by the allowlist. Additionally, commands that create but do not *use*
+a client (such as commands run with `--dry-run`) will likewise remain
+unaffected.
+
+In future updates, other allowlist entry fields MAY be added. Specifically,
+fields allowing for verification by digest or public key have been discussed.
+The initial design MUST accommodate such future additions.
+
 ### Test Plan
 
 <!--

keps/sig-cli/3104-introduce-kuberc/kep.yaml

@@ -5,14 +5,16 @@ authors:
   - "@soltysh"
 owning-sig: sig-cli
 participating-sigs:
-  - sig-cli
+  - sig-auth
+  - sig-api-machinery
 status: implementable
 creation-date: 2022-06-13
 reviewers:
   - "@liggitt"
   - "@eddiezane"
   - "@soltysh"
   - "@mpuckett159"
+  - "@enj"
 approvers:
   - "@ardaguclu"