From 1ef6e93da1166de2bde8ee181ffdca4d320abd59 Mon Sep 17 00:00:00 2001
From: Imre Farkas <ifarkas@gitlab.com>
Date: Mon, 9 Mar 2020 17:56:57 +0100
Subject: [PATCH] Load multiple root certificates in
 Auth::Smartcard::Base.store

OpenSSL already supports this, we just need to call
OpenSSL::X509::Store#add_file instead of OpenSSL::X509::Store#add_cert.
When multiple certificates are loaded, validation checks the whole
certificate chain. If a root certificate is missing and only the
intermediate is loaded, validation fails.
---
 config/gitlab.yml.example                         |  2 +-
 ...97463-smartcard_multiple_root_certificates.yml |  5 +++++
 ee/lib/gitlab/auth/smartcard/base.rb              | 14 +++-----------
 ...smartcard_certificate_store_shared_examples.rb | 15 ++++++++-------
 4 files changed, 17 insertions(+), 19 deletions(-)
 create mode 100644 ee/changelogs/unreleased/197463-smartcard_multiple_root_certificates.yml

diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 9136bb0b569fb..1a042f0e349d2 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -770,7 +770,7 @@ production: &base
     # Allow smartcard authentication
     enabled: false
 
-    # Path to a file containing a CA certificate
+    # Path to a file containing a CA certificate bundle
     ca_file: '/etc/ssl/certs/CA.pem'
 
     # Host and port where the client side certificate is requested by the
diff --git a/ee/changelogs/unreleased/197463-smartcard_multiple_root_certificates.yml b/ee/changelogs/unreleased/197463-smartcard_multiple_root_certificates.yml
new file mode 100644
index 0000000000000..6da150426802e
--- /dev/null
+++ b/ee/changelogs/unreleased/197463-smartcard_multiple_root_certificates.yml
@@ -0,0 +1,5 @@
+---
+title: Allow multiple root certificates for smartcard auth
+merge_request: 26812
+author:
+type: added
diff --git a/ee/lib/gitlab/auth/smartcard/base.rb b/ee/lib/gitlab/auth/smartcard/base.rb
index 914534a323588..082666f75a667 100644
--- a/ee/lib/gitlab/auth/smartcard/base.rb
+++ b/ee/lib/gitlab/auth/smartcard/base.rb
@@ -4,7 +4,6 @@ module Gitlab
   module Auth
     module Smartcard
       class Base
-        InvalidCAFilePath = Class.new(StandardError)
         InvalidCertificate = Class.new(StandardError)
 
         delegate :allow_signup?,
@@ -12,17 +11,10 @@ class Base
 
         def self.store
           @store ||= OpenSSL::X509::Store.new.tap do |store|
-            store.add_cert(
-              OpenSSL::X509::Certificate.new(
-                File.read(Gitlab.config.smartcard.ca_file)))
+            store.add_file(Gitlab.config.smartcard.ca_file)
           end
-        rescue Errno::ENOENT => ex
-          logger.error(message: 'Failed to open Gitlab.config.smartcard.ca_file',
-                       error: ex)
-
-          raise InvalidCAFilePath
-        rescue OpenSSL::X509::CertificateError => ex
-          logger.error(message: 'Gitlab.config.smartcard.ca_file is not a valid certificate',
+        rescue OpenSSL::X509::StoreError => ex
+          logger.error(message: 'Gitlab.config.smartcard.ca_file is invalid or does not exist',
                        error: ex)
 
           raise InvalidCertificate
diff --git a/ee/spec/support/shared_examples/lib/gitlab/smartcard_certificate_store_shared_examples.rb b/ee/spec/support/shared_examples/lib/gitlab/smartcard_certificate_store_shared_examples.rb
index 82bf421d67d6a..9313ad4d601da 100644
--- a/ee/spec/support/shared_examples/lib/gitlab/smartcard_certificate_store_shared_examples.rb
+++ b/ee/spec/support/shared_examples/lib/gitlab/smartcard_certificate_store_shared_examples.rb
@@ -16,17 +16,18 @@
 
     subject { described_class.store }
 
-    context 'file does not exist' do
-      it 'raises error' do
-        expect { subject }.to(
-          raise_error(Gitlab::Auth::Smartcard::Certificate::InvalidCAFilePath))
+    context 'loads CA bundle' do
+      it 'uses correct method' do
+        expect_next_instance_of(OpenSSL::X509::Store) do |store|
+          expect(store).to receive(:add_file).and_return(true)
+        end
+
+        subject
       end
     end
 
-    context 'smartcard ca_file is not a valid certificate' do
+    context 'without valid CA file' do
       it 'raises error' do
-        expect(File).to(
-          receive(:read).with('ca_file').and_return('invalid certificate'))
         expect { subject }.to(
           raise_error(Gitlab::Auth::Smartcard::Certificate::InvalidCertificate))
       end
-- 
GitLab