Commit 68470602 authored by Nick Thomas's avatar Nick Thomas

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.
parent b49b7bc1
...@@ -66,8 +66,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -66,8 +66,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
end end
end end
params[:application_setting][:allowed_key_types]&.delete('')
enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources) enabled_oauth_sign_in_sources = params[:application_setting].delete(:enabled_oauth_sign_in_sources)
params[:application_setting][:disabled_oauth_sign_in_sources] = params[:application_setting][:disabled_oauth_sign_in_sources] =
...@@ -85,7 +83,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController ...@@ -85,7 +83,6 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
def visible_application_setting_attributes def visible_application_setting_attributes
ApplicationSettingsHelper.visible_attributes + [ ApplicationSettingsHelper.visible_attributes + [
:domain_blacklist_file, :domain_blacklist_file,
allowed_key_types: [],
disabled_oauth_sign_in_sources: [], disabled_oauth_sign_in_sources: [],
import_sources: [], import_sources: [],
repository_storages: [], repository_storages: [],
......
...@@ -81,18 +81,16 @@ module ApplicationSettingsHelper ...@@ -81,18 +81,16 @@ module ApplicationSettingsHelper
end end
end end
def allowed_key_types_checkboxes(help_block_id) def key_restriction_options_for_select(type)
Gitlab::SSHPublicKey.technology_names.map do |type| bit_size_options = Gitlab::SSHPublicKey.supported_sizes(type).map do |bits|
checked = current_application_settings.allowed_key_types.include?(type) ["Must be at least #{bits} bits", bits]
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
end end
[
['Are allowed', 0],
*bit_size_options,
['Are forbidden', ApplicationSetting::FORBIDDEN_KEY_VALUE]
]
end end
def repository_storages_options_for_select def repository_storages_options_for_select
...@@ -127,6 +125,9 @@ module ApplicationSettingsHelper ...@@ -127,6 +125,9 @@ module ApplicationSettingsHelper
:domain_blacklist_enabled, :domain_blacklist_enabled,
:domain_blacklist_raw, :domain_blacklist_raw,
:domain_whitelist_raw, :domain_whitelist_raw,
:dsa_key_restriction,
:ecdsa_key_restriction,
:ed25519_key_restriction,
:email_author_in_body, :email_author_in_body,
:enabled_git_access_protocol, :enabled_git_access_protocol,
:gravatar_enabled, :gravatar_enabled,
...@@ -155,10 +156,6 @@ module ApplicationSettingsHelper ...@@ -155,10 +156,6 @@ module ApplicationSettingsHelper
:metrics_port, :metrics_port,
:metrics_sample_interval, :metrics_sample_interval,
:metrics_timeout, :metrics_timeout,
:minimum_dsa_bits,
:minimum_ecdsa_bits,
:minimum_ed25519_bits,
:minimum_rsa_bits,
:password_authentication_enabled, :password_authentication_enabled,
:performance_bar_allowed_group_id, :performance_bar_allowed_group_id,
:performance_bar_enabled, :performance_bar_enabled,
...@@ -174,6 +171,7 @@ module ApplicationSettingsHelper ...@@ -174,6 +171,7 @@ module ApplicationSettingsHelper
:repository_storages, :repository_storages,
:require_two_factor_authentication, :require_two_factor_authentication,
:restricted_visibility_levels, :restricted_visibility_levels,
:rsa_key_restriction,
:send_user_confirmation_email, :send_user_confirmation_email,
:sentry_dsn, :sentry_dsn,
:sentry_enabled, :sentry_enabled,
......
...@@ -13,6 +13,15 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -13,6 +13,15 @@ class ApplicationSetting < ActiveRecord::Base
[\r\n] # any number of newline characters [\r\n] # any number of newline characters
}x }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 :restricted_visibility_levels # rubocop:disable Cop/ActiveRecordSerialize
serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize serialize :import_sources # rubocop:disable Cop/ActiveRecordSerialize
serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :disabled_oauth_sign_in_sources, Array # rubocop:disable Cop/ActiveRecordSerialize
...@@ -20,7 +29,6 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -20,7 +29,6 @@ class ApplicationSetting < ActiveRecord::Base
serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize
serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
serialize :sidekiq_throttling_queues, Array # 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 :sign_in_text
cache_markdown_field :help_page_text cache_markdown_field :help_page_text
...@@ -147,23 +155,11 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -147,23 +155,11 @@ class ApplicationSetting < ActiveRecord::Base
presence: true, presence: true,
numericality: { greater_than_or_equal_to: 0 } numericality: { greater_than_or_equal_to: 0 }
validates :allowed_key_types, presence: true SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction",
validates :minimum_rsa_bits, presence: true,
presence: true, inclusion: { in: ApplicationSetting.supported_key_restrictions(type) }
inclusion: { in: Gitlab::SSHPublicKey.allowed_sizes('rsa') } end
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') }
validates_each :restricted_visibility_levels do |record, attr, value| validates_each :restricted_visibility_levels do |record, attr, value|
value&.each do |level| value&.each do |level|
...@@ -189,14 +185,6 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -189,14 +185,6 @@ class ApplicationSetting < ActiveRecord::Base
end end
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_validation :ensure_uuid!
before_save :ensure_runners_registration_token before_save :ensure_runners_registration_token
...@@ -240,7 +228,6 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -240,7 +228,6 @@ class ApplicationSetting < ActiveRecord::Base
{ {
after_sign_up_text: nil, after_sign_up_text: nil,
akismet_enabled: false, akismet_enabled: false,
allowed_key_types: Gitlab::SSHPublicKey.technology_names,
container_registry_token_expire_delay: 5, container_registry_token_expire_delay: 5,
default_artifacts_expire_in: '30 days', default_artifacts_expire_in: '30 days',
default_branch_protection: Settings.gitlab['default_branch_protection'], default_branch_protection: Settings.gitlab['default_branch_protection'],
...@@ -250,6 +237,9 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -250,6 +237,9 @@ class ApplicationSetting < ActiveRecord::Base
default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'], default_group_visibility: Settings.gitlab.default_projects_features['visibility_level'],
disabled_oauth_sign_in_sources: [], disabled_oauth_sign_in_sources: [],
domain_whitelist: Settings.gitlab['domain_whitelist'], domain_whitelist: Settings.gitlab['domain_whitelist'],
dsa_key_restriction: 0,
ecdsa_key_restriction: 0,
ed25519_key_restriction: 0,
gravatar_enabled: Settings.gravatar['enabled'], gravatar_enabled: Settings.gravatar['enabled'],
help_page_text: nil, help_page_text: nil,
help_page_hide_commercial_content: false, help_page_hide_commercial_content: false,
...@@ -268,10 +258,7 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -268,10 +258,7 @@ class ApplicationSetting < ActiveRecord::Base
max_attachment_size: Settings.gitlab['max_attachment_size'], max_attachment_size: Settings.gitlab['max_attachment_size'],
password_authentication_enabled: Settings.gitlab['password_authentication_enabled'], password_authentication_enabled: Settings.gitlab['password_authentication_enabled'],
performance_bar_allowed_group_id: nil, performance_bar_allowed_group_id: nil,
minimum_rsa_bits: 1024, rsa_key_restriction: 0,
minimum_dsa_bits: 1024,
minimum_ecdsa_bits: 256,
minimum_ed25519_bits: 256,
plantuml_enabled: false, plantuml_enabled: false,
plantuml_url: nil, plantuml_url: nil,
project_export_enabled: true, project_export_enabled: true,
...@@ -446,6 +433,19 @@ class ApplicationSetting < ActiveRecord::Base ...@@ -446,6 +433,19 @@ class ApplicationSetting < ActiveRecord::Base
usage_ping_can_be_configured? && super usage_ping_can_be_configured? && super
end 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 private
def ensure_uuid! def ensure_uuid!
......
...@@ -24,7 +24,7 @@ class Key < ActiveRecord::Base ...@@ -24,7 +24,7 @@ class Key < ActiveRecord::Base
uniqueness: true, uniqueness: true,
presence: { message: 'cannot be generated' } 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 delegate :name, :email, to: :user, prefix: true
...@@ -100,37 +100,24 @@ class Key < ActiveRecord::Base ...@@ -100,37 +100,24 @@ class Key < ActiveRecord::Base
self.fingerprint = public_key.fingerprint self.fingerprint = public_key.fingerprint
end end
def key_meets_minimum_bit_length def key_meets_restrictions
case public_key.type restriction = current_application_settings.key_restriction_for(public_key.type)
when :rsa
if public_key.bits < current_application_settings.minimum_rsa_bits if restriction == ApplicationSetting::FORBIDDEN_KEY_VALUE
errors.add(:key, "length must be at least #{current_application_settings.minimum_rsa_bits} bits") errors.add(:key, forbidden_key_type_message)
end elsif public_key.bits < restriction
when :dsa errors.add(:key, "must be at least #{restriction} bits")
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
end end
end end
def key_type_is_allowed def forbidden_key_type_message
unless current_application_settings.allowed_key_types.include?(public_key.type.to_s) allowed_types =
allowed_types = current_application_settings
current_application_settings
.allowed_key_types .allowed_key_types
.map(&:upcase) .map(&:upcase)
.to_sentence(last_word_connector: ', or ', two_words_connector: ' or ') .to_sentence(last_word_connector: ', or ', two_words_connector: ' or ')
errors.add(:key, "type is not allowed. Must be #{allowed_types}") "type is forbidden. Must be #{allowed_types}"
end
end end
def notify_user def notify_user
......
...@@ -57,42 +57,12 @@ ...@@ -57,42 +57,12 @@
%span.help-block#clone-protocol-help %span.help-block#clone-protocol-help
Allow only the selected protocols to be used for Git access. Allow only the selected protocols to be used for Git access.
.form-group - ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
= f.label :allowed_key_types, 'Allowed SSH keys', class: 'control-label col-sm-2' - field_name = :"#{type}_key_restriction"
.col-sm-10 .form-group
= hidden_field_tag 'application_setting[allowed_key_types][]', nil, id: 'allowed_key_types-none' = f.label field_name, "#{type.upcase} SSH keys", class: 'control-label col-sm-2'
- allowed_key_types_checkboxes('allowed-key-types-help').each do |key_type_checkbox| .col-sm-10
.checkbox= key_type_checkbox = f.select field_name, key_restriction_options_for_select(type), {}, class: 'form-control'
%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)
%fieldset %fieldset
%legend Account and Limit Settings %legend Account and Limit Settings
......
...@@ -7,18 +7,22 @@ class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration ...@@ -7,18 +7,22 @@ class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration
disable_ddl_transaction! disable_ddl_transaction!
def up def up
add_column_with_default :application_settings, :minimum_rsa_bits, :integer, default: 1024 # A key restriction has two possible states:
add_column_with_default :application_settings, :minimum_dsa_bits, :integer, default: 1024 #
add_column_with_default :application_settings, :minimum_ecdsa_bits, :integer, default: 256 # * -1 means "this key type is completely disabled"
add_column_with_default :application_settings, :minimum_ed25519_bits, :integer, default: 256 # * >= 0 means "keys must have at least this many bits to be valid"
add_column_with_default :application_settings, :allowed_key_types, :string, default: %w[rsa dsa ecdsa ed25519].to_yaml #
# 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 end
def down def down
remove_column :application_settings, :minimum_rsa_bits remove_column :application_settings, :rsa_key_restriction
remove_column :application_settings, :minimum_dsa_bits remove_column :application_settings, :dsa_key_restriction
remove_column :application_settings, :minimum_ecdsa_bits remove_column :application_settings, :ecdsa_key_restriction
remove_column :application_settings, :minimum_ed25519_bits remove_column :application_settings, :ed25519_key_restriction
remove_column :application_settings, :allowed_key_types
end end
end end
...@@ -129,11 +129,10 @@ ActiveRecord::Schema.define(version: 20170824162758) do ...@@ -129,11 +129,10 @@ ActiveRecord::Schema.define(version: 20170824162758) do
t.boolean "password_authentication_enabled" t.boolean "password_authentication_enabled"
t.boolean "project_export_enabled", default: true, null: false t.boolean "project_export_enabled", default: true, null: false
t.boolean "hashed_storage_enabled", default: false, null: false t.boolean "hashed_storage_enabled", default: false, null: false
t.integer "minimum_rsa_bits", default: 1024, null: false t.integer "rsa_key_restriction", default: 0, null: false
t.integer "minimum_dsa_bits", default: 1024, null: false t.integer "dsa_key_restriction", default: 0, null: false
t.integer "minimum_ecdsa_bits", default: 256, null: false t.integer "ecdsa_key_restriction", default: 0, null: false
t.integer "minimum_ed25519_bits", default: 256, null: false t.integer "ed25519_key_restriction", default: 0, null: false
t.string "allowed_key_types", default: "---\n- rsa\n- dsa\n- ecdsa\n- ed25519\n", null: false
end end
create_table "audit_events", force: :cascade do |t| create_table "audit_events", force: :cascade do |t|
......
...@@ -49,11 +49,10 @@ Example response: ...@@ -49,11 +49,10 @@ Example response:
"plantuml_url": null, "plantuml_url": null,
"terminal_max_session_time": 0, "terminal_max_session_time": 0,
"polling_interval_multiplier": 1.0, "polling_interval_multiplier": 1.0,
"minimum_rsa_bits": 1024, "rsa_key_restriction": 0,
"minimum_dsa_bits": 1024, "dsa_key_restriction": 0,
"minimum_ecdsa_bits": 256, "ecdsa_key_restriction": 0,
"minimum_ed25519_bits": 256, "ed25519_key_restriction": 0,
"allowed_key_types": ["rsa", "dsa", "ecdsa", "ed25519"]
} }
``` ```
...@@ -93,11 +92,10 @@ PUT /application/settings ...@@ -93,11 +92,10 @@ PUT /application/settings
| `plantuml_url` | string | yes (if `plantuml_enabled` is `true`) | The PlantUML instance URL for integration. | | `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. | | `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. | | `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`. | `rsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `0` (no restriction). `-1` disables RSA keys.
| `minimum_dsa_bits` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `1024`. | `dsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded DSA key. Default is `0` (no restriction). `-1` disables DSA keys.
| `minimum_ecdsa_bits` | integer | no | The minimum allowed curve size (in bits) of an uploaded ECDSA key. Default is `256`. | `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.
| `minimum_ed25519_bits` | integer | no | The minimum allowed curve size (in bits) of an uploaded ED25519 key. Default is `256`. | `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.
| `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"]`.
```bash ```bash
curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/application/settings?signup_enabled=false&default_project_visibility=internal 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: ...@@ -136,10 +134,9 @@ Example response:
"plantuml_url": null, "plantuml_url": null,
"terminal_max_session_time": 0, "terminal_max_session_time": 0,
"polling_interval_multiplier": 1.0, "polling_interval_multiplier": 1.0,
"minimum_rsa_bits": 1024, "rsa_key_restriction": 0,
"minimum_dsa_bits": 1024, "dsa_key_restriction": 0,
"minimum_ecdsa_bits": 256, "ecdsa_key_restriction": 0,
"minimum_ed25519_bits": 256, "ed25519_key_restriction": 0,
"allowed_key_types": ["rsa", "dsa", "ecdsa", "ed25519"]
} }
``` ```
...@@ -744,7 +744,6 @@ module API ...@@ -744,7 +744,6 @@ module API
expose(:default_snippet_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_snippet_visibility) } 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(:default_group_visibility) { |setting, _options| Gitlab::VisibilityLevel.string_level(setting.default_group_visibility) }
expose :password_authentication_enabled, as: :signin_enabled expose :password_authentication_enabled, as: :signin_enabled
expose :allowed_key_types
end end
class Release < Grape::Entity class Release < Grape::Entity
......
...@@ -122,11 +122,12 @@ module API ...@@ -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 :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 :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.' ApplicationSetting::SUPPORTED_KEY_TYPES.each do |type|
optional :minimum_dsa_bits, type: Integer, values: Gitlab::SSHPublicKey.allowed_sizes('dsa'), desc: 'The minimum allowed bit length of an uploaded DSA key.' optional :"#{type}_key_restriction",
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.' type: Integer,
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.' values: ApplicationSetting.supported_key_restrictions(type),
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`).' 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) optional(*::ApplicationSettingsHelper.visible_attributes)
at_least_one_of(*::ApplicationSettingsHelper.visible_attributes) at_least_one_of(*::ApplicationSettingsHelper.visible_attributes)
......
module Gitlab module Gitlab
class SSHPublicKey class SSHPublicKey
TYPES = %w[rsa dsa ecdsa ed25519].freeze Technology = Struct.new(:name, :key_class, :supported_sizes)
Technology = Struct.new(:name, :allowed_sizes)
Technologies = [ Technologies = [
Technology.new('rsa', [1024, 2048, 3072, 4096]), Technology.new(:rsa, OpenSSL::PKey::RSA, [1024, 2048, 3072, 4096]),
Technology.new('dsa', [1024, 2048, 3072]), Technology.new(:dsa, OpenSSL::PKey::DSA, [1024, 2048, 3072]),
Technology.new('ecdsa', [256, 384, 521]), Technology.new(:ecdsa, OpenSSL::PKey::EC, [256, 384, 521]),
Technology.new('ed25519', [256]) Technology.new(:ed25519, Net::SSH::Authentication::ED25519::PubKey, [256])
].freeze ].freeze
def self.technology_names
Technologies.map(&:name)
end
def self.technology(name) 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 end
private_class_method :technology
def self.allowed_sizes(name) def self.supported_sizes(name)
technology(name).allowed_sizes technology(name)&.supported_sizes
end
def self.allowed_type?(type)
technology_names.include?(type.to_s)
end end
attr_reader :key_text, :key attr_reader :key_text, :key
...@@ -50,18 +39,7 @@ module Gitlab ...@@ -50,18 +39,7 @@ module Gitlab
def type def type
return unless valid? return unless valid?
case key technology.name
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
end end
def bits def bits
...@@ -80,5 +58,17 @@ module Gitlab ...@@ -80,5 +58,17 @@ module Gitlab
raise "Unsupported key type: #{type}" raise "Unsupported key type: #{type}"
end end
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
end end
...@@ -80,23 +80,19 @@ feature 'Admin updates settings' do ...@@ -80,23 +80,19 @@ feature 'Admin updates settings' do
end end
scenario 'Change Keys settings' do scenario 'Change Keys settings' do
uncheck 'RSA' select 'Are forbidden', from: 'RSA SSH keys'
uncheck 'DSA' select 'Are allowed', from: 'DSA SSH keys'
uncheck 'ED25519' select 'Must be at least 384 bits', from: 'ECDSA SSH keys'
select '384', from: 'Minimum ECDSA key length' select 'Are forbidden', from: 'ED25519 SSH keys'
click_on 'Save' click_on 'Save'
expect(page).to have_content 'Application settings saved successfully' forbidden = ApplicationSetting::FORBIDDEN_KEY_VALUE.to_s
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'
uncheck 'ECDSA' expect(page).to have_content 'Application settings saved successfully'
click_on 'Save' expect(find_field('RSA SSH keys').value).to eq(forbidden)
expect(find_field('DSA SSH keys').value).to eq('0')
expect(page).to have_content "Allowed key types can't be blank" expect(find_field('ECDSA SSH keys').value).to eq('384')
expect(find_field('ED25519 SSH keys').value).to eq(forbidden)
end end
def check_all_events def check_all_events
......
...@@ -31,7 +31,8 @@ feature 'Profile > SSH Keys' do ...@@ -31,7 +31,8 @@ feature 'Profile > SSH Keys' do
context 'when only DSA and ECDSA keys are allowed' do context 'when only DSA and ECDSA keys are allowed' do
before 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 end
scenario 'shows a validation error' do scenario 'shows a validation error' do
...@@ -41,7 +42,7 @@ feature 'Profile > SSH Keys' do ...@@ -41,7 +42,7 @@ feature 'Profile > SSH Keys' do
fill_in('Title', with: attrs[:title]) fill_in('Title', with: attrs[:title])
click_button('Add key') 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 end
end end
......
...@@ -162,28 +162,28 @@ describe Gitlab::GitAccess do ...@@ -162,28 +162,28 @@ describe Gitlab::GitAccess do
context 'key is too small' do context 'key is too small' do
before do before do
stub_application_setting(minimum_rsa_bits: 4096) stub_application_setting(rsa_key_restriction: 4096)
end end
it 'does not allow keys which are too small' do it 'does not allow keys which are too small' do
aggregate_failures do aggregate_failures do
expect(actor).not_to be_valid expect(actor).not_to be_valid
expect { pull_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 length 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 end
end end
context 'key type is not allowed' do context 'key type is not allowed' do
before do before do
stub_application_setting(allowed_key_types: ['ecdsa']) stub_application_setting(rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE)
end end
it 'does not allow keys which are too small' do it 'does not allow keys which are too small' do
aggregate_failures do aggregate_failures do
expect(actor).not_to be_valid expect(actor).not_to be_valid
expect { pull_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 not allowed. Must be ECDSA.') expect { push_access_check }.to raise_unauthorized(/Your SSH key type is forbidden/)
end end
end end
end end
......
...@@ -4,32 +4,36 @@ describe Gitlab::SSHPublicKey, lib: true do ...@@ -4,32 +4,36 @@ describe Gitlab::SSHPublicKey, lib: true do
let(:key) { attributes_for(:rsa_key_2048)[:key] } let(:key) { attributes_for(:rsa_key_2048)[:key] }
let(:public_key) { described_class.new(key) } let(:public_key) { described_class.new(key) }
describe '.technology_names' do describe '.technology(name)' do
it 'returns the available technology names' do it 'returns nil for an unrecognised name' do
expect(described_class.technology_names).to eq(%w[rsa dsa ecdsa ed25519]) 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
end end
describe '.allowed_sizes(name)' do describe '.supported_sizes(name)' do
where(:name, :sizes) do where(:name, :sizes) do
[ [
['rsa', [1024, 2048, 3072, 4096]], [:rsa, [1024, 2048, 3072, 4096]],
['dsa', [1024, 2048, 3072]], [:dsa, [1024, 2048, 3072]],
['ecdsa', [256, 384, 521]], [:ecdsa, [256, 384, 521]],
['ed25519', [256]] [:ed25519, [256]]
] ]
end end
subject { described_class.allowed_sizes(name) } subject { described_class.supported_sizes(name) }
with_them do with_them do
it { is_expected.to eq(sizes) } it { expect(described_class.supported_sizes(name)).to eq(sizes) }
end it { expect(described_class.supported_sizes(name.to_s)).to eq(sizes) }
end
describe '.allowed_type?' do
it 'determines the key type' do
expect(described_class.allowed_type?('foo')).to be(false)
end end
end end
......
...@@ -72,25 +72,22 @@ describe ApplicationSetting do ...@@ -72,25 +72,22 @@ describe ApplicationSetting do
.is_greater_than(0) .is_greater_than(0)
end end
it { is_expected.to validate_presence_of(:minimum_rsa_bits) } context 'key restrictions' do
it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('rsa')).for(:minimum_rsa_bits) } it 'supports all key types' do
it { is_expected.not_to allow_value(128).for(:minimum_rsa_bits) } expect(described_class::SUPPORTED_KEY_TYPES).to contain_exactly(:rsa, :dsa, :ecdsa, :ed25519)
end
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) }
it { is_expected.to validate_presence_of(:minimum_ecdsa_bits) } where(:type) do
it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ecdsa')).for(:minimum_ecdsa_bits) } described_class::SUPPORTED_KEY_TYPES
it { is_expected.not_to allow_value(128).for(:minimum_ecdsa_bits) } end
it { is_expected.to validate_presence_of(:minimum_ed25519_bits) } with_them do
it { is_expected.to allow_value(*Gitlab::SSHPublicKey.allowed_sizes('ed25519')).for(:minimum_ed25519_bits) } let(:field) { :"#{type}_key_restriction" }
it { is_expected.not_to allow_value(128).for(:minimum_ed25519_bits) }
describe 'allowed_key_types validations' do it { is_expected.to validate_presence_of(field) }
it { is_expected.to allow_value(Gitlab::SSHPublicKey.technology_names).for(:allowed_key_types) } it { is_expected.to allow_value(*described_class.supported_key_restrictions(type)).for(field) }
it { is_expected.not_to allow_value(['foo']).for(:allowed_key_types) } it { is_expected.not_to allow_value(128).for(field) }
end
end end
it_behaves_like 'an object with email-formated attributes', :admin_notification_email do it_behaves_like 'an object with email-formated attributes', :admin_notification_email do
...@@ -463,15 +460,35 @@ describe ApplicationSetting do ...@@ -463,15 +460,35 @@ describe ApplicationSetting do
end end
end end
context 'allowed key types attribute' do describe '#allowed_key_types' do
it 'set value with array of symbols' do it 'includes all key types by default' do
setting.allowed_key_types = [:rsa] expect(setting.allowed_key_types).to contain_exactly(*described_class::SUPPORTED_KEY_TYPES)
expect(setting.allowed_key_types).to contain_exactly(:rsa) 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 end
it 'get value as array of symbols' do it 'returns forbidden for unrecognised type' do
setting.allowed_key_types = ['rsa'] expect(setting.key_restriction_for(:foo)).to eq(described_class::FORBIDDEN_KEY_VALUE)
expect(setting.allowed_key_types).to eq(['rsa'])
end end
end end
end end
...@@ -104,19 +104,34 @@ describe Key, :mailer do ...@@ -104,19 +104,34 @@ describe Key, :mailer do
end end
end end
context 'validate it meets minimum bit length' do context 'validate it meets key restrictions' do
where(:factory, :minimum, :result) 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, 1024, true],
[:rsa_key_2048, 2048, true], [:rsa_key_2048, 2048, true],
[:rsa_key_2048, 4096, false], [:rsa_key_2048, 4096, false],
[:dsa_key_2048, 1024, true], [:dsa_key_2048, 1024, true],
[:dsa_key_2048, 2048, true], [:dsa_key_2048, 2048, true],
[:dsa_key_2048, 4096, false], [:dsa_key_2048, 4096, false],
[:ecdsa_key_256, 256, true], [:ecdsa_key_256, 256, true],
[:ecdsa_key_256, 384, false], [:ecdsa_key_256, 384, false],
[:ed25519_key_256, 256, true], [: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 end
...@@ -124,58 +139,13 @@ describe Key, :mailer do ...@@ -124,58 +139,13 @@ describe Key, :mailer do
subject(:key) { build(factory) } subject(:key) { build(factory) }
before do before do
stub_application_setting("minimum_#{key.public_key.type}_bits" => minimum) stub_application_setting("#{key.public_key.type}_key_restriction" => minimum)
end end
it { expect(key.valid?).to eq(result) } it { expect(key.valid?).to eq(result) }
end end
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 context 'callbacks' do
it 'adds new key to authorized_file' do it 'adds new key to authorized_file' do
key = build(:personal_key, id: 7) key = build(:personal_key, id: 7)
......
...@@ -19,11 +19,10 @@ describe API::Settings, 'Settings' do ...@@ -19,11 +19,10 @@ describe API::Settings, 'Settings' do
expect(json_response['default_project_visibility']).to be_a String expect(json_response['default_project_visibility']).to be_a String
expect(json_response['default_snippet_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['default_group_visibility']).to be_a String
expect(json_response['minimum_rsa_bits']).to eq(1024) expect(json_response['rsa_key_restriction']).to eq(0)
expect(json_response['minimum_dsa_bits']).to eq(1024) expect(json_response['dsa_key_restriction']).to eq(0)
expect(json_response['minimum_ecdsa_bits']).to eq(256) expect(json_response['ecdsa_key_restriction']).to eq(0)
expect(json_response['minimum_ed25519_bits']).to eq(256) expect(json_response['ed25519_key_restriction']).to eq(0)
expect(json_response['allowed_key_types']).to contain_exactly('rsa', 'dsa', 'ecdsa', 'ed25519')
end end
end end
...@@ -50,11 +49,10 @@ describe API::Settings, 'Settings' do ...@@ -50,11 +49,10 @@ describe API::Settings, 'Settings' do
help_page_hide_commercial_content: true, help_page_hide_commercial_content: true,
help_page_support_url: 'http://example.com/help', help_page_support_url: 'http://example.com/help',
project_export_enabled: false, project_export_enabled: false,
minimum_rsa_bits: 2048, rsa_key_restriction: ApplicationSetting::FORBIDDEN_KEY_VALUE,
minimum_dsa_bits: 2048, dsa_key_restriction: 2048,
minimum_ecdsa_bits: 384, ecdsa_key_restriction: 384,
minimum_ed25519_bits: 256, ed25519_key_restriction: 256
allowed_key_types: ['rsa']
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['default_projects_limit']).to eq(3) expect(json_response['default_projects_limit']).to eq(3)
...@@ -71,11 +69,10 @@ describe API::Settings, 'Settings' do ...@@ -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_hide_commercial_content']).to be_truthy
expect(json_response['help_page_support_url']).to eq('http://example.com/help') 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['project_export_enabled']).to be_falsey
expect(json_response['minimum_rsa_bits']).to eq(2048) expect(json_response['rsa_key_restriction']).to eq(ApplicationSetting::FORBIDDEN_KEY_VALUE)
expect(json_response['minimum_dsa_bits']).to eq(2048) expect(json_response['dsa_key_restriction']).to eq(2048)
expect(json_response['minimum_ecdsa_bits']).to eq(384) expect(json_response['ecdsa_key_restriction']).to eq(384)
expect(json_response['minimum_ed25519_bits']).to eq(256) expect(json_response['ed25519_key_restriction']).to eq(256)
expect(json_response['allowed_key_types']).to eq(['rsa'])
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment