summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/form_helper.rb4
-rw-r--r--app/models/application_setting.rb3
-rw-r--r--app/models/key.rb1
-rw-r--r--app/views/profiles/keys/_key_details.html.haml2
-rw-r--r--changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml2
-rw-r--r--db/migrate/20161020180657_add_minimum_key_length_to_application_settings.rb7
-rw-r--r--doc/security/README.md2
-rw-r--r--doc/security/ssh_keys_restrictions.md7
-rw-r--r--lib/gitlab/git_access.rb2
-rw-r--r--lib/gitlab/ssh_public_key.rb15
-rw-r--r--spec/lib/gitlab/git_access_spec.rb20
11 files changed, 29 insertions, 36 deletions
diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
index 2e05b8c6d65..eeb130d5240 100644
--- a/app/helpers/form_helper.rb
+++ b/app/helpers/form_helper.rb
@@ -1,10 +1,10 @@
module FormHelper
- def form_errors(model, headline = 'The form contains the following')
+ def form_errors(model, type: 'form')
return unless model.errors.any?
pluralized = 'error'.pluralize(model.errors.count)
- headline = headline + ' ' + pluralized + ':'
+ headline = "The #{type} contains the following #{pluralized}:"
content_tag(:div, class: 'alert alert-danger', id: 'error_explanation') do
content_tag(:h4, headline) <<
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 0f9053262c2..fcf31694ab5 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -442,8 +442,7 @@ class ApplicationSetting < ActiveRecord::Base
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
+ has_attribute?(attr_name) ? public_send(attr_name) : FORBIDDEN_KEY_VALUE # rubocop:disable GitlabSecurity/PublicSend
end
private
diff --git a/app/models/key.rb b/app/models/key.rb
index 2334603b58b..a6b4dcfec0d 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -1,7 +1,6 @@
require 'digest/md5'
class Key < ActiveRecord::Base
- include AfterCommitQueue
include Gitlab::CurrentSettings
include Sortable
diff --git a/app/views/profiles/keys/_key_details.html.haml b/app/views/profiles/keys/_key_details.html.haml
index cf70320e7a8..77521417f47 100644
--- a/app/views/profiles/keys/_key_details.html.haml
+++ b/app/views/profiles/keys/_key_details.html.haml
@@ -16,7 +16,7 @@
%strong= @key.last_used_at.try(:to_s, :medium) || 'N/A'
.col-md-8
- = form_errors(@key, 'The key has the following') unless @key.valid?
+ = form_errors(@key, type: 'key') unless @key.valid?
%p
%span.light Fingerprint:
%code.key-fingerprint= @key.fingerprint
diff --git a/changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml b/changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml
index 2e0f509b17c..8ec78bbd41f 100644
--- a/changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml
+++ b/changelogs/unreleased/17849-allow-admin-to-restrict-min-key-length-and-techno.yml
@@ -1,5 +1,5 @@
---
-title: Add settings for minimum key strength and allowed key type
+title: Add settings for minimum SSH key strength and allowed key type
merge_request: 13712
author: Cory Hinshaw
type: added
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 2882e7f8b45..5b6079002c0 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,12 +7,13 @@ class AddMinimumKeyLengthToApplicationSettings < ActiveRecord::Migration
disable_ddl_transaction!
def up
- # A key restriction has two possible states:
+ # A key restriction has these possible states:
#
# * -1 means "this key type is completely disabled"
- # * >= 0 means "keys must have at least this many bits to be valid"
+ # * 0 means "all keys of this type are valid"
+ # * > 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"
+ # 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
diff --git a/doc/security/README.md b/doc/security/README.md
index 1f54948d113..0fea6be8b55 100644
--- a/doc/security/README.md
+++ b/doc/security/README.md
@@ -1,7 +1,7 @@
# Security
- [Password length limits](password_length_limits.md)
-- [Restrict allowed SSH key technologies and minimum length](ssh_keys_restrictions.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)
diff --git a/doc/security/ssh_keys_restrictions.md b/doc/security/ssh_keys_restrictions.md
index 32ca7dacab3..213fa5bfef5 100644
--- a/doc/security/ssh_keys_restrictions.md
+++ b/doc/security/ssh_keys_restrictions.md
@@ -2,12 +2,13 @@
`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
+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 or ECDSA over the older
-DSA and administrators may need to limit the allowed SSH key algorithms.
+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.
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 9eab26d111e..62d1ecae676 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -34,8 +34,8 @@ module Gitlab
end
def check(cmd, changes)
- check_valid_actor!
check_protocol!
+ check_valid_actor!
check_active_user!
check_project_accessibility!
check_project_moved!
diff --git a/lib/gitlab/ssh_public_key.rb b/lib/gitlab/ssh_public_key.rb
index a3f8730fb04..89ca1298120 100644
--- a/lib/gitlab/ssh_public_key.rb
+++ b/lib/gitlab/ssh_public_key.rb
@@ -13,6 +13,10 @@ module Gitlab
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
@@ -37,9 +41,7 @@ module Gitlab
end
def type
- return unless valid?
-
- technology.name
+ technology.name if valid?
end
def bits
@@ -63,12 +65,7 @@ module Gitlab
def technology
@technology ||=
- begin
- tech = Technologies.find { |tech| key.is_a?(tech.key_class) }
- raise "Unsupported key type: #{key.class}" unless tech
-
- tech
- end
+ self.class.technology_for_key(key) || raise("Unsupported key type: #{key.class}")
end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 9e4174ecdca..458627ee4de 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -165,12 +165,10 @@ describe Gitlab::GitAccess do
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 must be at least 4096 bits.')
- expect { push_access_check }.to raise_unauthorized('Your SSH key must be at least 4096 bits.')
- 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
@@ -179,12 +177,10 @@ describe Gitlab::GitAccess do
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 forbidden/)
- expect { push_access_check }.to raise_unauthorized(/Your SSH key type is forbidden/)
- 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
View Lorry Controller Status