Merge branch '355877_deprecate_md5_fingerprint_for_api_keys_endpoint' into 'master'

Add FIPS support for API keys endpoint

See merge request gitlab-org/gitlab!82911

app/finders/keys_finder.rb

@@ -52,12 +52,12 @@ class KeysFinder
   end
 
   def valid_fingerprint_param?
-    if fingerprint_type == "sha256"
-      Base64.decode64(fingerprint).length == 32
-    else
+    return Base64.decode64(fingerprint).length == 32 if fingerprint_type == "sha256"
+
+    return false if Gitlab::FIPS.enabled?
+
     fingerprint =~ /^(\h{2}:){15}\h{2}/
   end
-  end
 
   def fingerprint_query
     fingerprint_attribute = FINGERPRINT_ATTRIBUTES[fingerprint_type]

spec/factories/keys.rb

@@ -19,6 +19,12 @@ FactoryBot.define do
       user
     end
 
+    factory :personal_key_4096 do
+      user
+
+      key { SSHData::PrivateKey::RSA.generate(4096, unsafe_allow_small_key: true).public_key.openssh(comment: 'dummy@gitlab.com') }
+    end
+
     factory :another_key do
       factory :another_deploy_key, class: 'DeployKey'
     end
@@ -74,6 +80,8 @@ FactoryBot.define do
           qpPN5jAskkAUzOh5L/M+dmq2jNn03U9xwORCYPZj+fFM9bL99/0knsV0ypZDZyWH dummy@gitlab.com
         KEY
       end
+
+      factory :rsa_deploy_key_5120, class: 'DeployKey'
     end
 
     factory :rsa_key_8192 do

spec/finders/keys_finder_spec.rb

@@ -5,23 +5,22 @@ require 'spec_helper'
 RSpec.describe KeysFinder do
   subject { described_class.new(params).execute }
 
-  let(:user) { create(:user) }
-  let(:params) { {} }
-
-  let!(:key_1) do
-    create(:personal_key,
+  let_it_be(:user) { create(:user) }
+  let_it_be(:key_1) do
+    create(:rsa_key_4096,
       last_used_at: 7.days.ago,
       user: user,
-      key: 'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt1016k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=',
-      fingerprint: 'ba:81:59:68:d7:6c:cd:02:02:bf:6a:9b:55:4e:af:d1',
-      fingerprint_sha256: 'nUhzNyftwADy8AH3wFY31tAKs7HufskYTte2aXo/lCg')
+      fingerprint: 'df:73:db:29:3c:a5:32:cf:09:17:7e:8e:9d:de:d7:f7',
+      fingerprint_sha256: 'ByDU7hQ1JB95l6p53rHrffc4eXvEtqGUtQhS+Dhyy7g')
   end
 
-  let!(:key_2) { create(:personal_key, last_used_at: nil, user: user) }
-  let!(:key_3) { create(:personal_key, last_used_at: 2.days.ago) }
+  let_it_be(:key_2) { create(:personal_key_4096, last_used_at: nil, user: user) }
+  let_it_be(:key_3) { create(:personal_key_4096, last_used_at: 2.days.ago) }
+
+  let(:params) { {} }
 
   context 'key_type' do
-    let!(:deploy_key) { create(:deploy_key) }
+    let_it_be(:deploy_key) { create(:deploy_key) }
 
     context 'when `key_type` is `ssh`' do
       before do
@@ -64,35 +63,41 @@ RSpec.describe KeysFinder do
     end
 
     context 'with valid fingerprints' do
-      let!(:deploy_key) do
-        create(:deploy_key,
-          user: user,
-          key: 'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt1017k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=',
-          fingerprint: '8a:4a:12:92:0b:50:47:02:d4:5a:8e:a9:44:4e:08:b4',
-          fingerprint_sha256: '4DPHOVNh53i9dHb5PpY2vjfyf5qniTx1/pBFPoZLDdk')
-      end
+      let_it_be(:deploy_key) { create(:rsa_deploy_key_5120, user: user) }
 
       context 'personal key with valid MD5 params' do
         context 'with an existent fingerprint' do
           before do
-            params[:fingerprint] = 'ba:81:59:68:d7:6c:cd:02:02:bf:6a:9b:55:4e:af:d1'
+            params[:fingerprint] = 'df:73:db:29:3c:a5:32:cf:09:17:7e:8e:9d:de:d7:f7'
           end
 
           it 'returns the key' do
             expect(subject).to eq(key_1)
             expect(subject.user).to eq(user)
           end
+
+          context 'with FIPS mode', :fips_mode do
+            it 'raises InvalidFingerprint' do
+              expect { subject }.to raise_error(KeysFinder::InvalidFingerprint)
+            end
+          end
         end
 
         context 'deploy key with an existent fingerprint' do
           before do
-            params[:fingerprint] = '8a:4a:12:92:0b:50:47:02:d4:5a:8e:a9:44:4e:08:b4'
+            params[:fingerprint] = 'fe:fa:3a:4d:7d:51:ec:bf:c7:64:0c:96:d0:17:8a:d0'
           end
 
           it 'returns the key' do
             expect(subject).to eq(deploy_key)
             expect(subject.user).to eq(user)
           end
+
+          context 'with FIPS mode', :fips_mode do
+            it 'raises InvalidFingerprint' do
+              expect { subject }.to raise_error(KeysFinder::InvalidFingerprint)
+            end
+          end
         end
 
         context 'with a non-existent fingerprint' do
@@ -103,13 +108,19 @@ RSpec.describe KeysFinder do
           it 'returns nil' do
             expect(subject).to be_nil
           end
+
+          context 'with FIPS mode', :fips_mode do
+            it 'raises InvalidFingerprint' do
+              expect { subject }.to raise_error(KeysFinder::InvalidFingerprint)
+            end
+          end
         end
       end
 
       context 'personal key with valid SHA256 params' do
         context 'with an existent fingerprint' do
           before do
-            params[:fingerprint] = 'SHA256:nUhzNyftwADy8AH3wFY31tAKs7HufskYTte2aXo/lCg'
+            params[:fingerprint] = 'SHA256:ByDU7hQ1JB95l6p53rHrffc4eXvEtqGUtQhS+Dhyy7g'
           end
 
           it 'returns key' do
@@ -120,7 +131,7 @@ RSpec.describe KeysFinder do
 
         context 'deploy key with an existent fingerprint' do
           before do
-            params[:fingerprint] = 'SHA256:4DPHOVNh53i9dHb5PpY2vjfyf5qniTx1/pBFPoZLDdk'
+            params[:fingerprint] = 'SHA256:PCCupLbFHScm4AbEufbGDvhBU27IM0MVAor715qKQK8'
           end
 
           it 'returns key' do

spec/requests/api/keys_spec.rb

@@ -3,10 +3,11 @@
 require 'spec_helper'
 
 RSpec.describe API::Keys do
-  let(:user)  { create(:user) }
-  let(:admin) { create(:admin) }
-  let(:key)   { create(:key, user: user, expires_at: 1.day.from_now) }
-  let(:email) { create(:email, user: user) }
+  let_it_be(:user)  { create(:user) }
+  let_it_be(:admin) { create(:admin) }
+  let_it_be(:email) { create(:email, user: user) }
+  let_it_be(:key) { create(:rsa_key_4096, user: user, expires_at: 1.day.from_now) }
+  let_it_be(:fingerprint_md5) { 'df:73:db:29:3c:a5:32:cf:09:17:7e:8e:9d:de:d7:f7' }
 
   describe 'GET /keys/:uid' do
     context 'when unauthenticated' do
@@ -24,7 +25,6 @@ RSpec.describe API::Keys do
       end
 
       it 'returns single ssh key with user information' do
-        user.keys << key
         get api("/keys/#{key.id}", admin)
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response['title']).to eq(key.title)
@@ -43,18 +43,19 @@ RSpec.describe API::Keys do
 
   describe 'GET /keys?fingerprint=' do
     it 'returns authentication error' do
-      get api("/keys?fingerprint=#{key.fingerprint}")
+      get api("/keys?fingerprint=#{fingerprint_md5}")
 
       expect(response).to have_gitlab_http_status(:unauthorized)
     end
 
     it 'returns authentication error when authenticated as user' do
-      get api("/keys?fingerprint=#{key.fingerprint}", user)
+      get api("/keys?fingerprint=#{fingerprint_md5}", user)
 
       expect(response).to have_gitlab_http_status(:forbidden)
     end
 
     context 'when authenticated as admin' do
+      context 'MD5 fingerprint' do
         it 'returns 404 for non-existing SSH md5 fingerprint' do
           get api("/keys?fingerprint=11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11", admin)
 
@@ -62,17 +63,8 @@ RSpec.describe API::Keys do
           expect(json_response['message']).to eq('404 Key Not Found')
         end
 
-      it 'returns 404 for non-existing SSH sha256 fingerprint' do
-        get api("/keys?fingerprint=#{URI.encode_www_form_component("SHA256:nUhzNyftwADy8AH3wFY31tAKs7HufskYTte2aXo1lCg")}", admin)
-
-        expect(response).to have_gitlab_http_status(:not_found)
-        expect(json_response['message']).to eq('404 Key Not Found')
-      end
-
         it 'returns user if SSH md5 fingerprint found' do
-        user.keys << key
-
-        get api("/keys?fingerprint=#{key.fingerprint}", admin)
+          get api("/keys?fingerprint=#{fingerprint_md5}", admin)
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response['title']).to eq(key.title)
@@ -80,9 +72,31 @@ RSpec.describe API::Keys do
           expect(json_response['user']['username']).to eq(user.username)
         end
 
-      it 'returns user if SSH sha256 fingerprint found' do
-        user.keys << key
+        context 'with FIPS mode', :fips_mode do
+          it 'returns 404 for non-existing SSH md5 fingerprint' do
+            get api("/keys?fingerprint=11:11:11:11:11:11:11:11:11:11:11:11:11:11:11:11", admin)
+
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['message']).to eq('Failed to return the key')
+          end
+
+          it 'returns 404 for existing SSH md5 fingerprint' do
+            get api("/keys?fingerprint=#{fingerprint_md5}", admin)
 
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['message']).to eq('Failed to return the key')
+          end
+        end
+      end
+
+      it 'returns 404 for non-existing SSH sha256 fingerprint' do
+        get api("/keys?fingerprint=#{URI.encode_www_form_component("SHA256:nUhzNyftwADy8AH3wFY31tAKs7HufskYTte2aXo1lCg")}", admin)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(json_response['message']).to eq('404 Key Not Found')
+      end
+
+      it 'returns user if SSH sha256 fingerprint found' do
         get api("/keys?fingerprint=#{URI.encode_www_form_component("SHA256:" + key.fingerprint_sha256)}", admin)
 
         expect(response).to have_gitlab_http_status(:ok)
@@ -92,8 +106,6 @@ RSpec.describe API::Keys do
       end
 
       it 'returns user if SSH sha256 fingerprint found' do
-        user.keys << key
-
         get api("/keys?fingerprint=#{URI.encode_www_form_component("sha256:" + key.fingerprint_sha256)}", admin)
 
         expect(response).to have_gitlab_http_status(:ok)
@@ -103,7 +115,7 @@ RSpec.describe API::Keys do
       end
 
       it "does not include the user's `is_admin` flag" do
-        get api("/keys?fingerprint=#{key.fingerprint}", admin)
+        get api("/keys?fingerprint=#{URI.encode_www_form_component("sha256:" + key.fingerprint_sha256)}", admin)
 
         expect(json_response['user']['is_admin']).to be_nil
       end