Add latest changes from gitlab-org/gitlab@13-0-stable-ee

5 files changed, 63 insertions, 21 deletions

diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb
index e9a7c9bcf5c..7f8dd815103 100644
--- a/lib/gitlab/email/handler.rb
+++ b/lib/gitlab/email/handler.rb
@@ -3,18 +3,16 @@
 module Gitlab
   module Email
     module Handler
-      prepend_if_ee('::EE::Gitlab::Email::Handler') # rubocop: disable Cop/InjectEnterpriseEditionModule
-
       def self.handlers
         @handlers ||= load_handlers
       end
 
       def self.load_handlers
         [
-          UnsubscribeHandler,
           CreateNoteHandler,
-          CreateMergeRequestHandler,
-          CreateIssueHandler
+          CreateIssueHandler,
+          UnsubscribeHandler,
+          CreateMergeRequestHandler
         ]
       end
 
@@ -27,3 +25,5 @@ module Gitlab
     end
   end
 end
+
+Gitlab::Email::Handler.prepend_if_ee('::EE::Gitlab::Email::Handler')
diff --git a/lib/gitlab/email/hook/smime_signature_interceptor.rb b/lib/gitlab/email/hook/smime_signature_interceptor.rb
index 61c9c984f8e..fe39589d019 100644
--- a/lib/gitlab/email/hook/smime_signature_interceptor.rb
+++ b/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
diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb
index ec412e7a8b1..d55cf3202a6 100644
--- a/lib/gitlab/email/message/repository_push.rb
+++ b/lib/gitlab/email/message/repository_push.rb
@@ -52,7 +52,7 @@ module Gitlab
         end
 
         def compare
-          @opts[:compare] if @opts[:compare]
+          @opts[:compare]
         end
 
         def diff_refs
diff --git a/lib/gitlab/email/smime/certificate.rb b/lib/gitlab/email/smime/certificate.rb
index 59d7b0c3c5b..3607b95b4bc 100644
--- a/lib/gitlab/email/smime/certificate.rb
+++ b/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
diff --git a/lib/gitlab/email/smime/signer.rb b/lib/gitlab/email/smime/signer.rb
index db03e383ecf..6a445730463 100644
--- a/lib/gitlab/email/smime/signer.rb
+++ b/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