Merge branch 'v4.y-security-config-ssl_verify' into security-config-s…

…sl_verify

.github/workflows/actions.yml

@@ -10,8 +10,8 @@ on:
     - '**'
 jobs:
   build:
+    continue-on-error: true
     runs-on: ${{ matrix.os_and_command.os }}
-    continue-on-error: ${{ contains(matrix.ruby, '-head') }}
     strategy:
       matrix:
         ruby: [ '2.5', '2.6', '2.7', '3.0', '3.1', 'ruby-head', 'truffleruby-head' ]

CHANGELOG.md

@@ -4,10 +4,54 @@ Notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 Kubeclient release versioning follows [SemVer](https://semver.org/).
 
-## Unreleased
+## Unreleased — to become 5.y.z
+
 ### Changed
 - `Kubeclient::Client.new` now always requires an api version, use for example: `Kubeclient::Client.new(uri, 'v1')`
 
+## 4.9.3 — 2021-03-23
+
+### Fixed
+
+- VULNERABILITY FIX: Previously, whenever kubeconfig did not define custom CA
+  (normal situation for production clusters with public domain and certificate!),
+  `Config` was returning ssl_options[:verify_ssl] hard-coded to `VERIFY_NONE` :-(
+
+  Assuming you passed those ssl_options to Kubeclient::Client, this means that
+  instead of checking server's certificate against your system CA store,
+  it would accept ANY certificate, allowing easy man-in-the middle attacks.
+
+  This is especially dangerous with user/password or token credentials
+  because MITM attacker could simply steal those credentials to the cluster
+  and do anything you could do on the cluster.
+
+  This was broken IN ALL RELEASES MADE BEFORE 2022, ever since
+  [`Kubeclient::Config` was created](https://github.com/ManageIQ/kubeclient/pull/127/files#diff-32e70f2f6781a9e9c7b83ae5e7eaf5ffd068a05649077fa38f6789e72f3de837R41-R48).
+
+  [#554](https://github.com/ManageIQ/kubeclient/issues/554).
+
+- Bug fix: kubeconfig `insecure-skip-tls-verify` field was ignored.
+  When kubeconfig did define custom CA, `Config` was returning hard-coded `VERIFY_PEER`.
+
+  Now we honor it, return `VERIFY_NONE` iff kubeconfig has explicit
+  `insecure-skip-tls-verify: true`, otherwise `VERIFY_PEER`.
+
+  [#555](https://github.com/ManageIQ/kubeclient/issues/555).
+
+- `Config`: fixed parsing of `certificate-authority` file containing concatenation of
+  several certificates.  Previously, server's cert was checked against only first CA cert,
+  resulting in possible "certificate verify failed" errors.
+
+  An important use case is a chain of root & intermediate cert(s) - necessary when cluster's CA
+  itself is signed by another custom CA.
+  But also helps when you simply concatenate independent certs. (#461, #552)
+
+  - Still broken (#460): inline `certificate-authority-data` is still parsed using `add_cert`
+    method that handles only one cert.
+
+These don't affect code that supplies `Client` parameters directly,
+only code that uses `Config`.
+
 ## 4.9.2 — 2021-05-30
 
 ### Added

README.md

@@ -10,6 +10,12 @@ The client supports GET, POST, PUT, DELETE on all the entities available in kube
 The client currently supports Kubernetes REST api version v1.
 To learn more about groups and versions in kubernetes refer to [k8s docs](https://kubernetes.io/docs/api/)
 
+## VULNERABILITY❗
+
+If you use `Kubeclient::Config`, all gem versions released before 2022 could return incorrect `ssl_options[:verify_ssl]`,
+endangering your connection and cluster credentials.
+See https://github.com/ManageIQ/kubeclient/issues/554 for details and which versions got a fix.
+
 ## Installation
 
 Add this line to your application's Gemfile:

lib/kubeclient/config.rb

@@ -63,13 +63,16 @@ def context(context_name = nil)
 
       ssl_options = {}
 
+      ssl_options[:verify_ssl] = if cluster['insecure-skip-tls-verify'] == true
+                                   OpenSSL::SSL::VERIFY_NONE
+                                 else
+                                   OpenSSL::SSL::VERIFY_PEER
+                                 end
+
       if cluster_ca_data?(cluster)
         cert_store = OpenSSL::X509::Store.new
         populate_cert_store_from_cluster_ca_data(cluster, cert_store)
-        ssl_options[:verify_ssl] = OpenSSL::SSL::VERIFY_PEER
         ssl_options[:cert_store] = cert_store
-      else
-        ssl_options[:verify_ssl] = OpenSSL::SSL::VERIFY_NONE
       end
 
       unless client_cert_data.nil?

test/config/another-ca1.pem

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDADCCAeigAwIBAgIUQZjM/5qoAF78qIDyc+rKi4qBdOIwDQYJKoZIhvcNAQEL
+BQAwGDEWMBQGA1UEAxMNa3ViZXJuZXRlcy1jYTAeFw0yMjAzMjIxNDQzMDBaFw0z
+MjAzMTkxNDQzMDBaMBgxFjAUBgNVBAMTDWt1YmVybmV0ZXMtY2EwggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDGkG7g+UjpDhZ7A4Pm7Hme+RWs5IHz4I2X
+IclvtO3LuJ26yzz2S8VaXFFeUqzEPb2G1RxFGvoAVN7qrTw0n5MQJCFLAA4dI7oY
+8XLRJ7KgTBBIw1jYpgKb2zyHPIJE6VmslliKUiX+QDovdRU/dsbdup2EucrnGw4+
+QNNAc3XMbXgm6lubA6znYZlSpcQ8BKer3tq75q4KUZicIjS6gKQyZjk9a6fcOuCS
+ybtlAKp9lYzcwxZkNrx+V1PJMQ1qaJWPnMAVi7Oj5Dm3Jmf1WHBcNEh52Q/0vYlt
+4WSaeM5t/Py/m/7c4Ve97f5m2X6EhYyUbzov4qeZOnIJI3MnU1FxAgMBAAGjQjBA
+MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBSl1qyt
+jd96WstRE8h9x5qkCvZUvjANBgkqhkiG9w0BAQsFAAOCAQEAJt55qYvBaniAwvgO
+tbO79g1FcQGrxpMX45TuoCE/K+MWDjrr6bp+FbLOqT8MwOsbGwwJIRTHGvkEkVso
+5AWI5aSNs3hWnltOdz27ZSHeX77WB4daK1tLK6ggZrp3v9iIpbBwWBFdmAqsPvEs
+H17K2BgAzdh6xRKPQd0BGTUpJBfk50R2gDMj7FKyIzBN69IOGytBfAXBhHzEGy4+
+MvtTEIMUjR//KgCrpNeyDuaWHttR5FdnuRxFO7O3BAfyNSaNmd/IEHQf7DIGgzOy
++xWLyH/HRHj5C70qAqjbnrgBODI99BsA9U7oXTuyPLdIboAcFt2zD5DIYgZET52X
+53w4jA==
+-----END CERTIFICATE-----

test/config/another-ca2.pem

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDADCCAeigAwIBAgIUHW3OPnmuTquJ0YgbGpmm/blsY2QwDQYJKoZIhvcNAQEL
+BQAwGDEWMBQGA1UEAxMNa3ViZXJuZXRlcy1jYTAeFw0yMjAzMjIxNDQ0MDBaFw0z
+MjAzMTkxNDQ0MDBaMBgxFjAUBgNVBAMTDWt1YmVybmV0ZXMtY2EwggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDLMEJs5agS0hNQBxPTtsI6dIhIi/pY8liI
+sNukbi5KwKf80FYNyRXqE8ufDVyTFzOc+MG96jnHjDaBWjrVN9On0PgUBo4nPyd4
+DtyvYx2jMzwToSEIo/Z1aroMx1oGywCgdS4/3FWAbhlSbyXKJmhfh6gX0TxWz+dV
+zqNuqQq9EWuRhOMg9vgzjfp3mjiPE10lW8pT0j5JT3PI/eGO+C2Z7z33LJXb6GM2
+nXvhGFMGY+7XG65pqJ3L8g1mk+LjPiwyIItw8wPtrnrZ2VXMklMd5Mn+jgCTNe1B
+om0nPpPIiTblCr6gcNcVjy5WGN37OKlqrT0JTuSPHcxSUp05LFjDAgMBAAGjQjBA
+MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBQvV/sB
+wbR3UwjkLAMN+6P3fZ/3OjANBgkqhkiG9w0BAQsFAAOCAQEACAk4EQwCkw2EBsSR
+2SKoa1SjYFkZzIr/0/TB2YcMUvHF+RpvlD5vQ8/RJjeAl1kc6/niZ9TWCemjBLqI
+hPoFe49zr49DyQjC2ZfsXVJvFCr6g7o4q4DtQ6ltyBuTJbkn1hI+aB8zgvpofG44
+mKj18Y7tPvgXtRua4SaeBq777+22AOvKxPied9p4PTrMN4RKTP6+yIbLflej7dBD
+zQDjfmmYsH0T2ZRtBpE1dYrUbU3tkizcMZRJBgreoxoff+r5coibMIm/7gh+YoSb
+BCItCaeuGSKQ8CJb8DElcPUd6nKUjmeiQL68ztsG/+CXLiL/TZb914VaaCXvPInw
+49jJ7w==
+-----END CERTIFICATE-----

test/config/concatenated-ca.kubeconfig

@@ -0,0 +1,20 @@
+apiVersion: v1
+clusters:
+- cluster:
+    certificate-authority: concatenated-ca.pem
+    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/concatenated-ca.pem

@@ -0,0 +1,57 @@
+-----BEGIN CERTIFICATE-----
+MIIDADCCAeigAwIBAgIUQZjM/5qoAF78qIDyc+rKi4qBdOIwDQYJKoZIhvcNAQEL
+BQAwGDEWMBQGA1UEAxMNa3ViZXJuZXRlcy1jYTAeFw0yMjAzMjIxNDQzMDBaFw0z
+MjAzMTkxNDQzMDBaMBgxFjAUBgNVBAMTDWt1YmVybmV0ZXMtY2EwggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDGkG7g+UjpDhZ7A4Pm7Hme+RWs5IHz4I2X
+IclvtO3LuJ26yzz2S8VaXFFeUqzEPb2G1RxFGvoAVN7qrTw0n5MQJCFLAA4dI7oY
+8XLRJ7KgTBBIw1jYpgKb2zyHPIJE6VmslliKUiX+QDovdRU/dsbdup2EucrnGw4+
+QNNAc3XMbXgm6lubA6znYZlSpcQ8BKer3tq75q4KUZicIjS6gKQyZjk9a6fcOuCS
+ybtlAKp9lYzcwxZkNrx+V1PJMQ1qaJWPnMAVi7Oj5Dm3Jmf1WHBcNEh52Q/0vYlt
+4WSaeM5t/Py/m/7c4Ve97f5m2X6EhYyUbzov4qeZOnIJI3MnU1FxAgMBAAGjQjBA
+MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBSl1qyt
+jd96WstRE8h9x5qkCvZUvjANBgkqhkiG9w0BAQsFAAOCAQEAJt55qYvBaniAwvgO
+tbO79g1FcQGrxpMX45TuoCE/K+MWDjrr6bp+FbLOqT8MwOsbGwwJIRTHGvkEkVso
+5AWI5aSNs3hWnltOdz27ZSHeX77WB4daK1tLK6ggZrp3v9iIpbBwWBFdmAqsPvEs
+H17K2BgAzdh6xRKPQd0BGTUpJBfk50R2gDMj7FKyIzBN69IOGytBfAXBhHzEGy4+
+MvtTEIMUjR//KgCrpNeyDuaWHttR5FdnuRxFO7O3BAfyNSaNmd/IEHQf7DIGgzOy
++xWLyH/HRHj5C70qAqjbnrgBODI99BsA9U7oXTuyPLdIboAcFt2zD5DIYgZET52X
+53w4jA==
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIC/zCCAeegAwIBAgITPbfpy29aBG67ChRdB6lJegTkmDANBgkqhkiG9w0BAQsF
+ADAYMRYwFAYDVQQDEw1rdWJlcm5ldGVzLWNhMB4XDTIyMDIyMTA5MDIwMFoXDTMy
+MDIxOTA5MDIwMFowGDEWMBQGA1UEAxMNa3ViZXJuZXRlcy1jYTCCASIwDQYJKoZI
+hvcNAQEBBQADggEPADCCAQoCggEBAMLjZ2cFBalzoimaEcpT9Jz2JmdgsHMOagVd
+It7OQpTwDZ3npIICVpguEh9xtovR8m8/HYM+/a4vMQHT+3p8HPjiDaRYGg7OZ9L+
+Fp/9zhBuiaIvg8Z+Bbys9Q9Uuj6VEwfFJBcNH6TmzdiDgQUs5/k+6/vtuJ4ys3sD
+KkAOxqPXDaBoANnLpIxdIMQDcWSLFA0wmFhdZJq3KEAoJpEL0WYo1ZRBV3iH77yf
+sDbN1OBu2vNnRZ+DrV0ZJ5ApmbFXPX8i4KJaW9lCB62FN0j5XsNDoyTeAVpesfNs
+zYufVpBdqNZFkOKg9diMuTMika2aYfDuiVzdebDgcp9aMloKtbECAwEAAaNCMEAw
+DgYDVR0PAQH/BAQDAgEGMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFBdOiygC
+LcuJrq8rNa1xADr5Sp7CMA0GCSqGSIb3DQEBCwUAA4IBAQDCy4IlhASh6Br5XEcI
+TpP5ThD1OyRzQnsPe6P1qgWP3kBXK/AcsSl+VGtaZp2oEhJoUnsz7kE8yW3gK+PA
+51zY4aHTiF9xkyd5zOCAGB+cfp9Ys+szWzyu0QQ9IBjJ4+eDjg7W0/S+BM2Qn1iL
+jTFIe2Bdf+Q/J24/q3ksTXK17UNun14vDRsJgsNcrFt/rumfHPx1ytwsiqKyEKV7
+kFxSwa3d8/AvhGgFpPmfRjU7gAJCFcHz501zhi2a6L5TYBTecVRbqZoeHiZ0YNWI
+is5g4VmVB+BxMAM2WEd29v4l/3oI1Pey9rvt7NJqSe1im9uqZgVDeg/vP8zKs/dF
+ZYw8
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIDADCCAeigAwIBAgIUHW3OPnmuTquJ0YgbGpmm/blsY2QwDQYJKoZIhvcNAQEL
+BQAwGDEWMBQGA1UEAxMNa3ViZXJuZXRlcy1jYTAeFw0yMjAzMjIxNDQ0MDBaFw0z
+MjAzMTkxNDQ0MDBaMBgxFjAUBgNVBAMTDWt1YmVybmV0ZXMtY2EwggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQDLMEJs5agS0hNQBxPTtsI6dIhIi/pY8liI
+sNukbi5KwKf80FYNyRXqE8ufDVyTFzOc+MG96jnHjDaBWjrVN9On0PgUBo4nPyd4
+DtyvYx2jMzwToSEIo/Z1aroMx1oGywCgdS4/3FWAbhlSbyXKJmhfh6gX0TxWz+dV
+zqNuqQq9EWuRhOMg9vgzjfp3mjiPE10lW8pT0j5JT3PI/eGO+C2Z7z33LJXb6GM2
+nXvhGFMGY+7XG65pqJ3L8g1mk+LjPiwyIItw8wPtrnrZ2VXMklMd5Mn+jgCTNe1B
+om0nPpPIiTblCr6gcNcVjy5WGN37OKlqrT0JTuSPHcxSUp05LFjDAgMBAAGjQjBA
+MA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBQvV/sB
+wbR3UwjkLAMN+6P3fZ/3OjANBgkqhkiG9w0BAQsFAAOCAQEACAk4EQwCkw2EBsSR
+2SKoa1SjYFkZzIr/0/TB2YcMUvHF+RpvlD5vQ8/RJjeAl1kc6/niZ9TWCemjBLqI
+hPoFe49zr49DyQjC2ZfsXVJvFCr6g7o4q4DtQ6ltyBuTJbkn1hI+aB8zgvpofG44
+mKj18Y7tPvgXtRua4SaeBq777+22AOvKxPied9p4PTrMN4RKTP6+yIbLflej7dBD
+zQDjfmmYsH0T2ZRtBpE1dYrUbU3tkizcMZRJBgreoxoff+r5coibMIm/7gh+YoSb
+BCItCaeuGSKQ8CJb8DElcPUd6nKUjmeiQL68ztsG/+CXLiL/TZb914VaaCXvPInw
+49jJ7w==
+-----END CERTIFICATE-----

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/update_certs_k0s.rb

@@ -33,6 +33,8 @@ def sh!(*cmd)
 # The rest could easily be extracted from allinone.kubeconfig, but the test is more robust
 # if we don't reuse YAML and/or Kubeclient::Config parsing to construct test data.
 sh! "#{DOCKER} exec #{CONTAINER} cat /var/lib/k0s/pki/ca.crt     > test/config/external-ca.pem"
+sh! 'cat test/config/another-ca1.pem test/config/external-ca.pem '\
+    '    test/config/another-ca2.pem > test/config/concatenated-ca.pem'
 sh! "#{DOCKER} exec #{CONTAINER} cat /var/lib/k0s/pki/admin.crt  > test/config/external-cert.pem"
 sh! "#{DOCKER} exec #{CONTAINER} cat /var/lib/k0s/pki/admin.key  > test/config/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: