Merge branch 'feat/add-intermediate-cas-to-smime' into 'master'

Add intermediate CAs capability to S/MIME email signature

See merge request gitlab-org/gitlab!29352

changelogs/unreleased/feat-add-intermediate-cas-to-smime.yml

+---
+title: Add intermediate CAs capability to S/MIME email signature
+merge_request: 29352
+author: Diego Louzán
+type: added

config/gitlab.yml.example

@@ -107,6 +107,9 @@ production: &base
       # S/MIME public certificate key in PEM format, will be attached to signed messages
       # Default is '.gitlab_smime_cert' relative to Rails.root (i.e. root of the GitLab app).
       # cert_file: /home/git/gitlab/.gitlab_smime_cert
+      # S/MIME extra CA public certificates in PEM format, will be attached to signed messages
+      # Optional
+      # ca_certs_file: /home/git/gitlab/.gitlab_smime_ca_certs
 
     # Email server smtp settings are in config/initializers/smtp_settings.rb.sample
 

config/smime_signature_settings.rb

@@ -5,6 +5,7 @@ class SmimeSignatureSettings
     email_smime['enabled'] = false unless email_smime['enabled']
     email_smime['key_file'] ||= Rails.root.join('.gitlab_smime_key')
     email_smime['cert_file'] ||= Rails.root.join('.gitlab_smime_cert')
+    email_smime['ca_certs_file'] ||= nil
 
     email_smime
   end

doc/administration/smime_signing_email.md

@@ -18,6 +18,9 @@ files must be provided:
   intervention.
 - Only RSA keys are supported.
 
+Optionally, you can also provide a bundle of CA certs (PEM-encoded) to be
+included on each signature. This will typically be an intermediate CA.
+
 NOTE: **Note:** Be mindful of the access levels for your private keys and visibility to
 third parties.
 
@@ -29,6 +32,8 @@ third parties.
    gitlab_rails['gitlab_email_smime_enabled'] = true
    gitlab_rails['gitlab_email_smime_key_file'] = '/etc/gitlab/ssl/gitlab_smime.key'
    gitlab_rails['gitlab_email_smime_cert_file'] = '/etc/gitlab/ssl/gitlab_smime.crt'
+   # Optional
+   gitlab_rails['gitlab_email_smime_ca_certs_file'] = '/etc/gitlab/ssl/gitlab_smime_cas.crt'
    ```
 
 1. Save the file and [reconfigure GitLab](restart_gitlab.md#omnibus-gitlab-reconfigure) for the changes to take effect.
@@ -49,6 +54,9 @@ NOTE: **Note:** The key needs to be readable by the GitLab system user (`git` by
      # S/MIME public certificate key in PEM format, will be attached to signed messages
      # Default is '.gitlab_smime_cert' relative to Rails.root (i.e. root of the GitLab app).
      cert_file: /etc/pki/smime/certs/gitlab.crt
+     # S/MIME extra CA public certificates in PEM format, will be attached to signed messages
+     # Optional
+     ca_certs_file: /etc/pki/smime/certs/gitlab_cas.crt
    ```
 
 1. Save the file and [restart GitLab](restart_gitlab.md#installations-from-source) for the changes to take effect.

lib/gitlab/email/hook/smime_signature_interceptor.rb

@@ -10,6 +10,7 @@ module Gitlab
             signed_message = Gitlab::Email::Smime::Signer.sign(
               cert: certificate.cert,
               key: certificate.key,
+              ca_certs: certificate.ca_certs,
               data: message.encoded)
 
             signed_email = Mail.new(signed_message)
@@ -21,7 +22,7 @@ module Gitlab
           private
 
           def certificate
-            @certificate ||= Gitlab::Email::Smime::Certificate.from_files(key_path, cert_path)
+            @certificate ||= Gitlab::Email::Smime::Certificate.from_files(key_path, cert_path, ca_certs_path)
           end
 
           def key_path
@@ -32,6 +33,10 @@ module Gitlab
             Gitlab.config.gitlab.email_smime.cert_file
           end
 
+          def ca_certs_path
+            Gitlab.config.gitlab.email_smime.ca_certs_file
+          end
+
           def overwrite_body(message, signed_email)
             # since this is a multipart email, assignment to nil is important,
             # otherwise Message#body will add a new mail part

lib/gitlab/email/smime/certificate.rb

@@ -4,29 +4,53 @@ module Gitlab
   module Email
     module Smime
       class Certificate
-        attr_reader :key, :cert
+        CERT_REGEX = /-----BEGIN CERTIFICATE-----(?:.|\n)+?-----END CERTIFICATE-----/.freeze
+
+        attr_reader :key, :cert, :ca_certs
 
         def key_string
-          @key.to_s
+          key.to_s
         end
 
         def cert_string
-          @cert.to_pem
+          cert.to_pem
+        end
+
+        def ca_certs_string
+          ca_certs.map(&:to_pem).join('\n') unless ca_certs.blank?
         end
 
-        def self.from_strings(key_string, cert_string)
+        def self.from_strings(key_string, cert_string, ca_certs_string = nil)
           key = OpenSSL::PKey::RSA.new(key_string)
           cert = OpenSSL::X509::Certificate.new(cert_string)
-          new(key, cert)
+          ca_certs = load_ca_certs_bundle(ca_certs_string)
+
+          new(key, cert, ca_certs)
         end
 
-        def self.from_files(key_path, cert_path)
-          from_strings(File.read(key_path), File.read(cert_path))
+        def self.from_files(key_path, cert_path, ca_certs_path = nil)
+          ca_certs_string = File.read(ca_certs_path) if ca_certs_path
+
+          from_strings(File.read(key_path), File.read(cert_path), ca_certs_string)
+        end
+
+        # Returns an array of OpenSSL::X509::Certificate objects, empty array if none found
+        #
+        # Ruby OpenSSL::X509::Certificate.new will only load the first
+        # certificate if a bundle is presented, this allows to parse multiple certs
+        # in the same file
+        def self.load_ca_certs_bundle(ca_certs_string)
+          return [] unless ca_certs_string
+
+          ca_certs_string.scan(CERT_REGEX).map do |ca_cert_string|
+            OpenSSL::X509::Certificate.new(ca_cert_string)
+          end
         end
 
-        def initialize(key, cert)
+        def initialize(key, cert, ca_certs = nil)
           @key = key
           @cert = cert
+          @ca_certs = ca_certs
         end
       end
     end

lib/gitlab/email/smime/signer.rb

@@ -7,19 +7,32 @@ module Gitlab
     module Smime
       # Tooling for signing and verifying data with SMIME
       class Signer
-        def self.sign(cert:, key:, data:)
-          signed_data = OpenSSL::PKCS7.sign(cert, key, data, nil, OpenSSL::PKCS7::DETACHED)
+        # The `ca_certs` parameter, if provided, is an array of CA certificates
+        # that will be attached in the signature together with the main `cert`.
+        # This will be typically intermediate CAs
+        def self.sign(cert:, key:, ca_certs: nil, data:)
+          signed_data = OpenSSL::PKCS7.sign(cert, key, data, Array.wrap(ca_certs), OpenSSL::PKCS7::DETACHED)
           OpenSSL::PKCS7.write_smime(signed_data)
         end
 
-        # return nil if data cannot be verified, otherwise the signed content data
-        def self.verify_signature(cert:, ca_cert: nil, signed_data:)
+        # Return nil if data cannot be verified, otherwise the signed content data
+        #
+        # Be careful with the `ca_certs` parameter, it will implicitly trust all the CAs
+        # in the array by creating a trusted store, stopping validation at the first match
+        # This is relevant when using intermediate CAs, `ca_certs` should only
+        # include the trusted, root CA
+        def self.verify_signature(ca_certs: nil, signed_data:)
           store = OpenSSL::X509::Store.new
           store.set_default_paths
-          store.add_cert(ca_cert) if ca_cert
+          Array.wrap(ca_certs).compact.each { |ca_cert| store.add_cert(ca_cert) }
 
           signed_smime = OpenSSL::PKCS7.read_smime(signed_data)
-          signed_smime if signed_smime.verify([cert], store)
+
+          # The S/MIME certificate(s) are included in the message and the trusted
+          # CAs are in the store parameter, so we pass no certs as parameters
+          # to `PKCS7.verify`
+          # See https://www.openssl.org/docs/manmaster/man3/PKCS7_verify.html
+          signed_smime if signed_smime.verify(nil, store)
         end
       end
     end

spec/config/smime_signature_settings_spec.rb

@@ -6,6 +6,7 @@ describe SmimeSignatureSettings do
   describe '.parse' do
     let(:default_smime_key) { Rails.root.join('.gitlab_smime_key') }
     let(:default_smime_cert) { Rails.root.join('.gitlab_smime_cert') }
+    let(:default_smime_ca_certs) { nil }
 
     it 'sets correct default values to disabled' do
       parsed_settings = described_class.parse(nil)
@@ -13,6 +14,7 @@ describe SmimeSignatureSettings do
       expect(parsed_settings['enabled']).to be(false)
       expect(parsed_settings['key_file']).to eq(default_smime_key)
       expect(parsed_settings['cert_file']).to eq(default_smime_cert)
+      expect(parsed_settings['ca_certs_file']).to eq(default_smime_ca_certs)
     end
 
     context 'when providing custom values' do
@@ -24,6 +26,7 @@ describe SmimeSignatureSettings do
         expect(parsed_settings['enabled']).to be(false)
         expect(parsed_settings['key_file']).to eq(default_smime_key)
         expect(parsed_settings['cert_file']).to eq(default_smime_cert)
+        expect(parsed_settings['ca_certs_file']).to eq(default_smime_ca_certs)
       end
 
       it 'enables smime with default key and cert' do
@@ -36,15 +39,18 @@ describe SmimeSignatureSettings do
         expect(parsed_settings['enabled']).to be(true)
         expect(parsed_settings['key_file']).to eq(default_smime_key)
         expect(parsed_settings['cert_file']).to eq(default_smime_cert)
+        expect(parsed_settings['ca_certs_file']).to eq(default_smime_ca_certs)
       end
 
       it 'enables smime with custom key and cert' do
         custom_key = '/custom/key'
         custom_cert = '/custom/cert'
+        custom_ca_certs = '/custom/ca_certs'
         custom_settings = Settingslogic.new({
           'enabled' => true,
           'key_file' => custom_key,
-          'cert_file' => custom_cert
+          'cert_file' => custom_cert,
+          'ca_certs_file' => custom_ca_certs
         })
 
         parsed_settings = described_class.parse(custom_settings)
@@ -52,6 +58,7 @@ describe SmimeSignatureSettings do
         expect(parsed_settings['enabled']).to be(true)
         expect(parsed_settings['key_file']).to eq(custom_key)
         expect(parsed_settings['cert_file']).to eq(custom_cert)
+        expect(parsed_settings['ca_certs_file']).to eq(custom_ca_certs)
       end
     end
   end

spec/lib/gitlab/email/hook/smime_signature_interceptor_spec.rb

@@ -5,19 +5,24 @@ require 'spec_helper'
 describe Gitlab::Email::Hook::SmimeSignatureInterceptor do
   include SmimeHelper
 
-  # cert generation is an expensive operation and they are used read-only,
+  # certs generation is an expensive operation and they are used read-only,
   # so we share them as instance variables in all tests
   before :context do
     @root_ca = generate_root
-    @cert = generate_cert(root_ca: @root_ca)
+    @intermediate_ca = generate_intermediate(signer_ca: @root_ca)
+    @cert = generate_cert(signer_ca: @intermediate_ca)
   end
 
   let(:root_certificate) do
     Gitlab::Email::Smime::Certificate.new(@root_ca[:key], @root_ca[:cert])
   end
 
+  let(:intermediate_certificate) do
+    Gitlab::Email::Smime::Certificate.new(@intermediate_ca[:key], @intermediate_ca[:cert])
+  end
+
   let(:certificate) do
-    Gitlab::Email::Smime::Certificate.new(@cert[:key], @cert[:cert])
+    Gitlab::Email::Smime::Certificate.new(@cert[:key], @cert[:cert], [intermediate_certificate.cert])
   end
 
   let(:mail_body) { "signed hello with Unicode €áø and\r\n newlines\r\n" }
@@ -48,17 +53,19 @@ describe Gitlab::Email::Hook::SmimeSignatureInterceptor do
 
     # verify signature and obtain pkcs7 encoded content
     p7enc = Gitlab::Email::Smime::Signer.verify_signature(
-      cert: certificate.cert,
-      ca_cert: root_certificate.cert,
+      ca_certs: root_certificate.cert,
       signed_data: mail.encoded)
 
+    expect(p7enc).not_to be_nil
+
     # re-verify signature from a new Mail object content
     # See https://gitlab.com/gitlab-org/gitlab/issues/197386
-    Gitlab::Email::Smime::Signer.verify_signature(
-      cert: certificate.cert,
-      ca_cert: root_certificate.cert,
+    p7_re_enc = Gitlab::Email::Smime::Signer.verify_signature(
+      ca_certs: root_certificate.cert,
       signed_data: Mail.new(mail).encoded)
 
+    expect(p7_re_enc).not_to be_nil
+
     # envelope in a Mail object and obtain the body
     decoded_mail = Mail.new(p7enc.data)
 

spec/lib/gitlab/email/smime/certificate_spec.rb

@@ -9,7 +9,8 @@ describe Gitlab::Email::Smime::Certificate do
   # so we share them as instance variables in all tests
   before :context do
     @root_ca = generate_root
-    @cert = generate_cert(root_ca: @root_ca)
+    @intermediate_ca = generate_intermediate(signer_ca: @root_ca)
+    @cert = generate_cert(signer_ca: @intermediate_ca)
   end
 
   describe 'testing environment setup' do
@@ -21,11 +22,23 @@ describe Gitlab::Email::Smime::Certificate do
       end
     end
 
+    describe 'generate_intermediate' do
+      subject { @intermediate_ca }
+
+      it 'generates an intermediate CA that expires a long way in the future' do
+        expect(subject[:cert].not_after).to be > 999.years.from_now
+      end
+
+      it 'generates an intermediate CA properly signed by the root CA' do
+        expect(subject[:cert].issuer).to eq(@root_ca[:cert].subject)
+      end
+    end
+
     describe 'generate_cert' do
       subject { @cert }
 
-      it 'generates a cert properly signed by the root CA' do
-        expect(subject[:cert].issuer).to eq(@root_ca[:cert].subject)
+      it 'generates a cert properly signed by the intermediate CA' do
+        expect(subject[:cert].issuer).to eq(@intermediate_ca[:cert].subject)
       end
 
       it 'generates a cert that expires soon' do
@@ -37,7 +50,7 @@ describe Gitlab::Email::Smime::Certificate do
       end
 
       context 'passing in INFINITE_EXPIRY' do
-        subject { generate_cert(root_ca: @root_ca, expires_in: SmimeHelper::INFINITE_EXPIRY) }
+        subject { generate_cert(signer_ca: @intermediate_ca, expires_in: SmimeHelper::INFINITE_EXPIRY) }
 
         it 'generates a cert that expires a long way in the future' do
           expect(subject[:cert].not_after).to be > 999.years.from_now
@@ -50,7 +63,7 @@ describe Gitlab::Email::Smime::Certificate do
     it 'parses correctly a certificate and key' do
       parsed_cert = described_class.from_strings(@cert[:key].to_s, @cert[:cert].to_pem)
 
-      common_cert_tests(parsed_cert, @cert, @root_ca)
+      common_cert_tests(parsed_cert, @cert, @intermediate_ca)
     end
   end
 
@@ -61,17 +74,43 @@ describe Gitlab::Email::Smime::Certificate do
 
       parsed_cert = described_class.from_files('a_key', 'a_cert')
 
-      common_cert_tests(parsed_cert, @cert, @root_ca)
+      common_cert_tests(parsed_cert, @cert, @intermediate_ca)
+    end
+
+    context 'with optional ca_certs' do
+      it 'parses correctly certificate, key and ca_certs' do
+        allow(File).to receive(:read).with('a_key').and_return(@cert[:key].to_s)
+        allow(File).to receive(:read).with('a_cert').and_return(@cert[:cert].to_pem)
+        allow(File).to receive(:read).with('a_ca_cert').and_return(@intermediate_ca[:cert].to_pem)
+
+        parsed_cert = described_class.from_files('a_key', 'a_cert', 'a_ca_cert')
+
+        common_cert_tests(parsed_cert, @cert, @intermediate_ca, with_ca_certs: [@intermediate_ca[:cert]])
+      end
+    end
+  end
+
+  context 'with no intermediate CA' do
+    it 'parses correctly a certificate and key' do
+      cert = generate_cert(signer_ca: @root_ca)
+
+      allow(File).to receive(:read).with('a_key').and_return(cert[:key].to_s)
+      allow(File).to receive(:read).with('a_cert').and_return(cert[:cert].to_pem)
+
+      parsed_cert = described_class.from_files('a_key', 'a_cert')
+
+      common_cert_tests(parsed_cert, cert, @root_ca)
     end
   end
 
-  def common_cert_tests(parsed_cert, cert, root_ca)
+  def common_cert_tests(parsed_cert, cert, signer_ca, with_ca_certs: nil)
     expect(parsed_cert.cert).to be_a(OpenSSL::X509::Certificate)
     expect(parsed_cert.cert.subject).to eq(cert[:cert].subject)
-    expect(parsed_cert.cert.issuer).to eq(root_ca[:cert].subject)
+    expect(parsed_cert.cert.issuer).to eq(signer_ca[:cert].subject)
     expect(parsed_cert.cert.not_before).to eq(cert[:cert].not_before)
     expect(parsed_cert.cert.not_after).to eq(cert[:cert].not_after)
     expect(parsed_cert.cert.extensions).to include(an_object_having_attributes(oid: 'extendedKeyUsage', value: match('E-mail Protection')))
     expect(parsed_cert.key).to be_a(OpenSSL::PKey::RSA)
+    expect(parsed_cert.ca_certs).to match_array(Array.wrap(with_ca_certs)) if with_ca_certs
   end
 end

spec/lib/gitlab/email/smime/signer_spec.rb

@@ -5,22 +5,39 @@ require 'spec_helper'
 describe Gitlab::Email::Smime::Signer do
   include SmimeHelper
 
+  let_it_be(:root_ca) { generate_root }
+  let_it_be(:intermediate_ca) { generate_intermediate(signer_ca: root_ca) }
+
+  context 'when using an intermediate CA' do
+    it 'signs data appropriately with SMIME' do
+      cert = generate_cert(signer_ca: intermediate_ca)
+
+      sign_and_verify('signed content', cert[:cert], cert[:key], root_ca[:cert], ca_certs: intermediate_ca[:cert])
+    end
+  end
+
+  context 'when not using an intermediate CA' do
     it 'signs data appropriately with SMIME' do
-    root_certificate = generate_root
-    certificate = generate_cert(root_ca: root_certificate)
+      cert = generate_cert(signer_ca: root_ca)
 
+      sign_and_verify('signed content', cert[:cert], cert[:key], root_ca[:cert])
+    end
+  end
+
+  def sign_and_verify(data, cert, key, root_ca_cert, ca_certs: nil)
     signed_content = described_class.sign(
-      cert: certificate[:cert],
-      key: certificate[:key],
-      data: 'signed content')
+      cert: cert,
+      key: key,
+      ca_certs: ca_certs,
+      data: data)
+
     expect(signed_content).not_to be_nil
 
     p7enc = described_class.verify_signature(
-      cert: certificate[:cert],
-      ca_cert: root_certificate[:cert],
+      ca_certs: root_ca_cert,
       signed_data: signed_content)
 
     expect(p7enc).not_to be_nil
-    expect(p7enc.data).to eq('signed content')
+    expect(p7enc.data).to eq(data)
   end
 end

spec/support/helpers/smime_helper.rb

@@ -5,20 +5,24 @@ module SmimeHelper
   SHORT_EXPIRY = 30.minutes
 
   def generate_root
-    issue(signed_by: nil, expires_in: INFINITE_EXPIRY, certificate_authority: true)
+    issue(cn: 'RootCA', signed_by: nil, expires_in: INFINITE_EXPIRY, certificate_authority: true)
   end
 
-  def generate_cert(root_ca:, expires_in: SHORT_EXPIRY)
-    issue(signed_by: root_ca, expires_in: expires_in, certificate_authority: false)
+  def generate_intermediate(signer_ca:)
+    issue(cn: 'IntermediateCA', signed_by: signer_ca, expires_in: INFINITE_EXPIRY, certificate_authority: true)
+  end
+
+  def generate_cert(signer_ca:, expires_in: SHORT_EXPIRY)
+    issue(signed_by: signer_ca, expires_in: expires_in, certificate_authority: false)
   end
 
   # returns a hash { key:, cert: } containing a generated key, cert pair
-  def issue(email_address: 'test@example.com', signed_by:, expires_in:, certificate_authority:)
+  def issue(email_address: 'test@example.com', cn: nil, signed_by:, expires_in:, certificate_authority:)
     key = OpenSSL::PKey::RSA.new(4096)
     public_key = key.public_key
 
     subject = if certificate_authority
-                OpenSSL::X509::Name.parse("/CN=EU")
+                OpenSSL::X509::Name.parse("/CN=#{cn}")
               else
                 OpenSSL::X509::Name.parse("/CN=#{email_address}")
               end