Merge branch 'issue-58747' into 'master'

Fix GPG signature verification with recent versions of GnuPG

Closes #58747

See merge request gitlab-org/gitlab-ce!29388

3 files changed, 99 insertions, 8 deletions

diff --git a/changelogs/unreleased/issue-58747.yml b/changelogs/unreleased/issue-58747.yml
new file mode 100644
index 00000000000..01b610576f7
--- /dev/null
+++ b/changelogs/unreleased/issue-58747.yml
@@ -0,0 +1,5 @@
+---
+title: Fix GPG signature verification with recent GnuPG versions
+merge_request: 29388
+author: David Palubin
+type: fixed
diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb
index 5ff415b6126..1d317c389d2 100644
--- a/lib/gitlab/gpg/commit.rb
+++ b/lib/gitlab/gpg/commit.rb
@@ -52,12 +52,13 @@ module Gitlab
 
       def using_keychain
         Gitlab::Gpg.using_tmp_keychain do
-          # first we need to get the keyid from the signature to query the gpg
-          # key belonging to the keyid.
+          # 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 it's only returning
-          # 16 characters (the format used by keyid) instead of 40.
+          # 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.
           fingerprint = verified_signature&.fingerprint
 
           break unless fingerprint
@@ -128,11 +129,13 @@ module Gitlab
         gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {}
       end
 
-      # rubocop: disable CodeReuse/ActiveRecord
-      def find_gpg_key(keyid)
-        GpgKey.find_by(primary_keyid: keyid) || GpgKeySubkey.find_by(keyid: keyid)
+      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
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end
diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb
index 8229f0eb794..47e6f5d4220 100644
--- a/spec/lib/gitlab/gpg/commit_spec.rb
+++ b/spec/lib/gitlab/gpg/commit_spec.rb
@@ -109,6 +109,89 @@ describe Gitlab::Gpg::Commit do
           end
         end
 
+        context 'valid key signed using recent version of Gnupg' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first }
+
+          let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) }
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          let!(:crypto) { instance_double(GPGME::Crypto) }
+
+          before do
+            fake_signature = [
+              GpgHelpers::User1.signed_commit_signature,
+              GpgHelpers::User1.signed_commit_base_data
+            ]
+
+            allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+              .with(Gitlab::Git::Repository, commit_sha)
+              .and_return(fake_signature)
+          end
+
+          it 'returns a valid signature' do
+            verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true)
+            allow(GPGME::Crypto).to receive(:new).and_return(crypto)
+            allow(crypto).to receive(:verify).and_return(verified_signature)
+
+            signature = described_class.new(commit).signature
+
+            expect(signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              verification_status: 'verified'
+            )
+          end
+        end
+
+        context 'valid key signed using older version of Gnupg' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first }
+
+          let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) }
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          let!(:crypto) { instance_double(GPGME::Crypto) }
+
+          before do
+            fake_signature = [
+              GpgHelpers::User1.signed_commit_signature,
+              GpgHelpers::User1.signed_commit_base_data
+            ]
+
+            allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+              .with(Gitlab::Git::Repository, commit_sha)
+              .and_return(fake_signature)
+          end
+
+          it 'returns a valid signature' do
+            keyid = GpgHelpers::User1.fingerprint.last(16)
+            verified_signature = double('verified-signature', fingerprint: keyid, valid?: true)
+            allow(GPGME::Crypto).to receive(:new).and_return(crypto)
+            allow(crypto).to receive(:verify).and_return(verified_signature)
+
+            signature = described_class.new(commit).signature
+
+            expect(signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              verification_status: 'verified'
+            )
+          end
+        end
+
         context 'commit signed with a subkey' do
           let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User3.emails.first }