diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index 3b08ac449f4..f4d7f05d1ac 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -1451,6 +1451,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA pkgconfigv1alpha1.CommandOptionDefault{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1alpha1_CommandOptionDefault(ref), pkgconfigv1alpha1.Preference{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1alpha1_Preference(ref), pkgconfigv1beta1.AliasOverride{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1beta1_AliasOverride(ref), + pkgconfigv1beta1.AllowlistEntry{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1beta1_AllowlistEntry(ref), pkgconfigv1beta1.CommandDefaults{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1beta1_CommandDefaults(ref), pkgconfigv1beta1.CommandOptionDefault{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1beta1_CommandOptionDefault(ref), pkgconfigv1beta1.Preference{}.OpenAPIModelName(): schema_kubectl_pkg_config_v1beta1_Preference(ref), @@ -70426,6 +70427,28 @@ func schema_kubectl_pkg_config_v1beta1_AliasOverride(ref common.ReferenceCallbac } } +func schema_kubectl_pkg_config_v1beta1_AllowlistEntry(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "AllowlistEntry is an entry in the allowlist. For each allowlist item, at least one field must be nonempty. A struct with all empty fields is considered a misconfiguration error. Each field is a criterion for execution. If multiple fields are specified, then the criteria of all specified fields must be met. That is, the result of an individual entry is the logical AND of all checks corresponding to the specified fields within the entry.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name matching is performed by first resolving the absolute path of both the plugin and the name in the allowlist entry using `exec.LookPath`. It will be called on both, and the resulting strings must be equal. If either call to `exec.LookPath` results in an error, the `Name` check will be considered a failure.", + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"name"}, + }, + }, + } +} + func schema_kubectl_pkg_config_v1beta1_CommandDefaults(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -70558,12 +70581,38 @@ func schema_kubectl_pkg_config_v1beta1_Preference(ref common.ReferenceCallback) }, }, }, + "credentialPluginPolicy": { + SchemaProps: spec.SchemaProps{ + Description: "credentialPluginPolicy specifies the policy governing which, if any, client-go credential plugins may be executed. It MUST be one of { \"\", \"AllowAll\", \"DenyAll\", \"Allowlist\" }. If the policy is \"\", then it falls back to \"AllowAll\" (this is required to maintain backward compatibility). If the policy is DenyAll, no credential plugins may run. If the policy is Allowlist, only those plugins meeting the criteria specified in the `credentialPluginAllowlist` field may run.", + Type: []string{"string"}, + Format: "", + }, + }, + "credentialPluginAllowlist": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Allowlist is a slice of allowlist entries. If any of them is a match, then the executable in question may execute. That is, the result is the logical OR of all entries in the allowlist. This list MUST NOT be supplied if the policy is not \"Allowlist\".\n\ne.g. credentialPluginAllowlist: - name: cloud-provider-plugin - name: /usr/local/bin/my-plugin In the above example, the user allows the credential plugins `cloud-provider-plugin` (found somewhere in PATH), and the plugin found at the explicit path `/usr/local/bin/my-plugin`.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref(pkgconfigv1beta1.AllowlistEntry{}.OpenAPIModelName()), + }, + }, + }, + }, + }, }, Required: []string{"defaults", "aliases"}, }, }, Dependencies: []string{ - pkgconfigv1beta1.AliasOverride{}.OpenAPIModelName(), pkgconfigv1beta1.CommandDefaults{}.OpenAPIModelName()}, + pkgconfigv1beta1.AliasOverride{}.OpenAPIModelName(), pkgconfigv1beta1.AllowlistEntry{}.OpenAPIModelName(), pkgconfigv1beta1.CommandDefaults{}.OpenAPIModelName()}, } } diff --git a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go index ed47e99942f..04712fb497b 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go +++ b/staging/src/k8s.io/cli-runtime/pkg/genericclioptions/command_headers.go @@ -19,6 +19,7 @@ package genericclioptions import ( "net/http" "strings" + "sync/atomic" "github.com/google/uuid" "github.com/spf13/cobra" @@ -33,8 +34,9 @@ const ( // round tripper to add Request headers before delegation. Implements // the go standard library "http.RoundTripper" interface. type CommandHeaderRoundTripper struct { - Delegate http.RoundTripper - Headers map[string]string + Delegate http.RoundTripper + Headers map[string]string + SkipHeaders *atomic.Bool } // CommandHeaderRoundTripper adds Request headers before delegating to standard @@ -43,9 +45,14 @@ type CommandHeaderRoundTripper struct { // // https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/859-kubectl-headers func (c *CommandHeaderRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + if c.shouldSkipHeaders() { + return c.Delegate.RoundTrip(req) + } + for header, value := range c.Headers { req.Header.Set(header, value) } + return c.Delegate.RoundTrip(req) } @@ -92,3 +99,11 @@ func (c *CommandHeaderRoundTripper) CancelRequest(req *http.Request) { cr.CancelRequest(req) } } + +func (c *CommandHeaderRoundTripper) shouldSkipHeaders() bool { + if c.SkipHeaders == nil { + return false + } + + return c.SkipHeaders.Load() +} diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go index b471f5cc648..1af2afdb912 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go @@ -27,6 +27,7 @@ import ( "net/http" "os" "os/exec" + "path/filepath" "reflect" "strings" "sync" @@ -39,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/util/dump" utilnet "k8s.io/apimachinery/pkg/util/net" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/pkg/apis/clientauthentication" "k8s.io/client-go/pkg/apis/clientauthentication/install" clientauthenticationv1 "k8s.io/client-go/pkg/apis/clientauthentication/v1" @@ -177,13 +179,29 @@ func newAuthenticator(c *cache, isTerminalFunc func(int) bool, config *api.ExecC connTracker, ) + if err := ValidatePluginPolicy(config.PluginPolicy); err != nil { + return nil, fmt.Errorf("invalid plugin policy: %w", err) + } + + allowlistLookup := sets.New[string]() + for _, entry := range config.PluginPolicy.Allowlist { + if entry.Name != "" { + allowlistLookup.Insert(entry.Name) + } + } + a := &Authenticator{ - cmd: config.Command, + // Clean is called to normalize the path to facilitate comparison with + // the allowlist, when present + cmd: filepath.Clean(config.Command), args: config.Args, group: gv, cluster: cluster, provideClusterInfo: config.ProvideClusterInfo, + allowlistLookup: allowlistLookup, + execPluginPolicy: config.PluginPolicy, + installHint: config.InstallHint, sometimes: &sometimes{ threshold: 10, @@ -250,6 +268,9 @@ type Authenticator struct { cluster *clientauthentication.Cluster provideClusterInfo bool + allowlistLookup sets.Set[string] + execPluginPolicy api.PluginPolicy + // Used to avoid log spew by rate limiting install hint printing. We didn't do // this by interval based rate limiting alone since that way may have prevented // the install hint from showing up for kubectl users. @@ -441,6 +462,12 @@ func (a *Authenticator) refreshCredsLocked() error { cmd.Stdin = a.stdin } + err = a.updateCommandAndCheckAllowlistLocked(cmd) + incrementPolicyMetric(err) + if err != nil { + return err + } + err = cmd.Run() incrementCallsMetric(err) if err != nil { @@ -545,3 +572,131 @@ func (a *Authenticator) wrapCmdRunErrorLocked(err error) error { return fmt.Errorf("exec: %v", err) } } + +// `updateCommandAndCheckAllowlistLocked` determines whether or not the specified executable may run +// according to the credential plugin policy. If the plugin is allowed, `nil` +// is returned. If the plugin is not allowed, an error must be returned +// explaining why. +func (a *Authenticator) updateCommandAndCheckAllowlistLocked(cmd *exec.Cmd) error { + switch a.execPluginPolicy.PolicyType { + case "", api.PluginPolicyAllowAll: + return nil + case api.PluginPolicyDenyAll: + return fmt.Errorf("plugin %q not allowed: policy set to %q", a.cmd, api.PluginPolicyDenyAll) + case api.PluginPolicyAllowlist: + return a.checkAllowlistLocked(cmd) + default: + return fmt.Errorf("unknown plugin policy %q", a.execPluginPolicy.PolicyType) + } +} + +// `checkAllowlistLocked` checks the specified plugin against the allowlist, +// and may update the Authenticator's allowlistLookup set. +func (a *Authenticator) checkAllowlistLocked(cmd *exec.Cmd) error { + // a.cmd is the original command as specified in the configuration, then filepath.Clean(). + // cmd.Path is the possibly-resolved command. + // If either are an exact match in the allowlist, return success. + if a.allowlistLookup.Has(a.cmd) || a.allowlistLookup.Has(cmd.Path) { + return nil + } + + var cmdResolvedPath string + var cmdResolvedErr error + if cmd.Path != a.cmd { + // cmd.Path changed, use the already-resolved LookPath results + cmdResolvedPath = cmd.Path + cmdResolvedErr = cmd.Err + } else { + // cmd.Path is unchanged, do LookPath ourselves + cmdResolvedPath, cmdResolvedErr = exec.LookPath(cmd.Path) + // update cmd.Path to cmdResolvedPath so we only run the resolved path + if cmdResolvedPath != "" { + cmd.Path = cmdResolvedPath + } + } + + if cmdResolvedErr != nil { + return fmt.Errorf("plugin path %q cannot be resolved for credential plugin allowlist check: %w", cmd.Path, cmdResolvedErr) + } + + // cmdResolvedPath may have changed, and the changed value may be in the allowlist + if a.allowlistLookup.Has(cmdResolvedPath) { + return nil + } + + // There is no verbatim match + a.resolveAllowListEntriesLocked(cmd.Path) + + // allowlistLookup may have changed, recheck + if a.allowlistLookup.Has(cmdResolvedPath) { + return nil + } + + return fmt.Errorf("plugin path %q is not permitted by the credential plugin allowlist", cmd.Path) +} + +// resolveAllowListEntriesLocked tries to resolve allowlist entries with LookPath, +// and adds successfully resolved entries to allowlistLookup. +// The optional commandHint can be used to limit which entries are resolved to ones which match the hint basename. +func (a *Authenticator) resolveAllowListEntriesLocked(commandHint string) { + hintName := filepath.Base(commandHint) + for _, entry := range a.execPluginPolicy.Allowlist { + entryBasename := filepath.Base(entry.Name) + if hintName != "" && hintName != entryBasename { + // we got a hint, and this allowlist entry does not match it + continue + } + entryResolvedPath, err := exec.LookPath(entry.Name) + if err != nil { + klog.V(5).ErrorS(err, "resolving credential plugin allowlist", "name", entry.Name) + continue + } + if entryResolvedPath != "" { + a.allowlistLookup.Insert(entryResolvedPath) + } + } +} + +func ValidatePluginPolicy(policy api.PluginPolicy) error { + switch policy.PolicyType { + // "" is equivalent to "AllowAll" + case "", api.PluginPolicyAllowAll, api.PluginPolicyDenyAll: + if policy.Allowlist != nil { + return fmt.Errorf("misconfigured credential plugin allowlist: plugin policy is %q but allowlist is non-nil", policy.PolicyType) + } + return nil + case api.PluginPolicyAllowlist: + return validateAllowlist(policy.Allowlist) + default: + return fmt.Errorf("unknown plugin policy: %q", policy.PolicyType) + } +} + +var emptyAllowlistEntry api.AllowlistEntry + +func validateAllowlist(list []api.AllowlistEntry) error { + // This will be the case if the user has misspelled the field name for the + // allowlist. Because this is a security knob, fail immediately rather than + // proceed when the user has made a mistake. + if list == nil { + return fmt.Errorf("credential plugin policy set to %q, but allowlist is unspecified", api.PluginPolicyAllowlist) + } + + if len(list) == 0 { + return fmt.Errorf("credential plugin policy set to %q, but allowlist is empty; use %q policy instead", api.PluginPolicyAllowlist, api.PluginPolicyDenyAll) + } + + for i, item := range list { + if item == emptyAllowlistEntry { + return fmt.Errorf("misconfigured credential plugin allowlist: empty allowlist entry #%d", i+1) + } + + if cleaned := filepath.Clean(item.Name); cleaned != item.Name { + return fmt.Errorf("non-normalized file path: %q vs %q", item.Name, cleaned) + } else if item.Name == "" { + return fmt.Errorf("empty file path: %q", item.Name) + } + } + + return nil +} diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go index 291590eca86..ada512c66a6 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go @@ -29,8 +29,12 @@ import ( "fmt" "io" "math/big" + mathrand "math/rand/v2" "net/http" "net/http/httptest" + "os" + "os/exec" + "path/filepath" "reflect" "strconv" "strings" @@ -845,6 +849,354 @@ func TestRefreshCreds(t *testing.T) { } } +type pluginPolicyTest struct { + name string + wantErr bool + wantErrSubstr string + config *api.ExecConfig + pluginExists bool + entryExists bool + usePluginAbsPath bool + useEntryAbsPath bool + allowlistLength int + policyType api.PolicyType + allowlist []api.AllowlistEntry +} + +type dualBool struct{ plugin, entry bool } + +type pluginPolicyTestMatrix struct { + exists []dualBool + absolute []dualBool + allowlistLengths []int + policies []api.PolicyType + + // the logic of whether or not a given test configuration should produce an error + shouldErrFunc func(tt *pluginPolicyTest) (bool, string) +} + +var allPermutations = []dualBool{ + {plugin: false, entry: false}, + {plugin: false, entry: true}, + {plugin: true, entry: false}, + {plugin: true, entry: true}, +} + +// TestPluginPolicy tests the functioning of various plugin policies, as well +// as the the validity of a wide variety of policies. +func TestPluginPolicy(t *testing.T) { + // this is inlined to make highly visible and explicit the logic of which + // test configurations should pass and which should fail + shouldErrFunc := func(test *pluginPolicyTest) (bool, string) { + switch test.policyType { + case "", api.PluginPolicyAllowAll: + if test.allowlist != nil { // invalid + return true, "allowlist is non-nil" + } + + if test.pluginExists { + return false, "" + } + + return true, "not found" + case api.PluginPolicyDenyAll: + if test.allowlist != nil { // invalid + return true, "allowlist is non-nil" + } + + return true, `policy set to "DenyAll"` + case api.PluginPolicyAllowlist: + if test.allowlist == nil { // invalid + return true, "allowlist is unspecified" + } + + if len(test.allowlist) == 0 { // invalid + return true, "allowlist is empty; use \"DenyAll\" policy instead" + } + + switch { + case test.pluginExists && test.entryExists: + return false, "" + case test.pluginExists && !test.entryExists: + return true, "is not permitted by the credential plugin allowlist" + case !test.pluginExists && test.entryExists: + // error message varies depending on whether the paths are relative or absolute + // what is important is that an error arises here + return true, "" + case !test.pluginExists && !test.entryExists: + // error message varies depending on whether the paths are relative or absolute + // what is important is that an error arises here + return true, "" + } + + panic("unreachable") + } + + return true, "unknown plugin policy" + } + + wd, err := os.Getwd() + if err != nil { + t.Fatalf("could not get working directory: %s", err) + } + + testdataDir := filepath.Join(wd, "testdata") + + path := os.Getenv("PATH") + defer func() { + if err = os.Setenv("PATH", path); err != nil { + t.Fatal(err) + } + }() + + matrix := pluginPolicyTestMatrix{ + shouldErrFunc: shouldErrFunc, + exists: allPermutations, + absolute: allPermutations, + allowlistLengths: []int{-1 /*nil*/, 0, 1, 3}, + policies: []api.PolicyType{ + "", + api.PluginPolicyAllowAll, + api.PluginPolicyDenyAll, + api.PluginPolicyAllowlist, + api.PolicyType("HIGHLYILLEGAL"), + }, + } + + tests := matrix.makeTests(t, testdataDir, path) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := test.config + a, err := newAuthenticator(newCache(), func(_ int) bool { return false }, c, &clientauthentication.Cluster{}) + if err != nil { + if !test.wantErr { + t.Fatalf("unexpected validation error: %v", err) + } else if !strings.Contains(err.Error(), test.wantErrSubstr) { + t.Fatalf("expected error with substring '%v' got '%v'", test.wantErrSubstr, err.Error()) + } + + return + } + + stderr := &bytes.Buffer{} + a.stderr = stderr + a.environ = func() []string { return nil } + + if err := a.refreshCredsLocked(); err != nil { + if !test.wantErr { + t.Fatalf("unexpected allows plugin error: %v", err) + } else if !strings.Contains(err.Error(), test.wantErrSubstr) { + t.Fatalf("expected error with substring '%v' got '%v'", test.wantErrSubstr, err.Error()) + } + return + } + + if test.wantErr { + t.Fatal("expected allowlist error, but error was nil") + } + }) + } +} + +func (m *pluginPolicyTestMatrix) makeTests(t *testing.T, testdataDir, path string) []pluginPolicyTest { + const existingPluginInPATHBasename = "test-plugin.sh" + existingPluginInPATHAbsolutePath := filepath.Join(testdataDir, existingPluginInPATHBasename) + + err := os.Setenv("PATH", fmt.Sprintf("%s:%s", testdataDir, path)) + if err != nil { + t.Fatalf("error setting PATH: %s", err) + } + + resolved, err := exec.LookPath(existingPluginInPATHBasename) + if err != nil { + t.Fatal(err) + } + if existingPluginInPATHAbsolutePath != resolved { + t.Fatalf("test plugin basename resolved incorrectly: did not resolve to %s", existingPluginInPATHAbsolutePath) + } + + resolvedAbs, err := exec.LookPath(existingPluginInPATHAbsolutePath) + if err != nil { + t.Fatal(err) + } + if existingPluginInPATHAbsolutePath != resolvedAbs { + t.Fatalf("test plugin absolute path resolved incorrectly: did not resolve to %s", existingPluginInPATHAbsolutePath) + } + + tests := make([]pluginPolicyTest, 0, len(m.exists)*2+len(m.absolute)*2+len(m.allowlistLengths)+len(m.policies)) + + var tt *pluginPolicyTest + for _, exists := range m.exists { + tt = new(pluginPolicyTest) + + tt.setExists(exists) + for _, absolute := range m.absolute { + tt.setAbsolute(absolute) + + for _, length := range m.allowlistLengths { + tt.setAllowlist(length, existingPluginInPATHAbsolutePath, existingPluginInPATHBasename) + + for _, p := range m.policies { + tt.policyType = p + tt.setName() + tt.setExecConfig(existingPluginInPATHAbsolutePath, existingPluginInPATHBasename) + tt.setErrDetails(m) + + tests = append(tests, *tt) + } + } + } + } + + return tests +} + +func (tt *pluginPolicyTest) setErrDetails(m *pluginPolicyTestMatrix) { + tt.wantErr, tt.wantErrSubstr = m.shouldErrFunc(tt) +} + +func (tt *pluginPolicyTest) setAbsolute(absolute dualBool) { + tt.usePluginAbsPath = absolute.plugin + tt.useEntryAbsPath = absolute.entry +} + +func (tt *pluginPolicyTest) setExists(exists dualBool) { + tt.pluginExists = exists.plugin + tt.entryExists = exists.entry +} + +func (tt *pluginPolicyTest) setAllowlist(l int, existingPluginInPATHAbsolutePath string, existingPluginInPATHBasename string) { + tt.allowlistLength = l + if tt.allowlistLength >= 0 { + tt.allowlist = make([]api.AllowlistEntry, 0, tt.allowlistLength) + } + + if tt.allowlistLength >= 1 { + entry := tt.makeAllowlistEntry(existingPluginInPATHAbsolutePath, existingPluginInPATHBasename) + tt.allowlist = append(tt.allowlist, entry) + } + + for i := 1; i < tt.allowlistLength; i++ { + tt.allowlist = append(tt.allowlist, api.AllowlistEntry{Name: fmt.Sprintf("foo-%d", i)}) + } + + // shuffle the allowlist to guarantee ordering doesn't matter + for i := range tt.allowlist { + j := mathrand.IntN(i + 1) + tt.allowlist[i], tt.allowlist[j] = tt.allowlist[j], tt.allowlist[i] + } +} + +func (tt *pluginPolicyTest) makeAllowlistEntry(existingPluginInPATHAbsolutePath string, existingPluginInPATHBasename string) api.AllowlistEntry { + var entry api.AllowlistEntry + + switch { + case tt.entryExists && tt.useEntryAbsPath: + entry.Name = existingPluginInPATHAbsolutePath + case tt.entryExists && !tt.useEntryAbsPath: + entry.Name = existingPluginInPATHBasename + case !tt.entryExists && tt.useEntryAbsPath: + entry.Name = "/this/path/does/not/exist" + case !tt.entryExists && !tt.useEntryAbsPath: + entry.Name = "does not exist" + } + + return entry +} + +func (tt *pluginPolicyTest) setExecConfig(existingPluginInPATHAbsolutePath string, existingPluginInPATHBasename string) { + var cmd string + + switch { + case tt.pluginExists && tt.usePluginAbsPath: + cmd = existingPluginInPATHAbsolutePath + case tt.pluginExists && !tt.usePluginAbsPath: + cmd = existingPluginInPATHBasename + case !tt.pluginExists && tt.usePluginAbsPath: + cmd = "/this/path/does/not/exist" + case !tt.pluginExists && !tt.usePluginAbsPath: + cmd = "does not exist" + default: // verifiably unreachable + cmd = "does not exist" + } + + config := api.ExecConfig{} + if strings.HasSuffix(cmd, "test-plugin.sh") { + config.Env = append(config.Env, api.ExecEnvVar{ + Name: "TEST_OUTPUT", + Value: `{ + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1", + "status": { + "token": "foo-bar" + } + }`, + }) + config.Env = append(config.Env, api.ExecEnvVar{ + Name: "TEST_EXIT_CODE", + Value: strconv.Itoa(0), + }) + } + + config.APIVersion = "client.authentication.k8s.io/v1" + config.InteractiveMode = api.IfAvailableExecInteractiveMode + config.Command = cmd + config.PluginPolicy.PolicyType = tt.policyType + config.PluginPolicy.Allowlist = tt.allowlist + + tt.config = &config +} + +func makeExistsString(s string, t bool) string { + ne := "nonexistent" + if t { + ne = "existing" + } + + return fmt.Sprintf("%s-%s-path", ne, s) +} + +func makePathString(s string, t bool) string { + ba := "basename" + if t { + ba = "absolute-path" + } + + return fmt.Sprintf("%s-%s", ba, s) +} + +func (tt *pluginPolicyTest) setName() { + p := string(tt.policyType) + if string(tt.policyType) == "" { + p = "unspecified" + } + var lengthStr string + switch tt.allowlistLength { + case -1: + lengthStr = "nil-allowlist" + case 0: + lengthStr = "empty-allowlist" + case 1: + lengthStr = "single-entry-allowlist" + default: + lengthStr = "multiple-entry-allowlist" + } + pluginExistsString := makeExistsString("plugin", tt.pluginExists) + entryExistsString := makeExistsString("entry", tt.entryExists) + pluginPathString := makePathString("plugin", tt.usePluginAbsPath) + entryPathString := makePathString("entry", tt.useEntryAbsPath) + policyString := fmt.Sprintf("with-%s-policy", strings.ToLower(p)) + + tt.name = filepath.Join( + policyString, + lengthStr, + pluginExistsString, + entryExistsString, + pluginPathString, + entryPathString, + ) +} + func TestRoundTripper(t *testing.T) { wantToken := "" diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go index 51210975a66..fb300ae12a4 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go @@ -49,6 +49,13 @@ const ( // used in some failure modes (e.g., plugin not found, client internal error) so that someone // can more easily monitor all unsuccessful invocations. failureExitCode = 1 + + // pluginAllowed represents an exec plugin invocation that was allowed by + // the plugin policy and/or the allowlist + pluginAllowed = "allowed" + // pluginAllowed represents an exec plugin invocation that was denied by + // the plugin policy and/or the allowlist + pluginDenied = "denied" ) type certificateExpirationTracker struct { @@ -109,3 +116,12 @@ func incrementCallsMetric(err error) { metrics.ExecPluginCalls.Increment(failureExitCode, clientInternalError) } } + +func incrementPolicyMetric(err error) { + if err != nil { + metrics.ExecPluginPolicyCalls.Increment(pluginDenied) + return + } + + metrics.ExecPluginPolicyCalls.Increment(pluginAllowed) +} diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics_test.go index bf588dcdeb8..b50ac89b668 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics_test.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics_test.go @@ -196,3 +196,114 @@ func TestCallsMetric(t *testing.T) { t.Fatalf("got unexpected metrics calls; -want, +got:\n%s", diff) } } + +type mockPolicyCallsMetric struct { + status string +} + +type mockPolicyCallsMetricCounter struct { + policyCalls []mockPolicyCallsMetric +} + +func (f *mockPolicyCallsMetricCounter) Increment(status string) { + f.policyCalls = append(f.policyCalls, mockPolicyCallsMetric{status: status}) +} + +func TestPolicyCallsMetric(t *testing.T) { + const ( + goodOutput = `{ + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1beta1", + "status": { + "token": "foo-bar" + } + }` + ) + + policyCallsMetricCounter := &mockPolicyCallsMetricCounter{} + originalExecPluginPolicyCalls := metrics.ExecPluginPolicyCalls + t.Cleanup(func() { metrics.ExecPluginPolicyCalls = originalExecPluginPolicyCalls }) + metrics.ExecPluginPolicyCalls = policyCallsMetricCounter + + tests := []struct { + wantDenied bool + policy api.PluginPolicy + }{ + { + wantDenied: false, + policy: api.PluginPolicy{PolicyType: api.PluginPolicyAllowAll}, + }, + { + wantDenied: true, + policy: api.PluginPolicy{PolicyType: api.PluginPolicyDenyAll}, + }, + { + wantDenied: false, + policy: api.PluginPolicy{ + PolicyType: api.PluginPolicyAllowlist, + Allowlist: []api.AllowlistEntry{ + { + Name: "foobar", + }, + { + Name: "testdata/test-plugin.sh", + }, + }, + }, + }, + { + wantDenied: true, + policy: api.PluginPolicy{ + PolicyType: api.PluginPolicyAllowlist, + Allowlist: []api.AllowlistEntry{ + {Name: "foobar"}, + {Name: "baz"}, + }, + }, + }, + } + + var wantPolicyCallsMetrics []mockPolicyCallsMetric + for _, test := range tests { + c := api.ExecConfig{ + Command: "./testdata/test-plugin.sh", + APIVersion: "client.authentication.k8s.io/v1beta1", + Env: []api.ExecEnvVar{ + {Name: "TEST_EXIT_CODE", Value: fmt.Sprintf("%d", 0)}, + {Name: "TEST_OUTPUT", Value: goodOutput}, + }, + InteractiveMode: api.IfAvailableExecInteractiveMode, + PluginPolicy: test.policy, + } + + a, err := newAuthenticator(newCache(), func(_ int) bool { return false }, &c, nil) + if err != nil { + t.Fatal(err) + } + a.stderr = io.Discard + + err = a.refreshCredsLocked() + if err != nil && !test.wantDenied { + t.Fatalf("wanted no error, but got %q", err.Error()) + } + if err == nil && test.wantDenied { + t.Fatal("wanted error, but got nil") + } + + mockPolicyCallsMetric := mockPolicyCallsMetric{status: "allowed"} + if test.wantDenied { + mockPolicyCallsMetric.status = "denied" + } + + wantPolicyCallsMetrics = append(wantPolicyCallsMetrics, mockPolicyCallsMetric) + } + + policyCallsMetricComparer := cmp.Comparer(func(a, b mockPolicyCallsMetric) bool { + return a.status == b.status + }) + + actualPolicyCallsMetrics := policyCallsMetricCounter.policyCalls + if diff := cmp.Diff(wantPolicyCallsMetrics, actualPolicyCallsMetrics, policyCallsMetricComparer); diff != "" { + t.Fatalf("got unexpected metrics calls; -want, +got:\n%s", diff) + } +} diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go index 8c64adb841e..cb21c040a3b 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go @@ -283,8 +283,57 @@ type ExecConfig struct { // read user instructions might set this to "used by my-program to read user instructions". // +k8s:conversion-gen=false StdinUnavailableMessage string `json:"-"` + + // PluginPolicy is the policy governing whether or not the configured + // `Command` may run. + // +k8s:conversion-gen=false + PluginPolicy PluginPolicy `json:"-"` } +// AllowlistEntry is an entry in the allowlist. For each allowlist item, at +// least one field must be nonempty. A struct with all empty fields is +// considered a misconfiguration error. Each field is a criterion for +// execution. If multiple fields are specified, then the criteria of all +// specified fields must be met. That is, the result of an individual entry is +// the logical AND of all checks corresponding to the specified fields within +// the entry. +type AllowlistEntry struct { + // Name matching is performed by first resolving the absolute path of both + // the plugin and the name in the allowlist entry using `exec.LookPath`. It + // will be called on both, and the resulting strings must be equal. If + // either call to `exec.LookPath` results in an error, the `Name` check + // will be considered a failure. + Name string `json:"-"` +} + +// PluginPolicy describes the policy type and allowlist (if any) for client-go +// credential plugins. +type PluginPolicy struct { + // PolicyType specifies the policy governing which, if any, client-go + // credential plugins may be executed. It MUST be one of { "", "AllowAll", "DenyAll", "Allowlist" }. + // If the policy is "", then it falls back to "AllowAll" (this is required + // to maintain backward compatibility). If the policy is DenyAll, no + // credential plugins may run. If the policy is Allowlist, only those + // plugins meeting the criteria specified in the `credentialPluginAllowlist` + // field may run. If the policy is not `Allowlist` but one is provided, it + // is considered a configuration error. + PolicyType PolicyType `json:"-"` + + // Allowlist is a slice of allowlist entries. If any of them is a match, + // then the executable in question may execute. That is, the result is the + // logical OR of all entries in the allowlist. This list MUST be nil + // whenever the policy is not "Allowlist". + Allowlist []AllowlistEntry `json:"-"` +} + +type PolicyType string + +const ( + PluginPolicyAllowAll PolicyType = "AllowAll" + PluginPolicyDenyAll PolicyType = "DenyAll" + PluginPolicyAllowlist PolicyType = "Allowlist" +) + var _ fmt.Stringer = new(ExecConfig) var _ fmt.GoStringer = new(ExecConfig) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go index bdedc166695..ef1e9b8eb24 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go @@ -401,6 +401,7 @@ func autoConvert_api_ExecConfig_To_v1_ExecConfig(in *api.ExecConfig, out *ExecCo out.InteractiveMode = ExecInteractiveMode(in.InteractiveMode) // INFO: in.StdinUnavailable opted out of conversion generation // INFO: in.StdinUnavailableMessage opted out of conversion generation + // INFO: in.PluginPolicy opted out of conversion generation return nil } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/zz_generated.deepcopy.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/zz_generated.deepcopy.go index 86e4ddef1b1..aba8add9bec 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/zz_generated.deepcopy.go @@ -25,6 +25,22 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AllowlistEntry) DeepCopyInto(out *AllowlistEntry) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllowlistEntry. +func (in *AllowlistEntry) DeepCopy() *AllowlistEntry { + if in == nil { + return nil + } + out := new(AllowlistEntry) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AuthInfo) DeepCopyInto(out *AuthInfo) { *out = *in @@ -271,6 +287,7 @@ func (in *ExecConfig) DeepCopyInto(out *ExecConfig) { if in.Config != nil { out.Config = in.Config.DeepCopyObject() } + in.PluginPolicy.DeepCopyInto(&out.PluginPolicy) return } @@ -300,6 +317,27 @@ func (in *ExecEnvVar) DeepCopy() *ExecEnvVar { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PluginPolicy) DeepCopyInto(out *PluginPolicy) { + *out = *in + if in.Allowlist != nil { + in, out := &in.Allowlist, &out.Allowlist + *out = make([]AllowlistEntry, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PluginPolicy. +func (in *PluginPolicy) DeepCopy() *PluginPolicy { + if in == nil { + return nil + } + out := new(PluginPolicy) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Preferences) DeepCopyInto(out *Preferences) { *out = *in diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go index 088972ef65c..0389ad6dcb7 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go @@ -25,6 +25,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation" + authexec "k8s.io/client-go/plugin/pkg/client/auth/exec" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) @@ -327,6 +328,10 @@ func validateAuthInfo(authInfoName string, authInfo clientcmdapi.AuthInfo) []err default: validationErrors = append(validationErrors, fmt.Errorf("invalid interactiveMode for %v: %q", authInfoName, authInfo.Exec.InteractiveMode)) } + + if err := authexec.ValidatePluginPolicy(authInfo.Exec.PluginPolicy); err != nil { + validationErrors = append(validationErrors, fmt.Errorf("allowlist misconfiguration: %w", err)) + } } // authPath also provides information for the client to identify the server, so allow multiple auth methods in that case diff --git a/staging/src/k8s.io/client-go/tools/metrics/metrics.go b/staging/src/k8s.io/client-go/tools/metrics/metrics.go index 99d3d8e239c..e364b7e1cf0 100644 --- a/staging/src/k8s.io/client-go/tools/metrics/metrics.go +++ b/staging/src/k8s.io/client-go/tools/metrics/metrics.go @@ -62,6 +62,12 @@ type CallsMetric interface { Increment(exitCode int, callStatus string) } +// CallsMetric counts the success or failure of execution for exec plugins. +type PolicyCallsMetric interface { + // Increment increments a counter per status { "allowed", "denied" } + Increment(status string) +} + // RetryMetric counts the number of retries sent to the server // partitioned by code, method, and host. type RetryMetric interface { @@ -99,6 +105,9 @@ var ( // ExecPluginCalls is the number of calls made to an exec plugin, partitioned by // exit code and call status. ExecPluginCalls CallsMetric = noopCalls{} + // ExecPluginPolicyCalls is the number of plugin policy check calls, partitioned + // by {"allowed", "denied"} + ExecPluginPolicyCalls PolicyCallsMetric = noopPolicy{} // RequestRetry is the retry metric that tracks the number of // retries sent to the server. RequestRetry RetryMetric = noopRetry{} @@ -121,6 +130,7 @@ type RegisterOpts struct { RateLimiterLatency LatencyMetric RequestResult ResultMetric ExecPluginCalls CallsMetric + ExecPluginPolicyCalls PolicyCallsMetric RequestRetry RetryMetric TransportCacheEntries TransportCacheMetric TransportCreateCalls TransportCreateCallsMetric @@ -157,6 +167,9 @@ func Register(opts RegisterOpts) { if opts.ExecPluginCalls != nil { ExecPluginCalls = opts.ExecPluginCalls } + if opts.ExecPluginPolicyCalls != nil { + ExecPluginCalls = opts.ExecPluginCalls + } if opts.RequestRetry != nil { RequestRetry = opts.RequestRetry } @@ -198,6 +211,10 @@ type noopCalls struct{} func (noopCalls) Increment(int, string) {} +type noopPolicy struct{} + +func (noopPolicy) Increment(string) {} + type noopRetry struct{} func (noopRetry) IncrementRetry(context.Context, string, string, string) {} diff --git a/staging/src/k8s.io/component-base/metrics/prometheus/restclient/metrics.go b/staging/src/k8s.io/component-base/metrics/prometheus/restclient/metrics.go index d0c80de03fa..fd2c5f754cb 100644 --- a/staging/src/k8s.io/component-base/metrics/prometheus/restclient/metrics.go +++ b/staging/src/k8s.io/component-base/metrics/prometheus/restclient/metrics.go @@ -165,6 +165,17 @@ var ( []string{"code", "call_status"}, ) + execPluginPolicyCalls = k8smetrics.NewCounterVec( + &k8smetrics.CounterOpts{ + StabilityLevel: k8smetrics.ALPHA, + Name: "rest_client_exec_plugin_policy_call_total", + Help: "Number of comparisons of an exec plugin to the plugin policy " + + "and allowlist (if any), partitioned by whether or not the policy " + + "permits the plugin", + }, + []string{"allowed", "denied"}, + ) + transportCacheEntries = k8smetrics.NewGauge( &k8smetrics.GaugeOpts{ Name: "rest_client_transport_cache_entries", @@ -208,6 +219,7 @@ func init() { RequestResult: &resultAdapter{requestResult}, RequestRetry: &retryAdapter{requestRetry}, ExecPluginCalls: &callsAdapter{m: execPluginCalls}, + ExecPluginPolicyCalls: &policyAdapter{m: execPluginPolicyCalls}, TransportCacheEntries: &transportCacheAdapter{m: transportCacheEntries}, TransportCreateCalls: &transportCacheCallsAdapter{m: transportCacheCalls}, }) @@ -269,6 +281,14 @@ func (r *callsAdapter) Increment(code int, callStatus string) { r.m.WithLabelValues(fmt.Sprintf("%d", code), callStatus).Inc() } +type policyAdapter struct { + m *k8smetrics.CounterVec +} + +func (r *policyAdapter) Increment(status string) { + r.m.WithLabelValues(status).Inc() +} + type retryAdapter struct { m *k8smetrics.CounterVec } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index 28099196700..964abeaedcc 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -21,6 +21,7 @@ import ( "net/http" "os" "strings" + "sync/atomic" "github.com/spf13/cobra" @@ -233,15 +234,17 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) matchVersionKubeConfigFlags.AddFlags(flags) // Updates hooks to add kubectl command headers: SIG CLI KEP 859. - addCmdHeaderHooks(cmds, kubeConfigFlags) + var isProxyCmd atomic.Bool + addCmdHeaderHooks(cmds, kubeConfigFlags, &isProxyCmd) f := cmdutil.NewFactory(matchVersionKubeConfigFlags) - // Proxy command is incompatible with CommandHeaderRoundTripper, so - // clear the WrapConfigFn before running proxy command. + // Proxy command is incompatible with the headers set by + // CommandHeaderRoundTripper, so the RoundTripper hooks set in + // `addCmdHeaderHooks` needs to be aware that the subcommand is `proxy` proxyCmd := proxy.NewCmdProxy(f, o.IOStreams) proxyCmd.PreRun = func(cmd *cobra.Command, args []string) { - kubeConfigFlags.WrapConfigFn = nil + isProxyCmd.Store(true) } // Avoid import cycle by setting ValidArgsFunction here instead of in NewCmdGet() @@ -366,7 +369,7 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { } return existingPreRunE(cmd, args) } - _, err := pref.Apply(cmds, o.Arguments, o.IOStreams.ErrOut) + _, err := pref.Apply(cmds, kubeConfigFlags, o.Arguments, o.IOStreams.ErrOut) if err != nil { fmt.Fprintf(o.IOStreams.ErrOut, "error occurred while applying preferences %v\n", err) os.Exit(1) @@ -387,7 +390,7 @@ func NewKubectlCommand(o KubectlOptions) *cobra.Command { // See SIG CLI KEP 859 for more information: // // https://github.com/kubernetes/enhancements/tree/master/keps/sig-cli/859-kubectl-headers -func addCmdHeaderHooks(cmds *cobra.Command, kubeConfigFlags *genericclioptions.ConfigFlags) { +func addCmdHeaderHooks(cmds *cobra.Command, kubeConfigFlags *genericclioptions.ConfigFlags, isProxyCmd *atomic.Bool) { crt := &genericclioptions.CommandHeaderRoundTripper{} existingPreRunE := cmds.PersistentPreRunE // Add command parsing to the existing persistent pre-run function. @@ -397,7 +400,7 @@ func addCmdHeaderHooks(cmds *cobra.Command, kubeConfigFlags *genericclioptions.C } wrapConfigFn := kubeConfigFlags.WrapConfigFn // Wraps CommandHeaderRoundTripper around standard RoundTripper. - kubeConfigFlags.WrapConfigFn = func(c *rest.Config) *rest.Config { + kubeConfigFlags.WithWrapConfigFn(func(c *rest.Config) *rest.Config { if wrapConfigFn != nil { c = wrapConfigFn(c) } @@ -405,12 +408,13 @@ func addCmdHeaderHooks(cmds *cobra.Command, kubeConfigFlags *genericclioptions.C // Must be separate RoundTripper; not "crt" closure. // Fixes: https://github.com/kubernetes/kubectl/issues/1098 return &genericclioptions.CommandHeaderRoundTripper{ - Delegate: rt, - Headers: crt.Headers, + Delegate: rt, + Headers: crt.Headers, + SkipHeaders: isProxyCmd, // proxy command is incompatible with these headers } }) return c - } + }) } func runHelp(cmd *cobra.Command, args []string) { diff --git a/staging/src/k8s.io/kubectl/pkg/config/scheme/scheme_test.go b/staging/src/k8s.io/kubectl/pkg/config/scheme/scheme_test.go index e14549da4ae..372bf2f28a9 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/scheme/scheme_test.go +++ b/staging/src/k8s.io/kubectl/pkg/config/scheme/scheme_test.go @@ -19,10 +19,28 @@ package scheme import ( "testing" + "k8s.io/apimachinery/pkg/api/apitesting/fuzzer" "k8s.io/apimachinery/pkg/api/apitesting/roundtrip" - "k8s.io/kubectl/pkg/config/fuzzer" + runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/kubectl/pkg/config" + kubectlfuzzer "k8s.io/kubectl/pkg/config/fuzzer" + "sigs.k8s.io/randfill" ) func TestRoundTripTypes(t *testing.T) { - roundtrip.RoundTripTestForScheme(t, Scheme, fuzzer.Funcs) + // Because v1alpha1 does not have fields of these types, the fuzzing will + // be incorrect, so we need to manually intervene here + customFuzzerFuncs := func(codecs runtimeserializer.CodecFactory) []interface{} { + return []interface{}{ + func(s *config.CredentialPluginPolicy, c randfill.Continue) { + *s = "" + }, + func(s *[]config.AllowlistEntry, c randfill.Continue) { + *s = nil + }, + } + } + + funcs := fuzzer.MergeFuzzerFuncs(kubectlfuzzer.Funcs, customFuzzerFuncs) + roundtrip.RoundTripTestForScheme(t, Scheme, funcs) } diff --git a/staging/src/k8s.io/kubectl/pkg/config/types.go b/staging/src/k8s.io/kubectl/pkg/config/types.go index df5bb208440..b310ceae4e8 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/types.go +++ b/staging/src/k8s.io/kubectl/pkg/config/types.go @@ -16,7 +16,9 @@ limitations under the License. package config -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -62,6 +64,63 @@ type Preference struct { // "kubectl getn control-plane-1 --output=json" expands to "kubectl get node --output=json control-plane-1" // +optional Aliases []AliasOverride + + // credentialPluginPolicy specifies the policy governing which, if any, client-go + // credential plugins may be executed. It MUST be one of { "", "AllowAll", "DenyAll", "Allowlist" }. + // If the policy is "", then it falls back to "AllowAll" (this is required + // to maintain backward compatibility). If the policy is DenyAll, no + // credential plugins may run. If the policy is Allowlist, only those + // plugins meeting the criteria specified in the `credentialPluginAllowlist` + // field may run. + // +optional + CredentialPluginPolicy CredentialPluginPolicy + + // Allowlist is a slice of allowlist entries. If any of them is a match, + // then the executable in question may execute. That is, the result is the + // logical OR of all entries in the allowlist. This list MUST NOT be + // supplied if the policy is not "Allowlist". + // + // e.g. + // credentialPluginAllowlist: + // - name: cloud-provider-plugin + // - name: /usr/local/bin/my-plugin + // In the above example, the user allows the credential plugins + // `cloud-provider-plugin` (found somewhere in PATH), and the plugin found + // at the explicit path `/usr/local/bin/my-plugin`. + // +optional + CredentialPluginAllowlist []AllowlistEntry +} + +// CredentialPluginPolicy specifies the policy governing which, if any, client-go +// credential plugins may be executed. It MUST be one of { "", "AllowAll", "DenyAll", "Allowlist" }. +// If the policy is "", then it falls back to "AllowAll" (this is required +// to maintain backward compatibility). If the policy is DenyAll, no +// credential plugins may run. If the policy is Allowlist, only those +// plugins meeting the criteria specified in the `credentialPluginAllowlist` +// field may run. If the policy is not `Allowlist` but one is provided, it +// is considered a configuration error. +type CredentialPluginPolicy string + +const ( + PluginPolicyAllowAll CredentialPluginPolicy = "AllowAll" + PluginPolicyDenyAll CredentialPluginPolicy = "DenyAll" + PluginPolicyAllowlist CredentialPluginPolicy = "Allowlist" +) + +// AllowlistEntry is an entry in the allowlist. For each allowlist item, at +// least one field must be nonempty. A struct with all empty fields is +// considered a misconfiguration error. Each field is a criterion for +// execution. If multiple fields are specified, then the criteria of all +// specified fields must be met. That is, the result of an individual entry is +// the logical AND of all checks corresponding to the specified fields within +// the entry. +type AllowlistEntry struct { + // Name matching is performed by first resolving the absolute path of both + // the plugin and the name in the allowlist entry using `exec.LookPath`. It + // will be called on both, and the resulting strings must be equal. If + // either call to `exec.LookPath` results in an error, the `Name` check + // will be considered a failure. + Name string } // AliasOverride stores the alias definitions. diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/conversion.go b/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/conversion.go new file mode 100644 index 00000000000..05ddfdace7f --- /dev/null +++ b/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/conversion.go @@ -0,0 +1,31 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + conversion "k8s.io/apimachinery/pkg/conversion" + config "k8s.io/kubectl/pkg/config" +) + +// v1alpha1 Preference does not have `CredentialPluginPolicy` or `CredentialPluginAllowlist` fields. They can be left blank, so the autoConvert functions will suffice. +func Convert_config_Preference_To_v1alpha1_Preference(in *config.Preference, out *Preference, s conversion.Scope) error { + return autoConvert_config_Preference_To_v1alpha1_Preference(in, out, s) +} + +func Convert_v1alpha1_Preference_To_config_Preference(in *Preference, out *config.Preference, s conversion.Scope) error { + return autoConvert_v1alpha1_Preference_To_config_Preference(in, out, s) +} diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/zz_generated.conversion.go b/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/zz_generated.conversion.go index af0b9bba7f0..5ad464c8755 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/zz_generated.conversion.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1alpha1/zz_generated.conversion.go @@ -66,13 +66,13 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*Preference)(nil), (*config.Preference)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha1_Preference_To_config_Preference(a.(*Preference), b.(*config.Preference), scope) + if err := s.AddConversionFunc((*config.Preference)(nil), (*Preference)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_config_Preference_To_v1alpha1_Preference(a.(*config.Preference), b.(*Preference), scope) }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*config.Preference)(nil), (*Preference)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_config_Preference_To_v1alpha1_Preference(a.(*config.Preference), b.(*Preference), scope) + if err := s.AddConversionFunc((*Preference)(nil), (*config.Preference)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha1_Preference_To_config_Preference(a.(*Preference), b.(*config.Preference), scope) }); err != nil { return err } @@ -157,18 +157,10 @@ func autoConvert_v1alpha1_Preference_To_config_Preference(in *Preference, out *c return nil } -// Convert_v1alpha1_Preference_To_config_Preference is an autogenerated conversion function. -func Convert_v1alpha1_Preference_To_config_Preference(in *Preference, out *config.Preference, s conversion.Scope) error { - return autoConvert_v1alpha1_Preference_To_config_Preference(in, out, s) -} - func autoConvert_config_Preference_To_v1alpha1_Preference(in *config.Preference, out *Preference, s conversion.Scope) error { out.Defaults = *(*[]CommandDefaults)(unsafe.Pointer(&in.Defaults)) out.Aliases = *(*[]AliasOverride)(unsafe.Pointer(&in.Aliases)) + // WARNING: in.CredentialPluginPolicy requires manual conversion: does not exist in peer-type + // WARNING: in.CredentialPluginAllowlist requires manual conversion: does not exist in peer-type return nil } - -// Convert_config_Preference_To_v1alpha1_Preference is an autogenerated conversion function. -func Convert_config_Preference_To_v1alpha1_Preference(in *config.Preference, out *Preference, s conversion.Scope) error { - return autoConvert_config_Preference_To_v1alpha1_Preference(in, out, s) -} diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/register.go b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/register.go index c576a0c0224..d414d10e6ce 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/register.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/register.go @@ -37,7 +37,7 @@ func init() { // We only register manually written functions here. The registration of the // generated functions takes place in the generated files. The separation // makes the code compile even when the generated files are missing. - localSchemeBuilder.Register(addKnownTypes) + localSchemeBuilder.Register(addKnownTypes, RegisterDefaults) } // addKnownTypes registers known types to the given scheme diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/types.go b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/types.go index 219cfe5590b..e1d5409a185 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/types.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/types.go @@ -16,7 +16,9 @@ limitations under the License. package v1beta1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -62,6 +64,64 @@ type Preference struct { // "kubectl getn control-plane-1 --output=json" expands to "kubectl get node --output=json control-plane-1" // +listType=atomic Aliases []AliasOverride `json:"aliases"` + + // credentialPluginPolicy specifies the policy governing which, if any, client-go + // credential plugins may be executed. It MUST be one of { "", "AllowAll", "DenyAll", "Allowlist" }. + // If the policy is "", then it falls back to "AllowAll" (this is required + // to maintain backward compatibility). If the policy is DenyAll, no + // credential plugins may run. If the policy is Allowlist, only those + // plugins meeting the criteria specified in the `credentialPluginAllowlist` + // field may run. + // +optional + CredentialPluginPolicy CredentialPluginPolicy `json:"credentialPluginPolicy,omitempty"` + + // Allowlist is a slice of allowlist entries. If any of them is a match, + // then the executable in question may execute. That is, the result is the + // logical OR of all entries in the allowlist. This list MUST NOT be + // supplied if the policy is not "Allowlist". + // + // e.g. + // credentialPluginAllowlist: + // - name: cloud-provider-plugin + // - name: /usr/local/bin/my-plugin + // In the above example, the user allows the credential plugins + // `cloud-provider-plugin` (found somewhere in PATH), and the plugin found + // at the explicit path `/usr/local/bin/my-plugin`. + // +optional + // +listType=atomic + CredentialPluginAllowlist []AllowlistEntry `json:"credentialPluginAllowlist,omitempty"` +} + +// CredentialPluginPolicy specifies the policy governing which, if any, client-go +// credential plugins may be executed. It MUST be one of { "", "AllowAll", "DenyAll", "Allowlist" }. +// If the policy is "", then it falls back to "AllowAll" (this is required +// to maintain backward compatibility). If the policy is DenyAll, no +// credential plugins may run. If the policy is Allowlist, only those +// plugins meeting the criteria specified in the `credentialPluginAllowlist` +// field may run. If the policy is not `Allowlist` but one is provided, it +// is considered a configuration error. +type CredentialPluginPolicy string + +const ( + PluginPolicyAllowAll CredentialPluginPolicy = "AllowAll" + PluginPolicyDenyAll CredentialPluginPolicy = "DenyAll" + PluginPolicyAllowlist CredentialPluginPolicy = "Allowlist" +) + +// AllowlistEntry is an entry in the allowlist. For each allowlist item, at +// least one field must be nonempty. A struct with all empty fields is +// considered a misconfiguration error. Each field is a criterion for +// execution. If multiple fields are specified, then the criteria of all +// specified fields must be met. That is, the result of an individual entry is +// the logical AND of all checks corresponding to the specified fields within +// the entry. +type AllowlistEntry struct { + // Name matching is performed by first resolving the absolute path of both + // the plugin and the name in the allowlist entry using `exec.LookPath`. It + // will be called on both, and the resulting strings must be equal. If + // either call to `exec.LookPath` results in an error, the `Name` check + // will be considered a failure. + Name string `json:"name"` } // AliasOverride stores the alias definitions. diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.conversion.go b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.conversion.go index fbbd9510203..dcbe9948120 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.conversion.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.conversion.go @@ -46,6 +46,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddGeneratedConversionFunc((*AllowlistEntry)(nil), (*config.AllowlistEntry)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_AllowlistEntry_To_config_AllowlistEntry(a.(*AllowlistEntry), b.(*config.AllowlistEntry), scope) + }); err != nil { + return err + } + if err := s.AddGeneratedConversionFunc((*config.AllowlistEntry)(nil), (*AllowlistEntry)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_config_AllowlistEntry_To_v1beta1_AllowlistEntry(a.(*config.AllowlistEntry), b.(*AllowlistEntry), scope) + }); err != nil { + return err + } if err := s.AddGeneratedConversionFunc((*CommandDefaults)(nil), (*config.CommandDefaults)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_CommandDefaults_To_config_CommandDefaults(a.(*CommandDefaults), b.(*config.CommandDefaults), scope) }); err != nil { @@ -107,6 +117,26 @@ func Convert_config_AliasOverride_To_v1beta1_AliasOverride(in *config.AliasOverr return autoConvert_config_AliasOverride_To_v1beta1_AliasOverride(in, out, s) } +func autoConvert_v1beta1_AllowlistEntry_To_config_AllowlistEntry(in *AllowlistEntry, out *config.AllowlistEntry, s conversion.Scope) error { + out.Name = in.Name + return nil +} + +// Convert_v1beta1_AllowlistEntry_To_config_AllowlistEntry is an autogenerated conversion function. +func Convert_v1beta1_AllowlistEntry_To_config_AllowlistEntry(in *AllowlistEntry, out *config.AllowlistEntry, s conversion.Scope) error { + return autoConvert_v1beta1_AllowlistEntry_To_config_AllowlistEntry(in, out, s) +} + +func autoConvert_config_AllowlistEntry_To_v1beta1_AllowlistEntry(in *config.AllowlistEntry, out *AllowlistEntry, s conversion.Scope) error { + out.Name = in.Name + return nil +} + +// Convert_config_AllowlistEntry_To_v1beta1_AllowlistEntry is an autogenerated conversion function. +func Convert_config_AllowlistEntry_To_v1beta1_AllowlistEntry(in *config.AllowlistEntry, out *AllowlistEntry, s conversion.Scope) error { + return autoConvert_config_AllowlistEntry_To_v1beta1_AllowlistEntry(in, out, s) +} + func autoConvert_v1beta1_CommandDefaults_To_config_CommandDefaults(in *CommandDefaults, out *config.CommandDefaults, s conversion.Scope) error { out.Command = in.Command out.Options = *(*[]config.CommandOptionDefault)(unsafe.Pointer(&in.Options)) @@ -154,6 +184,8 @@ func Convert_config_CommandOptionDefault_To_v1beta1_CommandOptionDefault(in *con func autoConvert_v1beta1_Preference_To_config_Preference(in *Preference, out *config.Preference, s conversion.Scope) error { out.Defaults = *(*[]config.CommandDefaults)(unsafe.Pointer(&in.Defaults)) out.Aliases = *(*[]config.AliasOverride)(unsafe.Pointer(&in.Aliases)) + out.CredentialPluginPolicy = config.CredentialPluginPolicy(in.CredentialPluginPolicy) + out.CredentialPluginAllowlist = *(*[]config.AllowlistEntry)(unsafe.Pointer(&in.CredentialPluginAllowlist)) return nil } @@ -165,6 +197,8 @@ func Convert_v1beta1_Preference_To_config_Preference(in *Preference, out *config func autoConvert_config_Preference_To_v1beta1_Preference(in *config.Preference, out *Preference, s conversion.Scope) error { out.Defaults = *(*[]CommandDefaults)(unsafe.Pointer(&in.Defaults)) out.Aliases = *(*[]AliasOverride)(unsafe.Pointer(&in.Aliases)) + out.CredentialPluginPolicy = CredentialPluginPolicy(in.CredentialPluginPolicy) + out.CredentialPluginAllowlist = *(*[]AllowlistEntry)(unsafe.Pointer(&in.CredentialPluginAllowlist)) return nil } diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.deepcopy.go b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.deepcopy.go index e6422ad5d2c..35565bee4bf 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.deepcopy.go @@ -56,6 +56,22 @@ func (in *AliasOverride) DeepCopy() *AliasOverride { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AllowlistEntry) DeepCopyInto(out *AllowlistEntry) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllowlistEntry. +func (in *AllowlistEntry) DeepCopy() *AllowlistEntry { + if in == nil { + return nil + } + out := new(AllowlistEntry) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CommandDefaults) DeepCopyInto(out *CommandDefaults) { *out = *in @@ -111,6 +127,11 @@ func (in *Preference) DeepCopyInto(out *Preference) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.CredentialPluginAllowlist != nil { + in, out := &in.CredentialPluginAllowlist, &out.CredentialPluginAllowlist + *out = make([]AllowlistEntry, len(*in)) + copy(*out, *in) + } return } diff --git a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.model_name.go b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.model_name.go index fe07b0d5d86..a071ba0c71a 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.model_name.go +++ b/staging/src/k8s.io/kubectl/pkg/config/v1beta1/zz_generated.model_name.go @@ -26,6 +26,11 @@ func (in AliasOverride) OpenAPIModelName() string { return "io.k8s.kubectl.pkg.config.v1beta1.AliasOverride" } +// OpenAPIModelName returns the OpenAPI model name for this type. +func (in AllowlistEntry) OpenAPIModelName() string { + return "io.k8s.kubectl.pkg.config.v1beta1.AllowlistEntry" +} + // OpenAPIModelName returns the OpenAPI model name for this type. func (in CommandDefaults) OpenAPIModelName() string { return "io.k8s.kubectl.pkg.config.v1beta1.CommandDefaults" diff --git a/staging/src/k8s.io/kubectl/pkg/config/zz_generated.deepcopy.go b/staging/src/k8s.io/kubectl/pkg/config/zz_generated.deepcopy.go index e27a1533708..dd504ae72e5 100644 --- a/staging/src/k8s.io/kubectl/pkg/config/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/kubectl/pkg/config/zz_generated.deepcopy.go @@ -56,6 +56,22 @@ func (in *AliasOverride) DeepCopy() *AliasOverride { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AllowlistEntry) DeepCopyInto(out *AllowlistEntry) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AllowlistEntry. +func (in *AllowlistEntry) DeepCopy() *AllowlistEntry { + if in == nil { + return nil + } + out := new(AllowlistEntry) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CommandDefaults) DeepCopyInto(out *CommandDefaults) { *out = *in @@ -111,6 +127,11 @@ func (in *Preference) DeepCopyInto(out *Preference) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.CredentialPluginAllowlist != nil { + in, out := &in.CredentialPluginAllowlist, &out.CredentialPluginAllowlist + *out = make([]AllowlistEntry, len(*in)) + copy(*out, *in) + } return } diff --git a/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc.go b/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc.go index ab43babde2f..fed7fc8a722 100644 --- a/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc.go +++ b/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc.go @@ -29,7 +29,11 @@ import ( "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/plugin/pkg/client/auth/exec" + "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/util/homedir" "k8s.io/kubectl/pkg/config" ) @@ -47,11 +51,14 @@ var ( shortHandRegex = regexp.MustCompile("^-[a-zA-Z]+$") ) +// compile-time check that these two types are aligned +var _ = clientcmdapi.AllowlistEntry(config.AllowlistEntry{}) + // PreferencesHandler is responsible for setting default flags // arguments based on user's kuberc configuration. type PreferencesHandler interface { AddFlags(flags *pflag.FlagSet) - Apply(rootCmd *cobra.Command, args []string, errOut io.Writer) ([]string, error) + Apply(rootCmd *cobra.Command, kubeConfigFlags *genericclioptions.ConfigFlags, args []string, errOut io.Writer) ([]string, error) } // Preferences stores the kuberc file coming either from environment variable @@ -60,6 +67,7 @@ type Preferences struct { getPreferencesFunc func(kuberc string, errOut io.Writer) (*config.Preference, error) aliases map[string]struct{} + policy clientcmdapi.PluginPolicy } // NewPreferences returns initialized Prefrences object. @@ -85,7 +93,7 @@ func (p *Preferences) AddFlags(flags *pflag.FlagSet) { // Apply firstly applies the aliases in the preferences file and secondly overrides // the default values of flags. -func (p *Preferences) Apply(rootCmd *cobra.Command, args []string, errOut io.Writer) ([]string, error) { +func (p *Preferences) Apply(rootCmd *cobra.Command, kubeConfigFlags *genericclioptions.ConfigFlags, args []string, errOut io.Writer) ([]string, error) { if len(args) <= 1 { return args, nil } @@ -103,11 +111,15 @@ func (p *Preferences) Apply(rootCmd *cobra.Command, args []string, errOut io.Wri return args, nil } - err = validate(kuberc) + p.convertPluginPolicy(kuberc) + + err = p.validate(kuberc) if err != nil { return args, err } + p.applyPluginPolicy(kubeConfigFlags, kuberc) + args, err = p.applyAliases(rootCmd, kuberc, args, errOut) if err != nil { return args, err @@ -119,6 +131,38 @@ func (p *Preferences) Apply(rootCmd *cobra.Command, args []string, errOut io.Wri return args, nil } +func (p *Preferences) convertPluginPolicy(kuberc *config.Preference) { + var allowlist []clientcmdapi.AllowlistEntry + if kuberc.CredentialPluginAllowlist != nil { + allowlist = make([]clientcmdapi.AllowlistEntry, len(kuberc.CredentialPluginAllowlist)) + for i := range kuberc.CredentialPluginAllowlist { + allowlist[i] = clientcmdapi.AllowlistEntry(kuberc.CredentialPluginAllowlist[i]) + } + } + p.policy = clientcmdapi.PluginPolicy{ + PolicyType: clientcmdapi.PolicyType(kuberc.CredentialPluginPolicy), + Allowlist: allowlist, + } +} + +// `applyPluginPolicy` wraps the rest client getter with one that propagates +// the allowlist, via the rest config, to the code handling credential exec +// plugins. +func (p *Preferences) applyPluginPolicy(kubeConfigFlags *genericclioptions.ConfigFlags, kuberc *config.Preference) { + wrapConfigFn := kubeConfigFlags.WrapConfigFn + kubeConfigFlags.WithWrapConfigFn(func(c *rest.Config) *rest.Config { + if wrapConfigFn != nil { + c = wrapConfigFn(c) + } + + if c.ExecProvider != nil { + c.ExecProvider.PluginPolicy = p.policy + } + + return c + }) +} + // applyOverrides finds the command and sets the defaulted flag values in kuberc. func (p *Preferences) applyOverrides(rootCmd *cobra.Command, kuberc *config.Preference, args []string, errOut io.Writer) error { args = args[1:] @@ -455,7 +499,7 @@ func searchInArgs(flagName string, shorthand string, allShorthands map[string]st return false } -func validate(plugin *config.Preference) error { +func (p *Preferences) validate(plugin *config.Preference) error { validateFlag := func(flags []config.CommandOptionDefault) error { for _, flag := range flags { if strings.HasPrefix(flag.Name, "-") { @@ -486,5 +530,9 @@ func validate(plugin *config.Preference) error { } } + if err := exec.ValidatePluginPolicy(p.policy); err != nil { + return err + } + return nil } diff --git a/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc_test.go b/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc_test.go index e26d0da5d79..01afec48915 100644 --- a/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc_test.go +++ b/staging/src/k8s.io/kubectl/pkg/kuberc/kuberc_test.go @@ -31,6 +31,8 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/cli-runtime/pkg/genericclioptions" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/kubectl/pkg/config" ) @@ -856,6 +858,8 @@ func TestApplyOverride(t *testing.T) { rootCmd := &cobra.Command{ Use: "root", } + + opts := genericclioptions.NewConfigFlags(false) prefHandler := NewPreferences() prefHandler.AddFlags(rootCmd.PersistentFlags()) pref, ok := prefHandler.(*Preferences) @@ -866,7 +870,7 @@ func TestApplyOverride(t *testing.T) { pref.getPreferencesFunc = test.getPreferencesFunc errWriter := &bytes.Buffer{} - _, err := pref.Apply(rootCmd, test.args, errWriter) + _, err := pref.Apply(rootCmd, opts, test.args, errWriter) if test.expectedErr == nil && err != nil { t.Fatalf("unexpected error %v\n", err) } @@ -1114,7 +1118,8 @@ func TestApplyOverrideBool(t *testing.T) { addCommands(rootCmd, test.nestedCmds) pref.getPreferencesFunc = test.getPreferencesFunc errWriter := &bytes.Buffer{} - _, err := pref.Apply(rootCmd, test.args, errWriter) + opts := genericclioptions.NewConfigFlags(false) + _, err := pref.Apply(rootCmd, opts, test.args, errWriter) if err != nil { t.Fatalf("unexpected error %v\n", err) } @@ -1403,7 +1408,8 @@ func TestApplyAliasBool(t *testing.T) { addCommands(rootCmd, test.nestedCmds) pref.getPreferencesFunc = test.getPreferencesFunc errWriter := &bytes.Buffer{} - lastArgs, err := pref.Apply(rootCmd, test.args, errWriter) + opts := genericclioptions.NewConfigFlags(false) + lastArgs, err := pref.Apply(rootCmd, opts, test.args, errWriter) if test.expectedErr == nil && err != nil { t.Fatalf("unexpected error %v\n", err) } @@ -2606,7 +2612,8 @@ func TestApplyAlias(t *testing.T) { addCommands(rootCmd, test.nestedCmds) pref.getPreferencesFunc = test.getPreferencesFunc errWriter := &bytes.Buffer{} - lastArgs, err := pref.Apply(rootCmd, test.args, errWriter) + opts := genericclioptions.NewConfigFlags(false) + lastArgs, err := pref.Apply(rootCmd, opts, test.args, errWriter) if test.expectedErr == nil && err != nil { t.Fatalf("unexpected error %v\n", err) } @@ -2922,3 +2929,244 @@ unknownField: value`, }) } } + +func TestApplyPluginPolicy(t *testing.T) { + kubeconfigData := ` +apiVersion: v1 +clusters: +- cluster: + server: https://example.test:443 + name: foo +contexts: +- context: + cluster: foo + user: me + name: foo +current-context: foo +kind: Config +preferences: {} +users: +- name: me + user: + exec: + apiVersion: client.authentication.k8s.io/v1beta1 + args: + - get-token + - --login + command: foo` + + tmpDir := t.TempDir() + kubeconfig := filepath.Join(tmpDir, "kubeconfig") + err := os.WriteFile(kubeconfig, []byte(kubeconfigData), 0o644) + require.NoError(t, err, "writing fake kubeconfig") + + rootCmd := &cobra.Command{ + Use: "root", + } + + args := []string{"placeholder", "two"} + + opts := genericclioptions.NewConfigFlags(false) + opts.KubeConfig = &kubeconfig + + p := NewPreferences() + pref, ok := p.(*Preferences) + require.True(t, ok, "preference type") + + t.Run("plumbing", func(t *testing.T) { + pref.getPreferencesFunc = func(_ string, _ io.Writer) (*config.Preference, error) { + return &config.Preference{ + CredentialPluginPolicy: config.CredentialPluginPolicy("Allowlist"), + CredentialPluginAllowlist: []config.AllowlistEntry{ + {Name: "bar"}, + {Name: "baz"}, + }, + }, nil + } + + _, err = p.Apply(rootCmd, opts, args, io.Discard) + require.NoError(t, err, "error applying preferences") + + cfg, err := opts.ToRESTConfig() + require.NoError(t, err, "unexpected error") + require.NotNil(t, cfg, "rest config") + require.NotNil(t, cfg.ExecProvider, "exec config") + require.Equal(t, clientcmdapi.PolicyType("Allowlist"), cfg.ExecProvider.PluginPolicy.PolicyType) + require.Equal(t, "bar", cfg.ExecProvider.PluginPolicy.Allowlist[0].Name) + require.Equal(t, "baz", cfg.ExecProvider.PluginPolicy.Allowlist[1].Name) + }) + + type pluginPolicyTest struct { + name string + kuberc string + shouldErr bool + } + tests := []pluginPolicyTest{ + { + name: "invalid-plugin-policy-with-nil-allowlist", + shouldErr: true, + kuberc: `kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "foo" +`, + }, + { + name: "invalid-policy-with-empty-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "foo" +credentialPluginAllowlist: [] +`, + }, + { + name: "invalid-policy-with-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "foo" +credentialPluginAllowlist: +- name: "bar" +`, + }, + { + name: "allowlist-policy-with-nil-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "Allowlist" +`, + }, + { + name: "allowlist-policy-with-empty-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "Allowlist" +credentialPluginAllowlist: [] +`, + }, + { + name: "unspecified-policy-with-non-nil-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginAllowlist: +- name: "bar" +- name: "baz" +`, + }, + { + name: "allowall-policy-with-non-nil-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "AllowAll" +credentialPluginAllowlist: []clientcmdapi.AllowlistEntry{ +- name: "bar" +- name: "baz" +`, + }, + { + name: "allowall-policy-with-non-nil-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "DenyAll" +credentialPluginAllowlist: +- name: "bar" +- name: "baz" +`, + }, + { + name: "non-allowlist-policy-with-non-nil-empty-allowlist", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "DenyAll" +credentialPluginAllowlist: [] +`, + }, + { + name: "allowlist-policy-with-one-empty-allowlist-entry", + shouldErr: true, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "Allowlist" +credentialPluginAllowlist: +- name: "foo" +- name: "" +`, + }, + { + name: "allowlist-policy-with-nonempty-allowlist", + shouldErr: false, + kuberc: `apiVersion: kubectl.config.k8s.io/v1beta1 +kind: Preference +credentialPluginPolicy: "Allowlist" +credentialPluginAllowlist: +- name: "foo" +`, + }, + { + name: "allowall-policy-with-nil-allowlist", + shouldErr: false, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "AllowAll" +`, + }, + { + name: "denyall-policy-with-nil-allowlist", + shouldErr: false, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "DenyAll" +`, + }, + { + name: "unspecified-policy-with-nil-allowlist", + shouldErr: false, + kuberc: ` +kind: Preference +apiVersion: kubectl.config.k8s.io/v1beta1 +credentialPluginPolicy: "" +`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + tmpDir := t.TempDir() + kuberc := filepath.Join(tmpDir, "kuberc") + require.NoError(t, os.WriteFile(kuberc, []byte(test.kuberc), 0o644)) + + pref.getPreferencesFunc = func(_ string, errOut io.Writer) (*config.Preference, error) { + pref, err := DefaultGetPreferences(kuberc, errOut) + if err != nil { + return nil, err + } + + return pref, nil + } + + _, err := p.Apply(rootCmd, opts, args, io.Discard) + if test.shouldErr { + require.Error(t, err, "expected error, but error was nil") + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh index 04c43bce783..ab6c4f638ab 100644 --- a/test/cmd/authentication.sh +++ b/test/cmd/authentication.sh @@ -178,6 +178,8 @@ EOF kube::log::status "exec credential plugin not triggered since kubeconfig was configured with --client-certificate/--client-key for authentication" fi + run_allowlist_tests_version + kubectl delete csr testuser rm "${TMPDIR:-/tmp}"/invalid_execcredential.sh rm "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml @@ -187,6 +189,112 @@ EOF set +o errexit } +run_allowlist_tests_version() { + set +o nounset + set +o errexit + + cat >"${TMPDIR:-/tmp}/kuberc-deny-all.yaml" <&1) + rc7="$?" + + if [ "$rc7" -ne 0 ] && [[ "$output7" =~ "DenyAll" ]]; then + kube::log::status "exec credential plugin not triggered because plugin policy set to DenyAll" + else + kube::log::status "Unexpected output when kuberc was configured with credentialPluginPolicy: DenyAll. Exec credential plugin ran despite DenyAll policy" + exit 1 + fi + + output8=$(kubectl --kubeconfig="${TMPDIR:-/tmp}/valid_exec_plugin.yaml" --kuberc="${TMPDIR:-/tmp}/kuberc-deny-all.yaml" "${kube_flags_without_token[@]:?}" config view) + rc8="$?" + + if [ "$rc8" -eq 0 ] && kube::test::if_has_string "$output8" "kind: Config"; then + kube::log::status "plugin policy has no effect on 'kubectl config view'" + else + kube::log::status "plugin policy 'DenyAll' prevented 'kubectl config view' from running" + exit 1 + fi + + cat >"${TMPDIR:-/tmp}/kuberc-allowlist-should-fail.yaml" <&1) + rc9="$?" + + if [ "$rc9" -ne 0 ] && kube::test::if_has_string "$output9" "is not permitted by the credential plugin allowlist"; then + kube::log::status "exec credential plugin not triggered because allowlist does not permit it" + else + kube::log::status "Unexpected output when kuberc was configured with credentialPluginPolicy: Allowlist. Exec credential plugin ran despite not appearing in allowlist" + exit 1 + fi + + cat >"${TMPDIR:-/tmp}/kuberc-allowlist-should-succeed.yaml" <"${TMPDIR:-/tmp}/kuberc-allowlist-not-given.yaml" <"${TMPDIR:-/tmp}/kuberc-invalid-allowlist.yaml" <&1) + rc12="$?" + + if [ "$rc12" -ne 0 ] && [[ "$output12" =~ "misconfigured credential plugin allowlist" ]]; then + kube::log::status "exec credential plugin denied by invalid allowlist" + else + kube::log::status "Unexpected output when kuberc was configured with invalid allowlist" + exit 1 + fi + + set -o nounset + set -o errexit +} + run_exec_credentials_interactive_tests() { run_exec_credentials_interactive_tests_version client.authentication.k8s.io/v1beta1 run_exec_credentials_interactive_tests_version client.authentication.k8s.io/v1 diff --git a/test/integration/client/exec_test.go b/test/integration/client/exec_test.go index 8c872e90295..5e4b907a6f5 100644 --- a/test/integration/client/exec_test.go +++ b/test/integration/client/exec_test.go @@ -91,15 +91,29 @@ type execPluginCall struct { callStatus string } -type execPluginMetrics struct { - calls []execPluginCall +type execPluginPolicyCall struct { + status string } +type wantMetrics struct { + calls []execPluginCall + policyCalls []execPluginPolicyCall +} + +// go does not allow implementing different interfaces that share method names, +// so implement the interfaces on type aliases instead, casting them as needed +type execPluginMetrics wantMetrics +type execPluginPolicyMetrics wantMetrics + func (m *execPluginMetrics) Increment(exitCode int, callStatus string) { m.calls = append(m.calls, execPluginCall{exitCode: exitCode, callStatus: callStatus}) } -var execPluginMetricsComparer = cmp.Comparer(func(a, b *execPluginMetrics) bool { +func (m *execPluginPolicyMetrics) Increment(status string) { + m.policyCalls = append(m.policyCalls, execPluginPolicyCall{status: status}) +} + +var wantMetricsComparer = cmp.Comparer(func(a, b *wantMetrics) bool { return reflect.DeepEqual(a, b) }) @@ -110,7 +124,7 @@ type execPluginClientTestData struct { wantCertificate *tls.Certificate wantGetCertificateErrorPrefix string wantClientErrorPrefix string - wantMetrics *execPluginMetrics + wantMetrics *wantMetrics } func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byte, clientAuthorizedToken, clientCertFileName, clientKeyFileName string) []execPluginClientTestData { @@ -134,12 +148,17 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt wantAuthorizationHeaderValues: [][]string{{"Bearer unauthorized"}}, wantCertificate: &tls.Certificate{}, wantClientErrorPrefix: "Unauthorized", - wantMetrics: &execPluginMetrics{ + wantMetrics: &wantMetrics{ calls: []execPluginCall{ // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. {exitCode: 0, callStatus: "no_error"}, {exitCode: 0, callStatus: "no_error"}, }, + policyCalls: []execPluginPolicyCall{ + // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. + {status: "allowed"}, + {status: "allowed"}, + }, }, }, { @@ -162,12 +181,17 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt wantAuthorizationHeaderValues: [][]string{nil}, wantCertificate: x509KeyPair(unauthorizedCert, unauthorizedKey, true), wantClientErrorPrefix: "Unauthorized", - wantMetrics: &execPluginMetrics{ + wantMetrics: &wantMetrics{ calls: []execPluginCall{ // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. {exitCode: 0, callStatus: "no_error"}, {exitCode: 0, callStatus: "no_error"}, }, + policyCalls: []execPluginPolicyCall{ + // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. + {status: "allowed"}, + {status: "allowed"}, + }, }, }, { @@ -188,7 +212,10 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}}, wantCertificate: &tls.Certificate{}, - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "authorized certificate", @@ -209,7 +236,10 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{nil}, wantCertificate: loadX509KeyPair(clientCertFileName, clientKeyFileName), - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "authorized token and certificate", @@ -231,7 +261,10 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}}, wantCertificate: loadX509KeyPair(clientCertFileName, clientKeyFileName), - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "unauthorized token and authorized certificate favors authorized certificate", @@ -253,7 +286,10 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Bearer client-unauthorized-token"}}, wantCertificate: loadX509KeyPair(clientCertFileName, clientKeyFileName), - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "authorized token and unauthorized certificate favors authorized token", @@ -275,7 +311,10 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}}, wantCertificate: x509KeyPair([]byte(unauthorizedCert), []byte(unauthorizedKey), true), - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "unauthorized token and unauthorized certificate", @@ -298,12 +337,17 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt wantAuthorizationHeaderValues: [][]string{{"Bearer client-unauthorized-token"}}, wantCertificate: x509KeyPair(unauthorizedCert, unauthorizedKey, true), wantClientErrorPrefix: "Unauthorized", - wantMetrics: &execPluginMetrics{ + wantMetrics: &wantMetrics{ calls: []execPluginCall{ // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. {exitCode: 0, callStatus: "no_error"}, {exitCode: 0, callStatus: "no_error"}, }, + policyCalls: []execPluginPolicyCall{ + // 2 calls since we preemptively refresh the creds upon a 401 HTTP response. + {status: "allowed"}, + {status: "allowed"}, + }, }, }, { @@ -326,7 +370,7 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Basic " + basicAuthHeaderValue("unauthorized", "unauthorized")}}, wantClientErrorPrefix: "Unauthorized", - wantMetrics: &execPluginMetrics{}, + wantMetrics: &wantMetrics{}, }, { name: "good token with static auth bearer token favors static auth bearer token", @@ -347,7 +391,7 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantAuthorizationHeaderValues: [][]string{{"Bearer some-unauthorized-token"}}, wantClientErrorPrefix: "Unauthorized", - wantMetrics: &execPluginMetrics{}, + wantMetrics: &wantMetrics{}, }, { name: "good token with static auth cert and key favors static cert", @@ -370,7 +414,7 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt wantAuthorizationHeaderValues: [][]string{nil}, wantClientErrorPrefix: "Unauthorized", wantCertificate: x509KeyPair(unauthorizedCert, unauthorizedKey, false), - wantMetrics: &execPluginMetrics{}, + wantMetrics: &wantMetrics{}, }, { name: "unknown binary", @@ -379,16 +423,22 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantGetCertificateErrorPrefix: "exec: executable does not exist not found", wantClientErrorPrefix: `Get "https`, - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 1, callStatus: "plugin_not_found_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 1, callStatus: "plugin_not_found_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "binary not executable", clientConfigFunc: func(c *rest.Config) { c.ExecProvider.Command = "./testdata/exec-plugin-not-executable.sh" }, - wantGetCertificateErrorPrefix: "exec: fork/exec ./testdata/exec-plugin-not-executable.sh: permission denied", + wantGetCertificateErrorPrefix: "exec: fork/exec testdata/exec-plugin-not-executable.sh: permission denied", wantClientErrorPrefix: `Get "https`, - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 1, callStatus: "plugin_not_found_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 1, callStatus: "plugin_not_found_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, { name: "binary fails", @@ -402,7 +452,88 @@ func execPluginClientTests(t *testing.T, unauthorizedCert, unauthorizedKey []byt }, wantGetCertificateErrorPrefix: "exec: executable testdata/exec-plugin.sh failed with exit code 10", wantClientErrorPrefix: `Get "https`, - wantMetrics: &execPluginMetrics{calls: []execPluginCall{{exitCode: 10, callStatus: "plugin_execution_error"}}}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 10, callStatus: "plugin_execution_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, + }, + { + name: "binary denied by denyall policy", + clientConfigFunc: func(c *rest.Config) { + c.ExecProvider.PluginPolicy.PolicyType = clientcmdapi.PluginPolicyDenyAll + }, + wantGetCertificateErrorPrefix: "plugin", + wantClientErrorPrefix: `Get "https`, + wantMetrics: &wantMetrics{ + policyCalls: []execPluginPolicyCall{{status: "denied"}}, + }, + }, + { + name: "binary denied by allowlist policy", + clientConfigFunc: func(c *rest.Config) { + c.ExecProvider.PluginPolicy.PolicyType = clientcmdapi.PluginPolicyAllowlist + c.ExecProvider.PluginPolicy.Allowlist = []clientcmdapi.AllowlistEntry{ + {Name: "/only/my/very/secure/binary"}, + {Name: "other-very-secure-binary"}, + } + }, + wantGetCertificateErrorPrefix: `plugin path "testdata/exec-plugin.sh" is not permitted by the credential plugin allowlist`, + wantClientErrorPrefix: `Get "https`, + wantMetrics: &wantMetrics{ + policyCalls: []execPluginPolicyCall{{status: "denied"}}, + }, + }, + { + name: "allowall policy happy path", + clientConfigFunc: func(c *rest.Config) { + c.ExecProvider.PluginPolicy.PolicyType = clientcmdapi.PluginPolicyAllowAll + c.ExecProvider.Env = []clientcmdapi.ExecEnvVar{ + { + Name: outputEnvVar, + Value: fmt.Sprintf(`{ + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1", + "status": { + "token": "%s" + } + }`, clientAuthorizedToken), + }, + } + }, + wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}}, + wantCertificate: &tls.Certificate{}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, + }, + { + name: "allowlist policy happy path", + clientConfigFunc: func(c *rest.Config) { + c.ExecProvider.PluginPolicy.PolicyType = clientcmdapi.PluginPolicyAllowlist + c.ExecProvider.PluginPolicy.Allowlist = []clientcmdapi.AllowlistEntry{ + {Name: "testdata/exec-plugin.sh"}, + {Name: "other-very-secure-binary"}, + } + c.ExecProvider.Env = []clientcmdapi.ExecEnvVar{ + { + Name: outputEnvVar, + Value: fmt.Sprintf(`{ + "kind": "ExecCredential", + "apiVersion": "client.authentication.k8s.io/v1", + "status": { + "token": "%s" + } + }`, clientAuthorizedToken), + }, + } + }, + wantAuthorizationHeaderValues: [][]string{{"Bearer " + clientAuthorizedToken}}, + wantCertificate: &tls.Certificate{}, + wantMetrics: &wantMetrics{ + calls: []execPluginCall{{exitCode: 0, callStatus: "no_error"}}, + policyCalls: []execPluginPolicyCall{{status: "allowed"}}, + }, }, } return append(v1Tests, v1beta1TestsFromV1Tests(v1Tests)...) @@ -486,7 +617,7 @@ func TestExecPluginViaClient(t *testing.T) { } // Validate that the proper metrics were set. - if diff := cmp.Diff(test.wantMetrics, actualMetrics, execPluginMetricsComparer); diff != "" { + if diff := cmp.Diff(test.wantMetrics, actualMetrics, wantMetricsComparer); diff != "" { t.Error("unexpected metrics; -want, +got:\n" + diff) } @@ -521,14 +652,18 @@ func TestExecPluginViaClient(t *testing.T) { } } -func captureMetrics(t *testing.T) *execPluginMetrics { +func captureMetrics(t *testing.T) *wantMetrics { previousCallsMetric := metrics.ExecPluginCalls + previousPolicyCallsMetric := metrics.ExecPluginPolicyCalls t.Cleanup(func() { metrics.ExecPluginCalls = previousCallsMetric + metrics.ExecPluginPolicyCalls = previousPolicyCallsMetric }) - actualMetrics := &execPluginMetrics{} - metrics.ExecPluginCalls = actualMetrics + actualMetrics := &wantMetrics{} + + metrics.ExecPluginCalls = (*execPluginMetrics)(actualMetrics) + metrics.ExecPluginPolicyCalls = (*execPluginPolicyMetrics)(actualMetrics) return actualMetrics }