Merge branch 'feat/x509-signed-commits' into 'master'

feat: x509 signed commits using openssl

See merge request gitlab-org/gitlab!17773

app/assets/stylesheets/pages/commits.scss

@@ -321,6 +321,16 @@
   }
 }
 
+.gpg-popover-certificate-details {
+  ul {
+    padding-left: $gl-padding;
+  }
+
+  li.unstyled {
+    list-style-type: none;
+  }
+}
+
 .gpg-popover-status {
   display: flex;
   align-items: center;

app/models/commit.rb

@@ -25,7 +25,7 @@ class Commit
   attr_accessor :redacted_description_html
   attr_accessor :redacted_title_html
   attr_accessor :redacted_full_title_html
-  attr_reader :gpg_commit, :container
+  attr_reader :container
 
   delegate :repository, to: :container
   delegate :project, to: :repository, allow_nil: true
@@ -123,7 +123,6 @@ class Commit
 
     @raw = raw_commit
     @container = container
-    @gpg_commit = Gitlab::Gpg::Commit.new(self) if container
   end
 
   delegate \
@@ -320,13 +319,34 @@ class Commit
     )
   end
 
-  def signature
-    return @signature if defined?(@signature)
+  def has_signature?
+    signature_type && signature_type != :NONE
+  end
+
+  def raw_signature_type
+    strong_memoize(:raw_signature_type) do
+      next unless @raw.instance_of?(Gitlab::Git::Commit)
+
+      @raw.raw_commit.signature_type if defined? @raw.raw_commit.signature_type
+    end
+  end
 
-    @signature = gpg_commit.signature
+  def signature_type
+    @signature_type ||= raw_signature_type || :NONE
   end
 
-  delegate :has_signature?, to: :gpg_commit
+  def signature
+    strong_memoize(:signature) do
+      case signature_type
+      when :PGP
+        Gitlab::Gpg::Commit.new(self).signature
+      when :X509
+        Gitlab::X509::Commit.new(self).signature
+      else
+        nil
+      end
+    end
+  end
 
   def revert_branch_name
     "revert-#{short_id}"

app/models/concerns/x509_serial_number_attribute.rb

+# frozen_string_literal: true
+
+module X509SerialNumberAttribute
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def x509_serial_number_attribute(name)
+      return if ENV['STATIC_VERIFICATION']
+
+      validate_binary_column_exists!(name) unless Rails.env.production?
+
+      attribute(name, Gitlab::Database::X509SerialNumberAttribute.new)
+    end
+
+    # This only gets executed in non-production environments as an additional check to ensure
+    # the column is the correct type.  In production it should behave like any other attribute.
+    # See https://gitlab.com/gitlab-org/gitlab/merge_requests/5502 for more discussion
+    def validate_binary_column_exists!(name)
+      return unless database_exists?
+
+      unless table_exists?
+        warn "WARNING: x509_serial_number_attribute #{name.inspect} is invalid since the table doesn't exist - you may need to run database migrations"
+        return
+      end
+
+      column = columns.find { |c| c.name == name.to_s }
+
+      unless column
+        warn "WARNING: x509_serial_number_attribute #{name.inspect} is invalid since the column doesn't exist - you may need to run database migrations"
+        return
+      end
+
+      unless column.type == :binary
+        raise ArgumentError.new("x509_serial_number_attribute #{name.inspect} is invalid since the column type is not :binary")
+      end
+    rescue => error
+      Gitlab::AppLogger.error "X509SerialNumberAttribute initialization: #{error.message}"
+      raise
+    end
+
+    def database_exists?
+      Gitlab::Database.exists?
+    end
+  end
+end

app/models/x509_certificate.rb

+# frozen_string_literal: true
+
+class X509Certificate < ApplicationRecord
+  include X509SerialNumberAttribute
+
+  x509_serial_number_attribute :serial_number
+
+  enum certificate_status: {
+    good: 0,
+    revoked: 1
+  }
+
+  belongs_to :x509_issuer, class_name: 'X509Issuer', foreign_key: 'x509_issuer_id', optional: false
+
+  has_many :x509_commit_signatures, inverse_of: 'x509_certificate'
+
+  # rfc 5280 - 4.2.1.2  Subject Key Identifier
+  validates :subject_key_identifier, presence: true, format: { with: /\A(\h{2}:){19}\h{2}\z/ }
+  # rfc 5280 - 4.1.2.6  Subject
+  validates :subject, presence: true
+  # rfc 5280 - 4.1.2.6  Subject (subjectAltName contains the email address)
+  validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP }
+  # rfc 5280 - 4.1.2.2  Serial number
+  validates :serial_number, presence: true, numericality: { only_integer: true }
+
+  validates :x509_issuer_id, presence: true
+
+  def self.safe_create!(attributes)
+    create_with(attributes)
+      .safe_find_or_create_by!(subject_key_identifier: attributes[:subject_key_identifier])
+  end
+end

app/models/x509_commit_signature.rb

+# frozen_string_literal: true
+
+class X509CommitSignature < ApplicationRecord
+  include ShaAttribute
+
+  sha_attribute :commit_sha
+
+  enum verification_status: {
+    unverified: 0,
+    verified: 1
+  }
+
+  belongs_to :project, class_name: 'Project', foreign_key: 'project_id', optional: false
+  belongs_to :x509_certificate, class_name: 'X509Certificate', foreign_key: 'x509_certificate_id', optional: false
+
+  validates :commit_sha, presence: true
+  validates :project_id, presence: true
+  validates :x509_certificate_id, presence: true
+
+  scope :by_commit_sha, ->(shas) { where(commit_sha: shas) }
+
+  def self.safe_create!(attributes)
+    create_with(attributes)
+      .safe_find_or_create_by!(commit_sha: attributes[:commit_sha])
+  end
+
+  # Find commits that are lacking a signature in the database at present
+  def self.unsigned_commit_shas(commit_shas)
+    return [] if commit_shas.empty?
+
+    signed = by_commit_sha(commit_shas).pluck(:commit_sha)
+    commit_shas - signed
+  end
+
+  def commit
+    project.commit(commit_sha)
+  end
+
+  def x509_commit
+    return unless commit
+
+    Gitlab::X509::Commit.new(commit)
+  end
+end

app/models/x509_issuer.rb

+# frozen_string_literal: true
+
+class X509Issuer < ApplicationRecord
+  has_many :x509_certificates, inverse_of: 'x509_issuer'
+
+  # rfc 5280 - 4.2.1.1  Authority Key Identifier
+  validates :subject_key_identifier, presence: true, format: { with: /\A(\h{2}:){19}\h{2}\z/ }
+  # rfc 5280 - 4.1.2.4  Issuer
+  validates :subject, presence: true
+  # rfc 5280 - 4.2.1.14  CRL Distribution Points
+  # cRLDistributionPoints extension using URI:http
+  validates :crl_url, presence: true, public_url: true
+
+  def self.safe_create!(attributes)
+    create_with(attributes)
+      .safe_find_or_create_by!(subject_key_identifier: attributes[:subject_key_identifier])
+  end
+end

app/services/git/branch_hooks_service.rb

@@ -6,7 +6,7 @@ module Git
       execute_branch_hooks
 
       super.tap do
-        enqueue_update_gpg_signatures
+        enqueue_update_signatures
       end
     end
 
@@ -103,14 +103,22 @@ module Git
       end
     end
 
-    def enqueue_update_gpg_signatures
-      unsigned = GpgSignature.unsigned_commit_shas(limited_commits.map(&:sha))
+    def unsigned_x509_shas(commits)
+      X509CommitSignature.unsigned_commit_shas(commits.map(&:sha))
+    end
+
+    def unsigned_gpg_shas(commits)
+      GpgSignature.unsigned_commit_shas(commits.map(&:sha))
+    end
+
+    def enqueue_update_signatures
+      unsigned = unsigned_x509_shas(commits) & unsigned_gpg_shas(commits)
       return if unsigned.empty?
 
       signable = Gitlab::Git::Commit.shas_with_signatures(project.repository, unsigned)
       return if signable.empty?
 
-      CreateGpgSignatureWorker.perform_async(signable, project.id)
+      CreateCommitSignatureWorker.perform_async(signable, project.id)
     end
 
     # It's not sufficient to just check for a blank SHA as it's possible for the

app/views/projects/commit/_signature.html.haml

 - if signature
-  = render partial: "projects/commit/#{signature.verification_status}_signature_badge", locals: { signature: signature }
+  - uri = "projects/commit/#{"x509/" if signature.instance_of?(X509CommitSignature)}"
+  = render partial: "#{uri}#{signature.verification_status}_signature_badge", locals: { signature: signature }

app/views/projects/commit/_signature_badge.html.haml

@@ -17,12 +17,18 @@
 - content = capture do
   - if show_user
     .clearfix
-      = render partial: 'projects/commit/signature_badge_user', locals: { signature: signature }
+      - uri_signature_badge_user = "projects/commit/#{"x509/" if signature.instance_of?(X509CommitSignature)}signature_badge_user"
+      = render partial: "#{uri_signature_badge_user}", locals: { signature: signature }
 
-  = _('GPG Key ID:')
-  %span.monospace= signature.gpg_key_primary_keyid
+  - if signature.instance_of?(X509CommitSignature)
+    = render partial: "projects/commit/x509/certificate_details", locals: { signature: signature }
 
-  = link_to(_('Learn more about signing commits'), help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link')
+    = link_to(_('Learn more about x509 signed commits'), help_page_path('user/project/repository/x509_signed_commits/index.md'), class: 'gpg-popover-help-link')
+  - else
+    = _('GPG Key ID:')
+    %span.monospace= signature.gpg_key_primary_keyid
+
+    = link_to(_('Learn more about signing commits'), help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link')
 
 %button{ tabindex: 0, class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'top', title: title, content: content } }
   = label

app/views/projects/commit/x509/_certificate_details.html.haml

+.gpg-popover-certificate-details
+  %strong= _('Certificate Subject')
+  %ul
+    - signature.x509_certificate.subject.split(",").each do |i|
+      - if i.start_with?("CN", "O")
+        %li= i
+    %li= _('Subject Key Identifier:')
+    %li.unstyled= signature.x509_certificate.subject_key_identifier.gsub(":", " ")
+
+.gpg-popover-certificate-details
+  %strong= _('Certificate Issuer')
+  %ul
+    - signature.x509_certificate.x509_issuer.subject.split(",").each do |i|
+      - if i.start_with?("CN", "OU", "O")
+        %li= i
+    %li= _('Subject Key Identifier:')
+    %li.unstyled= signature.x509_certificate.x509_issuer.subject_key_identifier.gsub(":", " ")

app/views/projects/commit/x509/_signature_badge_user.html.haml

+- user = signature.commit.committer
+- user_email = signature.x509_certificate.email
+
+- if user
+  = link_to user_path(user), class: 'gpg-popover-user-link' do
+    %div
+      = user_avatar_without_link(user: user, size: 32)
+
+    %div
+      %strong= user.name
+      %div= user.to_reference
+
+- else
+  = mail_to user_email do
+    %div
+      = user_avatar_without_link(user_email: user_email, size: 32)
+
+    %div
+      %strong= user_email

app/views/projects/commit/x509/_unverified_signature_badge.html.haml

+- title = capture do
+  = _('This commit was signed with an <strong>unverified</strong> signature.').html_safe
+
+- locals = { signature: signature, title: title, label: _('Unverified'), css_class: 'invalid', icon: 'status_notfound_borderless', show_user: true }
+
+= render partial: 'projects/commit/signature_badge', locals: locals

app/views/projects/commit/x509/_verified_signature_badge.html.haml

+- title = capture do
+  = _('This commit was signed with a <strong>verified</strong> signature and the committer email is verified to belong to the same user.').html_safe
+
+- locals = { signature: signature, title: title, label: _('Verified'), css_class: 'valid', icon: 'status_success_borderless', show_user: true }
+
+= render partial: 'projects/commit/signature_badge', locals: locals

app/workers/all_queues.yml

@@ -699,14 +699,14 @@
   :latency_sensitive: true
   :resource_boundary: :unknown
   :weight: 2
-- :name: create_evidence
-  :feature_category: :release_governance
+- :name: create_commit_signature
+  :feature_category: :source_code_management
   :has_external_dependencies: 
   :latency_sensitive: 
   :resource_boundary: :unknown
   :weight: 2
-- :name: create_gpg_signature
-  :feature_category: :source_code_management
+- :name: create_evidence
+  :feature_category: :release_governance
   :has_external_dependencies: 
   :latency_sensitive: 
   :resource_boundary: :unknown

app/workers/create_gpg_signature_worker.rb → app/workers/create_commit_signature_worker.rb

 # frozen_string_literal: true
 
-class CreateGpgSignatureWorker
+class CreateCommitSignatureWorker
   include ApplicationWorker
 
   feature_category :source_code_management
@@ -23,7 +23,12 @@ class CreateGpgSignatureWorker
 
     # This calculates and caches the signature in the database
     commits.each do |commit|
-      Gitlab::Gpg::Commit.new(commit).signature
+      case commit.signature_type
+      when :PGP
+        Gitlab::Gpg::Commit.new(commit).signature
+      when :X509
+        Gitlab::X509::Commit.new(commit).signature
+      end
     rescue => e
       Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") # rubocop:disable Gitlab/RailsLogger
     end

changelogs/unreleased/feat-x509-signed-commits.yml

+---
+title: x509 signed commits using openssl
+merge_request: 17773
+author: Roger Meier
+type: added

config/sidekiq_queues.yml

@@ -42,12 +42,12 @@
   - 2
 - - container_repository
   - 1
+- - create_commit_signature
+  - 2
 - - create_evidence
   - 2
 - - create_github_webhook
   - 2
-- - create_gpg_signature
-  - 2
 - - create_note_diff_file
   - 1
 - - cronjob

db/migrate/20190926225633_create_x509_signatures.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CreateX509Signatures < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :x509_issuers do |t|
+      t.timestamps_with_timezone null: false
+
+      t.string :subject_key_identifier, index: true, null: false, unique: true, limit: 255
+      t.string :subject, null: false, limit: 255
+      t.string :crl_url, null: false, limit: 255
+    end
+
+    create_table :x509_certificates do |t|
+      t.timestamps_with_timezone null: false
+
+      t.string :subject_key_identifier, index: true, null: false, unique: true, limit: 255
+      t.string :subject, null: false, limit: 255
+      t.string :email, null: false, limit: 255
+      t.binary :serial_number, null: false
+
+      t.integer :certificate_status, limit: 2, default: 0, null: false
+
+      t.references :x509_issuer, index: true, null: false, foreign_key: { on_delete: :cascade }
+    end
+
+    create_table :x509_commit_signatures do |t|
+      t.timestamps_with_timezone null: false
+
+      t.references :project, index: true, null: false, foreign_key: { on_delete: :cascade }
+      t.references :x509_certificate, index: true, null: false, foreign_key: { on_delete: :cascade }
+
+      t.binary :commit_sha, index: true, null: false
+      t.integer :verification_status, limit: 2, default: 0, null: false
+    end
+  end
+end

db/post_migrate/20200206091544_migrate_create_commit_signature_worker_sidekiq_queue.rb

+# frozen_string_literal: true
+
+class MigrateCreateCommitSignatureWorkerSidekiqQueue < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sidekiq_queue_migrate 'create_gpg_signature', to: 'create_commit_signature'
+  end
+
+  def down
+    sidekiq_queue_migrate 'create_commit_signature', to: 'create_gpg_signature'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_02_05_143231) do
+ActiveRecord::Schema.define(version: 2020_02_06_091544) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -4482,6 +4482,40 @@ ActiveRecord::Schema.define(version: 2020_02_05_143231) do
     t.index ["type"], name: "index_web_hooks_on_type"
   end
 
+  create_table "x509_certificates", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "subject_key_identifier", limit: 255, null: false
+    t.string "subject", limit: 255, null: false
+    t.string "email", limit: 255, null: false
+    t.binary "serial_number", null: false
+    t.integer "certificate_status", limit: 2, default: 0, null: false
+    t.bigint "x509_issuer_id", null: false
+    t.index ["subject_key_identifier"], name: "index_x509_certificates_on_subject_key_identifier"
+    t.index ["x509_issuer_id"], name: "index_x509_certificates_on_x509_issuer_id"
+  end
+
+  create_table "x509_commit_signatures", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.bigint "project_id", null: false
+    t.bigint "x509_certificate_id", null: false
+    t.binary "commit_sha", null: false
+    t.integer "verification_status", limit: 2, default: 0, null: false
+    t.index ["commit_sha"], name: "index_x509_commit_signatures_on_commit_sha"
+    t.index ["project_id"], name: "index_x509_commit_signatures_on_project_id"
+    t.index ["x509_certificate_id"], name: "index_x509_commit_signatures_on_x509_certificate_id"
+  end
+
+  create_table "x509_issuers", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.string "subject_key_identifier", limit: 255, null: false
+    t.string "subject", limit: 255, null: false
+    t.string "crl_url", limit: 255, null: false
+    t.index ["subject_key_identifier"], name: "index_x509_issuers_on_subject_key_identifier"
+  end
+
   create_table "zoom_meetings", force: :cascade do |t|
     t.bigint "project_id", null: false
     t.bigint "issue_id", null: false
@@ -4973,6 +5007,9 @@ ActiveRecord::Schema.define(version: 2020_02_05_143231) do
   add_foreign_key "vulnerability_scanners", "projects", on_delete: :cascade
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
   add_foreign_key "web_hooks", "projects", name: "fk_0c8ca6d9d1", on_delete: :cascade
+  add_foreign_key "x509_certificates", "x509_issuers", on_delete: :cascade
+  add_foreign_key "x509_commit_signatures", "projects", on_delete: :cascade
+  add_foreign_key "x509_commit_signatures", "x509_certificates", on_delete: :cascade
   add_foreign_key "zoom_meetings", "issues", on_delete: :cascade
   add_foreign_key "zoom_meetings", "projects", on_delete: :cascade
 end

doc/user/project/repository/x509_signed_commits/index.md

+---
+type: concepts, howto
+---
+
+# Signing commits with x509
+
+[x509](https://en.wikipedia.org/wiki/X.509) is a standard format for public key
+certificates issued by a public or private Public Key Infrastructure (PKI).
+Personal x509 certificates are used for authentication or signing purposes
+such as SMIME, but beside that, Git supports signing of commits and tags
+with x509 certificates in a similar way as with [GPG](../gpg_signed_commits/index.md).
+The main difference is the trust anchor which is the PKI for x509 certificates
+instead of a web of trust with GPG.
+
+## How GitLab handles x509
+
+GitLab uses its own certificate store and therefore defines the trust chain.
+
+For a commit to be *verified* by GitLab:
+
+- The signing certificate email must match a verified email address used by the committer in GitLab.
+- The Certificate Authority has to be trusted by the GitLab instance, see also
+  [Omnibus install custom public certificates](https://docs.gitlab.com/omnibus/settings/ssl.html#install-custom-public-certificates).
+- The signing time has to be within the time range of the [certificate validity](https://www.rfc-editor.org/rfc/rfc5280.html#section-4.1.2.5)
+  which is usually up to three years.
+- The signing time is equal or later then commit time.
+
+NOTE: **Note:** There is no certificate revocation list check in place at the moment.
+
+## Obtaining an x509 key pair
+
+If your organization has Public Key Infrastructure (PKI), that PKI will provide
+an S/MIME key.
+
+If you do not have an S/MIME key pair from a PKI, you can either create your
+own self-signed one, or purchase one. MozillaZine keeps a nice collection
+of [S/MIME-capable signing authorities](http://kb.mozillazine.org/Getting_an_SMIME_certificate)
+and some of them generate keys for free.
+
+## Associating your x509 certificate with Git
+
+To take advantage of X509 signing, you will need Git 2.19.0 or later. You can
+check your Git version with:
+
+```sh
+git --version
+```
+
+If you have the correct version, you can proceed to configure Git.
+
+### Linux
+
+Configure Git to use your key for signing:
+
+```sh
+signingkey = $( gpgsm --list-secret-keys | egrep '(key usage|ID)' | grep -B 1 digitalSignature | awk '/ID/ {print $2}' )
+git config --global user.signingkey $signingkey
+git config --global gpg.format x509
+```
+
+### Windows and MacOS
+
+Install [smimesign](https://github.com/github/smimesign) by downloading the
+installer or via `brew install smimesign` on MacOS.
+
+Get the ID of your certificate with `smimesign --list-keys` and set your
+signingkey `git config --global user.signingkey ID`, then configure x509:
+
+```sh
+git config --global gpg.x509.program smimesign
+git config --global gpg.format x509
+```
+
+## Signing commits
+
+After you have [associated your x509 certificate with Git](#associating-your-x509-certificate-with-git) you
+can start signing your commits:
+
+1. Commit like you used to, the only difference is the addition of the `-S` flag:
+
+   ```sh
+   git commit -S -m "feat: x509 signed commits"
+   ```
+
+1. Push to GitLab and check that your commits [are verified](#verifying-commits).
+
+If you don't want to type the `-S` flag every time you commit, you can tell Git
+to sign your commits automatically:
+
+```sh
+git config --global commit.gpgsign true
+```
+
+## Verifying commits
+
+To verify that a commit is signed, you can use the `--show-signature` flag:
+
+```sh
+git log --show-signature
+```

lib/gitlab/database/x509_serial_number_attribute.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Database
+    # Class for casting binary data to int.
+    #
+    # Using X509SerialNumberAttribute allows you to store X509 certificate
+    # serial number values as binary while still using integer to access them.
+    # rfc 5280 - 4.1.2.2  Serial number (20 octets is the maximum), could be:
+    # - 1461501637330902918203684832716283019655932542975
+    # - 0xffffffffffffffffffffffffffffffffffffffff
+    class X509SerialNumberAttribute < ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Bytea
+      PACK_FORMAT = 'H*'
+
+      def deserialize(value)
+        value = super(value)
+        value ? value.unpack1(PACK_FORMAT).to_i : nil
+      end
+
+      def serialize(value)
+        arg = value ? [value.to_s].pack(PACK_FORMAT) : nil
+        super(arg)
+      end
+    end
+  end
+end

lib/gitlab/gpg/commit.rb

@@ -2,36 +2,9 @@
 
 module Gitlab
   module Gpg
-    class Commit
-      include Gitlab::Utils::StrongMemoize
-
-      def initialize(commit)
-        @commit = commit
-
-        repo = commit.container.repository.raw_repository
-        @signature_data = Gitlab::Git::Commit.extract_signature_lazily(repo, commit.sha || commit.id)
-
-        lazy_signature
-      end
-
-      def signature_text
-        strong_memoize(:signature_text) do
-          @signature_data&.itself && @signature_data[0] # rubocop:disable Lint/SafeNavigationConsistency
-        end
-      end
-
-      def signed_text
-        strong_memoize(:signed_text) do
-          @signature_data&.itself && @signature_data[1] # rubocop:disable Lint/SafeNavigationConsistency
-        end
-      end
-
-      def has_signature?
-        !!(signature_text && signed_text)
-      end
-
+    class Commit < Gitlab::SignedCommit
       def signature
-        return unless has_signature?
+        super
 
         return @signature if @signature
 

lib/gitlab/signed_commit.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class SignedCommit
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(commit)
+      @commit = commit
+
+      if commit.project
+        repo = commit.project.repository.raw_repository
+        @signature_data = Gitlab::Git::Commit.extract_signature_lazily(repo, commit.sha || commit.id)
+      end
+
+      lazy_signature
+    end
+
+    def signature
+      return unless @commit.has_signature?
+    end
+
+    def signature_text
+      strong_memoize(:signature_text) do
+        @signature_data.itself ? @signature_data[0] : nil
+      end
+    end
+
+    def signed_text
+      strong_memoize(:signed_text) do
+        @signature_data.itself ? @signature_data[1] : nil
+      end
+    end
+  end
+end

lib/gitlab/x509/commit.rb

+# frozen_string_literal: true
+require 'openssl'
+require 'digest'
+
+module Gitlab
+  module X509
+    class Commit < Gitlab::SignedCommit
+      def signature
+        super
+
+        return @signature if @signature
+
+        cached_signature = lazy_signature&.itself
+        return @signature = cached_signature if cached_signature.present?
+
+        @signature = create_cached_signature!
+      end
+
+      def update_signature!(cached_signature)
+        cached_signature.update!(attributes)
+        @signature = cached_signature
+      end
+
+      private
+
+      def lazy_signature
+        BatchLoader.for(@commit.sha).batch do |shas, loader|
+          X509CommitSignature.by_commit_sha(shas).each do |signature|
+            loader.call(signature.commit_sha, signature)
+          end
+        end
+      end
+
+      def verified_signature
+        strong_memoize(:verified_signature) { verified_signature? }
+      end
+
+      def cert
+        strong_memoize(:cert) do
+          signer_certificate(p7) if valid_signature?
+        end
+      end
+
+      def cert_store
+        strong_memoize(:cert_store) do
+          store = OpenSSL::X509::Store.new
+          store.set_default_paths
+          # valid_signing_time? checks the time attributes already
+          # this flag is required, otherwise expired certificates would become
+          # unverified when notAfter within certificate attribute is reached
+          store.flags = OpenSSL::X509::V_FLAG_NO_CHECK_TIME
+          store
+        end
+      end
+
+      def p7
+        strong_memoize(:p7) do
+          pkcs7_text = signature_text.sub('-----BEGIN SIGNED MESSAGE-----', '-----BEGIN PKCS7-----')
+          pkcs7_text = pkcs7_text.sub('-----END SIGNED MESSAGE-----', '-----END PKCS7-----')
+
+          OpenSSL::PKCS7.new(pkcs7_text)
+        rescue
+          nil
+        end
+      end
+
+      def valid_signing_time?
+        # rfc 5280 - 4.1.2.5  Validity
+        # check if signed_time is within the time range (notBefore/notAfter)
+        # non-rfc - git specific check: signed_time >= commit_time
+        p7.signers[0].signed_time.between?(cert.not_before, cert.not_after) &&
+          p7.signers[0].signed_time >= @commit.created_at
+      end
+
+      def valid_signature?
+        p7.verify([], cert_store, signed_text, OpenSSL::PKCS7::NOVERIFY)
+      rescue
+        nil
+      end
+
+      def verified_signature?
+        # verify has multiple options but only a boolean return value
+        # so first verify without certificate chain
+        if valid_signature?
+          if valid_signing_time?
+            # verify with system certificate chain
+            p7.verify([], cert_store, signed_text)
+          else
+            false
+          end
+        else
+          nil
+        end
+      rescue
+        nil
+      end
+
+      def signer_certificate(p7)
+        p7.certificates.each do |cert|
+          next if cert.serial != p7.signers[0].serial
+
+          return cert
+        end
+      end
+
+      def certificate_crl
+        extension = get_certificate_extension('crlDistributionPoints')
+        extension.split('URI:').each do |item|
+          item.strip
+
+          if item.start_with?("http")
+            return item.strip
+          end
+        end
+      end
+
+      def get_certificate_extension(extension)
+        cert.extensions.each do |ext|
+          if ext.oid == extension
+            return ext.value
+          end
+        end
+      end
+
+      def issuer_subject_key_identifier
+        get_certificate_extension('authorityKeyIdentifier').gsub("keyid:", "").delete!("\n")
+      end
+
+      def certificate_subject_key_identifier
+        get_certificate_extension('subjectKeyIdentifier')
+      end
+
+      def certificate_issuer
+        cert.issuer.to_s(OpenSSL::X509::Name::RFC2253)
+      end
+
+      def certificate_subject
+        cert.subject.to_s(OpenSSL::X509::Name::RFC2253)
+      end
+
+      def certificate_email
+        get_certificate_extension('subjectAltName').split('email:')[1]
+      end
+
+      def issuer_attributes
+        return if verified_signature.nil?
+
+        {
+          subject_key_identifier: issuer_subject_key_identifier,
+          subject: certificate_issuer,
+          crl_url: certificate_crl
+        }
+      end
+
+      def certificate_attributes
+        return if verified_signature.nil?
+
+        issuer = X509Issuer.safe_create!(issuer_attributes)
+
+        {
+          subject_key_identifier: certificate_subject_key_identifier,
+          subject: certificate_subject,
+          email: certificate_email,
+          serial_number: cert.serial,
+          x509_issuer_id: issuer.id
+        }
+      end
+
+      def attributes
+        return if verified_signature.nil?
+
+        certificate = X509Certificate.safe_create!(certificate_attributes)
+
+        {
+          commit_sha: @commit.sha,
+          project: @commit.project,
+          x509_certificate_id: certificate.id,
+          verification_status: verification_status
+        }
+      end
+
+      def verification_status
+        if verified_signature && certificate_email == @commit.committer_email
+          :verified
+        else
+          :unverified
+        end
+      end
+
+      def create_cached_signature!
+        return if verified_signature.nil?
+
+        return X509CommitSignature.new(attributes) if Gitlab::Database.read_only?
+
+        X509CommitSignature.safe_create!(attributes)
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -3194,6 +3194,12 @@ msgstr ""
 msgid "Certificate (PEM)"
 msgstr ""
 
+msgid "Certificate Issuer"
+msgstr ""
+
+msgid "Certificate Subject"
+msgstr ""
+
 msgid "Change assignee"
 msgstr ""
 
@@ -11130,6 +11136,9 @@ msgstr ""
 msgid "Learn more about the dependency list"
 msgstr ""
 
+msgid "Learn more about x509 signed commits"
+msgstr ""
+
 msgid "Learn more in the"
 msgstr ""
 
@@ -18173,6 +18182,9 @@ msgstr ""
 msgid "Subgroups and projects"
 msgstr ""
 
+msgid "Subject Key Identifier:"
+msgstr ""
+
 msgid "Subkeys"
 msgstr ""
 

spec/controllers/projects/compare_controller_spec.rb

@@ -281,7 +281,7 @@ describe Projects::CompareController do
       context 'when the user has access to the project' do
         render_views
 
-        let(:signature_commit) { build(:commit, project: project, safe_message: "message", sha: 'signature_commit') }
+        let(:signature_commit) { project.commit_by(oid: '0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
         let(:non_signature_commit) { build(:commit, project: project, safe_message: "message", sha: 'non_signature_commit') }
 
         before do

spec/factories/x509_certificate.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :x509_certificate do
+    subject_key_identifier { 'BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC:BC' }
+    subject { 'CN=gitlab@example.org,OU=Example,O=World' }
+
+    email { 'gitlab@example.org' }
+    serial_number { 278969561018901340486471282831158785578 }
+    x509_issuer
+  end
+end

spec/factories/x509_commit_signature.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :x509_commit_signature do
+    commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+    project
+    x509_certificate
+    verification_status { :verified }
+  end
+end

spec/factories/x509_issuer.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :x509_issuer do
+    subject_key_identifier { 'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB' }
+    subject { 'CN=PKI,OU=Example,O=World' }
+
+    crl_url { 'http://example.com/pki.crl' }
+  end
+end

spec/lib/gitlab/x509/commit_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::X509::Commit do
+  describe '#signature' do
+    let(:signature) { described_class.new(commit).signature }
+
+    let(:user1_certificate_attributes) do
+      {
+        subject_key_identifier: X509Helpers::User1.certificate_subject_key_identifier,
+        subject: X509Helpers::User1.certificate_subject,
+        email: X509Helpers::User1.certificate_email,
+        serial_number: X509Helpers::User1.certificate_serial
+      }
+    end
+
+    let(:user1_issuer_attributes) do
+      {
+        subject_key_identifier: X509Helpers::User1.issuer_subject_key_identifier,
+        subject: X509Helpers::User1.certificate_issuer,
+        crl_url: X509Helpers::User1.certificate_crl
+      }
+    end
+
+    shared_examples 'returns the cached signature on second call' do
+      it 'returns the cached signature on second call' do
+        x509_commit = described_class.new(commit)
+
+        expect(x509_commit).to receive(:create_cached_signature).and_call_original
+        signature
+
+        # consecutive call
+        expect(x509_commit).not_to receive(:create_cached_signature).and_call_original
+        signature
+      end
+    end
+
+    let!(:project) { create :project, :repository, path: X509Helpers::User1.path }
+    let!(:commit_sha) { X509Helpers::User1.commit }
+
+    context 'unsigned commit' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha }
+
+      it 'returns nil' do
+        expect(described_class.new(commit).signature).to be_nil
+      end
+    end
+
+    context 'valid signature from known user' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first }
+
+      let!(:user) { create(:user, email: X509Helpers::User1.emails.first) }
+
+      before do
+        allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                X509Helpers::User1.signed_commit_signature,
+                X509Helpers::User1.signed_commit_base_data
+              ]
+            )
+      end
+
+      it 'returns an unverified signature' do
+        expect(signature).to have_attributes(
+          commit_sha: commit_sha,
+          project: project,
+          verification_status: 'unverified'
+        )
+        expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes)
+        expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
+        expect(signature.persisted?).to be_truthy
+      end
+    end
+
+    context 'verified signature from known user' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first }
+
+      let!(:user) { create(:user, email: X509Helpers::User1.emails.first) }
+
+      before do
+        allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                X509Helpers::User1.signed_commit_signature,
+                X509Helpers::User1.signed_commit_base_data
+              ]
+            )
+      end
+
+      context 'with trusted certificate store' do
+        before do
+          store = OpenSSL::X509::Store.new
+          certificate = OpenSSL::X509::Certificate.new X509Helpers::User1.trust_cert
+          store.add_cert(certificate)
+          allow(OpenSSL::X509::Store).to receive(:new)
+              .and_return(
+                store
+              )
+        end
+
+        it 'returns a verified signature' do
+          expect(signature).to have_attributes(
+            commit_sha: commit_sha,
+            project: project,
+            verification_status: 'verified'
+          )
+          expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes)
+          expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
+          expect(signature.persisted?).to be_truthy
+        end
+      end
+
+      context 'without trusted certificate within store' do
+        before do
+          store = OpenSSL::X509::Store.new
+          allow(OpenSSL::X509::Store).to receive(:new)
+              .and_return(
+                store
+              )
+        end
+
+        it 'returns an unverified signature' do
+          expect(signature).to have_attributes(
+            commit_sha: commit_sha,
+            project: project,
+            verification_status: 'unverified'
+          )
+          expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes)
+          expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
+          expect(signature.persisted?).to be_truthy
+        end
+      end
+    end
+
+    context 'unverified signature from unknown user' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first }
+
+      before do
+        allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                X509Helpers::User1.signed_commit_signature,
+                X509Helpers::User1.signed_commit_base_data
+              ]
+            )
+      end
+
+      it 'returns an unverified signature' do
+        expect(signature).to have_attributes(
+          commit_sha: commit_sha,
+          project: project,
+          verification_status: 'unverified'
+        )
+        expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes)
+        expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes)
+        expect(signature.persisted?).to be_truthy
+      end
+    end
+
+    context 'invalid signature' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: X509Helpers::User1.emails.first }
+
+      let!(:user) { create(:user, email: X509Helpers::User1.emails.first) }
+
+      before do
+        allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                # Corrupt the key
+                X509Helpers::User1.signed_commit_signature.tr('A', 'B'),
+                X509Helpers::User1.signed_commit_base_data
+              ]
+            )
+      end
+
+      it 'returns nil' do
+        expect(described_class.new(commit).signature).to be_nil
+      end
+    end
+
+    context 'invalid commit message' do
+      let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: X509Helpers::User1.emails.first }
+
+      let!(:user) { create(:user, email: X509Helpers::User1.emails.first) }
+
+      before do
+        allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                X509Helpers::User1.signed_commit_signature,
+                # Corrupt the commit message
+                'x'
+              ]
+            )
+      end
+
+      it 'returns nil' do
+        expect(described_class.new(commit).signature).to be_nil
+      end
+    end
+  end
+end

spec/migrations/migrate_create_commit_signature_worker_sidekiq_queue_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200206091544_migrate_create_commit_signature_worker_sidekiq_queue.rb')
+
+describe MigrateCreateCommitSignatureWorkerSidekiqQueue, :sidekiq, :redis do
+  include Gitlab::Database::MigrationHelpers
+  include StubWorker
+
+  context 'when there are jobs in the queue' do
+    it 'correctly migrates queue when migrating up' do
+      Sidekiq::Testing.disable! do
+        stub_worker(queue: 'create_commit_signature').perform_async('Something', [1])
+        stub_worker(queue: 'create_gpg_signature').perform_async('Something', [1])
+
+        described_class.new.up
+
+        expect(sidekiq_queue_length('create_gpg_signature')).to eq 0
+        expect(sidekiq_queue_length('create_commit_signature')).to eq 2
+      end
+    end
+
+    it 'correctly migrates queue when migrating down' do
+      Sidekiq::Testing.disable! do
+        stub_worker(queue: 'create_gpg_signature').perform_async('Something', [1])
+
+        described_class.new.down
+
+        expect(sidekiq_queue_length('create_gpg_signature')).to eq 1
+        expect(sidekiq_queue_length('create_commit_signature')).to eq 0
+      end
+    end
+  end
+
+  context 'when there are no jobs in the queues' do
+    it 'does not raise error when migrating up' do
+      expect { described_class.new.up }.not_to raise_error
+    end
+
+    it 'does not raise error when migrating down' do
+      expect { described_class.new.down }.not_to raise_error
+    end
+  end
+end

spec/models/commit_spec.rb

@@ -671,4 +671,25 @@ eos
       expect(commit2.merge_requests).to contain_exactly(merge_request1)
     end
   end
+
+  describe 'signed commits' do
+    let(:gpg_signed_commit) { project.commit_by(oid: '0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
+    let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') }
+    let(:unsigned_commit) { project.commit_by(oid: '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51') }
+    let!(:commit) { create(:commit, project: project) }
+
+    it 'returns signature_type properly' do
+      expect(gpg_signed_commit.signature_type).to eq(:PGP)
+      expect(x509_signed_commit.signature_type).to eq(:X509)
+      expect(unsigned_commit.signature_type).to eq(:NONE)
+      expect(commit.signature_type).to eq(:NONE)
+    end
+
+    it 'returns has_signature? properly' do
+      expect(gpg_signed_commit.has_signature?).to be_truthy
+      expect(x509_signed_commit.has_signature?).to be_truthy
+      expect(unsigned_commit.has_signature?).to be_falsey
+      expect(commit.has_signature?).to be_falsey
+    end
+  end
 end

spec/models/concerns/x509_serial_number_attribute_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe X509SerialNumberAttribute do
+  let(:model) { Class.new { include X509SerialNumberAttribute } }
+
+  before do
+    columns = [
+      double(:column, name: 'name', type: :text),
+      double(:column, name: 'serial_number', type: :binary)
+    ]
+
+    allow(model).to receive(:columns).and_return(columns)
+  end
+
+  describe '#x509_serial_number_attribute' do
+    context 'when in non-production' do
+      before do
+        stub_rails_env('development')
+      end
+
+      context 'when the table exists' do
+        before do
+          allow(model).to receive(:table_exists?).and_return(true)
+        end
+
+        it 'defines a x509 serial number attribute for a binary column' do
+          expect(model).to receive(:attribute)
+            .with(:serial_number, an_instance_of(Gitlab::Database::X509SerialNumberAttribute))
+
+          model.x509_serial_number_attribute(:serial_number)
+        end
+
+        it 'raises ArgumentError when the column type is not :binary' do
+          expect { model.x509_serial_number_attribute(:name) }.to raise_error(ArgumentError)
+        end
+      end
+
+      context 'when the table does not exist' do
+        it 'allows the attribute to be added and issues a warning' do
+          allow(model).to receive(:table_exists?).and_return(false)
+
+          expect(model).not_to receive(:columns)
+          expect(model).to receive(:attribute)
+          expect(model).to receive(:warn)
+
+          model.x509_serial_number_attribute(:name)
+        end
+      end
+
+      context 'when the column does not exist' do
+        it 'allows the attribute to be added and issues a warning' do
+          allow(model).to receive(:table_exists?).and_return(true)
+
+          expect(model).to receive(:columns)
+          expect(model).to receive(:attribute)
+          expect(model).to receive(:warn)
+
+          model.x509_serial_number_attribute(:no_name)
+        end
+      end
+
+      context 'when other execeptions are raised' do
+        it 'logs and re-rasises the error' do
+          allow(model).to receive(:table_exists?).and_raise(ActiveRecord::NoDatabaseError.new('does not exist'))
+
+          expect(model).not_to receive(:columns)
+          expect(model).not_to receive(:attribute)
+          expect(Gitlab::AppLogger).to receive(:error)
+
+          expect { model.x509_serial_number_attribute(:name) }.to raise_error(ActiveRecord::NoDatabaseError)
+        end
+      end
+    end
+
+    context 'when in production' do
+      before do
+        stub_rails_env('production')
+      end
+
+      it 'defines a x509 serial number attribute' do
+        expect(model).not_to receive(:table_exists?)
+        expect(model).not_to receive(:columns)
+        expect(model).to receive(:attribute).with(:serial_number, an_instance_of(Gitlab::Database::X509SerialNumberAttribute))
+
+        model.x509_serial_number_attribute(:serial_number)
+      end
+    end
+  end
+end

spec/models/x509_certificate_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe X509Certificate do
+  describe 'validation' do
+    it { is_expected.to validate_presence_of(:subject_key_identifier) }
+    it { is_expected.to validate_presence_of(:subject) }
+    it { is_expected.to validate_presence_of(:email) }
+    it { is_expected.to validate_presence_of(:serial_number) }
+    it { is_expected.to validate_presence_of(:x509_issuer_id) }
+  end
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:x509_issuer).required }
+  end
+
+  describe '.safe_create!' do
+    let(:subject_key_identifier) { 'CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD' }
+    let(:subject) { 'CN=gitlab@example.com,OU=Example,O=World' }
+    let(:email) { 'gitlab@example.com' }
+    let(:serial_number) { '123456789' }
+    let(:issuer) { create(:x509_issuer) }
+
+    let(:attributes) do
+      {
+        subject_key_identifier: subject_key_identifier,
+        subject: subject,
+        email: email,
+        serial_number: serial_number,
+        x509_issuer_id: issuer.id
+      }
+    end
+
+    it 'creates a new certificate if it was not found' do
+      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
+    end
+
+    it 'assigns the correct attributes when creating' do
+      certificate = described_class.safe_create!(attributes)
+
+      expect(certificate.subject_key_identifier).to eq(subject_key_identifier)
+      expect(certificate.subject).to eq(subject)
+      expect(certificate.email).to eq(email)
+    end
+  end
+
+  describe 'validators' do
+    it 'accepts correct subject_key_identifier' do
+      subject_key_identifiers = [
+        'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB',
+        'CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD'
+      ]
+
+      subject_key_identifiers.each do |identifier|
+        expect(build(:x509_certificate, subject_key_identifier: identifier)).to be_valid
+      end
+    end
+
+    it 'rejects invalid subject_key_identifier' do
+      subject_key_identifiers = [
+        'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB',
+        'CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:GG',
+        'random string',
+        '12321342545356434523412341245452345623453542345234523453245'
+      ]
+
+      subject_key_identifiers.each do |identifier|
+        expect(build(:x509_certificate, subject_key_identifier: identifier)).to be_invalid
+      end
+    end
+
+    it 'accepts correct email address' do
+      emails = [
+        'smime@example.org',
+        'smime@example.com'
+      ]
+
+      emails.each do |email|
+        expect(build(:x509_certificate, email: email)).to be_valid
+      end
+    end
+
+    it 'rejects invalid email' do
+      emails = [
+        'this is not an email',
+        '@example.org'
+      ]
+
+      emails.each do |email|
+        expect(build(:x509_certificate, email: email)).to be_invalid
+      end
+    end
+
+    it 'accepts valid serial_number' do
+      expect(build(:x509_certificate, serial_number: 123412341234)).to be_valid
+
+      # rfc 5280 - 4.1.2.2  Serial number (20 octets is the maximum)
+      expect(build(:x509_certificate, serial_number: 1461501637330902918203684832716283019655932542975)).to be_valid
+      expect(build(:x509_certificate, serial_number: 'ffffffffffffffffffffffffffffffffffffffff'.to_i(16))).to be_valid
+    end
+
+    it 'rejects invalid serial_number' do
+      expect(build(:x509_certificate, serial_number: "sgsgfsdgdsfg")).to be_invalid
+    end
+  end
+end

spec/models/x509_commit_signature_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe X509CommitSignature do
+  let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' }
+  let(:project) { create(:project, :public, :repository) }
+  let!(:commit) { create(:commit, project: project, sha: commit_sha) }
+  let(:x509_certificate) { create(:x509_certificate) }
+  let(:x509_signature) { create(:x509_commit_signature, commit_sha: commit_sha) }
+
+  it_behaves_like 'having unique enum values'
+
+  describe 'validation' do
+    it { is_expected.to validate_presence_of(:commit_sha) }
+    it { is_expected.to validate_presence_of(:project_id) }
+    it { is_expected.to validate_presence_of(:x509_certificate_id) }
+  end
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:project).required }
+    it { is_expected.to belong_to(:x509_certificate).required }
+  end
+
+  describe '.safe_create!' do
+    let(:attributes) do
+      {
+        commit_sha: commit_sha,
+        project: project,
+        x509_certificate_id: x509_certificate.id,
+        verification_status: "verified"
+      }
+    end
+
+    it 'finds a signature by commit sha if it existed' do
+      x509_signature
+
+      expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(x509_signature)
+    end
+
+    it 'creates a new signature if it was not found' do
+      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
+    end
+
+    it 'assigns the correct attributes when creating' do
+      signature = described_class.safe_create!(attributes)
+
+      expect(signature.project).to eq(project)
+      expect(signature.commit_sha).to eq(commit_sha)
+      expect(signature.x509_certificate_id).to eq(x509_certificate.id)
+    end
+  end
+end

spec/models/x509_issuer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe X509Issuer do
+  describe 'validation' do
+    it { is_expected.to validate_presence_of(:subject_key_identifier) }
+    it { is_expected.to validate_presence_of(:subject) }
+    it { is_expected.to validate_presence_of(:crl_url) }
+  end
+
+  describe '.safe_create!' do
+    let(:issuer_subject_key_identifier) { 'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB' }
+    let(:issuer_subject) { 'CN=PKI,OU=Example,O=World' }
+    let(:issuer_crl_url) { 'http://example.com/pki.crl' }
+
+    let(:attributes) do
+      {
+        subject_key_identifier: issuer_subject_key_identifier,
+        subject: issuer_subject,
+        crl_url: issuer_crl_url
+      }
+    end
+
+    it 'creates a new issuer if it was not found' do
+      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
+    end
+
+    it 'assigns the correct attributes when creating' do
+      issuer = described_class.safe_create!(attributes)
+
+      expect(issuer.subject_key_identifier).to eq(issuer_subject_key_identifier)
+      expect(issuer.subject).to eq(issuer_subject)
+      expect(issuer.crl_url).to eq(issuer_crl_url)
+    end
+  end
+
+  describe 'validators' do
+    it 'accepts correct subject_key_identifier' do
+      subject_key_identifiers = [
+        'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB',
+        'CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD'
+      ]
+
+      subject_key_identifiers.each do |identifier|
+        expect(build(:x509_issuer, subject_key_identifier: identifier)).to be_valid
+      end
+    end
+
+    it 'rejects invalid subject_key_identifier' do
+      subject_key_identifiers = [
+        'AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB:AB',
+        'CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:CD:GG',
+        'random string',
+        '12321342545356434523412341245452345623453542345234523453245'
+      ]
+
+      subject_key_identifiers.each do |identifier|
+        expect(build(:x509_issuer, subject_key_identifier: identifier)).to be_invalid
+      end
+    end
+
+    it 'accepts valid crl_url' do
+      expect(build(:x509_issuer, crl_url: "https://pki.example.org")).to be_valid
+    end
+
+    it 'rejects invalid crl_url' do
+      expect(build(:x509_issuer, crl_url: "ht://pki.example.org")).to be_invalid
+    end
+  end
+end

spec/services/git/branch_hooks_service_spec.rb

@@ -214,23 +214,23 @@ describe Git::BranchHooksService do
     end
   end
 
-  describe 'GPG signatures' do
+  describe 'signatures' do
     context 'when the commit has a signature' do
       context 'when the signature is already cached' do
         before do
           create(:gpg_signature, commit_sha: commit.id)
         end
 
-        it 'does not queue a CreateGpgSignatureWorker' do
-          expect(CreateGpgSignatureWorker).not_to receive(:perform_async)
+        it 'does not queue a CreateCommitSignatureWorker' do
+          expect(CreateCommitSignatureWorker).not_to receive(:perform_async)
 
           service.execute
         end
       end
 
       context 'when the signature is not yet cached' do
-        it 'queues a CreateGpgSignatureWorker' do
-          expect(CreateGpgSignatureWorker).to receive(:perform_async).with([commit.id], project.id)
+        it 'queues a CreateCommitSignatureWorker' do
+          expect(CreateCommitSignatureWorker).to receive(:perform_async).with([commit.id], project.id)
 
           service.execute
         end
@@ -240,7 +240,7 @@ describe Git::BranchHooksService do
             .to receive(:shas_with_signatures)
             .and_return([sample_commit.id, another_sample_commit.id])
 
-          expect(CreateGpgSignatureWorker)
+          expect(CreateCommitSignatureWorker)
             .to receive(:perform_async)
             .with([sample_commit.id, another_sample_commit.id], project.id)
 
@@ -257,8 +257,8 @@ describe Git::BranchHooksService do
           .and_return([])
       end
 
-      it 'does not queue a CreateGpgSignatureWorker' do
-        expect(CreateGpgSignatureWorker)
+      it 'does not queue a CreateCommitSignatureWorker' do
+        expect(CreateCommitSignatureWorker)
           .not_to receive(:perform_async)
           .with(sample_commit.id, project.id)
 

spec/workers/create_gpg_signature_worker_spec.rb → spec/workers/create_commit_signature_worker_spec.rb

@@ -2,13 +2,14 @@
 
 require 'spec_helper'
 
-describe CreateGpgSignatureWorker do
+describe CreateCommitSignatureWorker do
   let(:project) { create(:project, :repository) }
   let(:commits) { project.repository.commits('HEAD', limit: 3).commits }
   let(:commit_shas) { commits.map(&:id) }
   let(:gpg_commit) { instance_double(Gitlab::Gpg::Commit) }
+  let(:x509_commit) { instance_double(Gitlab::X509::Commit) }
 
-  context 'when GpgKey is found' do
+  context 'when a signature is found' do
     before do
       allow(Project).to receive(:find_by).with(id: project.id).and_return(project)
       allow(project).to receive(:commits_by).with(oids: commit_shas).and_return(commits)
@@ -18,6 +19,7 @@ describe CreateGpgSignatureWorker do
 
     it 'calls Gitlab::Gpg::Commit#signature' do
       commits.each do |commit|
+        allow(commit).to receive(:signature_type).and_return(:PGP)
         expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit).once
       end
 
@@ -31,13 +33,46 @@ describe CreateGpgSignatureWorker do
       allow(Gitlab::Gpg::Commit).to receive(:new).and_return(gpg_commit)
       allow(Gitlab::Gpg::Commit).to receive(:new).with(commits.first).and_raise(StandardError)
 
+      allow(commits[1]).to receive(:signature_type).and_return(:PGP)
+      allow(commits[2]).to receive(:signature_type).and_return(:PGP)
+
       expect(gpg_commit).to receive(:signature).twice
 
       subject
     end
+
+    it 'calls Gitlab::X509::Commit#signature' do
+      commits.each do |commit|
+        allow(commit).to receive(:signature_type).and_return(:X509)
+        expect(Gitlab::X509::Commit).to receive(:new).with(commit).and_return(x509_commit).once
+      end
+
+      expect(x509_commit).to receive(:signature).exactly(commits.size).times
+
+      subject
+    end
+
+    it 'can recover from exception and continue the X509 signature process' do
+      allow(x509_commit).to receive(:signature)
+      allow(Gitlab::X509::Commit).to receive(:new).and_return(x509_commit)
+      allow(Gitlab::X509::Commit).to receive(:new).with(commits.first).and_raise(StandardError)
+
+      allow(commits[1]).to receive(:signature_type).and_return(:X509)
+      allow(commits[2]).to receive(:signature_type).and_return(:X509)
+
+      expect(x509_commit).to receive(:signature).twice
+
+      subject
+    end
   end
 
   context 'handles when a string is passed in for the commit SHA' do
+    before do
+      allow(Project).to receive(:find_by).with(id: project.id).and_return(project)
+      allow(project).to receive(:commits_by).with(oids: Array(commit_shas.first)).and_return(commits)
+      allow(commits.first).to receive(:signature_type).and_return(:PGP)
+    end
+
     it 'creates a signature once' do
       allow(Gitlab::Gpg::Commit).to receive(:new).with(commits.first).and_return(gpg_commit)
 
@@ -67,5 +102,11 @@ describe CreateGpgSignatureWorker do
 
       described_class.new.perform(commit_shas, nonexisting_project_id)
     end
+
+    it 'does not call Gitlab::X509::Commit#signature' do
+      expect_any_instance_of(Gitlab::X509::Commit).not_to receive(:signature)
+
+      described_class.new.perform(commit_shas, nonexisting_project_id)
+    end
   end
 end