Commit 184240e8 authored by Stan Hu's avatar Stan Hu

Gracefully handle unknown/invalid GPG keys

An unknown public GPG key will result in a GPGME::Error thrown from gpg,
which would cause an Error 500 on the signatures endpoint.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/54729
parent cfe48479
...@@ -44,10 +44,9 @@ module Gitlab ...@@ -44,10 +44,9 @@ module Gitlab
def update_signature!(cached_signature) def update_signature!(cached_signature)
using_keychain do |gpg_key| using_keychain do |gpg_key|
cached_signature.update!(attributes(gpg_key)) cached_signature.update!(attributes(gpg_key))
end
@signature = cached_signature @signature = cached_signature
end end
end
private private
...@@ -59,11 +58,15 @@ module Gitlab ...@@ -59,11 +58,15 @@ module Gitlab
# the proper signature. # the proper signature.
# NOTE: the invoked method is #fingerprint but it's only returning # NOTE: the invoked method is #fingerprint but it's only returning
# 16 characters (the format used by keyid) instead of 40. # 16 characters (the format used by keyid) instead of 40.
gpg_key = find_gpg_key(verified_signature.fingerprint) fingerprint = verified_signature&.fingerprint
break unless fingerprint
gpg_key = find_gpg_key(fingerprint)
if gpg_key if gpg_key
Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key)
@verified_signature = nil clear_memoization(:verified_signature)
end end
yield gpg_key yield gpg_key
...@@ -71,9 +74,16 @@ module Gitlab ...@@ -71,9 +74,16 @@ module Gitlab
end end
def verified_signature def verified_signature
@verified_signature ||= GPGME::Crypto.new.verify(signature_text, signed_text: signed_text) do |verified_signature| strong_memoize(:verified_signature) { gpgme_signature }
end
def gpgme_signature
GPGME::Crypto.new.verify(signature_text, signed_text: signed_text) do |verified_signature|
# Return the first signature for now: https://gitlab.com/gitlab-org/gitlab-ce/issues/54932
break verified_signature break verified_signature
end end
rescue GPGME::Error
nil
end end
def create_cached_signature! def create_cached_signature!
...@@ -92,7 +102,7 @@ module Gitlab ...@@ -92,7 +102,7 @@ module Gitlab
commit_sha: @commit.sha, commit_sha: @commit.sha,
project: @commit.project, project: @commit.project,
gpg_key: gpg_key, gpg_key: gpg_key,
gpg_key_primary_keyid: gpg_key&.keyid || verified_signature.fingerprint, gpg_key_primary_keyid: gpg_key&.keyid || verified_signature&.fingerprint,
gpg_key_user_name: user_infos[:name], gpg_key_user_name: user_infos[:name],
gpg_key_user_email: user_infos[:email], gpg_key_user_email: user_infos[:email],
verification_status: verification_status verification_status: verification_status
...@@ -102,7 +112,7 @@ module Gitlab ...@@ -102,7 +112,7 @@ module Gitlab
def verification_status(gpg_key) def verification_status(gpg_key)
return :unknown_key unless gpg_key return :unknown_key unless gpg_key
return :unverified_key unless gpg_key.verified? return :unverified_key unless gpg_key.verified?
return :unverified unless verified_signature.valid? return :unverified unless verified_signature&.valid?
if gpg_key.verified_and_belongs_to_email?(@commit.committer_email) if gpg_key.verified_and_belongs_to_email?(@commit.committer_email)
:verified :verified
......
...@@ -26,6 +26,28 @@ describe Gitlab::Gpg::Commit do ...@@ -26,6 +26,28 @@ describe Gitlab::Gpg::Commit do
end end
end end
context 'invalid signature' 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) }
before do
allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
.with(Gitlab::Git::Repository, commit_sha)
.and_return(
[
# Corrupt the key
GpgHelpers::User1.signed_commit_signature.tr('=', 'a'),
GpgHelpers::User1.signed_commit_base_data
]
)
end
it 'returns nil' do
expect(described_class.new(commit).signature).to be_nil
end
end
context 'known key' do context 'known key' do
context 'user matches the key uid' do context 'user matches the key uid' do
context 'user email matches the email committer' do context 'user email matches the email committer' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment