SECURITY: unit tests for Kubeclient::Config handling of ssl_options[:…

…verify_ssl]

- Removed `insecure-skip-tls-verify: true` from most test configs
  (that was one of the reasons the bug went unnoticed, VERIFY_NONE
  was what the unit tests expected.)

- Added new kubeconfig files + `Config` unit tests covering:
  - custom CA, omitted `insecure-skip-tls-verify`
  - custom CA, `insecure-skip-tls-verify: false`
  - custom CA, `insecure-skip-tls-verify: true`
  - no custom CA, omitted `insecure-skip-tls-verify`
  - no custom CA, `insecure-skip-tls-verify: false`
  - no custom CA, `insecure-skip-tls-verify: true`

test/config/execauth.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:6443
-    insecure-skip-tls-verify: true
   name: localhost:6443
 contexts:
 - context:

test/config/external-without-ca.kubeconfig

@@ -0,0 +1,21 @@
+apiVersion: v1
+clusters:
+- cluster:
+    # Not defining custom `certificate-authority`.
+    # Without it, the localhost cert should be rejected.
+    server: https://localhost:6443
+  name: local
+contexts:
+- context:
+    cluster: local
+    namespace: default
+    user: user
+  name: Default
+current-context: Default
+kind: Config
+preferences: {}
+users:
+- name: user
+  user:
+    client-certificate: external-cert.pem
+    client-key: external-key.rsa

test/config/gcpauth.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:8443
-    insecure-skip-tls-verify: true
   name: localhost:8443
 contexts:
 - context:

test/config/gcpcmdauth.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:8443
-    insecure-skip-tls-verify: true
   name: localhost:8443
 contexts:
 - context:

test/config/insecure-custom-ca.kubeconfig

@@ -0,0 +1,22 @@
+apiVersion: v1
+clusters:
+- cluster:
+    # This is a silly configuration, skip-tls-verify makes CA data useless, but testing for completeness.
+    certificate-authority: external-ca.pem
+    server: https://localhost:6443
+    insecure-skip-tls-verify: true
+  name: local
+contexts:
+- context:
+    cluster: local
+    namespace: default
+    user: user
+  name: Default
+current-context: Default
+kind: Config
+preferences: {}
+users:
+- name: user
+  user:
+    client-certificate: external-cert.pem
+    client-key: external-key.rsa

test/config/insecure.kubeconfig

@@ -0,0 +1,25 @@
+apiVersion: v1
+clusters:
+- cluster:
+    server: https://localhost:6443
+    insecure-skip-tls-verify: true
+  name: local
+contexts:
+- context:
+    cluster: local
+    namespace: default
+    user: user
+  name: Default
+current-context: Default
+kind: Config
+preferences: {}
+users:
+- name: user
+  user:
+    # Providing ANY credentials in `insecure-skip-tls-verify` mode is unwise due to MITM risk.
+    # At least client certs are not as catastrophic as bearer tokens.
+    #
+    # This combination of insecure + client certs was once broken in kubernetes but
+    # is meaningful since 2015 (https://github.com/kubernetes/kubernetes/pull/15430).
+    client-certificate: external-cert.pem
+    client-key: external-key.rsa

test/config/nouser.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:6443
-    insecure-skip-tls-verify: true
   name: localhost:6443
 contexts:
 - context:

test/config/oidcauth.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:8443
-    insecure-skip-tls-verify: true
   name: localhost:8443
 contexts:
 - context:

test/config/secure-without-ca.kubeconfig

@@ -0,0 +1,22 @@
+apiVersion: v1
+clusters:
+- cluster:
+    # Not defining custom `certificate-authority`.
+    # Without it, the localhost cert should be rejected.
+    server: https://localhost:6443
+    insecure-skip-tls-verify: false  # Same as external-without-ca.kubeconfig but with explicit false here.
+  name: local
+contexts:
+- context:
+    cluster: local
+    namespace: default
+    user: user
+  name: Default
+current-context: Default
+kind: Config
+preferences: {}
+users:
+- name: user
+  user:
+    client-certificate: external-cert.pem
+    client-key: external-key.rsa

test/config/secure.kubeconfig

@@ -0,0 +1,21 @@
+apiVersion: v1
+clusters:
+- cluster:
+    certificate-authority: external-ca.pem
+    server: https://localhost:6443
+    insecure-skip-tls-verify: false  # Same as external.kubeconfig but with explicit false here.
+  name: local
+contexts:
+- context:
+    cluster: local
+    namespace: default
+    user: user
+  name: Default
+current-context: Default
+kind: Config
+preferences: {}
+users:
+- name: user
+  user:
+    client-certificate: external-cert.pem
+    client-key: external-key.rsa

test/config/userauth.kubeconfig

@@ -2,7 +2,6 @@ apiVersion: v1
 clusters:
 - cluster:
     server: https://localhost:6443
-    insecure-skip-tls-verify: true
   name: localhost:6443
 contexts:
 - context:

test/test_config.rb

@@ -7,21 +7,56 @@ class KubeclientConfigTest < MiniTest::Test
   def test_allinone
     config = Kubeclient::Config.read(config_file('allinone.kubeconfig'))
     assert_equal(['Default'], config.contexts)
-    check_context(config.context, ssl: true)
+    check_context(config.context, ssl: true, custom_ca: true, client_cert: true)
   end
 
   def test_external
     config = Kubeclient::Config.read(config_file('external.kubeconfig'))
     assert_equal(['Default'], config.contexts)
-    check_context(config.context, ssl: true)
+    check_context(config.context, ssl: true, custom_ca: true, client_cert: true)
+  end
+
+  def test_explicit_secure
+    config = Kubeclient::Config.read(config_file('secure.kubeconfig'))
+    assert_equal(['Default'], config.contexts)
+    # Same as external.kubeconfig, but with explicit `insecure-skip-tls-verify: false`
+    check_context(config.context, ssl: true, custom_ca: true, client_cert: true)
+  end
+
+  def test_external_public_ca
+    config = Kubeclient::Config.read(config_file('external-without-ca.kubeconfig'))
+    assert_equal(['Default'], config.contexts)
+    # Same as external.kubeconfig, no custom CA data (cluster has a publicly trusted cert)
+    check_context(config.context, ssl: true, custom_ca: false, client_cert: true)
+  end
+
+  def test_secure_public_ca
+    config = Kubeclient::Config.read(config_file('secure-without-ca.kubeconfig'))
+    assert_equal(['Default'], config.contexts)
+    # no custom CA data + explicit `insecure-skip-tls-verify: false`
+    check_context(config.context, ssl: true, custom_ca: false, client_cert: true)
+  end
+
+  def test_insecure
+    config = Kubeclient::Config.read(config_file('insecure.kubeconfig'))
+    assert_equal(['Default'], config.contexts)
+    # Has explicit `insecure-skip-tls-verify: false`
+    check_context(config.context, ssl: false, custom_ca: false, client_cert: true)
+  end
+
+  def test_insecure_custom_ca
+    config = Kubeclient::Config.read(config_file('insecure-custom-ca.kubeconfig'))
+    assert_equal(['Default'], config.contexts)
+    # Has explicit `insecure-skip-tls-verify: false`
+    check_context(config.context, ssl: false, custom_ca: true, client_cert: true)
   end
 
   def test_allinone_nopath
     yaml = File.read(config_file('allinone.kubeconfig'))
     # A self-contained config shouldn't depend on kcfg_path.
     config = Kubeclient::Config.new(YAML.safe_load(yaml), nil)
     assert_equal(['Default'], config.contexts)
-    check_context(config.context, ssl: true)
+    check_context(config.context, ssl: true, custom_ca: true, client_cert: true)
   end
 
   def test_external_nopath
@@ -53,15 +88,15 @@ def test_concatenated_ca
   def test_nouser
     config = Kubeclient::Config.read(config_file('nouser.kubeconfig'))
     assert_equal(['default/localhost:6443/nouser'], config.contexts)
-    check_context(config.context, ssl: false)
+    check_context(config.context, ssl: true, custom_ca: false, client_cert: false)
   end
 
   def test_user_token
     config = Kubeclient::Config.read(config_file('userauth.kubeconfig'))
     assert_equal(['localhost/system:admin:token', 'localhost/system:admin:userpass'],
                  config.contexts)
     context = config.context('localhost/system:admin:token')
-    check_context(context, ssl: false)
+    check_context(context, ssl: true, custom_ca: false, client_cert: false)
     assert_equal('0123456789ABCDEF0123456789ABCDEF', context.auth_options[:bearer_token])
   end
 
@@ -70,7 +105,7 @@ def test_user_password
     assert_equal(['localhost/system:admin:token', 'localhost/system:admin:userpass'],
                  config.contexts)
     context = config.context('localhost/system:admin:userpass')
-    check_context(context, ssl: false)
+    check_context(context, ssl: true, custom_ca: false, client_cert: false)
     assert_equal('admin', context.auth_options[:username])
     assert_equal('pAssw0rd123', context.auth_options[:password])
   end
@@ -98,21 +133,21 @@ def test_user_exec
     # 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)
+      check_context(context, ssl: true, custom_ca: false, client_cert: false)
       assert_equal(token, context.auth_options[:bearer_token])
     end
 
     # 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)
+      check_context(context, ssl: true, custom_ca: false, client_cert: 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)
+      check_context(context, ssl: true, custom_ca: false, client_cert: false)
       assert_equal(token, context.auth_options[:bearer_token])
     end
   end
@@ -182,20 +217,33 @@ def test_oidc_auth_provider
 
   private
 
-  def check_context(context, ssl: true)
+  def check_context(context, ssl: true, custom_ca: true, client_cert: true)
     assert_equal('https://localhost:6443', context.api_endpoint)
     assert_equal('v1', context.api_version)
     assert_equal('default', context.namespace)
-    if ssl
-      assert_equal(OpenSSL::SSL::VERIFY_PEER, context.ssl_options[:verify_ssl])
+    if custom_ca
       assert_kind_of(OpenSSL::X509::Store, context.ssl_options[:cert_store])
+    else
+      assert_nil(context.ssl_options[:cert_store])
+    end
+    if client_cert
       assert_kind_of(OpenSSL::X509::Certificate, context.ssl_options[:client_cert])
       assert_kind_of(OpenSSL::PKey::RSA, context.ssl_options[:client_key])
-      # When certificates expire one way to recreate them is using a k0s single-node cluster:
-      #     test/config/update_certs_k0s.rb
-      assert(context.ssl_options[:cert_store].verify(context.ssl_options[:client_cert]))
+      if custom_ca
+        # When certificates expire one way to recreate them is using a k0s single-node cluster:
+        #     test/config/update_certs_k0s.rb
+        assert(context.ssl_options[:cert_store].verify(context.ssl_options[:client_cert]))
+      end
+    else
+      assert_nil(context.ssl_options[:client_cert])
+      assert_nil(context.ssl_options[:client_key])
+    end
+    if ssl
+      assert_equal(OpenSSL::SSL::VERIFY_PEER, context.ssl_options[:verify_ssl],
+                   'expected VERIFY_PEER')
     else
-      assert_equal(OpenSSL::SSL::VERIFY_NONE, context.ssl_options[:verify_ssl])
+      assert_equal(OpenSSL::SSL::VERIFY_NONE, context.ssl_options[:verify_ssl],
+                   'expected VERIFY_NONE')
     end
   end