Merge pull request #552 from cben/v4.y-openssl-x509-store-add-file

[v4.y] Load cluster ca certificates using OpenSSL::X509::Store#add_file

.github/workflows/actions.yml

@@ -10,6 +10,7 @@ on:
     - '**'
 jobs:
   build:
+    continue-on-error: true
     runs-on: ${{ matrix.os_and_command.os }}
     strategy:
       matrix:

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

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
 

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/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/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)

test/test_real_cluster.rb

@@ -16,59 +16,88 @@ 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
+
+  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.
+  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)