diff --git a/.rubocop.yml b/.rubocop.yml index 43597c55..7474171b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -25,3 +25,5 @@ Security/MarshalLoad: Style/MethodCallWithArgsParentheses: IgnoredMethods: - require_relative +Style/RegexpLiteral: + Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 924f6c0e..b81c4222 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,15 @@ Kubeclient release versioning follows [SemVer](https://semver.org/). ## Unreleased +### Added +- Support `user: exec: ...` credential plugins like in Go client (#363, #375). + ### Security -- Really made `Kubeclient::Config.new(data, nil)` prevent external file lookups. +- Really made `Kubeclient::Config.new(data, nil)` prevent external file lookups. (#372) README documented this since 3.1.1 (#334) but alas that was a lie — absolute paths always worked. + Now this also prevents credential plugin execution. + + Even in this mode, using config from untrusted sources is not recommended. ## 4.1.0 — 2018-11-28 diff --git a/README.md b/README.md index 9409500f..f304db92 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,11 @@ If you've been using `kubectl` and have a `.kube/config` file (possibly referenc config = Kubeclient::Config.read(ENV['KUBECONFIG'] || '/path/to/.kube/config') ``` +This will lookup external files; relative paths will be resolved relative to the file's directory, if config refers to them with relative path. +This includes external [`exec:` credential plugins][exec] to be executed. + +[exec]: https://kubernetes.io/docs/reference/access-authn-authz/authentication/#client-go-credential-plugins + You can also construct `Config` directly from nested data. For example if you have JSON or YAML config data in a variable: ```ruby @@ -277,7 +282,7 @@ config = Kubeclient::Config.new(JSON.parse(json_text), nil) ``` The 2nd argument is a base directory for finding external files, if config refers to them with relative path. -Setting it to `nil` disables file lookups. (A config can be self-contained by using inline fields such as `client-certificate-data`.) +Setting it to `nil` disables file lookups, and `exec:` execution - such configs will raise an exception. (A config can be self-contained by using inline fields such as `client-certificate-data`.) To create a client based on a Config object: @@ -297,7 +302,9 @@ Kubeclient::Client.new( #### Security: Don't use config from untrusted sources -Kubeclient was never reviewed for behaving safely with malicious / malformed config. +`Config.read` is catastrophically unsafe — it will execute arbitrary command lines specified by the config! + +`Config.new(data, nil)` is better but Kubeclient was never reviewed for behaving safely with malicious / malformed config. It might crash / misbehave in unexpected ways... #### namespace @@ -604,9 +611,9 @@ Checking the type of a resource can be done using: update_* delete_* and patch_* now return a RecursiveOpenStruct like the get_* methods -The gem raises Kubeclient::HttpError or subclasses now. Catching KubeException still works but is deprecated. +The `Kubeclient::Client` class raises `Kubeclient::HttpError` or subclasses now. Catching `KubeException` still works but is deprecated. -`Kubeclient::Config#context` raises KeyError instead of RuntimeError for non-existent context name. +`Kubeclient::Config#context` raises `KeyError` instead of `RuntimeError` for non-existent context name. diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index e3408101..ecd316b6 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -20,7 +20,7 @@ def initialize(api_endpoint, api_version, ssl_options, auth_options, namespace) # data (Hash) - Parsed kubeconfig data. # kcfg_path (string) - Base directory for resolving relative references to external files. - # If set to nil, all external lookups are disabled (even for absolute paths). + # If set to nil, all external lookups & commands are disabled (even for absolute paths). # See also the more convenient Config.read def initialize(data, kcfg_path) @kcfg = data @@ -81,6 +81,24 @@ def ext_file_path(path) Pathname(path).absolute? ? path : File.join(@kcfg_path, path) end + def ext_command_path(path) + unless allow_external_lookups? + raise "Kubeclient::Config: external lookups disabled, can't execute '#{path}'" + end + # Like go client https://github.com/kubernetes/kubernetes/pull/59495#discussion_r171138995, + # distinguish 3 cases: + # - absolute (e.g. /path/to/foo) + # - $PATH-based (e.g. curl) + # - relative to config file's dir (e.g. ./foo) + if Pathname(path).absolute? + path + elsif File.basename(path) == path + path + else + File.join(@kcfg_path, path) + end + end + def fetch_context(context_name) context = @kcfg['contexts'].detect do |x| break x['context'] if x['name'] == context_name @@ -132,7 +150,9 @@ def fetch_user_auth_options(user) if user.key?('token') options[:bearer_token] = user['token'] elsif user.key?('exec') - options[:bearer_token] = Kubeclient::ExecCredentials.token(user['exec']) + exec_opts = user['exec'].dup + exec_opts['command'] = ext_command_path(exec_opts['command']) if exec_opts['command'] + options[:bearer_token] = Kubeclient::ExecCredentials.token(exec_opts) else %w[username password].each do |attr| options[attr.to_sym] = user[attr] if user.key?(attr) diff --git a/test/config/execauth.kubeconfig b/test/config/execauth.kubeconfig index 2a0093d3..4076c705 100644 --- a/test/config/execauth.kubeconfig +++ b/test/config/execauth.kubeconfig @@ -8,13 +8,22 @@ contexts: - context: cluster: localhost:8443 namespace: default - user: system:admin:exec - name: localhost/system:admin:exec -current-context: localhost/system:admin:exec + user: system:admin:exec-search-path + name: localhost/system:admin:exec-search-path +- context: + cluster: localhost:8443 + namespace: default + user: system:admin:exec-relative-path + name: localhost/system:admin:exec-relative-path +- context: + cluster: localhost:8443 + namespace: default + user: system:admin:exec-absolute-path + name: localhost/system:admin:exec-absolute-path kind: Config preferences: {} users: -- name: system:admin:exec +- name: system:admin:exec-search-path user: exec: # Command to execute. Required. @@ -38,3 +47,16 @@ users: - "arg1" - "arg2" +- name: system:admin:exec-relative-path + user: + exec: + # Command to execute. Required. + command: "dir/example-exec-plugin" + apiVersion: "client.authentication.k8s.io/v1beta1" + +- name: system:admin:exec-absolute-path + user: + exec: + # Command to execute. Required. + command: "/abs/path/example-exec-plugin" + apiVersion: "client.authentication.k8s.io/v1beta1" diff --git a/test/test_config.rb b/test/test_config.rb index fe4e1762..f48c8d67 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -75,23 +75,51 @@ def test_timestamps end def test_user_exec - creds = JSON.dump( + token = '0123456789ABCDEF0123456789ABCDEF' + creds = { 'apiVersion': 'client.authentication.k8s.io/v1beta1', 'status': { - 'token': '0123456789ABCDEF0123456789ABCDEF' + 'token': token } - ) + } - st = Minitest::Mock.new - st.expect(:success?, true) + config = Kubeclient::Config.read(config_file('execauth.kubeconfig')) + assert_equal(['localhost/system:admin:exec-search-path', + 'localhost/system:admin:exec-relative-path', + 'localhost/system:admin:exec-absolute-path'], + config.contexts) + + # A bare command name in config means search PATH, so it's executed as bare command. + stub_exec(%r{^example-exec-plugin$}, creds) do + context = config.context('localhost/system:admin:exec-search-path') + check_context(context, ssl: false) + assert_equal(token, context.auth_options[:bearer_token]) + end - Open3.stub(:capture3, [creds, nil, st]) do - config = Kubeclient::Config.read(config_file('execauth.kubeconfig')) - assert_equal(['localhost/system:admin:exec'], config.contexts) - context = config.context('localhost/system:admin:exec') + # A relative path is taken relative to the dir of the kubeconfig. + stub_exec(%r{.*config/dir/example-exec-plugin$}, creds) do + context = config.context('localhost/system:admin:exec-relative-path') check_context(context, ssl: false) + assert_equal(token, context.auth_options[:bearer_token]) + end + + # An absolute path is taken as-is. + stub_exec(%r{^/abs/path/example-exec-plugin$}, creds) do + context = config.context('localhost/system:admin:exec-absolute-path') + check_context(context, ssl: false) + assert_equal(token, context.auth_options[:bearer_token]) + end + end - assert_equal('0123456789ABCDEF0123456789ABCDEF', context.auth_options[:bearer_token]) + def test_user_exec_nopath + yaml = File.read(config_file('execauth.kubeconfig')) + config = Kubeclient::Config.new(YAML.safe_load(yaml), nil) + config.contexts.each do |context_name| + Open3.stub(:capture3, proc { flunk 'should not execute command' }) do + assert_raises(StandardError) do + config.context(context_name) + end + end end end @@ -126,4 +154,18 @@ def check_context(context, ssl: true) def config_file(name) File.join(File.dirname(__FILE__), 'config', name) end + + def stub_exec(command_regexp, creds) + st = Minitest::Mock.new + st.expect(:success?, true) + + capture3_stub = lambda do |_env, command, *_args| + assert_match command_regexp, command + [JSON.dump(creds), nil, st] + end + + Open3.stub(:capture3, capture3_stub) do + yield + end + end end