diff --git a/app/models/concerns/commit_signature.rb b/app/models/concerns/commit_signature.rb
index 5dac3c7833a3059213ee6c1f2e6ee1dfea972d2e..5bdf6bb31bfa34e5952c4e926ff3c3ccc3a249f0 100644
--- a/app/models/concerns/commit_signature.rb
+++ b/app/models/concerns/commit_signature.rb
@@ -16,7 +16,8 @@ module CommitSignature
unverified_key: 4,
unknown_key: 5,
multiple_signatures: 6,
- revoked_key: 7
+ revoked_key: 7,
+ verified_system: 8
}
belongs_to :project, class_name: 'Project', foreign_key: 'project_id', optional: false
diff --git a/app/views/projects/commit/_verified_system_signature_badge.html.haml b/app/views/projects/commit/_verified_system_signature_badge.html.haml
new file mode 100644
index 0000000000000000000000000000000000000000..96ff26ecbd71d0d27ae324032382f0fb44b33353
--- /dev/null
+++ b/app/views/projects/commit/_verified_system_signature_badge.html.haml
@@ -0,0 +1,5 @@
+- title = _('Verified commit')
+- description = _('This commit was created in the GitLab UI, and signed with a GitLab-verified signature.')
+- locals = { signature: signature, title: title, description: description, label: _('Verified'), variant: 'success' }
+
+= render partial: 'projects/commit/signature_badge', locals: locals
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index a299927c71161eb6215064beb95da50473db90f2..92dce4c55f080ec135ce67dcd413519884aaf112 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -26727,6 +26727,7 @@ Verification status of a GPG or X.509 signature for a commit.
| `UNVERIFIED` | unverified verification status. |
| `UNVERIFIED_KEY` | unverified_key verification status. |
| `VERIFIED` | verified verification status. |
+| `VERIFIED_SYSTEM` | verified_system verification status. |
### `VisibilityLevelsEnum`
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 33557c6e2362fa360defca2e490bd4b339a28edc..c10f780665cf277949a6d790d0866734643d3a39 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -531,14 +531,24 @@ def get_commit_signatures(commit_ids)
request = Gitaly::GetCommitSignaturesRequest.new(repository: @gitaly_repo, commit_ids: commit_ids)
response = gitaly_client_call(@repository.storage, :commit_service, :get_commit_signatures, request, timeout: GitalyClient.fast_timeout)
- signatures = Hash.new { |h, k| h[k] = [+''.b, +''.b] }
+ signatures = Hash.new do |h, k|
+ h[k] = {
+ signature: +''.b,
+ signed_text: +''.b,
+ signer: :SIGNER_UNSPECIFIED
+ }
+ end
+
current_commit_id = nil
response.each do |message|
current_commit_id = message.commit_id if message.commit_id.present?
- signatures[current_commit_id].first << message.signature
- signatures[current_commit_id].last << message.signed_text
+ signatures[current_commit_id][:signature] << message.signature
+ signatures[current_commit_id][:signed_text] << message.signed_text
+
+ # The actual value is send once. All the other chunks send SIGNER_UNSPECIFIED
+ signatures[current_commit_id][:signer] = message.signer unless message.signer == :SIGNER_UNSPECIFIED
end
signatures
diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb
index a03aeb9c2936ed831e70e38cb5e131933966160c..1fc951817677d390560e6743eab18b7a9c31cfd7 100644
--- a/lib/gitlab/gpg/commit.rb
+++ b/lib/gitlab/gpg/commit.rb
@@ -87,6 +87,7 @@ def attributes(gpg_key)
end
def verification_status(gpg_key)
+ return :verified_system if verified_by_gitlab?
return :multiple_signatures if multiple_signatures?
return :unknown_key unless gpg_key
return :unverified_key unless gpg_key.verified?
@@ -101,6 +102,15 @@ def verification_status(gpg_key)
end
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 user_infos(gpg_key)
gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {}
end
diff --git a/lib/gitlab/signed_commit.rb b/lib/gitlab/signed_commit.rb
index 410e71f51a19b2f8cfaad64c98642682cdef1b85..be6592dd23129e48fdea3c7cbb1249e89c4f968f 100644
--- a/lib/gitlab/signed_commit.rb
+++ b/lib/gitlab/signed_commit.rb
@@ -34,13 +34,19 @@ def update_signature!(cached_signature)
def signature_text
strong_memoize(:signature_text) do
- @signature_data.itself ? @signature_data[0] : nil
+ @signature_data.itself ? @signature_data[:signature] : nil
end
end
def signed_text
strong_memoize(:signed_text) do
- @signature_data.itself ? @signature_data[1] : nil
+ @signature_data.itself ? @signature_data[:signed_text] : nil
+ end
+ end
+
+ def signer
+ strong_memoize(:signer) do
+ @signature_data.itself ? @signature_data[:signer] : nil
end
end
diff --git a/lib/gitlab/ssh/commit.rb b/lib/gitlab/ssh/commit.rb
index d9ac8c1b88127335b495d5841ed041aaaa19e7e0..7d7cc529b1a7679c4bb529249a441c3f964e76fc 100644
--- a/lib/gitlab/ssh/commit.rb
+++ b/lib/gitlab/ssh/commit.rb
@@ -10,7 +10,7 @@ def signature_class
end
def attributes
- signature = ::Gitlab::Ssh::Signature.new(signature_text, signed_text, @commit.committer_email)
+ signature = ::Gitlab::Ssh::Signature.new(signature_text, signed_text, signer, @commit.committer_email)
{
commit_sha: @commit.sha,
diff --git a/lib/gitlab/ssh/signature.rb b/lib/gitlab/ssh/signature.rb
index 763d89116f1931f27f0ed0e1b782207dd1a0829c..6b0cab75557ad8710c3a3565d48fd115d6df1b4e 100644
--- a/lib/gitlab/ssh/signature.rb
+++ b/lib/gitlab/ssh/signature.rb
@@ -11,15 +11,17 @@ class Signature
GIT_NAMESPACE = 'git'
- def initialize(signature_text, signed_text, committer_email)
+ def initialize(signature_text, signed_text, signer, committer_email)
@signature_text = signature_text
@signed_text = signed_text
+ @signer = signer
@committer_email = committer_email
end
def verification_status
strong_memoize(:verification_status) do
next :unverified unless all_attributes_present?
+ next :verified_system if verified_by_gitlab?
next :unverified unless valid_signature_blob?
next :unknown_key unless signed_by_key
next :other_user unless committer
@@ -81,6 +83,15 @@ def signature
nil
end
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
end
end
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 47d97df25db45a5033004a249a681998cf8776e2..9171a68f8a177ea14ea9517f10c47be4c6e1ddcb 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -46640,6 +46640,9 @@ msgstr ""
msgid "This commit is part of merge request %{link_to_merge_request}. Comments created here will be created in the context of that merge request."
msgstr ""
+msgid "This commit was created in the GitLab UI, and signed with a GitLab-verified signature."
+msgstr ""
+
msgid "This commit was signed with a %{strong_open}verified%{strong_close} signature and the committer email is verified to belong to the same user."
msgstr ""
diff --git a/spec/graphql/types/commit_signatures/verification_status_enum_spec.rb b/spec/graphql/types/commit_signatures/verification_status_enum_spec.rb
index a0d99f5f0c10e59656ba6cabfa8a426fbeb1a912..7fc600745dfc3b9735c4c2a33f53d2b1e780d104 100644
--- a/spec/graphql/types/commit_signatures/verification_status_enum_spec.rb
+++ b/spec/graphql/types/commit_signatures/verification_status_enum_spec.rb
@@ -10,7 +10,7 @@
.to match_array(%w[
UNVERIFIED UNVERIFIED_KEY VERIFIED
SAME_USER_DIFFERENT_EMAIL OTHER_USER UNKNOWN_KEY
- MULTIPLE_SIGNATURES REVOKED_KEY
+ MULTIPLE_SIGNATURES REVOKED_KEY VERIFIED_SYSTEM
])
end
end
diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb
index e5f8918f7bbedda13dcfd13b21f404bc0b039532..db2536b76334ad6c35eeae49af785679c681f3f2 100644
--- a/spec/lib/gitlab/git/commit_spec.rb
+++ b/spec/lib/gitlab/git/commit_spec.rb
@@ -476,7 +476,7 @@ def create_commit_with_large_message
let(:commit_id) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }
it 'returns signature and signed text' do
- signature, signed_text = subject
+ signature, signed_text, signer = subject.values_at(:signature, :signed_text, :signer)
expected_signature = <<~SIGNATURE
-----BEGIN PGP SIGNATURE-----
@@ -509,6 +509,7 @@ def create_commit_with_large_message
expect(signed_text).to eq(expected_signed_text)
expect(signed_text).to be_a_binary_string
+ expect(signer).to eq(:SIGNER_USER)
end
end
diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
index b08d11223f79ef84d8848ba772499e73a53e3f8a..fd66efe12c840224183e2d23e5c4a3918bd8d18c 100644
--- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb
@@ -1038,4 +1038,38 @@ def wrap_commits(commits)
end
end
end
+
+ describe '#get_commit_signatures' do
+ let(:project) { create(:project, :test_repo) }
+
+ it 'returns commit signatures for specified commit ids', :aggregate_failures do
+ without_signature = "e63f41fe459e62e1228fcef60d7189127aeba95a" # has no signature
+
+ signed_by_user = [
+ "a17a9f66543673edf0a3d1c6b93bdda3fe600f32", # has signature
+ "7b5160f9bb23a3d58a0accdbe89da13b96b1ece9" # SSH signature
+ ]
+
+ large_signed_text = "8cf8e80a5a0546e391823c250f2b26b9cf15ce88" # has signature and commit message > 4MB
+
+ signatures = client.get_commit_signatures(
+ [without_signature, large_signed_text, *signed_by_user]
+ )
+
+ expect(signatures.keys).to match_array([large_signed_text, *signed_by_user])
+
+ [large_signed_text, *signed_by_user].each do |commit_id|
+ expect(signatures[commit_id][:signature]).to be_present
+ expect(signatures[commit_id][:signer]).to eq(:SIGNER_USER)
+ end
+
+ signed_by_user.each do |commit_id|
+ commit = project.commit(commit_id)
+ expect(signatures[commit_id][:signed_text]).to include(commit.message)
+ expect(signatures[commit_id][:signed_text]).to include(commit.description)
+ end
+
+ expect(signatures[large_signed_text][:signed_text].size).to eq(4971878)
+ end
+ end
end
diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb
index 819a5633a785cb1b98033a4db9d2732367ddbfc8..6cd5cda69b86080cd99d0ee54b02a04150ec88ed 100644
--- a/spec/lib/gitlab/gpg/commit_spec.rb
+++ b/spec/lib/gitlab/gpg/commit_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Gitlab::Gpg::Commit do
+RSpec.describe Gitlab::Gpg::Commit, feature_category: :source_code_management do
let_it_be(:project) { create(:project, :repository, path: 'sample-project') }
let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
@@ -12,15 +12,17 @@
let(:user) { create(:user, email: user_email) }
let(:commit) { create(:commit, project: project, sha: commit_sha, committer_email: committer_email) }
let(:crypto) { instance_double(GPGME::Crypto) }
+ let(:signer) { :SIGNER_USER }
let(:mock_signature_data?) { true }
# gpg_keys must be pre-loaded so that they can be found during signature verification.
let!(:gpg_key) { create(:gpg_key, key: public_key, user: user) }
let(:signature_data) do
- [
- GpgHelpers::User1.signed_commit_signature,
- GpgHelpers::User1.signed_commit_base_data
- ]
+ {
+ signature: GpgHelpers::User1.signed_commit_signature,
+ signed_text: GpgHelpers::User1.signed_commit_base_data,
+ signer: signer
+ }
end
before do
@@ -55,11 +57,12 @@
context 'invalid signature' do
let(:signature_data) do
- [
+ {
# Corrupt the key
- GpgHelpers::User1.signed_commit_signature.tr('=', 'a'),
- GpgHelpers::User1.signed_commit_base_data
- ]
+ signature: GpgHelpers::User1.signed_commit_signature.tr('=', 'a'),
+ signed_text: GpgHelpers::User1.signed_commit_base_data,
+ signer: signer
+ }
end
it 'returns nil' do
@@ -185,10 +188,11 @@
end
let(:signature_data) do
- [
- GpgHelpers::User3.signed_commit_signature,
- GpgHelpers::User3.signed_commit_base_data
- ]
+ {
+ signature: GpgHelpers::User3.signed_commit_signature,
+ signed_text: GpgHelpers::User3.signed_commit_base_data,
+ signer: signer
+ }
end
it 'returns a valid signature' do
@@ -339,6 +343,25 @@
expect(recorder.count).to eq(1)
end
end
+
+ context 'when signature created by GitLab' do
+ let(:signer) { :SIGNER_SYSTEM }
+ let(:gpg_key) { nil }
+
+ it 'returns a valid signature' do
+ expect(described_class.new(commit).signature).to have_attributes(
+ commit_sha: commit_sha,
+ project: project,
+ gpg_key: nil,
+ gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+ gpg_key_user_name: nil,
+ gpg_key_user_email: nil,
+ verification_status: 'verified_system'
+ )
+ end
+
+ it_behaves_like 'returns the cached signature on second call'
+ end
end
describe '#update_signature!' do
diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
index 5d444775e53c027732250e6a5d8e9bb617e9844b..db88e99970c2e52ddeb498803f4db77225bb951a 100644
--- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
+++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
@@ -4,7 +4,13 @@
RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
describe '#run' do
- let(:signature) { [GpgHelpers::User1.signed_commit_signature, GpgHelpers::User1.signed_commit_base_data] }
+ let(:signature) do
+ {
+ signature: GpgHelpers::User1.signed_commit_signature,
+ signed_text: GpgHelpers::User1.signed_commit_base_data
+ }
+ end
+
let(:committer_email) { GpgHelpers::User1.emails.first }
let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
let!(:project) { create :project, :repository, path: 'sample-project' }
@@ -183,7 +189,13 @@
end
context 'gpg signature did not have an associated gpg subkey' do
- let(:signature) { [GpgHelpers::User3.signed_commit_signature, GpgHelpers::User3.signed_commit_base_data] }
+ let(:signature) do
+ {
+ signature: GpgHelpers::User3.signed_commit_signature,
+ signed_text: GpgHelpers::User3.signed_commit_base_data
+ }
+ end
+
let(:committer_email) { GpgHelpers::User3.emails.first }
let!(:user) { create :user, email: GpgHelpers::User3.emails.first }
diff --git a/spec/lib/gitlab/ssh/commit_spec.rb b/spec/lib/gitlab/ssh/commit_spec.rb
index 77f37857c82a2632448d8bab339e4f43fb7d45d4..3b53ed9d1db4686140e404cbe587b66353999c7a 100644
--- a/spec/lib/gitlab/ssh/commit_spec.rb
+++ b/spec/lib/gitlab/ssh/commit_spec.rb
@@ -9,7 +9,8 @@
let(:commit) { create(:commit, project: project) }
let(:signature_text) { 'signature_text' }
let(:signed_text) { 'signed_text' }
- let(:signature_data) { [signature_text, signed_text] }
+ let(:signer) { :SIGNER_USER }
+ let(:signature_data) { { signature: signature_text, signed_text: signed_text, signer: signer } }
let(:verifier) { instance_double('Gitlab::Ssh::Signature') }
let(:verification_status) { :verified }
@@ -27,7 +28,7 @@
})
allow(Gitlab::Ssh::Signature).to receive(:new)
- .with(signature_text, signed_text, commit.committer_email)
+ .with(signature_text, signed_text, signer, commit.committer_email)
.and_return(verifier)
end
diff --git a/spec/lib/gitlab/ssh/signature_spec.rb b/spec/lib/gitlab/ssh/signature_spec.rb
index ee9b38cae7d7142abc4f43f772867ff05d09c766..cb0b1ff049c7bd2cf3e00243b3856d5e129fcc39 100644
--- a/spec/lib/gitlab/ssh/signature_spec.rb
+++ b/spec/lib/gitlab/ssh/signature_spec.rb
@@ -10,6 +10,7 @@
let_it_be_with_reload(:key) { create(:key, usage_type: :signing, key: public_key_text, user: user) }
let(:signed_text) { 'This message was signed by an ssh key' }
+ let(:signer) { :SIGNER_USER }
let(:signature_text) do
# ssh-keygen -Y sign -n git -f id_test message.txt
@@ -27,6 +28,7 @@
described_class.new(
signature_text,
signed_text,
+ signer,
committer_email
)
end
@@ -266,6 +268,15 @@
expect(signature.verification_status).to eq(:other_user)
end
end
+
+ context 'when signature created by GitLab' do
+ let(:signer) { :SIGNER_SYSTEM }
+
+ it 'reports verified_system status' do
+ expect(signature.verification_status).to eq(:verified_system)
+ expect(signature.key_fingerprint).to eq('dw7gPSvYtkCBU+BbTolbbckUEX3sL6NsGIJTQ4PYEnM')
+ end
+ end
end
describe '#key_fingerprint' do
diff --git a/spec/views/projects/commit/show.html.haml_spec.rb b/spec/views/projects/commit/show.html.haml_spec.rb
index 6d2237e773e5dcef6f0315bcf0befa5205aeb2bb..4cfff00d390b47abd193c8d6211a6a8ad078cf12 100644
--- a/spec/views/projects/commit/show.html.haml_spec.rb
+++ b/spec/views/projects/commit/show.html.haml_spec.rb
@@ -71,14 +71,12 @@
let(:title) { badge_attributes['data-title'].value }
let(:content) { badge_attributes['data-content'].value }
- before do
- render
- end
-
context 'with GPG' do
let(:commit) { project.commit(GpgHelpers::SIGNED_COMMIT_SHA) }
it 'renders unverified badge' do
+ render
+
expect(title).to include('This commit was signed with an unverified signature.')
expect(content).to include(commit.signature.gpg_key_primary_keyid)
end
@@ -88,15 +86,34 @@
let(:commit) { project.commit('7b5160f9bb23a3d58a0accdbe89da13b96b1ece9') }
it 'renders unverified badge' do
+ render
+
expect(title).to include('This commit was signed with an unverified signature.')
expect(content).to match(/SSH key fingerprint:[\s\S].+#{commit.signature.key_fingerprint_sha256}/)
end
+
+ context 'when the commit has been signed by GitLab' do
+ it 'renders verified badge' do
+ allow_next_instance_of(Gitlab::Ssh::Commit) do |instance|
+ allow(instance).to receive(:signer).and_return(:SIGNER_SYSTEM)
+ end
+
+ render
+
+ expect(content).to match(/SSH key fingerprint:[\s\S].+#{commit.signature.key_fingerprint_sha256}/)
+ expect(title).to include(
+ 'This commit was created in the GitLab UI, and signed with a GitLab-verified signature.'
+ )
+ end
+ end
end
context 'with X.509' do
let(:commit) { project.commit('189a6c924013fc3fe40d6f1ec1dc20214183bc97') }
it 'renders unverified badge' do
+ render
+
expect(title).to include('This commit was signed with an unverified signature.')
expect(content).to include(commit.signature.x509_certificate.subject_key_identifier.tr(":", " "))
end