From 529f33932f3541cce5e48085ff921534d054e2d9 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Fri, 8 Aug 2025 16:40:58 -0400 Subject: [PATCH 1/4] Extract a Signature class from Gpg::Commit --- lib/gitlab/gpg/signature.rb | 116 +++++++++++++++ spec/lib/gitlab/gpg/signature_spec.rb | 207 ++++++++++++++++++++++++++ 2 files changed, 323 insertions(+) create mode 100644 lib/gitlab/gpg/signature.rb create mode 100644 spec/lib/gitlab/gpg/signature_spec.rb diff --git a/lib/gitlab/gpg/signature.rb b/lib/gitlab/gpg/signature.rb new file mode 100644 index 00000000000000..14ff27ffc49709 --- /dev/null +++ b/lib/gitlab/gpg/signature.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +module Gitlab + module Gpg + class Signature + include Gitlab::Utils::StrongMemoize + include SignatureType + + def initialize(signature_text, signed_text, signer, email) + @signature_text = signature_text + @signed_text = signed_text + @signer = signer + @email = email + end + + attr_reader :signature_text, :signed_text, :email, :signer + + def type + :gpg + end + + def verification_status + using_keychain do + break :verified_system if verified_by_gitlab? + break :multiple_signatures if multiple_signatures? + break :unknown_key unless gpg_key + break :unverified_key unless gpg_key.verified? + break :unverified unless verified_signature&.valid? + + if gpg_key.verified_and_belongs_to_email?(email) + :verified + elsif gpg_key.user.all_emails.include?(email) + :same_user_different_email + else + :other_user + end + end + end + + def gpg_key_primary_keyid + gpg_key&.keyid || fingerprint + end + + def gpg_key + return unless fingerprint + + find_gpg_key(fingerprint) + end + strong_memoize_attr :gpg_key + + def fingerprint + verified_signature&.fingerprint + end + + private + + def using_keychain + Gitlab::Gpg.using_tmp_keychain do + # first we need to get the fingerprint from the signature to query the gpg + # key belonging to the fingerprint. + # This way we can add the key to the temporary keychain and extract + # the proper signature. + # NOTE: the invoked method is #fingerprint but versions of GnuPG + # prior to 2.2.13 return 16 characters (the format used by keyid) + # instead of 40. + break unless fingerprint + + if gpg_key + Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) + clear_memoization(:gpg_signatures) + end + + yield gpg_key + end + end + + def verified_signature + gpg_signatures.first + end + + # If a commit is signed by Gitaly, the Gitaly returns `SIGNER_SYSTEM` as a signer + # In order to calculate it, the signature is Verified using the Gitaly's public key: + # https://gitlab.com/gitlab-org/gitaly/-/blob/v16.2.0-rc2/internal/gitaly/service/commit/commit_signatures.go#L63 + # + # It is safe to skip verification step if the commit has been signed by Gitaly + def verified_by_gitlab? + signer == :SIGNER_SYSTEM + end + + def multiple_signatures? + gpg_signatures.size > 1 + end + + def gpg_signatures + signatures = [] + + GPGME::Crypto.new.verify(signature_text, signed_text: signed_text) do |verified_signature| + signatures << verified_signature + end + + signatures + rescue GPGME::Error + [] + end + strong_memoize_attr :gpg_signatures + + def find_gpg_key(fingerprint) + if fingerprint.length > 16 + GpgKey.find_by_fingerprint(fingerprint) || GpgKeySubkey.find_by_fingerprint(fingerprint) + else + GpgKey.find_by_primary_keyid(fingerprint) || GpgKeySubkey.find_by_keyid(fingerprint) + end + end + end + end +end diff --git a/spec/lib/gitlab/gpg/signature_spec.rb b/spec/lib/gitlab/gpg/signature_spec.rb new file mode 100644 index 00000000000000..265af2e4a669f9 --- /dev/null +++ b/spec/lib/gitlab/gpg/signature_spec.rb @@ -0,0 +1,207 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Gpg::Signature, feature_category: :source_code_management do + let_it_be(:project) { create(:project, :repository, path: 'sample-project') } + + let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let(:committer_email) { GpgHelpers::User1.emails.first } + let(:user_email) { committer_email } + let(:signature) { GpgHelpers::User1.signed_commit_signature } + let(:signed_text) { GpgHelpers::User1.signed_commit_base_data } + let(:signer) { :SIGNER_USER } + let(:crypto) { instance_double(GPGME::Crypto) } + let(:user) { create(:user, email: user_email) } + let!(:gpg_key) { create(:gpg_key, key: public_key, user: user) } + let(:public_key) { GpgHelpers::User1.public_key } + let(:commit) { create(:commit, project: project, sha: commit_sha, committer_email: committer_email) } + let(:gpg_signature) { described_class.new(signature, signed_text, signer, committer_email) } + + it_behaves_like 'signature with type checking', :gpg do + subject { gpg_signature } + end + + describe '#verification_status' do + subject { gpg_signature.verification_status } + + context 'when the fingerprint in the signature matches a key belonging to a matching user' do + it { is_expected.to eq(:verified) } + end + + context 'when the signature is invalid' do + let(:signature) { GpgHelpers::User1.signed_commit_signature.tr('=', 'a') } + + it { is_expected.to be_nil } + end + + context 'when commit has multiple signatures' do + before do + verified_signature = instance_double(GPGME::Signature, fingerprint: GpgHelpers::User1.fingerprint, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(instance_double(GPGME::Signature)) + end + + it { is_expected.to eq(:multiple_signatures) } + end + + context 'when commit signed with a subkey' do + let(:committer_email) { GpgHelpers::User3.emails.first } + let(:public_key) { GpgHelpers::User3.public_key } + let(:signature) { GpgHelpers::User3.signed_commit_signature } + let(:signed_text) { GpgHelpers::User3.signed_commit_base_data } + + it { is_expected.to eq(:verified) } + end + + context 'when gpg key email does not match the committer_email but is the same user when the committer_email \ + belongs to the user as a confirmed secondary email' do + let(:committer_email) { GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, :confirmed, user: user, email: committer_email + end + end + + it { is_expected.to eq(:same_user_different_email) } + end + + context 'when gpg key email does not match the committer_email when the committer_email belongs to the user \ + as a unconfirmed secondary email' do + let(:committer_email) { GpgHelpers::User2.emails.first } + + let(:user) do + create(:user, email: GpgHelpers::User1.emails.first).tap do |user| + create :email, user: user, email: committer_email + end + end + + it { is_expected.to eq(:other_user) } + end + + context 'when user email does not match the committer email' do + let(:committer_email) { GpgHelpers::User2.emails.first } + let(:user_email) { GpgHelpers::User1.emails.first } + + it { is_expected.to eq(:other_user) } + end + + context 'when user does not match the key uid' do + let(:user_email) { GpgHelpers::User2.emails.first } + let(:public_key) { GpgHelpers::User1.public_key } + + it { is_expected.to eq(:unverified_key) } + end + + context 'when there is no matching gpg key' do + let(:gpg_key) { nil } + + it { is_expected.to eq(:unknown_key) } + end + + context 'when signature created by GitLab' do + let(:signer) { :SIGNER_SYSTEM } + let(:gpg_key) { nil } + + it { is_expected.to eq(:verified_system) } + end + end + + describe '#gpg_key' do + subject { gpg_signature.gpg_key } + + context 'when a valid key signed using recent version of Gnupg' do + before do + verified_signature = instance_double(GPGME::Signature, fingerprint: GpgHelpers::User1.fingerprint, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature) + end + + it { is_expected.to eq(gpg_key) } + end + + context 'when a valid key signed using older version of Gnupg' do + before do + keyid = GpgHelpers::User1.fingerprint.last(16) + verified_signature = instance_double(GPGME::Signature, fingerprint: keyid, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature) + end + + it { is_expected.to eq(gpg_key) } + end + + context 'when commit has multiple signatures' do + before do + verified_signature = instance_double(GPGME::Signature, fingerprint: GpgHelpers::User1.fingerprint, valid?: true) + allow(GPGME::Crypto).to receive(:new).and_return(crypto) + allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(instance_double(GPGME::Signature)) + end + + it 'returns the key matching the first signature' do + is_expected.to eq(gpg_key) + end + end + + context 'when commit signed with a subkey' do + let(:committer_email) { GpgHelpers::User3.emails.first } + let(:public_key) { GpgHelpers::User3.public_key } + let(:signature) { GpgHelpers::User3.signed_commit_signature } + let(:signed_text) { GpgHelpers::User3.signed_commit_base_data } + + let(:gpg_key_subkey) do + gpg_key.subkeys.find_by(fingerprint: GpgHelpers::User3.subkey_fingerprints.last) + end + + it { is_expected.to eq(gpg_key_subkey) } + end + + context 'when there is no matching gpg key' do + let(:gpg_key) { nil } + + it { is_expected.to be_nil } + end + + context 'when signature created by GitLab' do + let(:signer) { :SIGNER_SYSTEM } + let(:gpg_key) { nil } + + it { is_expected.to be_nil } + end + end + + describe '#gpg_key_primary_keyid' do + subject(:gpg_key_primary_keyid) { gpg_signature.gpg_key_primary_keyid } + + context 'when a gpg key exists' do + it { is_expected.to eq(gpg_key.keyid) } + end + + context 'when a no gpg key does not exist' do + let(:gpg_key) { nil } + + it 'returns fingerprint' do + allow(gpg_signature).to receive(:fingerprint).and_return('stubbed_fingerprint') + + expect(gpg_key_primary_keyid).to eq('stubbed_fingerprint') + end + end + end + + describe '#fingerprint' do + subject(:fingerprint) { gpg_signature.fingerprint } + + let(:signature_double) { instance_double(GPGME::Signature, fingerprint: 'stubbed_fingerprint') } + + it 'delegates to signature' do + expect_next_instance_of(GPGME::Crypto) do |instance| + expect(instance).to receive(:verify) + .with(signature, signed_text: signed_text) + .and_yield(signature_double) + end + + expect(fingerprint).to eq('stubbed_fingerprint') + end + end +end -- GitLab From 2e01c831cd4052776cab78369f9415cdc8b198f0 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Fri, 8 Aug 2025 16:46:21 -0400 Subject: [PATCH 2/4] Render verification status for gpg signed tags --- ...ender_gpg_signed_tags_verification_status.yml | 10 ++++++++++ lib/gitlab/git/tag.rb | 4 +++- lib/gitlab/gpg/tag.rb | 16 ++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml create mode 100644 lib/gitlab/gpg/tag.rb diff --git a/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml b/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml new file mode 100644 index 00000000000000..bc6d4e5143a8a8 --- /dev/null +++ b/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml @@ -0,0 +1,10 @@ +--- +name: render_gpg_signed_tags_verification_status +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/19260 +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560619 +milestone: '18.3' +group: group::source code +type: beta +default_enabled: false diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb index ab6c3c536e62df..95255c689aea05 100644 --- a/lib/gitlab/git/tag.rb +++ b/lib/gitlab/git/tag.rb @@ -101,7 +101,9 @@ def signature case signature_type when :PGP - nil # not implemented, see https://gitlab.com/gitlab-org/gitlab/issues/19260 + return unless Feature.enabled?(:render_gpg_signed_tags_verification_status, @repository.container) + + Gpg::Tag.new(@repository, self).signature when :X509 X509::Tag.new(@repository, self).signature end diff --git a/lib/gitlab/gpg/tag.rb b/lib/gitlab/gpg/tag.rb new file mode 100644 index 00000000000000..97a9c73ea02086 --- /dev/null +++ b/lib/gitlab/gpg/tag.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Gitlab + module Gpg + class Tag < Gitlab::SignedTag + include Gitlab::Utils::StrongMemoize + + def signature + super + + Gpg::Signature.new(signature_text, signed_text, nil, @tag.user_email) + end + strong_memoize_attr :signature + end + end +end -- GitLab From 31b33ebaec1c615de2239b5f30e88d841acdb390 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Mon, 11 Aug 2025 10:26:17 -0400 Subject: [PATCH 3/4] Update ff config --- .../beta/render_gpg_signed_tags_verification_status.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml b/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml index bc6d4e5143a8a8..2902c442ae44bc 100644 --- a/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml +++ b/config/feature_flags/beta/render_gpg_signed_tags_verification_status.yml @@ -2,7 +2,7 @@ name: render_gpg_signed_tags_verification_status description: feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/19260 -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/200868 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560619 milestone: '18.3' group: group::source code -- GitLab From 74e50651fdae65a95a19925a079973b3154390e9 Mon Sep 17 00:00:00 2001 From: "j.seto" Date: Tue, 12 Aug 2025 16:08:50 -0400 Subject: [PATCH 4/4] Improve test coverage --- spec/lib/gitlab/git/tag_spec.rb | 37 +++++++++++++++++++++++++++++++++ spec/lib/gitlab/gpg/tag_spec.rb | 17 +++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 spec/lib/gitlab/gpg/tag_spec.rb diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index c53d6c432565e4..d4e16f667bdef3 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -37,6 +37,43 @@ it { expect(tag.date).to eq(Time.at(1574261780).utc) } end + describe 'gpg tag' do + let(:gitaly_commit_author) { create(:gitaly_commit_author, date: now) } + let(:now) { Google::Protobuf::Timestamp.new(seconds: 1574261780) } + let(:find_all_tags_double) { double(tags: [gitaly_tag]) } + let(:gitaly_tag) { create(:gitaly_tag, signature_type: :PGP, tagger: gitaly_commit_author, id: 'stubbed_id') } + let(:signature) { 'stubbed_signature' } + let(:gitaly_signature) do + Gitaly::GetTagSignaturesResponse::TagSignature.new(signature: signature, tag_id: gitaly_tag.id) + end + + let(:get_tag_signature_double) { double(signatures: [gitaly_signature]) } + let(:tag) { repository.tags.first } + + before do + allow(Gitlab::GitalyClient).to receive(:call).and_call_original + allow(Gitlab::GitalyClient).to receive(:call).with(repository.storage, :ref_service, :find_all_tags, + any_args).and_return([find_all_tags_double]) + allow(Gitlab::GitalyClient).to receive(:call).with(repository.storage, :ref_service, :get_tag_signatures, + any_args).and_return([get_tag_signature_double]) + end + + it { expect(tag.signature_type).to eq(:PGP) } + it { expect(tag.has_signature?).to be_truthy } + it { expect(tag.signature).not_to be_nil } + it { expect(tag.user_name).to eq(gitaly_commit_author.name) } + it { expect(tag.user_email).to eq(gitaly_commit_author.email) } + it { expect(tag.date).to eq(Time.at(1574261780).utc) } + + context 'when render_gpg_signed_tags_verification_status is not enabled' do + before do + stub_feature_flags(render_gpg_signed_tags_verification_status: false) + end + + it { expect(tag.signature).to be_nil } + end + end + it { expect(repository.tags.size).to be > 0 } end diff --git a/spec/lib/gitlab/gpg/tag_spec.rb b/spec/lib/gitlab/gpg/tag_spec.rb new file mode 100644 index 00000000000000..445e4ebab97397 --- /dev/null +++ b/spec/lib/gitlab/gpg/tag_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Gpg::Tag, feature_category: :source_code_management do + let_it_be(:project) { create :project, :repository } + + let(:git_tag) { project.repository.tags.first } + + subject(:tag) { described_class.new(project.repository, git_tag) } + + describe '#signature' do + it 'returns a signature' do + expect(tag.signature).to be_a_kind_of(Gitlab::Gpg::Signature) + end + end +end -- GitLab