Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Credential plugins: lookup command relative to config's dir, nil = opt-out #375

Merged
merged 2 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ Security/MarshalLoad:
Style/MethodCallWithArgsParentheses:
IgnoredMethods:
- require_relative
Style/RegexpLiteral:
Enabled: false
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 11 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:

Expand All @@ -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
Expand Down Expand Up @@ -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.

<a name="past_version_1.2.0">

Expand Down
24 changes: 22 additions & 2 deletions lib/kubeclient/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
30 changes: 26 additions & 4 deletions test/config/execauth.kubeconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"
62 changes: 52 additions & 10 deletions test/test_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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