Merge branch '17849-allow-admin-to-restrict-min-key-length-and-techno' into 'master'

Add settings for minimum key strength and allowed key type

Closes #17849

See merge request !13712

app/helpers/application_settings_helper.rb

@@ -84,6 +84,18 @@ module ApplicationSettingsHelper
     end
   end
 
+  def key_restriction_options_for_select(type)
+    bit_size_options = Gitlab::SSHPublicKey.supported_sizes(type).map do |bits|
+      ["Must be at least #{bits} bits", bits]
+    end
+
+    [
+      ['Are allowed', 0],
+      *bit_size_options,
+      ['Are forbidden', ApplicationSetting::FORBIDDEN_KEY_VALUE]
+    ]
+  end
+
   def repository_storages_options_for_select
     options = Gitlab.config.repositories.storages.map do |name, storage|
       ["#{name} - #{storage['path']}", name]
@@ -116,6 +128,9 @@ module ApplicationSettingsHelper
       :domain_blacklist_enabled,
       :domain_blacklist_raw,
       :domain_whitelist_raw,
+      :dsa_key_restriction,
+      :ecdsa_key_restriction,
+      :ed25519_key_restriction,
       :email_author_in_body,
       :enabled_git_access_protocol,
       :gravatar_enabled,
@@ -159,6 +174,7 @@ module ApplicationSettingsHelper
       :repository_storages,
       :require_two_factor_authentication,
       :restricted_visibility_levels,
+      :rsa_key_restriction,
       :send_user_confirmation_email,
       :sentry_dsn,
       :sentry_enabled,

app/helpers/form_helper.rb

 module FormHelper
-  def form_errors(model)
+  def form_errors(model, type: 'form')
     return unless model.errors.any?
 
     pluralized = 'error'.pluralize(model.errors.count)
-    headline   = "The form contains the following #{pluralized}:"
+    headline   = "The #{type} contains the following #{pluralized}:"
 
     content_tag(:div, class: 'alert alert-danger', id: 'error_explanation') do
       content_tag(:h4, headline) <<

app/models/application_setting.rb

@@ -13,6 +13,11 @@ class ApplicationSetting < ActiveRecord::Base
                             [\r\n]          # any number of newline characters
                           }x
 
+  # Setting a key restriction to `-1` means that all keys of this type are
+  # forbidden.
+  FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
+  SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze
+
   serialize :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize
   serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize
   serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize
@@ -146,6 +151,12 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { greater_than_or_equal_to: 0 }
 
+  SUPPORTED_KEY_TYPES.each do |type|
+    validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
+  end
+
+  validates :allowed_key_types, presence: true
+
   validates_each :restricted_visibility_levels do |record, attr, value|
     value&.each do |level|
       unless Gitlab::VisibilityLevel.options.value?(level)
@@ -171,6 +182,7 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   before_validation :ensure_uuid!
+
   before_save :ensure_runners_registration_token
   before_save :ensure_health_check_access_token
 
@@ -221,6 +233,9 @@ class ApplicationSetting < ActiveRecord::Base
       default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'],
       disabled_oauth_sign_in_sources: [],
       domain_whitelist: Settings.gitlab['domain_whitelist'],
+      dsa_key_restriction: 0,
+      ecdsa_key_restriction: 0,
+      ed25519_key_restriction: 0,
       gravatar_enabled: Settings.gravatar['enabled'],
       help_page_text: nil,
       help_page_hide_commercial_content: false,
@@ -239,6 +254,7 @@ class ApplicationSetting < ActiveRecord::Base
       max_attachment_size: Settings.gitlab['max_attachment_size'],
       password_authentication_enabled: Settings.gitlab['password_authentication_enabled'],
       performance_bar_allowed_group_id: nil,
+      rsa_key_restriction: 0,
       plantuml_enabled: false,
       plantuml_url: nil,
       project_export_enabled: true,
@@ -413,6 +429,18 @@ class ApplicationSetting < ActiveRecord::Base
     usage_ping_can_be_configured? && super
   end
 
+  def allowed_key_types
+    SUPPORTED_KEY_TYPES.select do |type|
+      key_restriction_for(type) != FORBIDDEN_KEY_VALUE
+    end
+  end
+
+  def key_restriction_for(type)
+    attr_name = "#{type}_key_restriction"
+
+    has_attribute?(attr_name) ? public_send(attr_name) : FORBIDDEN_KEY_VALUE # rubocop:disable GitlabSecurity/PublicSend
+  end
+
   private
 
   def ensure_uuid!

app/models/key.rb

 require 'digest/md5'
 
 class Key < ActiveRecord::Base
+  include Gitlab::CurrentSettings
   include Sortable
 
   LAST_USED_AT_REFRESH_TIME = 1.day.to_i
@@ -12,14 +13,18 @@ class Key < ActiveRecord::Base
   validates :title,
     presence: true,
     length: { maximum: 255 }
+
   validates :key,
     presence: true,
     length: { maximum: 5000 },
     format: { with: /\A(ssh|ecdsa)-.*\Z/ }
+
   validates :fingerprint,
     uniqueness: true,
     presence: { message: 'cannot be generated' }
 
+  validate :key_meets_restrictions
+
   delegate :name, :email, to: :user, prefix: true
 
   after_commit :add_to_shell, on: :create
@@ -80,6 +85,10 @@ class Key < ActiveRecord::Base
     SystemHooksService.new.execute_hooks_for(self, :destroy)
   end
 
+  def public_key
+    @public_key ||= Gitlab::SSHPublicKey.new(key)
+  end
+
   private
 
   def generate_fingerprint
@@ -87,7 +96,27 @@ class Key < ActiveRecord::Base
 
     return unless self.key.present?
 
-    self.fingerprint = Gitlab::KeyFingerprint.new(self.key).fingerprint
+    self.fingerprint = public_key.fingerprint
+  end
+
+  def key_meets_restrictions
+    restriction = current_application_settings.key_restriction_for(public_key.type)
+
+    if restriction == ApplicationSetting::FORBIDDEN_KEY_VALUE
+      errors.add(:key, forbidden_key_type_message)
+    elsif public_key.bits < restriction
+      errors.add(:key, "must be at least #{restriction} bits")
+    end
+  end
+
+  def forbidden_key_type_message
+    allowed_types =
+      current_application_settings
+        .allowed_key_types
+        .map(&:upcase)
+        .to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
+
+    "type is forbidden. Must be #{allowed_types}"
   end
 
   def notify_user

app/validators/key_restriction_validator.rb

+class KeyRestrictionValidator < ActiveModel::EachValidator
+  FORBIDDEN = -1
+
+  def self.supported_sizes(type)
+    Gitlab::SSHPublicKey.supported_sizes(type)
+  end
+
+  def self.supported_key_restrictions(type)
+    [0, *supported_sizes(type), FORBIDDEN]
+  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}")
+    end
+  end
+
+  private
+
+  def supported_sizes_message
+    sizes = self.class.supported_sizes(options[:type])
+    sizes.to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
+  end
+
+  def valid_restriction?(value)
+    choices = self.class.supported_key_restrictions(options[:type])
+    choices.include?(value)
+  end
+end

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

@@ -42,12 +42,7 @@
           = link_to "(?)", help_page_path("integration/bitbucket")
           and GitLab.com
           = link_to "(?)", help_page_path("integration/gitlab")
-    .form-group
-      %label.control-label.col-sm-2 Enabled Git access protocols
-      .col-sm-10
-        = select(:application_setting, :enabled_git_access_protocol, [['Both SSH and HTTP(S)', nil], ['Only SSH', 'ssh'], ['Only HTTP(S)', 'http']], {}, class: 'form-control')
-        %span.help-block#clone-protocol-help
-          Allow only the selected protocols to be used for Git access.
+
     .form-group
       .col-sm-offset-2.col-sm-10
         .checkbox
@@ -55,6 +50,20 @@
             = f.check_box :project_export_enabled
             Project export enabled
 
+    .form-group
+      %label.control-label.col-sm-2 Enabled Git access protocols
+      .col-sm-10
+        = select(:application_setting, :enabled_git_access_protocol, [['Both SSH and HTTP(S)', nil], ['Only SSH', 'ssh'], ['Only HTTP(S)', 'http']], {}, class: 'form-control')
+        %span.help-block#clone-protocol-help
+          Allow only the selected protocols to be used for Git access.
+
+    - ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
+      - field_name = :"#{type}_key_restriction"
+      .form-group
+        = f.label field_name, "#{type.upcase} SSH keys", class: 'control-label col-sm-2'
+        .col-sm-10
+          = f.select field_name, key_restriction_options_for_select(type), {}, class: 'form-control'
+
   %fieldset
     %legend Account and Limit Settings
     .form-group

app/views/profiles/keys/_key.html.haml

 %li.key-list-item
   .pull-left.append-right-10
-    = icon 'key', class: "settings-list-icon hidden-xs"
+    - if key.valid?
+      = icon 'key', class: 'settings-list-icon hidden-xs'
+    - else
+      = icon 'exclamation-triangle', class: 'settings-list-icon hidden-xs has-tooltip',
+        title: key.errors.full_messages.join(', ')
+
+
   .key-list-item-info
     = link_to path_to_key(key, is_admin), class: "title" do
       = key.title

app/views/profiles/keys/_key_details.html.haml

@@ -16,6 +16,7 @@
           %strong= @key.last_used_at.try(:to_s, :medium) || 'N/A'
 
   .col-md-8
+    = form_errors(@key, type: 'key') unless @key.valid?
     %p
       %span.light Fingerprint:
       %code.key-fingerprint= @key.fingerprint

changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml

+---
+title: Add settings for minimum SSH key strength and allowed key type
+merge_request: 13712
+author: Cory Hinshaw
+type: added

db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb

+class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    # A key restriction has these possible states:
+    #
+    #   * -1 means "this key type is completely disabled"
+    #   * 0 means "all keys of this type are valid"
+    #   * > 0 means "keys must have at least this many bits to be valid"
+    #
+    # The default is 0, for backward compatibility
+    add_column_with_default :application_settings, :rsa_key_restriction, :integer, default: 0
+    add_column_with_default :application_settings, :dsa_key_restriction, :integer, default: 0
+    add_column_with_default :application_settings, :ecdsa_key_restriction, :integer, default: 0
+    add_column_with_default :application_settings, :ed25519_key_restriction, :integer, default: 0
+  end
+
+  def down
+    remove_column :application_settings, :rsa_key_restriction
+    remove_column :application_settings, :dsa_key_restriction
+    remove_column :application_settings, :ecdsa_key_restriction
+    remove_column :application_settings, :ed25519_key_restriction
+  end
+end

db/schema.rb

@@ -129,6 +129,10 @@ ActiveRecord::Schema.define(version: 20170824162758) do
     t.boolean "password_authentication_enabled"
     t.boolean "project_export_enabled", default: true, null: false
     t.boolean "hashed_storage_enabled", default: false, null: false
+    t.integer "rsa_key_restriction", default: 0, null: false
+    t.integer "dsa_key_restriction", default: 0, null: false
+    t.integer "ecdsa_key_restriction", default: 0, null: false
+    t.integer "ed25519_key_restriction", default: 0, null: false
   end
 
   create_table "audit_events", force: :cascade do |t|

doc/api/settings.md

@@ -48,7 +48,11 @@ Example response:
    "plantuml_enabled": false,
    "plantuml_url": null,
    "terminal_max_session_time": 0,
-   "polling_interval_multiplier": 1.0
+   "polling_interval_multiplier": 1.0,
+   "rsa_key_restriction": 0,
+   "dsa_key_restriction": 0,
+   "ecdsa_key_restriction": 0,
+   "ed25519_key_restriction": 0,
 }
 ```
 
@@ -88,6 +92,10 @@ PUT /application/settings
 | `plantuml_url` | string | yes (if `plantuml_enabled` is `true`) |  The PlantUML instance URL for integration. |
 | `terminal_max_session_time` | integer | no | Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time. |
 | `polling_interval_multiplier` | decimal | no | Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling. |
+| `rsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `0` (no restriction). `-1` disables RSA keys.
+| `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys.
+| `ecdsa_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `0` (no restriction). `-1` disables ECDSA keys.
+| `ed25519_key_restriction` | integer | no | The minimum allowed curve size (in bits) of an uploaded ED25519 key. Default is `0` (no restriction). `-1` disables ED25519 keys.
 
 ```bash
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/application/settings?signup_enabled=false&default_project_visibility=internal
@@ -125,6 +133,10 @@ Example response:
   "plantuml_enabled": false,
   "plantuml_url": null,
   "terminal_max_session_time": 0,
-  "polling_interval_multiplier": 1.0
+  "polling_interval_multiplier": 1.0,
+  "rsa_key_restriction": 0,
+  "dsa_key_restriction": 0,
+  "ecdsa_key_restriction": 0,
+  "ed25519_key_restriction": 0,
 }
 ```

doc/security/README.md

 # Security
 
 - [Password length limits](password_length_limits.md)
+- [Restrict SSH key technologies and minimum length](ssh_keys_restrictions.md)
 - [Rack attack](rack_attack.md)
 - [Webhooks and insecure internal web services](webhooks.md)
 - [Information exclusivity](information_exclusivity.md)

doc/security/ssh_keys_restrictions.md

+# Restrict allowed SSH key technologies and minimum length
+
+`ssh-keygen` allows users to create RSA keys with as few as 768 bits, which
+falls well below recommendations from certain standards groups (such as the US
+NIST). Some organizations deploying GitLab will need to enforce minimum key
+strength, either to satisfy internal security policy or for regulatory
+compliance.
+
+Similarly, certain standards groups recommend using RSA, ECDSA, or ED25519 over
+the older DSA, and administrators may need to limit the allowed SSH key
+algorithms.
+
+GitLab allows you to restrict the allowed SSH key technology as well as specify
+the minimum key length for each technology.
+
+In the Admin area under **Settings** (`/admin/application_settings`), look for
+the "Visibility and Access Controls" area:
+
+![SSH keys restriction admin settings](img/ssh_keys_restrictions_settings.png)

lib/api/settings.rb

@@ -122,6 +122,13 @@ module API
       optional :terminal_max_session_time, type: Integer, desc: 'Maximum time for web terminal websocket connection (in seconds). Set to 0 for unlimited time.'
       optional :polling_interval_multiplier, type: BigDecimal, desc: 'Interval multiplier used by endpoints that perform polling. Set to 0 to disable polling.'
 
+      ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
+        optional :"#{type}_key_restriction",
+                 type: Integer,
+                 values: KeyRestrictionValidator.supported_key_restrictions(type),
+                 desc: "Restrictions on the complexity of uploaded #{type.upcase} keys. A value of #{ApplicationSetting::FORBIDDEN_KEY_VALUE} disables all #{type.upcase} keys."
+      end
+
       optional(*::ApplicationSettingsHelper.visible_attributes)
       at_least_one_of(*::ApplicationSettingsHelper.visible_attributes)
     end

lib/gitlab/git_access.rb

@@ -35,6 +35,7 @@ module Gitlab
 
     def check(cmd, changes)
       check_protocol!
+      check_valid_actor!
       check_active_user!
       check_project_accessibility!
       check_project_moved!
@@ -70,6 +71,14 @@ module Gitlab
 
     private
 
+    def check_valid_actor!
+      return unless actor.is_a?(Key)
+
+      unless actor.valid?
+        raise UnauthorizedError, "Your SSH key #{actor.errors[:key].first}."
+      end
+    end
+
     def check_protocol!
       unless protocol_allowed?
         raise UnauthorizedError, "Git access over #{protocol.upcase} is not allowed"

lib/gitlab/key_fingerprint.rb

-module Gitlab
-  class KeyFingerprint
-    attr_reader :key, :ssh_key
-
-    # Unqualified MD5 fingerprint for compatibility
-    delegate :fingerprint, to: :ssh_key, allow_nil: true
-
-    def initialize(key)
-      @key = key
-
-      @ssh_key =
-        begin
-          Net::SSH::KeyFactory.load_data_public_key(key)
-        rescue Net::SSH::Exception, NotImplementedError
-        end
-    end
-
-    def valid?
-      ssh_key.present?
-    end
-
-    def type
-      return unless valid?
-
-      parts = ssh_key.ssh_type.split('-')
-      parts.shift if parts[0] == 'ssh'
-
-      parts[0].upcase
-    end
-
-    def bits
-      return unless valid?
-
-      case type
-      when 'RSA'
-        ssh_key.n.num_bits
-      when 'DSS', 'DSA'
-        ssh_key.p.num_bits
-      when 'ECDSA'
-        ssh_key.group.order.num_bits
-      when 'ED25519'
-        256
-      else
-        raise "Unsupported key type: #{type}"
-      end
-    end
-  end
-end

lib/gitlab/ssh_public_key.rb

+module Gitlab
+  class SSHPublicKey
+    Technology = Struct.new(:name, :key_class, :supported_sizes)
+
+    Technologies = [
+      Technology.new(:rsa, OpenSSL::PKey::RSA, [1024, 2048, 3072, 4096]),
+      Technology.new(:dsa, OpenSSL::PKey::DSA, [1024, 2048, 3072]),
+      Technology.new(:ecdsa, OpenSSL::PKey::EC, [256, 384, 521]),
+      Technology.new(:ed25519, Net::SSH::Authentication::ED25519::PubKey, [256])
+    ].freeze
+
+    def self.technology(name)
+      Technologies.find { |tech| tech.name.to_s == name.to_s }
+    end
+
+    def self.technology_for_key(key)
+      Technologies.find { |tech| key.is_a?(tech.key_class) }
+    end
+
+    def self.supported_sizes(name)
+      technology(name)&.supported_sizes
+    end
+
+    attr_reader :key_text, :key
+
+    # Unqualified MD5 fingerprint for compatibility
+    delegate :fingerprint, to: :key, allow_nil: true
+
+    def initialize(key_text)
+      @key_text = key_text
+
+      @key =
+        begin
+          Net::SSH::KeyFactory.load_data_public_key(key_text)
+        rescue StandardError, NotImplementedError
+        end
+    end
+
+    def valid?
+      key.present?
+    end
+
+    def type
+      technology.name if valid?
+    end
+
+    def bits
+      return unless valid?
+
+      case type
+      when :rsa
+        key.n.num_bits
+      when :dsa
+        key.p.num_bits
+      when :ecdsa
+        key.group.order.num_bits
+      when :ed25519
+        256
+      else
+        raise "Unsupported key type: #{type}"
+      end
+    end
+
+    private
+
+    def technology
+      @technology ||=
+        self.class.technology_for_key(key) || raise("Unsupported key type: #{key.class}")
+    end
+  end
+end

spec/factories/keys.rb

@@ -18,5 +18,54 @@ FactoryGirl.define do
     factory :write_access_key, class: 'DeployKey' do
       can_push true
     end
+
+    factory :rsa_key_2048 do
+      key do
+        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O9' \
+        '6x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5' \
+        '/jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7' \
+        'M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaC' \
+        'rzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy0' \
+        '5qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz= dummy@gitlab.com'
+      end
+
+      factory :rsa_deploy_key_2048, class: 'DeployKey'
+    end
+
+    factory :dsa_key_2048 do
+      key do
+        'ssh-dss AAAAB3NzaC1kc3MAAAEBAO/3/NPLA/zSFkMOCaTtGo+uos1flfQ5f038Uk+G' \
+        'Y9AeLGzX+Srhw59GdVXmOQLYBrOt5HdGwqYcmLnE2VurUGmhtfeO5H+3p5pGJbkS0Gxp' \
+        'YH1HRO9lWsncF3Hh1w4lYsDjkclDiSTdfTuN8F4Kb3DXNnVSCieeonp+B25F/CXagyTQ' \
+        '/pvNmHFeYgGCVdnBtFdi+xfxaZ8NKdPrGggzokbKHElDZQ4Xo5EpdcyLajgM7nB2r2Rz' \
+        'OrmeaevKi5lV68ehRa9Yyrb7vxvwiwBwOgqR/mnN7Gnaq1jUdmJY+ct04Qwx37f5jvhv' \
+        '5gA4U40SGMoiHM8RFIN7Ksz0jsyX73MAAAAVALRWOfjfzHpK7KLz4iqDvvTUAevJAAAB' \
+        'AEa9NZ+6y9iQ5erGsdfLTXFrhSefTG0NhghoO/5IFkSGfd8V7kzTvCHaFrcfpEA5kP8t' \
+        'poeOG0TASB6tgGOxm1Bq4Wncry5RORBPJlAVpDGRcvZ931ddH7IgltEInS6za2uH6F/1' \
+        'M1QfKePSLr6xJ1ZLYfP0Og5KTp1x6yMQvfwV0a+XdA+EPgaJWLWp/pWwKWa0oLUgjsIH' \
+        'MYzuOGh5c708uZrmkzqvgtW2NgXhcIroRgynT3IfI2lP2rqqb3uuuE/qH5UCUFO+Dc3H' \
+        'nAFNeQDT/M25AERdPYBAY5a+iPjIgO+jT7BfmfByT+AZTqZySrCyc7nNZL3YgGLK0l6A' \
+        '1GgAAAEBAN9FpFOdIXE+YEZhKl1vPmbcn+b1y5zOl6N4x1B7Q8pD/pLMziWROIS8uLzb' \
+        'aZ0sMIWezHIkxuo1iROMeT+jtCubn7ragaN6AX7nMpxYUH9+mYZZs/fyElt6wCviVhTI' \
+        'zM+u7VdQsnZttOOlQfogHdL+SpeAft0DsfJjlcgQnsLlHQKv6aPqCPYUST2nE7RyW/Ex' \
+        'PrMxLtOWt0/j8RYHbwwqvyeZqBz3ESBgrS9c5tBdBfauwYUV/E7gPLOU3OZFw9ue7o+z' \
+        'wzoTZqW6Xouy5wtWvSLQSLT5XwOslmQz8QMBxD0AQyDfEFGsBCWzmbTgKv9uqrBjubsS' \
+        'Taja+Cf9kMo== dummy@gitlab.com'
+      end
+    end
+
+    factory :ecdsa_key_256 do
+      key do
+        'ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYA' \
+        'AABBBJZmkzTgY0fiCQ+DVReyH/fFwTFz0XoR3RUO0u+199H19KFw7mNPxRSMOVS7tEtO' \
+        'Nj3Q7FcZXfqthHvgAzDiHsc= dummy@gitlab.com'
+      end
+    end
+
+    factory :ed25519_key_256 do
+      key do
+        'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIETnVTgzqC1gatgSlC4zH6aYt2CAQzgJOhDRvf59ohL6 dummy@gitlab.com'
+      end
+    end
   end
 end

spec/features/admin/admin_settings_spec.rb

@@ -79,6 +79,22 @@ feature 'Admin updates settings' do
     end
   end
 
+  scenario 'Change Keys settings' do
+    select 'Are forbidden', from: 'RSA SSH keys'
+    select 'Are allowed', from: 'DSA SSH keys'
+    select 'Must be at least 384 bits', from: 'ECDSA SSH keys'
+    select 'Are forbidden', from: 'ED25519 SSH keys'
+    click_on 'Save'
+
+    forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE.to_s
+
+    expect(page).to have_content 'Application settings saved successfully'
+    expect(find_field('RSA SSH keys').value).to eq(forbidden)
+    expect(find_field('DSA SSH keys').value).to eq('0')
+    expect(find_field('ECDSA SSH keys').value).to eq('384')
+    expect(find_field('ED25519 SSH keys').value).to eq(forbidden)
+  end
+
   def check_all_events
     page.check('Active')
     page.check('Push')

spec/features/profiles/keys_spec.rb

@@ -28,6 +28,23 @@ feature 'Profile > SSH Keys' do
       expect(page).to have_content("Title: #{attrs[:title]}")
       expect(page).to have_content(attrs[:key])
     end
+
+    context 'when only DSA and ECDSA keys are allowed' do
+      before do
+        forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE
+        stub_application_setting(rsa_key_restriction: forbidden, ed25519_key_restriction: forbidden)
+      end
+
+      scenario 'shows a validation error' do
+        attrs = attributes_for(:key)
+
+        fill_in('Key', with: attrs[:key])
+        fill_in('Title', with: attrs[:title])
+        click_button('Add key')
+
+        expect(page).to have_content('Key type is forbidden. Must be DSA or ECDSA')
+      end
+    end
   end
 
   scenario 'User sees their keys' do

spec/lib/gitlab/git_access_spec.rb

@@ -155,6 +155,44 @@ describe Gitlab::GitAccess do
     end
   end
 
+  shared_examples '#check with a key that is not valid' do
+    before do
+      project.add_master(user)
+    end
+
+    context 'key is too small' do
+      before do
+        stub_application_setting(rsa_key_restriction: 4096)
+      end
+
+      it 'does not allow keys which are too small', aggregate_failures: true do
+        expect(actor).not_to be_valid
+        expect { pull_access_check }.to raise_unauthorized('Your SSH key must be at least 4096 bits.')
+        expect { push_access_check }.to raise_unauthorized('Your SSH key must be at least 4096 bits.')
+      end
+    end
+
+    context 'key type is not allowed' do
+      before do
+        stub_application_setting(rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE)
+      end
+
+      it 'does not allow keys which are too small', aggregate_failures: true do
+        expect(actor).not_to be_valid
+        expect { pull_access_check }.to raise_unauthorized(/Your SSH key type is forbidden/)
+        expect { push_access_check }.to raise_unauthorized(/Your SSH key type is forbidden/)
+      end
+    end
+  end
+
+  it_behaves_like '#check with a key that is not valid' do
+    let(:actor) { build(:rsa_key_2048, user: user) }
+  end
+
+  it_behaves_like '#check with a key that is not valid' do
+    let(:actor) { build(:rsa_deploy_key_2048, user: user) }
+  end
+
   describe '#check_project_moved!' do
     before do
       project.add_master(user)

spec/lib/gitlab/key_fingerprint_spec.rb

-require 'spec_helper'
-
-describe Gitlab::KeyFingerprint, lib: true do
-  KEYS = {
-    rsa:
-      'example.com ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC5z65PwQ1GE6foJgwk' \
-      '9rmQi/glaXbUeVa5uvQpnZ3Z5+forcI7aTngh3aZ/H2UDP2L70TGy7kKNyp0J3a8/OdG' \
-      'Z08y5yi3JlbjFARO1NyoFEjw2H1SJxeJ43L6zmvTlu+hlK1jSAlidl7enS0ufTlzEEj4' \
-      'iJcuTPKdVzKRgZuTRVm9woWNVKqIrdRC0rJiTinERnfSAp/vNYERMuaoN4oJt8p/NEek' \
-      'rmFoDsQOsyDW5RAnCnjWUU+jFBKDpfkJQ1U2n6BjJewC9dl6ODK639l3yN4WOLZEk4tN' \
-      'UysfbGeF3rmMeflaD6O1Jplpv3YhwVGFNKa7fMq6k3Z0tszTJPYh',
-    ecdsa:
-      'example.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAI' \
-      'bmlzdHAyNTYAAABBBKTJy43NZzJSfNxpv/e2E6Zy3qoHoTQbmOsU5FEfpWfWa1MdTeXQ' \
-      'YvKOi+qz/1AaNx6BK421jGu74JCDJtiZWT8=',
-    ed25519:
-      '@revoked example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAfuCHKVTjq' \
-      'uxvt6CM6tdG4SLp1Btn/nOeHHE5UOzRdf',
-    dss:
-      'example.com ssh-dss AAAAB3NzaC1kc3MAAACBAP1/U4EddRIpUt9KnC7s5Of2EbdS' \
-      'PO9EAMMeP4C2USZpRV1AIlH7WT2NWPq/xfW6MPbLm1Vs14E7gB00b/JmYLdrmVClpJ+f' \
-      '6AR7ECLCT7up1/63xhv4O1fnxqimFQ8E+4P208UewwI1VBNaFpEy9nXzrith1yrv8iID' \
-      'GZ3RSAHHAAAAFQCXYFCPFSMLzLKSuYKi64QL8Fgc9QAAAIEA9+GghdabPd7LvKtcNrhX' \
-      'uXmUr7v6OuqC+VdMCz0HgmdRWVeOutRZT+ZxBxCBgLRJFnEj6EwoFhO3zwkyjMim4TwW' \
-      'eotUfI0o4KOuHiuzpnWRbqN/C/ohNWLx+2J6ASQ7zKTxvqhRkImog9/hWuWfBpKLZl6A' \
-      'e1UlZAFMO/7PSSoAAACBAJcQ4JODqhuGbXIEpqxetm7PWbdbCcr3y/GzIZ066pRovpL6' \
-      'qm3qCVIym4cyChxWwb8qlyCIi+YRUUWm1z/wiBYT2Vf3S4FXBnyymCkKEaV/EY7+jd4X' \
-      '1bXI58OD2u+bLCB/sInM4fGB8CZUIWT9nJH0Ve9jJUge2ms348/QOJ1+'
-  }.freeze
-
-  MD5_FINGERPRINTS = {
-    rsa: '06:b2:8a:92:df:0e:11:2c:ca:7b:8f:a4:ba:6e:4b:fd',
-    ecdsa: '45:ff:5b:98:9a:b6:8a:41:13:c1:30:8b:09:5e:7b:4e',
-    ed25519: '2e:65:6a:c8:cf:bf:b2:8b:9a:bd:6d:9f:11:5c:12:16',
-    dss: '57:98:86:02:5f:9c:f4:9b:ad:5a:1e:51:92:0e:fd:2b'
-  }.freeze
-
-  BIT_COUNTS = {
-    rsa: 2048,
-    ecdsa: 256,
-    ed25519: 256,
-    dss: 1024
-  }.freeze
-
-  describe '#type' do
-    KEYS.each do |type, key|
-      it "calculates the type of #{type} keys" do
-        calculated_type = described_class.new(key).type
-
-        expect(calculated_type).to eq(type.to_s.upcase)
-      end
-    end
-  end
-
-  describe '#fingerprint' do
-    KEYS.each do |type, key|
-      it "calculates the MD5 fingerprint for #{type} keys" do
-        fp = described_class.new(key).fingerprint
-
-        expect(fp).to eq(MD5_FINGERPRINTS[type])
-      end
-    end
-  end
-
-  describe '#bits' do
-    KEYS.each do |type, key|
-      it "calculates the number of bits in #{type} keys" do
-        bits = described_class.new(key).bits
-
-        expect(bits).to eq(BIT_COUNTS[type])
-      end
-    end
-  end
-
-  describe '#key' do
-    it 'carries the unmodified key data' do
-      key = described_class.new(KEYS[:rsa]).key
-
-      expect(key).to eq(KEYS[:rsa])
-    end
-  end
-end

spec/lib/gitlab/ssh_public_key_spec.rb

+require 'spec_helper'
+
+describe Gitlab::SSHPublicKey, lib: true do
+  let(:key) { attributes_for(:rsa_key_2048)[:key] }
+  let(:public_key) { described_class.new(key) }
+
+  describe '.technology(name)' do
+    it 'returns nil for an unrecognised name' do
+      expect(described_class.technology(:foo)).to be_nil
+    end
+
+    where(:name) do
+      [:rsa, :dsa, :ecdsa, :ed25519]
+    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
+
+  describe '.supported_sizes(name)' do
+    where(:name, :sizes) do
+      [
+        [:rsa, [1024, 2048, 3072, 4096]],
+        [:dsa, [1024, 2048, 3072]],
+        [:ecdsa, [256, 384, 521]],
+        [:ed25519, [256]]
+      ]
+    end
+
+    subject { described_class.supported_sizes(name) }
+
+    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
+
+  describe '#valid?' do
+    subject { public_key }
+
+    context 'with a valid SSH key' do
+      it { is_expected.to be_valid }
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it { is_expected.not_to be_valid }
+    end
+  end
+
+  describe '#type' do
+    subject { public_key.type }
+
+    where(:factory, :type) do
+      [
+        [:rsa_key_2048, :rsa],
+        [:dsa_key_2048, :dsa],
+        [:ecdsa_key_256, :ecdsa],
+        [:ed25519_key_256, :ed25519]
+      ]
+    end
+
+    with_them do
+      let(:key) { attributes_for(factory)[:key] }
+
+      it { is_expected.to eq(type) }
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#bits' do
+    subject { public_key.bits }
+
+    where(:factory, :bits) do
+      [
+        [:rsa_key_2048, 2048],
+        [:dsa_key_2048, 2048],
+        [:ecdsa_key_256, 256],
+        [:ed25519_key_256, 256]
+      ]
+    end
+
+    with_them do
+      let(:key) { attributes_for(factory)[:key] }
+
+      it { is_expected.to eq(bits) }
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#fingerprint' do
+    subject { public_key.fingerprint }
+
+    where(:factory, :fingerprint) do
+      [
+        [:rsa_key_2048, '2e:ca:dc:e0:37:29:ed:fc:f0:1d:bf:66:d4:cd:51:b1'],
+        [:dsa_key_2048, 'bc:c1:a4:be:7e:8c:84:56:b3:58:93:53:c6:80:78:8c'],
+        [:ecdsa_key_256, '67:a3:a9:7d:b8:e1:15:d4:80:40:21:34:bb:ed:97:38'],
+        [:ed25519_key_256, 'e6:eb:45:8a:3c:59:35:5f:e9:5b:80:12:be:7e:22:73']
+      ]
+    end
+
+    with_them do
+      let(:key) { attributes_for(factory)[:key] }
+
+      it { is_expected.to eq(fingerprint) }
+    end
+
+    context 'with an invalid SSH key' do
+      let(:key) { 'this is not a key' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#key_text' do
+    let(:key) { 'this is not a key' }
+
+    it 'carries the unmodified key data' do
+      expect(public_key.key_text).to eq(key)
+    end
+  end
+end

spec/models/application_setting_spec.rb

@@ -72,6 +72,33 @@ describe ApplicationSetting do
         .is_greater_than(0)
     end
 
+    context 'key restrictions' do
+      it 'supports all key types' do
+        expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)
+      end
+
+      it 'does not allow all key types to be disabled' do
+        described_class::SUPPORTED_KEY_TYPES.each do |type|
+          setting["#{type}_key_restriction"] = described_class::FORBIDDEN_KEY_VALUE
+        end
+
+        expect(setting).not_to be_valid
+        expect(setting.errors.messages).to have_key(:allowed_key_types)
+      end
+
+      where(:type) do
+        described_class::SUPPORTED_KEY_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) }
+      end
+    end
+
     it_behaves_like 'an object with email-formated attributes', :admin_notification_email do
       subject { setting }
     end
@@ -441,4 +468,36 @@ describe ApplicationSetting do
       end
     end
   end
+
+  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)
+    end
+
+    it 'excludes disabled key types' do
+      expect(setting.allowed_key_types).to include(:ed25519)
+
+      setting.ed25519_key_restriction = described_class::FORBIDDEN_KEY_VALUE
+
+      expect(setting.allowed_key_types).not_to include(:ed25519)
+    end
+  end
+
+  describe '#key_restriction_for' do
+    it 'returns the restriction value for recognised types' do
+      setting.rsa_key_restriction = 1024
+
+      expect(setting.key_restriction_for(:rsa)).to eq(1024)
+    end
+
+    it 'allows types to be passed as a string' do
+      setting.rsa_key_restriction = 1024
+
+      expect(setting.key_restriction_for('rsa')).to eq(1024)
+    end
+
+    it 'returns forbidden for unrecognised type' do
+      expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE)
+    end
+  end
 end

spec/models/key_spec.rb

 require 'spec_helper'
 
 describe Key, :mailer do
+  include Gitlab::CurrentSettings
+
+  describe 'modules' do
+    subject { described_class }
+    it { is_expected.to include_module(Gitlab::CurrentSettings) }
+  end
+
   describe "Associations" do
     it { is_expected.to belong_to(:user) }
   end
@@ -11,8 +18,10 @@ describe Key, :mailer do
 
     it { is_expected.to validate_presence_of(:key) }
     it { is_expected.to validate_length_of(:key).is_at_most(5000) }
-    it { is_expected.to allow_value('ssh-foo').for(:key) }
-    it { is_expected.to allow_value('ecdsa-foo').for(:key) }
+    it { is_expected.to allow_value(attributes_for(:rsa_key_2048)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:dsa_key_2048)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:ecdsa_key_256)[:key]).for(:key) }
+    it { is_expected.to allow_value(attributes_for(:ed25519_key_256)[:key]).for(:key) }
     it { is_expected.not_to allow_value('foo-bar').for(:key) }
   end
 
@@ -95,6 +104,48 @@ describe Key, :mailer do
     end
   end
 
+  context 'validate it meets key restrictions' do
+    where(:factory, :minimum, :result) do
+      forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE
+
+      [
+        [:rsa_key_2048,    0, true],
+        [:dsa_key_2048,    0, true],
+        [:ecdsa_key_256,   0, true],
+        [:ed25519_key_256, 0, true],
+
+        [:rsa_key_2048, 1024, true],
+        [:rsa_key_2048, 2048, true],
+        [:rsa_key_2048, 4096, false],
+
+        [:dsa_key_2048, 1024, true],
+        [:dsa_key_2048, 2048, true],
+        [:dsa_key_2048, 4096, false],
+
+        [:ecdsa_key_256, 256, true],
+        [:ecdsa_key_256, 384, false],
+
+        [:ed25519_key_256, 256, true],
+        [:ed25519_key_256, 384, false],
+
+        [:rsa_key_2048,    forbidden, false],
+        [:dsa_key_2048,    forbidden, false],
+        [:ecdsa_key_256,   forbidden, false],
+        [:ed25519_key_256, forbidden, false]
+      ]
+    end
+
+    with_them do
+      subject(:key) { build(factory) }
+
+      before do
+        stub_application_setting("#{key.public_key.type}_key_restriction" => minimum)
+      end
+
+      it { expect(key.valid?).to eq(result) }
+    end
+  end
+
   context 'callbacks' do
     it 'adds new key to authorized_file' do
       key = build(:personal_key, id: 7)

spec/requests/api/settings_spec.rb

@@ -19,6 +19,10 @@ describe API::Settings, 'Settings' do
       expect(json_response['default_project_visibility']).to be_a String
       expect(json_response['default_snippet_visibility']).to be_a String
       expect(json_response['default_group_visibility']).to be_a String
+      expect(json_response['rsa_key_restriction']).to eq(0)
+      expect(json_response['dsa_key_restriction']).to eq(0)
+      expect(json_response['ecdsa_key_restriction']).to eq(0)
+      expect(json_response['ed25519_key_restriction']).to eq(0)
     end
   end
 
@@ -44,7 +48,11 @@ describe API::Settings, 'Settings' do
           help_page_text: 'custom help text',
           help_page_hide_commercial_content: true,
           help_page_support_url: 'http://example.com/help',
-          project_export_enabled: false
+          project_export_enabled: false,
+          rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE,
+          dsa_key_restriction: 2048,
+          ecdsa_key_restriction: 384,
+          ed25519_key_restriction: 256
 
         expect(response).to have_http_status(200)
         expect(json_response['default_projects_limit']).to eq(3)
@@ -61,6 +69,10 @@ describe API::Settings, 'Settings' do
         expect(json_response['help_page_hide_commercial_content']).to be_truthy
         expect(json_response['help_page_support_url']).to eq('http://example.com/help')
         expect(json_response['project_export_enabled']).to be_falsey
+        expect(json_response['rsa_key_restriction']).to eq(ApplicationSetting::FORBIDDEN_KEY_VALUE)
+        expect(json_response['dsa_key_restriction']).to eq(2048)
+        expect(json_response['ecdsa_key_restriction']).to eq(384)
+        expect(json_response['ed25519_key_restriction']).to eq(256)
       end
     end