Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2017-08-25 16:08:48 +0300
committerNick Thomas <nick@gitlab.com>2017-08-30 22:50:44 +0300
commit6847060266792471c9c14518a5106e0f622cd6c5 (patch)
tree291238748abd929e77aaf462b8833bd336e39f5d
parentb49b7bc147955df6589b13942d0437a3b4518c7b (diff)
Rework the permissions model for SSH key restrictions
`allowed_key_types` is removed and the `minimum_<type>_bits` fields are renamed to `<tech>_key_restriction`. A special sentinel value (`-1`) signifies that the key type is disabled. This also feeds through to the UI - checkboxes per key type are out, inline selection of "forbidden" and "allowed" (i.e., no restrictions) are in. As with the previous model, unknown key types are disallowed, even if the underlying ssh daemon happens to support them. The defaults have also been changed from the lowest known bit size to "no restriction". So if someone does happen to have a 768-bit RSA key, it will continue to work on upgrade, at least until the administrator restricts them.
-rw-r--r--app/controllers/admin/application_settings_controller.rb3
-rw-r--r--app/helpers/application_settings_helper.rb28
-rw-r--r--app/models/application_setting.rb62
-rw-r--r--app/models/key.rb37
-rw-r--r--app/views/admin/application_settings/_form.html.haml42
-rw-r--r--db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb24
-rw-r--r--db/schema.rb9
-rw-r--r--doc/api/settings.md27
-rw-r--r--doc/security/img/ssh_keys_restrictions_settings.pngbin41803 -> 13698 bytes
-rw-r--r--lib/api/entities.rb1
-rw-r--r--lib/api/settings.rb11
-rw-r--r--lib/gitlab/ssh_public_key.rb52
-rw-r--r--spec/features/admin/admin_settings_spec.rb24
-rw-r--r--spec/features/profiles/keys_spec.rb5
-rw-r--r--spec/lib/gitlab/git_access_spec.rb12
-rw-r--r--spec/lib/gitlab/ssh_public_key_spec.rb36
-rw-r--r--spec/models/application_setting_spec.rb63
-rw-r--r--spec/models/key_spec.rb66
-rw-r--r--spec/requests/api/settings_spec.rb27
19 files changed, 228 insertions, 301 deletions
diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb
index 8cc7111033f..8367c22d1ca 100644
--- a/app/controllers/admin/application_settings_controller.rb
+++ b/app/controllers/admin/application_settings_controller.rb
@@ -66,8 +66,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
end
end
- params[:application_setting][:allowed_key_types]&.delete('')
-
enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
params[:application_setting][:disabled_oauth_sign_in_sources] =
@@ -85,7 +83,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
def visible_application_setting_attributes
ApplicationSettingsHelper.visible_attributes + [
:domain_blacklist_file,
- allowed_key_types: [],
disabled_oauth_sign_in_sources: [],
import_sources: [],
repository_storages: [],
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 75d090359d0..0c74d464601 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -81,18 +81,16 @@ module ApplicationSettingsHelper
end
end
- def allowed_key_types_checkboxes(help_block_id)
- Gitlab::SSHPublicKey.technology_names.map do |type|
- checked = current_application_settings.allowed_key_types.include?(type)
- checkbox_id = "allowed_key_types-#{type}"
-
- label_tag(checkbox_id, class: checked ? 'active' : nil) do
- check_box_tag('application_setting[allowed_key_types][]', type, checked,
- autocomplete: 'off',
- 'aria-describedby' => help_block_id,
- id: checkbox_id) + type.upcase
- 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
@@ -127,6 +125,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,
@@ -155,10 +156,6 @@ module ApplicationSettingsHelper
:metrics_port,
:metrics_sample_interval,
:metrics_timeout,
- :minimum_dsa_bits,
- :minimum_ecdsa_bits,
- :minimum_ed25519_bits,
- :minimum_rsa_bits,
:password_authentication_enabled,
:performance_bar_allowed_group_id,
:performance_bar_enabled,
@@ -174,6 +171,7 @@ module ApplicationSettingsHelper
:repository_storages,
:require_two_factor_authentication,
:restricted_visibility_levels,
+ :rsa_key_restriction,
:send_user_confirmation_email,
:sentry_dsn,
:sentry_enabled,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 988ee4802b9..0f9053262c2 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -13,6 +13,15 @@ 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 = -1
+ SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze
+
+ def self.supported_key_restrictions(type)
+ [0, *Gitlab::SSHPublicKey.supported_sizes(type), FORBIDDEN_KEY_VALUE]
+ end
+
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
@@ -20,7 +29,6 @@ class ApplicationSetting < ActiveRecord::Base
serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize
serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
serialize :sidekiq_throttling_queues, Array # rubocop:disable Cop/ActiveRecordSerialize
- serialize :allowed_key_types, Array # rubocop:disable Cop/ActiveRecordSerialize
cache_markdown_field :sign_in_text
cache_markdown_field :help_page_text
@@ -147,23 +155,11 @@ class ApplicationSetting < ActiveRecord::Base
presence: true,
numericality: { greater_than_or_equal_to: 0 }
- validates :allowed_key_types, presence: true
-
- validates :minimum_rsa_bits,
- presence: true,
- inclusion: { in: Gitlab::SSHPublicKey.allowed_sizes('rsa') }
-
- validates :minimum_dsa_bits,
- presence: true,
- inclusion: { in: Gitlab::SSHPublicKey.allowed_sizes('dsa') }
-
- validates :minimum_ecdsa_bits,
- presence: true,
- inclusion: { in: Gitlab::SSHPublicKey.allowed_sizes('ecdsa') }
-
- validates :minimum_ed25519_bits,
- presence: true,
- inclusion: { in: Gitlab::SSHPublicKey.allowed_sizes('ed25519') }
+ SUPPORTED_KEY_TYPES.each do |type|
+ validates :"#{type}_key_restriction",
+ presence: true,
+ inclusion: { in: ApplicationSetting.supported_key_restrictions(type) }
+ end
validates_each :restricted_visibility_levels do |record, attr, value|
value&.each do |level|
@@ -189,14 +185,6 @@ class ApplicationSetting < ActiveRecord::Base
end
end
- validates_each :allowed_key_types do |record, attr, value|
- value&.each do |type|
- unless Gitlab::SSHPublicKey.allowed_type?(type)
- record.errors.add(attr, "'#{type}' is not a valid SSH key type")
- end
- end
- end
-
before_validation :ensure_uuid!
before_save :ensure_runners_registration_token
@@ -240,7 +228,6 @@ class ApplicationSetting < ActiveRecord::Base
{
after_sign_up_text: nil,
akismet_enabled: false,
- allowed_key_types: Gitlab::SSHPublicKey.technology_names,
container_registry_token_expire_delay: 5,
default_artifacts_expire_in: '30 days',
default_branch_protection: Settings.gitlab['default_branch_protection'],
@@ -250,6 +237,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,
@@ -268,10 +258,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,
- minimum_rsa_bits: 1024,
- minimum_dsa_bits: 1024,
- minimum_ecdsa_bits: 256,
- minimum_ed25519_bits: 256,
+ rsa_key_restriction: 0,
plantuml_enabled: false,
plantuml_url: nil,
project_export_enabled: true,
@@ -446,6 +433,19 @@ 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"
+
+ # rubocop:disable GitlabSecurity/PublicSend
+ has_attribute?(attr_name) ? public_send(attr_name) : FORBIDDEN_KEY_VALUE
+ end
+
private
def ensure_uuid!
diff --git a/app/models/key.rb b/app/models/key.rb
index 27c91679ec9..2334603b58b 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -24,7 +24,7 @@ class Key < ActiveRecord::Base
uniqueness: true,
presence: { message: 'cannot be generated' }
- validate :key_meets_minimum_bit_length, :key_type_is_allowed
+ validate :key_meets_restrictions
delegate :name, :email, to: :user, prefix: true
@@ -100,37 +100,24 @@ class Key < ActiveRecord::Base
self.fingerprint = public_key.fingerprint
end
- def key_meets_minimum_bit_length
- case public_key.type
- when :rsa
- if public_key.bits < current_application_settings.minimum_rsa_bits
- errors.add(:key, "length must be at least #{current_application_settings.minimum_rsa_bits} bits")
- end
- when :dsa
- if public_key.bits < current_application_settings.minimum_dsa_bits
- errors.add(:key, "length must be at least #{current_application_settings.minimum_dsa_bits} bits")
- end
- when :ecdsa
- if public_key.bits < current_application_settings.minimum_ecdsa_bits
- errors.add(:key, "elliptic curve size must be at least #{current_application_settings.minimum_ecdsa_bits} bits")
- end
- when :ed25519
- if public_key.bits < current_application_settings.minimum_ed25519_bits
- errors.add(:key, "length must be at least #{current_application_settings.minimum_ed25519_bits} bits")
- 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 key_type_is_allowed
- unless current_application_settings.allowed_key_types.include?(public_key.type.to_s)
- allowed_types =
- current_application_settings
+ 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 ')
- errors.add(:key, "type is not allowed. Must be #{allowed_types}")
- end
+ "type is forbidden. Must be #{allowed_types}"
end
def notify_user
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 1cda98ffea8..fd083c03633 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -57,42 +57,12 @@
%span.help-block#clone-protocol-help
Allow only the selected protocols to be used for Git access.
- .form-group
- = f.label :allowed_key_types, 'Allowed SSH keys', class: 'control-label col-sm-2'
- .col-sm-10
- = hidden_field_tag 'application_setting[allowed_key_types][]', nil, id: 'allowed_key_types-none'
- - allowed_key_types_checkboxes('allowed-key-types-help').each do |key_type_checkbox|
- .checkbox= key_type_checkbox
- %span.help-block#allowed-key-types-help
- Only SSH keys with allowed algorithms can be uploaded.
-
- .form-group
- = f.label :minimum_rsa_bits, 'Minimum RSA key length', class: 'control-label col-sm-2'
- .col-sm-10
- = f.select :minimum_rsa_bits, Gitlab::SSHPublicKey.allowed_sizes('rsa'), {}, class: 'form-control'
- .help-block
- The minimum length for user RSA SSH keys (in bits)
-
- .form-group
- = f.label :minimum_dsa_bits, 'Minimum DSA key length', class: 'control-label col-sm-2'
- .col-sm-10
- = f.select :minimum_dsa_bits, Gitlab::SSHPublicKey.allowed_sizes('dsa'), {}, class: 'form-control'
- .help-block
- The minimum length for user DSA SSH keys (in bits)
-
- .form-group
- = f.label :minimum_ecdsa_bits, 'Minimum ECDSA key length', class: 'control-label col-sm-2'
- .col-sm-10
- = f.select :minimum_ecdsa_bits, Gitlab::SSHPublicKey.allowed_sizes('ecdsa'), {}, class: 'form-control'
- .help-block
- The minimum elliptic curve size for user ECDSA SSH keys (in bits)
-
- .form-group
- = f.label :minimum_ed25519_bits, 'Minimum ED25519 key length', class: 'control-label col-sm-2'
- .col-sm-10
- = f.select :minimum_ed25519_bits, Gitlab::SSHPublicKey.allowed_sizes('ed25519'), {}, class: 'form-control'
- .help-block
- The minimum length for user ED25519 SSH keys (in bits)
+ - 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
diff --git a/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb b/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
index ce87d8a26b6..2882e7f8b45 100644
--- a/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
+++ b/db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb
@@ -7,18 +7,22 @@ class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration
disable_ddl_transaction!
def up
- add_column_with_default :application_settings, :minimum_rsa_bits, :integer, default: 1024
- add_column_with_default :application_settings, :minimum_dsa_bits, :integer, default: 1024
- add_column_with_default :application_settings, :minimum_ecdsa_bits, :integer, default: 256
- add_column_with_default :application_settings, :minimum_ed25519_bits, :integer, default: 256
- add_column_with_default :application_settings, :allowed_key_types, :string, default: %w[rsa dsa ecdsa ed25519].to_yaml
+ # A key restriction has two possible states:
+ #
+ # * -1 means "this key type is completely disabled"
+ # * >= 0 means "keys must have at least this many bits to be valid"
+ #
+ # A value of 0 is equivalent to "there are no restrictions on keys of this type"
+ 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, :minimum_rsa_bits
- remove_column :application_settings, :minimum_dsa_bits
- remove_column :application_settings, :minimum_ecdsa_bits
- remove_column :application_settings, :minimum_ed25519_bits
- remove_column :application_settings, :allowed_key_types
+ 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
diff --git a/db/schema.rb b/db/schema.rb
index 49ae4b48627..434d1326419 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -129,11 +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 "minimum_rsa_bits", default: 1024, null: false
- t.integer "minimum_dsa_bits", default: 1024, null: false
- t.integer "minimum_ecdsa_bits", default: 256, null: false
- t.integer "minimum_ed25519_bits", default: 256, null: false
- t.string "allowed_key_types", default: "---\n- rsa\n- dsa\n- ecdsa\n- ed25519\n", 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|
diff --git a/doc/api/settings.md b/doc/api/settings.md
index a43e13e6217..b78f1252108 100644
--- a/doc/api/settings.md
+++ b/doc/api/settings.md
@@ -49,11 +49,10 @@ Example response:
"plantuml_url": null,
"terminal_max_session_time": 0,
"polling_interval_multiplier": 1.0,
- "minimum_rsa_bits": 1024,
- "minimum_dsa_bits": 1024,
- "minimum_ecdsa_bits": 256,
- "minimum_ed25519_bits": 256,
- "allowed_key_types": ["rsa", "dsa", "ecdsa", "ed25519"]
+ "rsa_key_restriction": 0,
+ "dsa_key_restriction": 0,
+ "ecdsa_key_restriction": 0,
+ "ed25519_key_restriction": 0,
}
```
@@ -93,11 +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. |
-| `minimum_rsa_bits` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `1024`.
-| `minimum_dsa_bits` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `1024`.
-| `minimum_ecdsa_bits` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `256`.
-| `minimum_ed25519_bits` | integer | no | The minimum allowed curve size (in bits) of an uploaded ED25519 key. Default is `256`.
-| `allowed_key_types` | array of strings | no | Array of SSH key types accepted by the application. Allowed values are: `rsa`, `dsa`, `ecdsa`, and `ed25519`. Default is `["rsa", "dsa", "ecdsa", "ed25519"]`.
+| `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
@@ -136,10 +134,9 @@ Example response:
"plantuml_url": null,
"terminal_max_session_time": 0,
"polling_interval_multiplier": 1.0,
- "minimum_rsa_bits": 1024,
- "minimum_dsa_bits": 1024,
- "minimum_ecdsa_bits": 256,
- "minimum_ed25519_bits": 256,
- "allowed_key_types": ["rsa", "dsa", "ecdsa", "ed25519"]
+ "rsa_key_restriction": 0,
+ "dsa_key_restriction": 0,
+ "ecdsa_key_restriction": 0,
+ "ed25519_key_restriction": 0,
}
```
diff --git a/doc/security/img/ssh_keys_restrictions_settings.png b/doc/security/img/ssh_keys_restrictions_settings.png
index b62bfc2f7e0..7b8bbb05bce 100644
--- a/doc/security/img/ssh_keys_restrictions_settings.png
+++ b/doc/security/img/ssh_keys_restrictions_settings.png
Binary files differ
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 8f766ba4f8d..803b48dd88a 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -744,7 +744,6 @@ module API
expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) }
expose(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) }
expose :password_authentication_enabled, as: :signin_enabled
- expose :allowed_key_types
end
class Release < Grape::Entity
diff --git a/lib/api/settings.rb b/lib/api/settings.rb
index 6ace0e1e390..01123e45ee0 100644
--- a/lib/api/settings.rb
+++ b/lib/api/settings.rb
@@ -122,11 +122,12 @@ 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.'
- optional :minimum_rsa_bits, type: Integer, values: Gitlab::SSHPublicKey.allowed_sizes('rsa'), desc: 'The minimum allowed bit length of an uploaded RSA key.'
- optional :minimum_dsa_bits, type: Integer, values: Gitlab::SSHPublicKey.allowed_sizes('dsa'), desc: 'The minimum allowed bit length of an uploaded DSA key.'
- optional :minimum_ecdsa_bits, type: Integer, values: Gitlab::SSHPublicKey.allowed_sizes('ecdsa'), desc: 'The minimum allowed curve size (in bits) of an uploaded ECDSA key.'
- optional :minimum_ed25519_bits, type: Integer, values: Gitlab::SSHPublicKey.allowed_sizes('ed25519'), desc: 'The minimum allowed curve size (in bits) of an uploaded ED25519 key.'
- optional :allowed_key_types, type: Array[String], values: Gitlab::SSHPublicKey.technology_names, desc: 'The SSH key types accepted by the application (`rsa`, `dsa`, `ecdsa` or `ed25519`).'
+ ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
+ optional :"#{type}_key_restriction",
+ type: Integer,
+ values: ApplicationSetting.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)
diff --git a/lib/gitlab/ssh_public_key.rb b/lib/gitlab/ssh_public_key.rb
index 2df31bcc246..a3f8730fb04 100644
--- a/lib/gitlab/ssh_public_key.rb
+++ b/lib/gitlab/ssh_public_key.rb
@@ -1,31 +1,20 @@
module Gitlab
class SSHPublicKey
- TYPES = %w[rsa dsa ecdsa ed25519].freeze
-
- Technology = Struct.new(:name, :allowed_sizes)
+ Technology = Struct.new(:name, :key_class, :supported_sizes)
Technologies = [
- Technology.new('rsa', [1024, 2048, 3072, 4096]),
- Technology.new('dsa', [1024, 2048, 3072]),
- Technology.new('ecdsa', [256, 384, 521]),
- Technology.new('ed25519', [256])
+ 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_names
- Technologies.map(&:name)
- end
-
def self.technology(name)
- Technologies.find { |ssh_key_technology| ssh_key_technology.name == name }
+ Technologies.find { |tech| tech.name.to_s == name.to_s }
end
- private_class_method :technology
- def self.allowed_sizes(name)
- technology(name).allowed_sizes
- end
-
- def self.allowed_type?(type)
- technology_names.include?(type.to_s)
+ def self.supported_sizes(name)
+ technology(name)&.supported_sizes
end
attr_reader :key_text, :key
@@ -50,18 +39,7 @@ module Gitlab
def type
return unless valid?
- case key
- when OpenSSL::PKey::EC
- :ecdsa
- when OpenSSL::PKey::RSA
- :rsa
- when OpenSSL::PKey::DSA
- :dsa
- when Net::SSH::Authentication::ED25519::PubKey
- :ed25519
- else
- raise "Unsupported key type: #{key.class}"
- end
+ technology.name
end
def bits
@@ -80,5 +58,17 @@ module Gitlab
raise "Unsupported key type: #{type}"
end
end
+
+ private
+
+ def technology
+ @technology ||=
+ begin
+ tech = Technologies.find { |tech| key.is_a?(tech.key_class) }
+ raise "Unsupported key type: #{key.class}" unless tech
+
+ tech
+ end
+ end
end
end
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index 9698dead67a..563818e8761 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -80,23 +80,19 @@ feature 'Admin updates settings' do
end
scenario 'Change Keys settings' do
- uncheck 'RSA'
- uncheck 'DSA'
- uncheck 'ED25519'
- select '384', from: 'Minimum ECDSA key length'
+ 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'
- expect(page).to have_content 'Application settings saved successfully'
-
- expect(find_field('RSA', checked: false)).not_to be_checked
- expect(find_field('DSA', checked: false)).not_to be_checked
- expect(find_field('ED25519', checked: false)).not_to be_checked
- expect(find_field('Minimum ECDSA key length').value).to eq '384'
+ forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE.to_s
- uncheck 'ECDSA'
- click_on 'Save'
-
- expect(page).to have_content "Allowed key types can't be blank"
+ 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
diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb
index a4ccf222b50..aa71c4dbba4 100644
--- a/spec/features/profiles/keys_spec.rb
+++ b/spec/features/profiles/keys_spec.rb
@@ -31,7 +31,8 @@ feature 'Profile > SSH Keys' do
context 'when only DSA and ECDSA keys are allowed' do
before do
- stub_application_setting(allowed_key_types: %w[dsa ecdsa])
+ forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE
+ stub_application_setting(rsa_key_restriction: forbidden, ed25519_key_restriction: forbidden)
end
scenario 'shows a validation error' do
@@ -41,7 +42,7 @@ feature 'Profile > SSH Keys' do
fill_in('Title', with: attrs[:title])
click_button('Add key')
- expect(page).to have_content('Key type is not allowed. Must be DSA or ECDSA')
+ expect(page).to have_content('Key type is forbidden. Must be DSA or ECDSA')
end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index a67902c7209..9e4174ecdca 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -162,28 +162,28 @@ describe Gitlab::GitAccess do
context 'key is too small' do
before do
- stub_application_setting(minimum_rsa_bits: 4096)
+ stub_application_setting(rsa_key_restriction: 4096)
end
it 'does not allow keys which are too small' do
aggregate_failures do
expect(actor).not_to be_valid
- expect { pull_access_check }.to raise_unauthorized('Your SSH key length must be at least 4096 bits.')
- expect { push_access_check }.to raise_unauthorized('Your SSH key length must be at least 4096 bits.')
+ 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
end
context 'key type is not allowed' do
before do
- stub_application_setting(allowed_key_types: ['ecdsa'])
+ stub_application_setting(rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE)
end
it 'does not allow keys which are too small' do
aggregate_failures do
expect(actor).not_to be_valid
- expect { pull_access_check }.to raise_unauthorized('Your SSH key type is not allowed. Must be ECDSA.')
- expect { push_access_check }.to raise_unauthorized('Your SSH key type is not allowed. Must be ECDSA.')
+ 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
diff --git a/spec/lib/gitlab/ssh_public_key_spec.rb b/spec/lib/gitlab/ssh_public_key_spec.rb
index d3314552d31..93d538141ce 100644
--- a/spec/lib/gitlab/ssh_public_key_spec.rb
+++ b/spec/lib/gitlab/ssh_public_key_spec.rb
@@ -4,32 +4,36 @@ describe Gitlab::SSHPublicKey, lib: true do
let(:key) { attributes_for(:rsa_key_2048)[:key] }
let(:public_key) { described_class.new(key) }
- describe '.technology_names' do
- it 'returns the available technology names' do
- expect(described_class.technology_names).to eq(%w[rsa dsa ecdsa ed25519])
+ 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 '.allowed_sizes(name)' do
+ describe '.supported_sizes(name)' do
where(:name, :sizes) do
[
- ['rsa', [1024, 2048, 3072, 4096]],
- ['dsa', [1024, 2048, 3072]],
- ['ecdsa', [256, 384, 521]],
- ['ed25519', [256]]
+ [:rsa, [1024, 2048, 3072, 4096]],
+ [:dsa, [1024, 2048, 3072]],
+ [:ecdsa, [256, 384, 521]],
+ [:ed25519, [256]]
]
end
- subject { described_class.allowed_sizes(name) }
+ subject { described_class.supported_sizes(name) }
with_them do
- it { is_expected.to eq(sizes) }
- end
- end
-
- describe '.allowed_type?' do
- it 'determines the key type' do
- expect(described_class.allowed_type?('foo')).to be(false)
+ it { expect(described_class.supported_sizes(name)).to eq(sizes) }
+ it { expect(described_class.supported_sizes(name.to_s)).to eq(sizes) }
end
end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 44d473db07d..0435aa9dfe1 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -72,25 +72,22 @@ describe ApplicationSetting do
.is_greater_than(0)
end
- it { is_expected.to validate_presence_of(:minimum_rsa_bits) }
- it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('rsa')).for(:minimum_rsa_bits) }
- it { is_expected.not_to allow_value(128).for(:minimum_rsa_bits) }
-
- it { is_expected.to validate_presence_of(:minimum_dsa_bits) }
- it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('dsa')).for(:minimum_dsa_bits) }
- it { is_expected.not_to allow_value(128).for(:minimum_dsa_bits) }
+ 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 { is_expected.to validate_presence_of(:minimum_ecdsa_bits) }
- it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ecdsa')).for(:minimum_ecdsa_bits) }
- it { is_expected.not_to allow_value(128).for(:minimum_ecdsa_bits) }
+ where(:type) do
+ described_class::SUPPORTED_KEY_TYPES
+ end
- it { is_expected.to validate_presence_of(:minimum_ed25519_bits) }
- it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ed25519')).for(:minimum_ed25519_bits) }
- it { is_expected.not_to allow_value(128).for(:minimum_ed25519_bits) }
+ with_them do
+ let(:field) { :"#{type}_key_restriction" }
- describe 'allowed_key_types validations' do
- it { is_expected.to allow_value(Gitlab::SSHPublicKey.technology_names).for(:allowed_key_types) }
- it { is_expected.not_to allow_value(['foo']).for(:allowed_key_types) }
+ it { is_expected.to validate_presence_of(field) }
+ it { is_expected.to allow_value(*described_class.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
@@ -463,15 +460,35 @@ describe ApplicationSetting do
end
end
- context 'allowed key types attribute' do
- it 'set value with array of symbols' do
- setting.allowed_key_types = [:rsa]
- expect(setting.allowed_key_types).to contain_exactly(:rsa)
+ 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 'get value as array of symbols' do
- setting.allowed_key_types = ['rsa']
- expect(setting.allowed_key_types).to eq(['rsa'])
+ it 'returns forbidden for unrecognised type' do
+ expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE)
end
end
end
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 83b11baa371..96baeaff0a4 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -104,19 +104,34 @@ describe Key, :mailer do
end
end
- context 'validate it meets minimum bit length' do
+ 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]
+ [: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
@@ -124,58 +139,13 @@ describe Key, :mailer do
subject(:key) { build(factory) }
before do
- stub_application_setting("minimum_#{key.public_key.type}_bits" => minimum)
+ stub_application_setting("#{key.public_key.type}_key_restriction" => minimum)
end
it { expect(key.valid?).to eq(result) }
end
end
- context 'validate the key type is allowed' do
- it 'accepts RSA, DSA, ECDSA and ED25519 keys by default' do
- expect(build(:rsa_key_2048)).to be_valid
- expect(build(:dsa_key_2048)).to be_valid
- expect(build(:ecdsa_key_256)).to be_valid
- expect(build(:ed25519_key_256)).to be_valid
- end
-
- it 'rejects RSA, ECDSA and ED25519 keys if DSA is the only allowed type' do
- stub_application_setting(allowed_key_types: ['dsa'])
-
- expect(build(:rsa_key_2048)).not_to be_valid
- expect(build(:dsa_key_2048)).to be_valid
- expect(build(:ecdsa_key_256)).not_to be_valid
- expect(build(:ed25519_key_256)).not_to be_valid
- end
-
- it 'rejects RSA, DSA and ED25519 keys if ECDSA is the only allowed type' do
- stub_application_setting(allowed_key_types: ['ecdsa'])
-
- expect(build(:rsa_key_2048)).not_to be_valid
- expect(build(:dsa_key_2048)).not_to be_valid
- expect(build(:ecdsa_key_256)).to be_valid
- expect(build(:ed25519_key_256)).not_to be_valid
- end
-
- it 'rejects DSA, ECDSA and ED25519 keys if RSA is the only allowed type' do
- stub_application_setting(allowed_key_types: ['rsa'])
-
- expect(build(:rsa_key_2048)).to be_valid
- expect(build(:dsa_key_2048)).not_to be_valid
- expect(build(:ecdsa_key_256)).not_to be_valid
- expect(build(:ed25519_key_256)).not_to be_valid
- end
-
- it 'rejects RSA, DSA and ECDSA keys if ED25519 is the only allowed type' do
- stub_application_setting(allowed_key_types: ['ed25519'])
-
- expect(build(:rsa_key_2048)).not_to be_valid
- expect(build(:dsa_key_2048)).not_to be_valid
- expect(build(:ecdsa_key_256)).not_to be_valid
- expect(build(:ed25519_key_256)).to be_valid
- end
- end
-
context 'callbacks' do
it 'adds new key to authorized_file' do
key = build(:personal_key, id: 7)
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 60e7c2d0da3..0b9a4b5c3db 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -19,11 +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['minimum_rsa_bits']).to eq(1024)
- expect(json_response['minimum_dsa_bits']).to eq(1024)
- expect(json_response['minimum_ecdsa_bits']).to eq(256)
- expect(json_response['minimum_ed25519_bits']).to eq(256)
- expect(json_response['allowed_key_types']).to contain_exactly('rsa', 'dsa', 'ecdsa', 'ed25519')
+ 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
@@ -50,11 +49,10 @@ describe API::Settings, 'Settings' do
help_page_hide_commercial_content: true,
help_page_support_url: 'http://example.com/help',
project_export_enabled: false,
- minimum_rsa_bits: 2048,
- minimum_dsa_bits: 2048,
- minimum_ecdsa_bits: 384,
- minimum_ed25519_bits: 256,
- allowed_key_types: ['rsa']
+ 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)
@@ -71,11 +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['minimum_rsa_bits']).to eq(2048)
- expect(json_response['minimum_dsa_bits']).to eq(2048)
- expect(json_response['minimum_ecdsa_bits']).to eq(384)
- expect(json_response['minimum_ed25519_bits']).to eq(256)
- expect(json_response['allowed_key_types']).to eq(['rsa'])
+ 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