From 7c27fae77d8f870801b52f1af4f4948038b08912 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 20 Mar 2022 13:20:08 +0200 Subject: [PATCH 01/12] test refactor: test_real_cluster.rb --- test/test_real_cluster.rb | 62 +++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/test/test_real_cluster.rb b/test/test_real_cluster.rb index 59c9deb0..568c9374 100644 --- a/test/test_real_cluster.rb +++ b/test/test_real_cluster.rb @@ -16,59 +16,71 @@ 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 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) From 7c331c1cfe87cca54826d74a2f23550006eb1d83 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Tue, 22 Mar 2022 16:57:26 +0200 Subject: [PATCH 02/12] Tests for concatenated CA data (#460) Test sandwitches the real CA cert between two unrelated CA certs (another-ca1.pem, another-ca2.pem, simply copied from two runs of update_certs_k0s.rb). No test for root+intermediate scenario. Fails before the backport of https://github.com/ManageIQ/kubeclient/pull/461: KubeclientConfigTest#test_concatenated_ca [/home/beni/kubeclient/test/test_config.rb:196]: Expected false to be truthy. (some experimenting with order suggests only first cert is honored.) Passes with the fix. --- test/config/another-ca1.pem | 19 +++++++++ test/config/another-ca2.pem | 19 +++++++++ test/config/concatenated-ca.kubeconfig | 20 +++++++++ test/config/concatenated-ca.pem | 57 ++++++++++++++++++++++++++ test/config/update_certs_k0s.rb | 2 + test/test_config.rb | 6 +++ test/test_real_cluster.rb | 17 ++++++++ 7 files changed, 140 insertions(+) create mode 100644 test/config/another-ca1.pem create mode 100644 test/config/another-ca2.pem create mode 100644 test/config/concatenated-ca.kubeconfig create mode 100644 test/config/concatenated-ca.pem 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/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/test_config.rb b/test/test_config.rb index 98d2114f..d246624c 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -44,6 +44,12 @@ 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) diff --git a/test/test_real_cluster.rb b/test/test_real_cluster.rb index 568c9374..5c1f4cf9 100644 --- a/test/test_real_cluster.rb +++ b/test/test_real_cluster.rb @@ -57,6 +57,23 @@ def test_real_cluster_verify_none check_cert_accepted(client2) end + 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.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), + auth_options: context.auth_options + ) + check_cert_accepted(client1) + client2 = Kubeclient::Client.new( + HOSTNAME_NOT_ON_CERT, 'v1', + ssl_options: context.ssl_options.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), + auth_options: context.auth_options + ) + check_cert_rejected(client2) + end + private # Test cert checking on discovery, CRUD, and watch code paths. From d1cd26d2702d0d74bf19fc9549eefc8ba1bc7ff8 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 6 Sep 2020 11:55:48 +0300 Subject: [PATCH 03/12] Merge pull request #461 from PerfectMemory/openssl-x509-store-add-file Load cluster ca certificates using OpenSSL::X509::Store#add_file (cherry picked from commit 53408c10e6f5b39178edec51fbe77b143e2523df) --- lib/kubeclient/config.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index 22885908..2520eb6b 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -51,16 +51,15 @@ def context(context_name = nil) user['exec_result'] = ExecCredentials.run(exec_opts) end - ca_cert_data = fetch_cluster_ca_data(cluster) client_cert_data = fetch_user_cert_data(user) client_key_data = fetch_user_key_data(user) auth_options = fetch_user_auth_options(user) ssl_options = {} - if !ca_cert_data.nil? + if cluster_ca_data?(cluster) cert_store = OpenSSL::X509::Store.new - cert_store.add_cert(OpenSSL::X509::Certificate.new(ca_cert_data)) + 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 @@ -131,11 +130,16 @@ def fetch_context(context_name) [cluster, user, namespace] end - def fetch_cluster_ca_data(cluster) + def cluster_ca_data?(cluster) + cluster.key?('certificate-authority') || cluster.key?('certificate-authority-data') + end + + def populate_cert_store_from_cluster_ca_data(cluster, cert_store) if cluster.key?('certificate-authority') - File.read(ext_file_path(cluster['certificate-authority'])) + cert_store.add_file(ext_file_path(cluster['certificate-authority'])) elsif cluster.key?('certificate-authority-data') - Base64.decode64(cluster['certificate-authority-data']) + ca_cert_data = Base64.decode64(cluster['certificate-authority-data']) + cert_store.add_cert(OpenSSL::X509::Certificate.new(ca_cert_data)) end end From 2dd7f64055575ec48baa48b011215b771f267c44 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Tue, 22 Mar 2022 17:34:53 +0200 Subject: [PATCH 04/12] Changelog for fix for #460 --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be0fab5d..4d8174eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ 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 4.9.z + +### Fixed + +- `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. + ## 4.9.2 — 2021-05-30 ### Added From b1824ed8a770ca22aa9520c8e3479c4c84b17f1d Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 23 Mar 2022 09:45:48 +0200 Subject: [PATCH 05/12] [v4.y] CI: don't abort other builds when one fails error - Helps confirm suspected OS-specific failures. - Normally when one build fails, most other builds already started and some almost complete `bundle install` / started tests. So it's not a big "waste" expensive to let them finish, arguably it's actually a waste to abort them! (sunken cost fallacy? :-) - In case rubocop complains, while I do consider it a merge blocker, it's better contributor (and maintainer) experience to also see test results. --- .github/workflows/actions.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index fd1de5f2..5a74326e 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -10,6 +10,7 @@ on: - '**' jobs: build: + continue-on-error: true runs-on: ${{ matrix.os_and_command.os }} strategy: matrix: From 88483cff28d39340a3206fb8dde3dfc730d1cc28 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 16 Mar 2022 23:30:20 +0200 Subject: [PATCH 06/12] SECURITY: Kubeclient::Config: return ssl_options[:verify_ssl] correctly - VULNERABILITY FIX: Previously, whenever kubeconfig did not define custom CA (normal situation for production clusters with public domain and certificate!), `Config` was returning hard-coded `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. - 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`. These don't affect code that supplies `Client` parameters directly, only code that uses `Config`. (To ease back-porting, this commit is rebased directly on the 6-year-old PR that introduced Kubeclient::Config - this was broken from day 1! https://github.com/ManageIQ/kubeclient/pull/127 Tests come in separate commits based on later points.) --- lib/kubeclient/config.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/kubeclient/config.rb b/lib/kubeclient/config.rb index 6700371a..b88845ff 100644 --- a/lib/kubeclient/config.rb +++ b/lib/kubeclient/config.rb @@ -38,13 +38,16 @@ def context(context_name = nil) ssl_options = {} - if !ca_cert_data.nil? + ssl_options[:verify_ssl] = if cluster['insecure-skip-tls-verify'] == true + OpenSSL::SSL::VERIFY_NONE + else + OpenSSL::SSL::VERIFY_PEER + end + + unless ca_cert_data.nil? cert_store = OpenSSL::X509::Store.new cert_store.add_cert(OpenSSL::X509::Certificate.new(ca_cert_data)) - 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? From c21e2b5261c9f8a7f67e2fda6bcb0bdb691ec628 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 20 Mar 2022 13:12:18 +0200 Subject: [PATCH 07/12] 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 | 1 - test/config/external-without-ca.kubeconfig | 21 ++++++ test/config/gcpauth.kubeconfig | 1 - test/config/gcpcmdauth.kubeconfig | 1 - test/config/insecure-custom-ca.kubeconfig | 22 ++++++ test/config/insecure.kubeconfig | 25 +++++++ test/config/nouser.kubeconfig | 1 - test/config/oidcauth.kubeconfig | 1 - test/config/secure-without-ca.kubeconfig | 22 ++++++ test/config/secure.kubeconfig | 21 ++++++ test/config/userauth.kubeconfig | 1 - test/test_config.rb | 80 +++++++++++++++++----- 12 files changed, 175 insertions(+), 22 deletions(-) create mode 100644 test/config/external-without-ca.kubeconfig create mode 100644 test/config/insecure-custom-ca.kubeconfig create mode 100644 test/config/insecure.kubeconfig create mode 100644 test/config/secure-without-ca.kubeconfig create mode 100644 test/config/secure.kubeconfig 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/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 d246624c..d47827e0 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -7,13 +7,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 @@ -21,7 +56,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 @@ -53,7 +88,7 @@ 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 @@ -61,7 +96,7 @@ def test_user_token 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 From aa36e8959dce8515fc905990475315fb102b59fb Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 20 Mar 2022 13:50:26 +0200 Subject: [PATCH 08/12] SECURITY: integration tests for Config+Client verify_ssl with real cluster --- test/test_real_cluster.rb | 59 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/test/test_real_cluster.rb b/test/test_real_cluster.rb index 5c1f4cf9..7ce9493a 100644 --- a/test/test_real_cluster.rb +++ b/test/test_real_cluster.rb @@ -57,23 +57,74 @@ def test_real_cluster_verify_none 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.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), - auth_options: context.auth_options + 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.merge(verify_ssl: OpenSSL::SSL::VERIFY_PEER), - auth_options: context.auth_options + 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. From e4cb7270532d580b2177e70e324b2caeed7b5f1e Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 23 Mar 2022 11:42:33 +0200 Subject: [PATCH 09/12] CHANGELOG and README about Config verify_ssl vulnerability --- CHANGELOG.md | 26 +++++++++++++++++++++++++- README.md | 7 +++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d8174eb..3237d4a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,31 @@ 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 4.9.z +## 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). + +- 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`. + - `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. @@ -19,6 +40,9 @@ Kubeclient release versioning follows [SemVer](https://semver.org/). - 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 b6f6f457..6cf0fb67 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,13 @@ 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 [latest CHANGELOG.md](https://github.com/ManageIQ/kubeclient/blob/master/CHANGELOG.md) for details and which versions got a fix. +Open an issue if you want a backport to another version. + ## Installation Add this line to your application's Gemfile: From d7bc03d51a43fd043e1c4e08ae902c5e3a55e90b Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 23 Mar 2022 13:40:49 +0200 Subject: [PATCH 10/12] Bump kubeclient to 4.9.3 --- lib/kubeclient/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kubeclient/version.rb b/lib/kubeclient/version.rb index 9186bc3a..c1a5f6c8 100644 --- a/lib/kubeclient/version.rb +++ b/lib/kubeclient/version.rb @@ -1,4 +1,4 @@ # Kubernetes REST-API Client module Kubeclient - VERSION = '4.9.2'.freeze + VERSION = '4.9.3'.freeze end From 6670fab571c466d7af4044bee9d742d5338f38d5 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 23 Mar 2022 11:42:33 +0200 Subject: [PATCH 11/12] CHANGELOG and README fixups about #554, #555 Tiny followup to https://github.com/ManageIQ/kubeclient/pull/556. Sorry for noise, had these locally but forgot to push before merging. If I start backporting, CHANGELOG.md on master branch might not always be updated with all backports (it SHOULD, but it will require separate merges to master). So I prefer pointing to the vulnerability issue as the "source of truth". Also, security impact will be better discussed on the issue. --- CHANGELOG.md | 4 ++++ README.md | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3237d4a3..c7b8607a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,12 +23,16 @@ Kubeclient release versioning follows [SemVer](https://semver.org/). 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. diff --git a/README.md b/README.md index 6cf0fb67..3414db23 100644 --- a/README.md +++ b/README.md @@ -13,8 +13,7 @@ To learn more about groups and versions in kubernetes refer to [k8s docs](https: 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 [latest CHANGELOG.md](https://github.com/ManageIQ/kubeclient/blob/master/CHANGELOG.md) for details and which versions got a fix. -Open an issue if you want a backport to another version. +See https://github.com/ManageIQ/kubeclient/issues/554 for details and which versions got a fix. ## Installation From 55709fa5916ba0849c57f9ec3449d750ec324fed Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 23 Mar 2022 12:34:20 +0200 Subject: [PATCH 12/12] test: silence rubocop about loop that gets aborted from another thread --- test/test_real_cluster.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_real_cluster.rb b/test/test_real_cluster.rb index ea15bc77..031e6611 100644 --- a/test/test_real_cluster.rb +++ b/test/test_real_cluster.rb @@ -154,7 +154,7 @@ def exercise_watcher_with_timeout(watcher) sleep(1) watcher.finish end - watcher.each do |_notice| + watcher.each do |_notice| # rubocop:disable Lint/UnreachableLoop break end thread.join