diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index 7deb752a..5a74326e 100644 --- a/.github/workflows/actions.yml +++ b/.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' ] diff --git a/CHANGELOG.md b/CHANGELOG.md index 2083eb48..b68dddd8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index e8b115c4..8e88aca0 100644 --- a/README.md +++ b/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: diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index 68b91739..2493faa3 100644 --- a/lib/kubeclient/config.rb +++ b/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? diff --git a/lib/kubeclient/version.rb b/lib/kubeclient/version.rb index 1dd4ac8c..80951488 100644 --- a/lib/kubeclient/version.rb +++ b/lib/kubeclient/version.rb @@ -2,5 +2,5 @@ # Kubernetes REST-API Client module Kubeclient - VERSION = '4.9.2' + VERSION = '4.9.3' end diff --git a/test/config/another-ca1.pem b/test/config/another-ca1.pem new file mode 100644 index 00000000..50825d47 --- /dev/null +++ b/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----- diff --git a/test/config/another-ca2.pem b/test/config/another-ca2.pem new file mode 100644 index 00000000..53be72e8 --- /dev/null +++ b/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----- diff --git a/test/config/concatenated-ca.kubeconfig b/test/config/concatenated-ca.kubeconfig new file mode 100644 index 00000000..ed20e4dd --- /dev/null +++ b/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 diff --git a/test/config/concatenated-ca.pem b/test/config/concatenated-ca.pem new file mode 100644 index 00000000..1117298f --- /dev/null +++ b/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----- diff --git a/test/config/execauth.kubeconfig b/test/config/execauth.kubeconfig index d6f654e9..c9a9773f 100644 --- a/test/config/execauth.kubeconfig +++ b/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: diff --git a/test/config/external-without-ca.kubeconfig b/test/config/external-without-ca.kubeconfig new file mode 100644 index 00000000..1f3d617a --- /dev/null +++ b/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 diff --git a/test/config/gcpauth.kubeconfig b/test/config/gcpauth.kubeconfig index e96af3db..0ee387eb 100644 --- a/test/config/gcpauth.kubeconfig +++ b/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: diff --git a/test/config/gcpcmdauth.kubeconfig b/test/config/gcpcmdauth.kubeconfig index 0f54ed35..2e2db395 100644 --- a/test/config/gcpcmdauth.kubeconfig +++ b/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: diff --git a/test/config/insecure-custom-ca.kubeconfig b/test/config/insecure-custom-ca.kubeconfig new file mode 100644 index 00000000..06c803c1 --- /dev/null +++ b/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 diff --git a/test/config/insecure.kubeconfig b/test/config/insecure.kubeconfig new file mode 100644 index 00000000..d7c28087 --- /dev/null +++ b/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 diff --git a/test/config/nouser.kubeconfig b/test/config/nouser.kubeconfig index 31f8240b..9289895c 100644 --- a/test/config/nouser.kubeconfig +++ b/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: diff --git a/test/config/oidcauth.kubeconfig b/test/config/oidcauth.kubeconfig index 4cc73767..e1f389da 100644 --- a/test/config/oidcauth.kubeconfig +++ b/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: diff --git a/test/config/secure-without-ca.kubeconfig b/test/config/secure-without-ca.kubeconfig new file mode 100644 index 00000000..1b1acefe --- /dev/null +++ b/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 diff --git a/test/config/secure.kubeconfig b/test/config/secure.kubeconfig new file mode 100644 index 00000000..b0a00bb8 --- /dev/null +++ b/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 diff --git a/test/config/update_certs_k0s.rb b/test/config/update_certs_k0s.rb index 84059bf4..2632d726 100755 --- a/test/config/update_certs_k0s.rb +++ b/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" diff --git a/test/config/userauth.kubeconfig b/test/config/userauth.kubeconfig index 63577de1..604e3bda 100644 --- a/test/config/userauth.kubeconfig +++ b/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: diff --git a/test/test_config.rb b/test/test_config.rb index 844e7849..49ed8263 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -11,13 +11,48 @@ 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 @@ -25,7 +60,7 @@ def test_allinone_nopath # 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 @@ -48,10 +83,16 @@ def test_external_nopath_absolute end end + def test_concatenated_ca + config = Kubeclient::Config.read(config_file('concatenated-ca.kubeconfig')) + assert_equal(['Default'], config.contexts) + check_context(config.context, ssl: true) + end + 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 @@ -61,7 +102,7 @@ def test_user_token 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 @@ -72,7 +113,7 @@ def test_user_password 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 @@ -104,21 +145,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 @@ -194,20 +235,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 diff --git a/test/test_real_cluster.rb b/test/test_real_cluster.rb index 9d0e76e2..031e6611 100644 --- a/test/test_real_cluster.rb +++ b/test/test_real_cluster.rb @@ -16,65 +16,145 @@ def teardown WebMock.disable_net_connect! # Don't allow any connections in other tests. end + # Partially isolated tests that check Client behavior with given `verify_ssl` value: + + # localhost and 127.0.0.1 are among names on the certificate + HOSTNAME_COVERED_BY_CERT = 'https://127.0.0.1:6443'.freeze + # 127.0.0.2 also means localhost but is not included in the certificate. + HOSTNAME_NOT_ON_CERT = 'https://127.0.0.2:6443'.freeze + def test_real_cluster_verify_peer config = Kubeclient::Config.read(config_file('external.kubeconfig')) context = config.context - # localhost and 127.0.0.1 are among names on the certificate client1 = Kubeclient::Client.new( - 'https://127.0.0.1:6443', 'v1', + HOSTNAME_COVERED_BY_CERT, 'v1', ssl_options: context.ssl_options.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), auth_options: context.auth_options ) - client1.discover - client1.get_nodes - exercise_watcher_with_timeout(client1.watch_nodes) - # 127.0.0.2 also means localhost but is not included in the certificate. + check_cert_accepted(client1) client2 = Kubeclient::Client.new( - 'https://127.0.0.2:6443', 'v1', + HOSTNAME_NOT_ON_CERT, 'v1', ssl_options: context.ssl_options.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), auth_options: context.auth_options ) - # TODO: all OpenSSL exceptions should be wrapped with Kubeclient error. - assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do - client2.discover - end - # Since discovery fails, methods like .get_nodes, .watch_nodes would all fail - # on method_missing -> discover. Call lower-level methods to test actual connection. - assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do - client2.get_entities('Node', 'nodes', {}) - end - assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do - exercise_watcher_with_timeout(client2.watch_entities('nodes')) - end + check_cert_rejected(client2) end def test_real_cluster_verify_none config = Kubeclient::Config.read(config_file('external.kubeconfig')) context = config.context - # localhost and 127.0.0.1 are among names on the certificate client1 = Kubeclient::Client.new( - 'https://127.0.0.1:6443', 'v1', + HOSTNAME_COVERED_BY_CERT, 'v1', ssl_options: context.ssl_options.merge(verify_ssl: OpenSSL::SSL::VERIFY_NONE), auth_options: context.auth_options ) - client1.get_nodes - # 127.0.0.2 also means localhost but is not included in the certificate. + check_cert_accepted(client1) client2 = Kubeclient::Client.new( - 'https://127.0.0.2:6443', 'v1', + HOSTNAME_NOT_ON_CERT, 'v1', ssl_options: context.ssl_options.merge(verify_ssl: OpenSSL::SSL::VERIFY_NONE), auth_options: context.auth_options ) - client2.get_nodes + check_cert_accepted(client2) + end + + # Integration tests that check combined Config -> Client behavior wrt. `verify_ssl`. + # Quite redundant, but this was an embarrasing vulnerability so want to confirm... + + def test_real_cluster_concatenated_ca + config = Kubeclient::Config.read(config_file('concatenated-ca.kubeconfig')) + context = config.context + client1 = Kubeclient::Client.new( + HOSTNAME_COVERED_BY_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_accepted(client1) + client2 = Kubeclient::Client.new( + HOSTNAME_NOT_ON_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_rejected(client2) + end + + def test_real_cluster_verify_ssl_with_ca + config = Kubeclient::Config.read(config_file('external.kubeconfig')) + context = config.context + client1 = Kubeclient::Client.new( + HOSTNAME_COVERED_BY_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_accepted(client1) + client2 = Kubeclient::Client.new( + HOSTNAME_NOT_ON_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_rejected(client2) + end + + def test_real_cluster_verify_ssl_without_ca + config = Kubeclient::Config.read(config_file('external-without-ca.kubeconfig')) + context = config.context + # Hostname matches cert but the local cluster uses self-signed certs from custom CA, + # and this config omits CA data, so verification can't succeed. + client1 = Kubeclient::Client.new( + HOSTNAME_COVERED_BY_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_rejected(client1) + client2 = Kubeclient::Client.new( + HOSTNAME_NOT_ON_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_rejected(client2) + end + + def test_real_cluster_insecure_without_ca + config = Kubeclient::Config.read(config_file('insecure.kubeconfig')) + context = config.context + # Hostname matches cert but the local cluster uses self-signed certs from custom CA, + # and this config omits CA data, so verification would fail; + # however, this config specifies `insecure-skip-tls-verify: true` so any cert goes. + client1 = Kubeclient::Client.new( + HOSTNAME_COVERED_BY_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_accepted(client1) + client2 = Kubeclient::Client.new( + HOSTNAME_NOT_ON_CERT, 'v1', + ssl_options: context.ssl_options, auth_options: context.auth_options + ) + check_cert_accepted(client2) end private + # Test cert checking on discovery, CRUD, and watch code paths. + def check_cert_accepted(client) + client.discover + client.get_nodes + exercise_watcher_with_timeout(client.watch_nodes) + end + + def check_cert_rejected(client) + # TODO: all OpenSSL exceptions should be wrapped with Kubeclient error. + assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do + client.discover + end + # Since discovery fails, methods like .get_nodes, .watch_nodes would all fail + # on method_missing -> discover. Call lower-level methods to test actual connection. + assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do + client.get_entities('Node', 'nodes', {}) + end + assert_raises(Kubeclient::HttpError, OpenSSL::SSL::SSLError) do + exercise_watcher_with_timeout(client.watch_entities('nodes')) + end + end + def exercise_watcher_with_timeout(watcher) thread = Thread.new do sleep(1) watcher.finish end - watcher.each do |_notice| + watcher.each do |_notice| # rubocop:disable Lint/UnreachableLoop break end thread.join