Merge branch '336421_dont_populate_md5_fingerprint_for_fips' into 'master'

Don't generate MD5 fingerprint in FIPS mode

See merge request gitlab-org/gitlab!84390

app/models/key.rb

@@ -26,7 +26,13 @@ class Key < ApplicationRecord
 
   validates :fingerprint,
     uniqueness: true,
-    presence: { message: 'cannot be generated' }
+    presence: { message: 'cannot be generated' },
+    unless: -> { Gitlab::FIPS.enabled? }
+
+  validates :fingerprint_sha256,
+    uniqueness: true,
+    presence: { message: 'cannot be generated' },
+    if: -> { Gitlab::FIPS.enabled? }
 
   validate :key_meets_restrictions
 
@@ -129,7 +135,7 @@ class Key < ApplicationRecord
 
     return unless public_key.valid?
 
-    self.fingerprint_md5 = public_key.fingerprint
+    self.fingerprint_md5 = public_key.fingerprint unless Gitlab::FIPS.enabled?
     self.fingerprint_sha256 = public_key.fingerprint_sha256.gsub("SHA256:", "")
   end
 

spec/controllers/profiles/keys_controller_spec.rb

@@ -17,7 +17,25 @@ RSpec.describe Profiles::KeysController do
         post :create, params: { key: build(:key, expires_at: expires_at).attributes }
       end.to change { Key.count }.by(1)
 
-      expect(Key.last.expires_at).to be_like_time(expires_at)
+      key = Key.last
+      expect(key.expires_at).to be_like_time(expires_at)
+      expect(key.fingerprint_md5).to be_present
+      expect(key.fingerprint_sha256).to be_present
+    end
+
+    context 'with FIPS mode', :fips_mode do
+      it 'creates a new key without MD5 fingerprint' do
+        expires_at = 3.days.from_now
+
+        expect do
+          post :create, params: { key: build(:rsa_key_4096, expires_at: expires_at).attributes }
+        end.to change { Key.count }.by(1)
+
+        key = Key.last
+        expect(key.expires_at).to be_like_time(expires_at)
+        expect(key.fingerprint_md5).to be_nil
+        expect(key.fingerprint_sha256).to be_present
+      end
     end
   end
 end

spec/models/key_spec.rb

@@ -123,21 +123,22 @@ RSpec.describe Key, :mailer do
     end
   end
 
-  context "validation of uniqueness (based on fingerprint uniqueness)" do
+  context 'validation of uniqueness (based on fingerprint uniqueness)' do
     let(:user) { create(:user) }
 
-    it "accepts the key once" do
-      expect(build(:key, user: user)).to be_valid
+    shared_examples 'fingerprint uniqueness' do
+      it 'accepts the key once' do
+        expect(build(:rsa_key_4096, user: user)).to be_valid
       end
 
-    it "does not accept the exact same key twice" do
-      first_key = create(:key, user: user)
+      it 'does not accept the exact same key twice' do
+        first_key = create(:rsa_key_4096, user: user)
 
         expect(build(:key, user: user, key: first_key.key)).not_to be_valid
       end
 
-    it "does not accept a duplicate key with a different comment" do
-      first_key = create(:key, user: user)
+      it 'does not accept a duplicate key with a different comment' do
+        first_key = create(:rsa_key_4096, user: user)
         duplicate = build(:key, user: user, key: first_key.key)
         duplicate.key << ' extra comment'
 
@@ -145,6 +146,35 @@ RSpec.describe Key, :mailer do
       end
     end
 
+    context 'with FIPS mode off' do
+      it_behaves_like 'fingerprint uniqueness'
+    end
+
+    context 'with FIPS mode', :fips_mode do
+      it_behaves_like 'fingerprint uniqueness'
+    end
+  end
+
+  context 'fingerprint generation' do
+    it 'generates both md5 and sha256 fingerprints' do
+      key = build(:rsa_key_4096)
+
+      expect(key).to be_valid
+      expect(key.fingerprint).to be_kind_of(String)
+      expect(key.fingerprint_sha256).to be_kind_of(String)
+    end
+
+    context 'with FIPS mode', :fips_mode do
+      it 'generates only sha256 fingerprint' do
+        key = build(:rsa_key_4096)
+
+        expect(key).to be_valid
+        expect(key.fingerprint).to be_nil
+        expect(key.fingerprint_sha256).to be_kind_of(String)
+      end
+    end
+  end
+
   context "validate it is a fingerprintable key" do
     it "accepts the fingerprintable key" do
       expect(build(:key)).to be_valid