From bab76f763748d746cdc019a642f995ecc7497605 Mon Sep 17 00:00:00 2001 From: David Palubin Date: Mon, 17 Jun 2019 13:33:39 +0000 Subject: Fix GPG signature verification with recent versions of GnuPG --- changelogs/unreleased/issue-58747.yml | 5 +++ lib/gitlab/gpg/commit.rb | 19 ++++---- spec/lib/gitlab/gpg/commit_spec.rb | 83 +++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 changelogs/unreleased/issue-58747.yml 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 } -- cgit v1.2.1