diff options
| -rw-r--r-- | app/models/commit.rb | 25 | ||||
| -rw-r--r-- | app/views/projects/commit/_signature.html.haml | 6 | ||||
| -rw-r--r-- | lib/gitlab/gpg/commit.rb | 51 | ||||
| -rw-r--r-- | spec/lib/gitlab/gpg/commit_spec.rb | 53 | ||||
| -rw-r--r-- | spec/models/commit_spec.rb | 88 |
5 files changed, 177 insertions, 46 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb index a6a11a2d3a5..6c5556902ec 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -239,29 +239,14 @@ class Commit @signature = nil - signature, signed_text = @raw.signature(project.repository) + cached_signature = GpgSignature.find_by(commit_sha: sha) + return cached_signature if cached_signature.present? - return unless signature && signed_text + gpg_commit = Gitlab::Gpg::Commit.new(self) - Gitlab::Gpg.using_tmp_keychain do - # first we need to get the keyid from the signature... - GPGME::Crypto.new.verify(signature, signed_text: signed_text) do |verified_signature| - @signature = verified_signature - end - - # ... then we query the gpg key belonging to the keyid. - gpg_key = GpgKey.find_by(primary_keyid: @signature.fingerprint) - - return @signature unless gpg_key - - Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) - - GPGME::Crypto.new.verify(signature, signed_text: signed_text) do |verified_signature| - @signature = verified_signature - end - end + return unless gpg_commit.has_signature? - @signature + @signature = gpg_commit.signature end def revert_branch_name diff --git a/app/views/projects/commit/_signature.html.haml b/app/views/projects/commit/_signature.html.haml index 7335b6b9597..48665ede6eb 100644 --- a/app/views/projects/commit/_signature.html.haml +++ b/app/views/projects/commit/_signature.html.haml @@ -1,4 +1,4 @@ - if signature - %a.btn.disabled.btn-xs{ class: ('btn-success' if signature.valid?) } - %i.fa.fa-key{ class: ('fa-inverse' if signature.valid?) } - = signature.valid? ? 'Verified': 'Unverified' + %a.btn.disabled.btn-xs{ class: ('btn-success' if signature.valid_signature?) } + %i.fa.fa-key{ class: ('fa-inverse' if signature.valid_signature?) } + = signature.valid_signature? ? 'Verified' : 'Unverified' diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb new file mode 100644 index 00000000000..f60e5125c13 --- /dev/null +++ b/lib/gitlab/gpg/commit.rb @@ -0,0 +1,51 @@ +module Gitlab + module Gpg + class Commit + attr_reader :commit + + def initialize(commit) + @commit = commit + + @signature_text, @signed_text = commit.raw.signature(commit.project.repository) + end + + def has_signature? + @signature_text && @signed_text + end + + def signature + 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. + # This way we can add the key to the temporary keychain and extract + # the proper signature. + gpg_key = GpgKey.find_by(primary_keyid: verified_signature.fingerprint) + + if gpg_key + Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) + end + + create_cached_signature!(gpg_key) + end + end + + private + + def verified_signature + GPGME::Crypto.new.verify(@signature_text, signed_text: @signed_text) do |verified_signature| + return verified_signature + end + end + + def create_cached_signature!(gpg_key) + GpgSignature.create!( + commit_sha: commit.sha, + project: commit.project, + gpg_key: gpg_key, + gpg_key_primary_keyid: gpg_key&.primary_keyid, + valid_signature: !!(gpg_key && verified_signature&.valid?) + ) + end + end + end +end diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb new file mode 100644 index 00000000000..8b1747eebcc --- /dev/null +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -0,0 +1,53 @@ +require 'rails_helper' + +RSpec.describe Gitlab::Gpg::Commit do + describe '#signature' do + let!(:project) { create :project, :repository, path: 'sample-project' } + + context 'known public key' do + it 'returns a valid signature' do + gpg_key = create :gpg_key, key: GpgHelpers::User1.public_key + + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! + + commit = create :commit, + git_commit: raw_commit, + project: project + + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33', + project: project, + gpg_key: gpg_key, + gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid, + valid_signature: true + ) + end + end + + context 'unknown public key' do + it 'returns an invalid signature', :gpg do + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! + + commit = create :commit, + git_commit: raw_commit, + project: project + + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33', + project: project, + gpg_key: nil, + gpg_key_primary_keyid: nil, + valid_signature: false + ) + end + end + end +end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 96af675c3f4..4370c78e6fd 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -421,36 +421,78 @@ eos end context 'signed commit', :gpg do - it 'returns a valid signature if the public key is known' do - create :gpg_key, key: GpgHelpers::User1.public_key + context 'known public key' do + it 'returns a valid signature' do + create :gpg_key, key: GpgHelpers::User1.public_key - raw_commit = double(:raw_commit, signature: [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ]) - allow(raw_commit).to receive :save! + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! - commit = create :commit, - git_commit: raw_commit, - project: project + commit = create :commit, + git_commit: raw_commit, + project: project - expect(commit.signature).to be_a GPGME::Signature - expect(commit.signature.valid?).to be_truthy + expect(commit.signature.valid_signature?).to be_truthy + end + + it 'returns the cached validation result on second call', :gpg do + create :gpg_key, key: GpgHelpers::User1.public_key + + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! + + commit = create :commit, + git_commit: raw_commit, + project: project + + expect(Gitlab::Gpg::Commit).to receive(:new).and_call_original + expect(commit.signature.valid_signature?).to be_truthy + + # second call returns the cache + expect(Gitlab::Gpg::Commit).not_to receive(:new).and_call_original + expect(commit.signature.valid_signature?).to be_truthy + end end - it 'returns an invalid signature if the public key is unknown', :gpg do - raw_commit = double(:raw_commit, signature: [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ]) - allow(raw_commit).to receive :save! + context 'unknown public key' do + it 'returns an invalid signature if the public key is unknown', :gpg do + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! - commit = create :commit, - git_commit: raw_commit, - project: project + commit = create :commit, + git_commit: raw_commit, + project: project - expect(commit.signature).to be_a GPGME::Signature - expect(commit.signature.valid?).to be_falsey + expect(commit.signature.valid_signature?).to be_falsey + end + + it 'returns the cached validation result on second call', :gpg do + raw_commit = double(:raw_commit, signature: [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ], sha: '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33') + allow(raw_commit).to receive :save! + + commit = create :commit, + git_commit: raw_commit, + project: project + + expect(Gitlab::Gpg::Commit).to receive(:new).and_call_original + expect(commit.signature.valid_signature?).to be_falsey + + # second call returns the cache + expect(Gitlab::Gpg::Commit).not_to receive(:new).and_call_original + expect(commit.signature.valid_signature?).to be_falsey + end end end end |