Merge branch '1007-add-ability-to-limit-the-lifetime-of-ssh-keys' into 'master'

Add a feature for setting the maximum allowable lifetime for SSH keys at an instance level

See merge request gitlab-org/gitlab!75098

app/helpers/profiles_helper.rb

@@ -61,6 +61,11 @@ module ProfilesHelper
   def ssh_key_expires_field_description
     s_('Profiles|Key can still be used after expiration.')
   end
+
+  # Overridden in EE::ProfilesHelper#ssh_key_expiration_policy_enabled?
+  def ssh_key_expiration_policy_enabled?
+    false
+  end
 end
 
 ProfilesHelper.prepend_mod

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

@@ -32,6 +32,7 @@
     = render_if_exists 'admin/application_settings/git_two_factor_session_expiry', form: f
     = render_if_exists 'admin/application_settings/personal_access_token_expiration_policy', form: f
     = render_if_exists 'admin/application_settings/enforce_pat_expiration', form: f
+    = render_if_exists 'admin/application_settings/ssh_key_expiration_policy', form: f
     = render_if_exists 'admin/application_settings/enforce_ssh_key_expiration', form: f
 
     .form-group

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

+- max_date = ::Gitlab::CurrentSettings.max_ssh_key_lifetime_from_now.to_date if ssh_key_expiration_policy_enabled?
 %div
   = form_for [:profile, @key], html: { class: 'js-requires-input' } do |f|
     = form_errors(@key)
@@ -13,8 +14,8 @@
         %p.form-text.text-muted= s_('Profiles|Give your individual key a title. This will be publicly visible.')
 
       .col.form-group
-        = f.label :expires_at, s_('Profiles|Expires at'), class: 'label-bold'
-        = f.date_field :expires_at, class: "form-control input-lg", min: Date.tomorrow, data: { qa_selector: 'key_expiry_date_field' }
+        = f.label :expires_at, s_('Profiles|Expiration date'), class: 'label-bold'
+        = f.date_field :expires_at, class: "form-control input-lg", min: Date.tomorrow, max: max_date, data: { qa_selector: 'key_expiry_date_field' }
         %p.form-text.text-muted{ data: { qa_selector: 'key_expiry_date_field_description' } }= ssh_key_expires_field_description
 
     .js-add-ssh-key-validation-warning.hide

db/migrate/20211118114228_add_max_ssh_key_lifetime_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddMaxSshKeyLifetimeToApplicationSettings < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :application_settings, :max_ssh_key_lifetime, :integer
+  end
+end

db/schema_migrations/20211118114228

+7686fd3e33b25b811aba459aba514cde8e88102277edb3be7e12378cb7e8de85
\ No newline at end of file

db/structure.sql

@@ -10477,6 +10477,7 @@ CREATE TABLE application_settings (
     sentry_dsn text,
     sentry_clientside_dsn text,
     sentry_environment text,
+    max_ssh_key_lifetime integer,
     static_objects_external_storage_auth_token_encrypted text,
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),

doc/api/settings.md

@@ -343,6 +343,7 @@ listed in the descriptions of the relevant settings.
 | `max_import_size`                        | integer          | no                                   | Maximum import size in MB. 0 for unlimited. Default = 0 (unlimited) [Modified](https://gitlab.com/gitlab-org/gitlab/-/issues/251106) from 50MB to 0 in GitLab 13.8. |
 | `max_pages_size`                         | integer          | no                                   | Maximum size of pages repositories in MB. |
 | `max_personal_access_token_lifetime`     | integer          | no                                   | **(ULTIMATE SELF)** Maximum allowable lifetime for personal access tokens in days. |
+| `max_ssh_key_lifetime`                   | integer          | no                                   | **(ULTIMATE SELF)** Maximum allowable lifetime for SSH keys in days. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/1007) in GitLab 14.6. |
 | `metrics_method_call_threshold`          | integer          | no                                   | A method call is only tracked when it takes longer than the given amount of milliseconds. |
 | `mirror_available`                       | boolean          | no                                   | Allow repository mirroring to configured by project Maintainers. If disabled, only Administrators can configure repository mirroring. |
 | `mirror_capacity_threshold`              | integer          | no                                   | **(PREMIUM)** Minimum capacity to be available before scheduling more mirrors preemptively. |

doc/user/admin_area/settings/account_and_limit_settings.md

@@ -192,38 +192,45 @@ To set a limit on how long these sessions are valid:
 1. Fill in the **Session duration for Git operations when 2FA is enabled (minutes)** field.
 1. Click **Save changes**.
 
-## Limit the lifetime of personal access tokens **(ULTIMATE SELF)**
+## Limit the lifetime of SSH keys **(ULTIMATE SELF)**
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3649) in GitLab 12.6.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/1007) in GitLab 14.6 [with a flag](../../../administration/feature_flags.md) named `ff_limit_ssh_key_lifetime`. Disabled by default. 
+
+FLAG:
+On self-managed GitLab, by default this feature is not available. To make it available,
+ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `ff_limit_ssh_key_lifetime`.
+On GitLab.com, this feature is not available. The feature is not ready for production use.
 
 Users can optionally specify a lifetime for
-[personal access tokens](../../profile/personal_access_tokens.md).
+[SSH keys](../../../ssh/index.md).
 This lifetime is not a requirement, and can be set to any arbitrary number of days.
 
-Personal access tokens are the only tokens needed for programmatic access to GitLab.
+SSH keys are user credentials to access GitLab.
 However, organizations with security requirements may want to enforce more protection by
-requiring the regular rotation of these tokens.
+requiring the regular rotation of these keys.
 
 ### Set a lifetime
 
 Only a GitLab administrator can set a lifetime. Leaving it empty means
 there are no restrictions.
 
-To set a lifetime on how long personal access tokens are valid:
+To set a lifetime on how long SSH keys are valid:
 
 1. On the top bar, select **Menu > Admin**.
 1. On the left sidebar, select **Settings > General**.
 1. Expand the **Account and limit** section.
-1. Fill in the **Maximum allowable lifetime for personal access tokens (days)** field.
+1. Fill in the **Maximum allowable lifetime for SSH keys (days)** field.
 1. Click **Save changes**.
 
-Once a lifetime for personal access tokens is set, GitLab:
+Once a lifetime for SSH keys is set, GitLab:
 
-- Applies the lifetime for new personal access tokens, and require users to set an expiration date
-  and a date no later than the allowed lifetime.
-- After three hours, revoke old tokens with no expiration date or with a lifetime longer than the
-  allowed lifetime. Three hours is given to allow administrators to change the allowed lifetime,
-  or remove it, before revocation takes place.
+- Requires users to set an expiration date that is no later than the allowed lifetime on new
+  SSH keys.
+- Applies the lifetime restriction to existing SSH keys. Keys with no expiry or a lifetime
+  greater than the maximum immediately become invalid.
+
+NOTE:
+When a user's SSH key becomes invalid they can delete and re-add the same key again.
 
 ## Allow expired SSH keys to be used **(ULTIMATE SELF)**
 
@@ -241,6 +248,39 @@ To allow the use of expired SSH keys:
 
 Disabling SSH key expiration immediately enables all expired SSH keys.
 
+## Limit the lifetime of personal access tokens **(ULTIMATE SELF)**
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/3649) in GitLab 12.6.
+
+Users can optionally specify a lifetime for
+[personal access tokens](../../profile/personal_access_tokens.md).
+This lifetime is not a requirement, and can be set to any arbitrary number of days.
+
+Personal access tokens are the only tokens needed for programmatic access to GitLab.
+However, organizations with security requirements may want to enforce more protection by
+requiring the regular rotation of these tokens.
+
+### Set a lifetime
+
+Only a GitLab administrator can set a lifetime. Leaving it empty means
+there are no restrictions.
+
+To set a lifetime on how long personal access tokens are valid:
+
+1. On the top bar, select **Menu > Admin**.
+1. On the left sidebar, select **Settings > General**.
+1. Expand the **Account and limit** section.
+1. Fill in the **Maximum allowable lifetime for personal access tokens (days)** field.
+1. Click **Save changes**.
+
+Once a lifetime for personal access tokens is set, GitLab:
+
+- Applies the lifetime for new personal access tokens, and require users to set an expiration date
+  and a date no later than the allowed lifetime.
+- After three hours, revoke old tokens with no expiration date or with a lifetime longer than the
+  allowed lifetime. Three hours is given to allow administrators to change the allowed lifetime,
+  or remove it, before revocation takes place.
+
 ## Allow expired Personal Access Tokens to be used **(ULTIMATE SELF)**
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/214723) in GitLab 13.1.

ee/app/helpers/ee/application_settings_helper.rb

@@ -58,6 +58,7 @@ module EE
         :help_text,
         :lock_memberships_to_ldap,
         :max_personal_access_token_lifetime,
+        :max_ssh_key_lifetime,
         :pseudonymizer_enabled,
         :repository_size_limit,
         :secret_detection_token_revocation_enabled,

ee/app/helpers/ee/profiles_helper.rb

@@ -15,7 +15,20 @@ module EE
     def ssh_key_expires_field_description
       return super unless ::Key.expiration_enforced?
 
-      s_('Profiles|Key will be deleted on this date.')
+      if ssh_key_expiration_policy_enabled?
+        s_('Profiles|Key becomes invalid on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days') % { max_ssh_key_lifetime: ::Gitlab::CurrentSettings.max_ssh_key_lifetime }
+      else
+        s_('Profiles|Key becomes invalid on this date.')
+      end
+    end
+
+    def ssh_key_expiration_policy_licensed?
+      License.feature_available?(:ssh_key_expiration_policy) && ::Feature.enabled?(:ff_limit_ssh_key_lifetime)
+    end
+
+    override :ssh_key_expiration_policy_enabled?
+    def ssh_key_expiration_policy_enabled?
+      ::Gitlab::CurrentSettings.max_ssh_key_lifetime && ssh_key_expiration_policy_licensed? && ::Feature.enabled?(:ff_limit_ssh_key_lifetime)
     end
   end
 end

ee/app/models/ee/application_setting.rb

@@ -116,6 +116,10 @@ module EE
                 presence: true,
                 numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: 10080 }
 
+      validates :max_ssh_key_lifetime,
+                allow_blank: true,
+                numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: 365 }
+
       alias_attribute :delayed_project_deletion, :delayed_project_removal
 
       after_commit :update_personal_access_tokens_lifetime, if: :saved_change_to_max_personal_access_token_lifetime?
@@ -155,6 +159,7 @@ module EE
           lock_memberships_to_ldap: false,
           maintenance_mode: false,
           max_personal_access_token_lifetime: nil,
+          max_ssh_key_lifetime: nil,
           mirror_capacity_threshold: Settings.gitlab['mirror_capacity_threshold'],
           mirror_max_capacity: Settings.gitlab['mirror_max_capacity'],
           mirror_max_delay: Settings.gitlab['mirror_max_delay'],
@@ -367,6 +372,10 @@ module EE
       max_personal_access_token_lifetime&.days&.from_now
     end
 
+    def max_ssh_key_lifetime_from_now
+      max_ssh_key_lifetime&.days&.from_now
+    end
+
     def compliance_frameworks=(values)
       cleaned = Array.wrap(values).reject(&:blank?).sort.uniq
 

ee/app/models/ee/key.rb

@@ -5,6 +5,7 @@ module EE
     extend ActiveSupport::Concern
 
     include Auditable
+    include ProfilesHelper
 
     prepended do
       include UsageStatistics
@@ -13,6 +14,10 @@ module EE
 
       validate :expiration, if: -> { ::Key.expiration_enforced? }
 
+      with_options if: :ssh_key_expiration_policy_enabled? do
+        validate :validate_expires_at_before_max_expiry_date
+      end
+
       def expiration
         errors.add(:key, :expired_and_enforced, message: 'has expired and the instance administrator has enforced expiration') if expired?
       end
@@ -26,6 +31,14 @@ module EE
 
         errors.map(&:type).reject { |t| t.eql?(:expired_and_enforced) }.empty?
       end
+
+      def validate_expires_at_before_max_expiry_date
+        return errors.add(:key, message: 'has no expiration date but an expiration date is required for SSH keys on this instance. Contact the instance administrator.') if expires_at.blank?
+
+        # when the key is not yet persisted the `created_at` field is nil
+        max_expiry_date = (created_at.presence || Time.current) + ::Gitlab::CurrentSettings.max_ssh_key_lifetime.days
+        errors.add(:key, message: 'has an invalid expiration date. Set a shorter lifetime for the key or contact the instance administrator.') if expires_at > max_expiry_date
+      end
     end
 
     class_methods do

ee/app/models/license.rb

@@ -192,6 +192,7 @@ class License < ApplicationRecord
     security_dashboard
     security_on_demand_scans
     security_orchestration_policies
+    ssh_key_expiration_policy
     status_page
     subepics
     threat_monitoring

ee/app/views/admin/application_settings/_ssh_key_expiration_policy.html.haml

+- return unless ssh_key_expiration_policy_licensed?
+- form = local_assigns.fetch(:form)
+
+.form-group
+  = form.label :max_ssh_key_lifetime, _('Maximum allowed lifetime for SSH keys (in days)'), class: 'label-light'
+  = form.number_field :max_ssh_key_lifetime, class: 'form-control gl-form-input input-xs'
+  %span.form-text.text-muted#max_ssh_key_lifetime= _('When enabled, SSH keys with no expiry date or an invalid expiration date are no longer accepted. Leave blank for no limit.')

ee/config/feature_flags/development/ff_limit_ssh_key_lifetime.yml

+---
+name: ff_limit_ssh_key_lifetime
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75098
+rollout_issue_url:
+milestone: '14.6'
+type: development
+group: group::compliance
+default_enabled: false

ee/spec/helpers/ee/profiles_helper_spec.rb

@@ -51,7 +51,7 @@ RSpec.describe ProfilesHelper do
     using RSpec::Parameterized::TableSyntax
 
     where(:expiration_enforced, :result) do
-      true  | "Key will be deleted on this date."
+      true  | "Key becomes invalid on this date."
       false | "Key can still be used after expiration."
     end
 
@@ -63,4 +63,74 @@ RSpec.describe ProfilesHelper do
       end
     end
   end
+
+  describe '#ssh_key_expiration_policy_licensed?' do
+    subject { helper.ssh_key_expiration_policy_licensed? }
+
+    context 'when is not licensed' do
+      before do
+        stub_licensed_features(ssh_key_expiration_policy: false)
+      end
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when is licensed' do
+      before do
+        stub_licensed_features(ssh_key_expiration_policy: true)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
+  describe '#ssh_key_expiration_policy_enabled?' do
+    subject { helper.ssh_key_expiration_policy_enabled? }
+
+    context 'when feature flag is enabled' do
+      before do
+        stub_feature_flags(ff_limit_ssh_key_lifetime: true)
+      end
+
+      context 'when is licensed and used' do
+        before do
+          stub_licensed_features(ssh_key_expiration_policy: true)
+          stub_application_setting(max_ssh_key_lifetime: 10)
+        end
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'when is not licensed' do
+        before do
+          stub_licensed_features(ssh_key_expiration_policy: false)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when is licensed but not used' do
+        before do
+          stub_licensed_features(ssh_key_expiration_policy: true)
+          stub_application_setting(max_ssh_key_lifetime: nil)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(ff_limit_ssh_key_lifetime: false)
+      end
+      context 'when is licensed and used' do
+        before do
+          stub_licensed_features(ssh_key_expiration_policy: true)
+          stub_application_setting(max_ssh_key_lifetime: 10)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+    end
+  end
 end

ee/spec/models/application_setting_spec.rb

@@ -96,6 +96,8 @@ RSpec.describe ApplicationSetting do
     it { is_expected.not_to allow_value(0).for(:git_two_factor_session_expiry) }
     it { is_expected.not_to allow_value(10081).for(:git_two_factor_session_expiry) }
 
+    it { is_expected.to validate_numericality_of(:max_ssh_key_lifetime).is_greater_than(0).is_less_than_or_equal_to(365).allow_nil }
+
     describe 'when additional email text is enabled' do
       before do
         stub_licensed_features(email_additional_text: true)
@@ -816,4 +818,36 @@ RSpec.describe ApplicationSetting do
       expect(subject.maintenance_mode).to be false
     end
   end
+
+  describe "#max_ssh_key_lifetime_from_now", :freeze_time do
+    subject { setting.max_ssh_key_lifetime_from_now }
+
+    let(:days_from_now) { nil }
+
+    before do
+      stub_application_setting(max_ssh_key_lifetime: days_from_now)
+    end
+
+    context 'when max_ssh_key_lifetime is defined' do
+      let(:days_from_now) { 30 }
+
+      it 'is a date time' do
+        expect(subject).to be_a Time
+      end
+
+      it 'is in the future' do
+        expect(subject).to be > Time.zone.now
+      end
+
+      it 'is in days_from_now' do
+        expect(subject.to_date - Date.today).to eq days_from_now
+      end
+    end
+
+    context 'when max_ssh_key_lifetime is nil' do
+      it 'is nil' do
+        expect(subject).to be_nil
+      end
+    end
+  end
 end

ee/spec/models/ee/key_spec.rb

@@ -22,6 +22,53 @@ RSpec.describe Key do
         end
       end
     end
+
+    describe '#validate_expires_at_before_max_expiry_date' do
+      using RSpec::Parameterized::TableSyntax
+
+      context 'for a range of key expiry combinations' do
+        where(:key, :max_ssh_key_lifetime, :valid) do
+          build(:personal_key, created_at: Time.current, expires_at: nil) | nil | true
+          build(:personal_key, created_at: Time.current, expires_at: 20.days.from_now) | nil | true
+          build(:personal_key, created_at: 1.day.ago, expires_at: 20.days.from_now) | 10 | false
+          build(:personal_key, created_at: 6.days.ago, expires_at: 3.days.from_now) | 10 | true
+          build(:personal_key, created_at: 10.days.ago, expires_at: 7.days.from_now) | 10 | false
+          build(:personal_key, created_at: Time.current, expires_at: nil) | 20 | false
+          build(:personal_key, expires_at: nil) | 30 | false
+        end
+
+        with_them do
+          before do
+            stub_licensed_features(ssh_key_expiration_policy: true)
+            stub_application_setting(max_ssh_key_lifetime: max_ssh_key_lifetime)
+          end
+          it 'checks if ssh key expiration is valid' do
+            expect(key.valid?).to eq(valid)
+          end
+        end
+      end
+
+      context 'when keys and max expiry are set' do
+        where(:key, :max_ssh_key_lifetime, :valid) do
+          build(:personal_key, created_at: Time.current, expires_at: 20.days.from_now) | 10 | false
+          build(:personal_key, created_at: Time.current, expires_at: 7.days.from_now) | 10 | true
+        end
+
+        with_them do
+          before do
+            stub_licensed_features(ssh_key_expiration_policy: true)
+            stub_application_setting(max_ssh_key_lifetime: max_ssh_key_lifetime)
+          end
+          it 'checks validity properly in the future too' do
+            # Travel to the day before the key is set to 'expire'.
+            # max_ssh_key_lifetime should still be enforced correctly.
+            travel_to(key.expires_at - 1) do
+              expect(key.valid?).to eq(valid)
+            end
+          end
+        end
+      end
+    end
   end
 
   describe 'only_expired_and_enforced?' do

locale/gitlab.pot

@@ -21496,6 +21496,9 @@ msgstr ""
 msgid "Maximum allowable lifetime for personal access token (days)"
 msgstr ""
 
+msgid "Maximum allowed lifetime for SSH keys (in days)"
+msgstr ""
+
 msgid "Maximum artifacts size"
 msgstr ""
 
@@ -26705,13 +26708,13 @@ msgstr ""
 msgid "Profiles|Enter your pronouns to let people know how to refer to you"
 msgstr ""
 
-msgid "Profiles|Expired key is not valid."
+msgid "Profiles|Expiration date"
 msgstr ""
 
-msgid "Profiles|Expired:"
+msgid "Profiles|Expired key is not valid."
 msgstr ""
 
-msgid "Profiles|Expires at"
+msgid "Profiles|Expired:"
 msgstr ""
 
 msgid "Profiles|Expires:"
@@ -26753,13 +26756,16 @@ msgstr ""
 msgid "Profiles|Key"
 msgstr ""
 
-msgid "Profiles|Key can still be used after expiration."
+msgid "Profiles|Key becomes invalid on this date."
 msgstr ""
 
-msgid "Profiles|Key usable beyond expiration date."
+msgid "Profiles|Key becomes invalid on this date. Maximum lifetime for SSH keys is %{max_ssh_key_lifetime} days"
 msgstr ""
 
-msgid "Profiles|Key will be deleted on this date."
+msgid "Profiles|Key can still be used after expiration."
+msgstr ""
+
+msgid "Profiles|Key usable beyond expiration date."
 msgstr ""
 
 msgid "Profiles|Last used:"
@@ -39221,6 +39227,9 @@ msgstr ""
 msgid "When a runner is locked, it cannot be assigned to other projects"
 msgstr ""
 
+msgid "When enabled, SSH keys with no expiry date or an invalid expiration date are no longer accepted. Leave blank for no limit."
+msgstr ""
+
 msgid "When enabled, existing personal access tokens may be revoked. Leave blank for no limit."
 msgstr ""
 

spec/views/profiles/keys/_form.html.haml_spec.rb

@@ -33,8 +33,8 @@ RSpec.describe 'profiles/keys/_form.html.haml' do
     end
 
     it 'has the expires at field', :aggregate_failures do
-      expect(rendered).to have_field('Expires at', type: 'date')
-      expect(page.find_field('Expires at')['min']).to eq(l(1.day.from_now, format: "%Y-%m-%d"))
+      expect(rendered).to have_field('Expiration date', type: 'date')
+      expect(page.find_field('Expiration date')['min']).to eq(l(1.day.from_now, format: "%Y-%m-%d"))
       expect(rendered).to have_text('Key can still be used after expiration.')
     end