Merge branch 'fips-ssh-key-restrictions' into 'master'

FIPS SSH key configuration settings

See merge request gitlab-org/gitlab!81917

app/models/application_setting.rb

@@ -387,7 +387,7 @@ class ApplicationSetting < ApplicationRecord
   validates :invisible_captcha_enabled,
             inclusion: { in: [true, false], message: _('must be a boolean value') }
 
-  SUPPORTED_KEY_TYPES.each do |type|
+  Gitlab::SSHPublicKey.supported_types.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
 

app/models/application_setting_implementation.rb

@@ -14,7 +14,6 @@ module ApplicationSettingImplementation
   # Setting a key restriction to `-1` means that all keys of this type are
   # forbidden.
   FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
-  SUPPORTED_KEY_TYPES = Gitlab::SSHPublicKey.supported_types
   VALID_RUNNER_REGISTRAR_TYPES = %w(project group).freeze
 
   DEFAULT_PROTECTED_PATHS = [
@@ -67,11 +66,11 @@ module ApplicationSettingImplementation
         disabled_oauth_sign_in_sources: [],
         dns_rebinding_protection_enabled: true,
         domain_allowlist: Settings.gitlab['domain_allowlist'],
-        dsa_key_restriction: 0,
-        ecdsa_key_restriction: 0,
-        ecdsa_sk_key_restriction: 0,
-        ed25519_key_restriction: 0,
-        ed25519_sk_key_restriction: 0,
+        dsa_key_restriction: default_min_key_size(:dsa),
+        ecdsa_key_restriction: default_min_key_size(:ecdsa),
+        ecdsa_sk_key_restriction: default_min_key_size(:ecdsa_sk),
+        ed25519_key_restriction: default_min_key_size(:ed25519),
+        ed25519_sk_key_restriction: default_min_key_size(:ed25519_sk),
         eks_access_key_id: nil,
         eks_account_id: nil,
         eks_integration_enabled: false,
@@ -143,7 +142,7 @@ module ApplicationSettingImplementation
         require_admin_approval_after_user_signup: true,
         require_two_factor_authentication: false,
         restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels'],
-        rsa_key_restriction: 0,
+        rsa_key_restriction: default_min_key_size(:rsa),
         send_user_confirmation_email: false,
         session_expire_delay: Settings.gitlab['session_expire_delay'],
         shared_runners_enabled: Settings.gitlab_ci['shared_runners_enabled'],
@@ -244,6 +243,20 @@ module ApplicationSettingImplementation
       "users.noreply.#{Gitlab.config.gitlab.host}"
     end
 
+    # Return the default allowed minimum key size for a type.
+    # By default this is 0 (unrestricted), but in FIPS mode
+    # this will return the smallest allowed key size. If no
+    # size is available, this type is denied.
+    #
+    # @return [Integer]
+    def default_min_key_size(name)
+      if Gitlab::FIPS.enabled?
+        Gitlab::SSHPublicKey.supported_sizes(name).select(&:positive?).min || -1
+      else
+        0
+      end
+    end
+
     def create_from_defaults
       build_from_defaults.tap(&:save)
     end
@@ -442,7 +455,7 @@ module ApplicationSettingImplementation
   alias_method :usage_ping_enabled?, :usage_ping_enabled
 
   def allowed_key_types
-    SUPPORTED_KEY_TYPES.select do |type|
+    Gitlab::SSHPublicKey.supported_types.select do |type|
       key_restriction_for(type) != FORBIDDEN_KEY_VALUE
     end
   end

app/validators/key_restriction_validator.rb

@@ -2,25 +2,34 @@
 
 class KeyRestrictionValidator < ActiveModel::EachValidator
   FORBIDDEN = -1
+  ALLOWED = 0
 
   def self.supported_sizes(type)
     Gitlab::SSHPublicKey.supported_sizes(type)
   end
 
   def self.supported_key_restrictions(type)
-    [0, *supported_sizes(type), FORBIDDEN]
+    if Gitlab::FIPS.enabled?
+      [*supported_sizes(type), FORBIDDEN]
+    else
+      [ALLOWED, *supported_sizes(type), FORBIDDEN]
+    end
   end
 
   def validate_each(record, attribute, value)
     unless valid_restriction?(value)
-      record.errors.add(attribute, "must be forbidden, allowed, or one of these sizes: #{supported_sizes_message}")
+      record.errors.add(attribute, "must be #{supported_sizes_message}")
     end
   end
 
   private
 
   def supported_sizes_message
-    sizes = self.class.supported_sizes(options[:type])
+    sizes = []
+
+    sizes << "forbidden" if valid_restriction?(FORBIDDEN)
+    sizes << "allowed" if valid_restriction?(ALLOWED)
+    sizes += self.class.supported_sizes(options[:type])
 
     Gitlab::Utils.to_exclusive_sentence(sizes)
   end

app/views/admin/application_settings/_visibility_and_access.html.haml

@@ -57,7 +57,7 @@
       %span.form-text.text-muted#custom_http_clone_url_root_help_block
         = _('Replaces the clone URL root.')
 
-    - ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
+    - Gitlab::SSHPublicKey.supported_types.each do |type|
       - field_name = :"#{type}_key_restriction"
       .form-group
         = f.label field_name, "#{type.upcase} SSH keys", class: 'label-bold'

lib/api/settings.rb

@@ -182,7 +182,7 @@ module API
       optional :group_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for group runners, in seconds'
       optional :project_runner_token_expiration_interval, type: Integer, desc: 'Token expiration interval for project runners, in seconds'
 
-      ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
+      Gitlab::SSHPublicKey.supported_types.each do |type|
         optional :"#{type}_key_restriction",
                  type: Integer,
                  values: KeyRestrictionValidator.supported_key_restrictions(type),

lib/gitlab/fips.rb

@@ -5,6 +5,17 @@ module Gitlab
   class FIPS
     # A simple utility class for FIPS-related helpers
 
+    Technology = Gitlab::SSHPublicKey::Technology
+
+    SSH_KEY_TECHNOLOGIES = [
+      Technology.new(:rsa, SSHData::PublicKey::RSA, [3072, 4096], %w(ssh-rsa)),
+      Technology.new(:dsa, SSHData::PublicKey::DSA, [], %w(ssh-dss)),
+      Technology.new(:ecdsa, SSHData::PublicKey::ECDSA, [256, 384, 521], %w(ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521)),
+      Technology.new(:ed25519, SSHData::PublicKey::ED25519, [256], %w(ssh-ed25519)),
+      Technology.new(:ecdsa_sk, SSHData::PublicKey::SKECDSA, [256], %w(sk-ecdsa-sha2-nistp256@openssh.com)),
+      Technology.new(:ed25519_sk, SSHData::PublicKey::SKED25519, [256], %w(sk-ssh-ed25519@openssh.com))
+    ].freeze
+
     class << self
       # Returns whether we should be running in FIPS mode or not
       #

lib/gitlab/ssh_public_key.rb

@@ -15,16 +15,24 @@ module Gitlab
       Technology.new(:ed25519_sk, SSHData::PublicKey::SKED25519, [256], %w(sk-ssh-ed25519@openssh.com))
     ].freeze
 
+    def self.technologies
+      if Gitlab::FIPS.enabled?
+        Gitlab::FIPS::SSH_KEY_TECHNOLOGIES
+      else
+        TECHNOLOGIES
+      end
+    end
+
     def self.technology(name)
-      TECHNOLOGIES.find { |tech| tech.name.to_s == name.to_s }
+      technologies.find { |tech| tech.name.to_s == name.to_s }
     end
 
     def self.technology_for_key(key)
-      TECHNOLOGIES.find { |tech| key.instance_of?(tech.key_class) }
+      technologies.find { |tech| key.instance_of?(tech.key_class) }
     end
 
     def self.supported_types
-      TECHNOLOGIES.map(&:name)
+      technologies.map(&:name)
     end
 
     def self.supported_sizes(name)
@@ -32,7 +40,7 @@ module Gitlab
     end
 
     def self.supported_algorithms
-      TECHNOLOGIES.flat_map { |tech| tech.supported_algorithms }
+      technologies.flat_map { |tech| tech.supported_algorithms }
     end
 
     def self.supported_algorithms_for_name(name)

spec/lib/gitlab/ssh_public_key_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::SSHPublicKey, lib: true do
+RSpec.describe Gitlab::SSHPublicKey, lib: true, fips_mode: false do
   let(:key) { attributes_for(:rsa_key_2048)[:key] }
   let(:public_key) { described_class.new(key) }
 
@@ -19,6 +19,17 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
       it { expect(described_class.technology(name).name).to eq(name) }
       it { expect(described_class.technology(name.to_s).name).to eq(name) }
     end
+
+    context 'FIPS mode', :fips_mode do
+      where(:name) do
+        [:rsa, :ecdsa, :ed25519, :ecdsa_sk, :ed25519_sk]
+      end
+
+      with_them do
+        it { expect(described_class.technology(name).name).to eq(name) }
+        it { expect(described_class.technology(name.to_s).name).to eq(name) }
+      end
+    end
   end
 
   describe '.supported_types' do
@@ -27,6 +38,14 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
         [:rsa, :dsa, :ecdsa, :ed25519, :ecdsa_sk, :ed25519_sk]
       )
     end
+
+    context 'FIPS mode', :fips_mode do
+      it 'returns array with the names of supported technologies' do
+        expect(described_class.supported_types).to eq(
+          [:rsa, :dsa, :ecdsa, :ed25519, :ecdsa_sk, :ed25519_sk]
+        )
+      end
+    end
   end
 
   describe '.supported_sizes(name)' do
@@ -45,6 +64,24 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
       it { expect(described_class.supported_sizes(name)).to eq(sizes) }
       it { expect(described_class.supported_sizes(name.to_s)).to eq(sizes) }
     end
+
+    context 'FIPS mode', :fips_mode do
+      where(:name, :sizes) do
+        [
+          [:rsa, [3072, 4096]],
+          [:dsa, []],
+          [:ecdsa, [256, 384, 521]],
+          [:ed25519, [256]],
+          [:ecdsa_sk, [256]],
+          [:ed25519_sk, [256]]
+        ]
+      end
+
+      with_them do
+        it { expect(described_class.supported_sizes(name)).to eq(sizes) }
+        it { expect(described_class.supported_sizes(name.to_s)).to eq(sizes) }
+      end
+    end
   end
 
   describe '.supported_algorithms' do
@@ -60,6 +97,21 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
         )
       )
     end
+
+    context 'FIPS mode', :fips_mode do
+      it 'returns all supported algorithms' do
+        expect(described_class.supported_algorithms).to eq(
+          %w(
+          ssh-rsa
+          ssh-dss
+          ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521
+          ssh-ed25519
+          sk-ecdsa-sha2-nistp256@openssh.com
+          sk-ssh-ed25519@openssh.com
+          )
+        )
+      end
+    end
   end
 
   describe '.supported_algorithms_for_name' do
@@ -80,6 +132,26 @@ RSpec.describe Gitlab::SSHPublicKey, lib: true do
         expect(described_class.supported_algorithms_for_name(name.to_s)).to eq(algorithms)
       end
     end
+
+    context 'FIPS mode', :fips_mode do
+      where(:name, :algorithms) do
+        [
+          [:rsa, %w(ssh-rsa)],
+          [:dsa, %w(ssh-dss)],
+          [:ecdsa, %w(ecdsa-sha2-nistp256 ecdsa-sha2-nistp384 ecdsa-sha2-nistp521)],
+          [:ed25519, %w(ssh-ed25519)],
+          [:ecdsa_sk, %w(sk-ecdsa-sha2-nistp256@openssh.com)],
+          [:ed25519_sk, %w(sk-ssh-ed25519@openssh.com)]
+        ]
+      end
+
+      with_them do
+        it "returns all supported algorithms for #{params[:name]}" do
+          expect(described_class.supported_algorithms_for_name(name)).to eq(algorithms)
+          expect(described_class.supported_algorithms_for_name(name.to_s)).to eq(algorithms)
+        end
+      end
+    end
   end
 
   describe '.sanitize(key_content)' do

spec/models/application_setting_spec.rb

@@ -512,12 +512,8 @@ RSpec.describe ApplicationSetting do
     end
 
     context 'key restrictions' do
-      it 'supports all key types' do
-        expect(described_class::SUPPORTED_KEY_TYPES).to eq(Gitlab::SSHPublicKey.supported_types)
-      end
-
       it 'does not allow all key types to be disabled' do
-        described_class::SUPPORTED_KEY_TYPES.each do |type|
+        Gitlab::SSHPublicKey.supported_types.each do |type|
           setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE
         end
 
@@ -526,15 +522,23 @@ RSpec.describe ApplicationSetting do
       end
 
       where(:type) do
-        described_class::SUPPORTED_KEY_TYPES
+        Gitlab::SSHPublicKey.supported_types
       end
 
       with_them do
         let(:field) { :"#{type}_key_restriction" }
 
-        it { is_expected.to validate_presence_of(field) }
-        it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) }
-        it { is_expected.not_to allow_value(128).for(field) }
+        shared_examples 'key validations' do
+          it { is_expected.to validate_presence_of(field) }
+          it { is_expected.to allow_value(*KeyRestrictionValidator.supported_key_restrictions(type)).for(field) }
+          it { is_expected.not_to allow_value(128).for(field) }
+        end
+
+        it_behaves_like 'key validations'
+
+        context 'FIPS mode', :fips_mode do
+          it_behaves_like 'key validations'
+        end
       end
     end
 

spec/requests/api/settings_spec.rb

@@ -91,7 +91,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
         end
       end
 
-      it "updates application settings" do
+      it "updates application settings", fips_mode: false do
         put api("/application/settings", admin),
           params: {
             default_ci_config_path: 'debian/salsa-ci.yml',
@@ -286,6 +286,55 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do
       expect(json_response['hashed_storage_enabled']).to eq(true)
     end
 
+    context 'SSH key restriction settings', :fips_mode do
+      let(:settings) do
+        {
+          dsa_key_restriction: -1,
+          ecdsa_key_restriction: 256,
+          ecdsa_sk_key_restriction: 256,
+          ed25519_key_restriction: 256,
+          ed25519_sk_key_restriction: 256,
+          rsa_key_restriction: 3072
+        }
+      end
+
+      it 'allows updating the settings' do
+        put api("/application/settings", admin), params: settings
+
+        expect(response).to have_gitlab_http_status(:ok)
+        settings.each do |attribute, value|
+          expect(ApplicationSetting.current.public_send(attribute)).to eq(value)
+        end
+      end
+
+      it 'does not allow DSA keys' do
+        put api("/application/settings", admin), params: { dsa_key_restriction: 1024 }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+
+      it 'does not allow short RSA key values' do
+        put api("/application/settings", admin), params: { rsa_key_restriction: 2048 }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+
+      it 'does not allow unrestricted key lengths' do
+        types = %w(dsa_key_restriction
+                   ecdsa_key_restriction
+                   ecdsa_sk_key_restriction
+                   ed25519_key_restriction
+                   ed25519_sk_key_restriction
+                   rsa_key_restriction)
+
+        types.each do |type|
+          put api("/application/settings", admin), params: { type => 0 }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+    end
+
     context 'external policy classification settings' do
       let(:settings) do
         {

spec/support/fips.rb

@@ -3,10 +3,22 @@
 
 RSpec.configure do |config|
   config.around(:each, :fips_mode) do |example|
+    set_fips_mode(true) do
+      example.run
+    end
+  end
+
+  config.around(:each, fips_mode: false) do |example|
+    set_fips_mode(false) do
+      example.run
+    end
+  end
+
+  def set_fips_mode(value)
     prior_value = ENV["FIPS_MODE"]
-    ENV["FIPS_MODE"] = "true"
+    ENV["FIPS_MODE"] = value.to_s
 
-    example.run
+    yield
 
     ENV["FIPS_MODE"] = prior_value
   end

spec/support/shared_examples/models/application_setting_shared_examples.rb

@@ -239,7 +239,7 @@ RSpec.shared_examples 'application settings examples' do
 
   describe '#allowed_key_types' do
     it 'includes all key types by default' do
-      expect(setting.allowed_key_types).to contain_exactly(*described_class::SUPPORTED_KEY_TYPES)
+      expect(setting.allowed_key_types).to contain_exactly(*Gitlab::SSHPublicKey.supported_types)
     end
 
     it 'excludes disabled key types' do