Merge branch 'security-12-1-enable-image-proxy' into '12-1-stable'

Use image proxy to mitigate stealing ip addresses

See merge request gitlab/gitlabhq!3231

4 files changed, 60 insertions, 15 deletions

diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 4bf9b708401..91466c13a8f 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -162,6 +162,10 @@ module ApplicationSettingsHelper
       :allow_local_requests_from_hooks_and_services,
       :dns_rebinding_protection_enabled,
       :archive_builds_in_human_readable,
+      :asset_proxy_enabled,
+      :asset_proxy_secret_key,
+      :asset_proxy_url,
+      :asset_proxy_whitelist,
       :authorized_keys_enabled,
       :auto_devops_enabled,
       :auto_devops_domain,
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 8e558487c1c..d00d0b728f0 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -16,12 +16,19 @@ class ApplicationSetting < ApplicationRecord
   # fix a lot of tests using allow_any_instance_of
   include ApplicationSettingImplementation
 
+  attr_encrypted :asset_proxy_secret_key,
+                 mode: :per_attribute_iv,
+                 insecure_mode: true,
+                 key: Settings.attr_encrypted_db_key_base_truncated,
+                 algorithm: 'aes-256-cbc'
+
   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
   serialize :domain_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
+  serialize :asset_proxy_whitelist, Array # rubocop:disable Cop/ActiveRecordSerialize
 
   ignore_column :koding_url
   ignore_column :koding_enabled
@@ -180,6 +187,17 @@ class ApplicationSetting < ApplicationRecord
             allow_nil: true,
             numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than: 65536 }
 
+  validates :asset_proxy_url,
+            presence: true,
+            allow_blank: false,
+            url: true,
+            if: :asset_proxy_enabled?
+
+  validates :asset_proxy_secret_key,
+            presence: true,
+            allow_blank: false,
+            if: :asset_proxy_enabled?
+
   SUPPORTED_KEY_TYPES.each do |type|
     validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
   end
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index df4caed175d..83546bb2a78 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -21,8 +21,9 @@ module ApplicationSettingImplementation
         after_sign_up_text: nil,
         akismet_enabled: false,
         allow_local_requests_from_hooks_and_services: false,
-        dns_rebinding_protection_enabled: true,
+        asset_proxy_enabled: false,
         authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
+        commit_email_hostname: default_commit_email_hostname,
         container_registry_token_expire_delay: 5,
         default_artifacts_expire_in: '30 days',
         default_branch_protection: Settings.gitlab['default_branch_protection'],
@@ -31,7 +32,9 @@ module ApplicationSettingImplementation
         default_project_visibility: Settings.gitlab.default_projects_features['visibility_level'],
         default_projects_limit: Settings.gitlab['default_projects_limit'],
         default_snippet_visibility: Settings.gitlab.default_projects_features['visibility_level'],
+        diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
         disabled_oauth_sign_in_sources: [],
+        dns_rebinding_protection_enabled: true,
         domain_whitelist: Settings.gitlab['domain_whitelist'],
         dsa_key_restriction: 0,
         ecdsa_key_restriction: 0,
@@ -50,6 +53,7 @@ module ApplicationSettingImplementation
         housekeeping_gc_period: 200,
         housekeeping_incremental_repack_period: 10,
         import_sources: Settings.gitlab['import_sources'],
+        local_markdown_version: 0,
         max_artifacts_size: Settings.artifacts['max_size'],
         max_attachment_size: Settings.gitlab['max_attachment_size'],
         mirror_available: true,
@@ -61,6 +65,7 @@ module ApplicationSettingImplementation
         plantuml_url: nil,
         polling_interval_multiplier: 1,
         project_export_enabled: true,
+        protected_ci_variables: false,
         recaptcha_enabled: false,
         repository_checks_enabled: true,
         repository_storages: ['default'],
@@ -92,11 +97,7 @@ module ApplicationSettingImplementation
         user_default_external: false,
         user_default_internal_regex: nil,
         user_show_add_ssh_key_message: true,
-        usage_stats_set_by_user_id: nil,
-        diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
-        commit_email_hostname: default_commit_email_hostname,
-        protected_ci_variables: false,
-        local_markdown_version: 0
+        usage_stats_set_by_user_id: nil
       }
     end
 
@@ -139,17 +140,20 @@ module ApplicationSettingImplementation
   end
 
   def domain_whitelist_raw=(values)
-    self.domain_whitelist = []
-    self.domain_whitelist = values.split(DOMAIN_LIST_SEPARATOR)
-    self.domain_whitelist.reject! { |d| d.empty? }
-    self.domain_whitelist
+    self.domain_whitelist = domain_string_to_array(values)
   end
 
   def domain_blacklist_raw=(values)
-    self.domain_blacklist = []
-    self.domain_blacklist = values.split(DOMAIN_LIST_SEPARATOR)
-    self.domain_blacklist.reject! { |d| d.empty? }
-    self.domain_blacklist
+    self.domain_blacklist = domain_string_to_array(values)
+  end
+
+  def asset_proxy_whitelist=(values)
+    values = domain_string_to_array(values) if values.is_a?(String)
+
+    # make sure we always whitelist the running host
+    values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
+
+    self[:asset_proxy_whitelist] = values
   end
 
   def domain_blacklist_file=(file)
@@ -276,4 +280,10 @@ module ApplicationSettingImplementation
   def expire_performance_bar_allowed_user_ids_cache
     Gitlab::PerformanceBar.expire_allowed_user_ids_cache
   end
+
+  def domain_string_to_array(values)
+    domain_list = values.split(DOMAIN_LIST_SEPARATOR).map(&:strip)
+    domain_list.reject! { |d| d.empty? }
+    domain_list
+  end
 end
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index 7eeaf8aade1..04931108873 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -6,6 +6,8 @@ module ApplicationSettings
 
     attr_reader :params, :application_setting
 
+    MARKDOWN_CACHE_INVALIDATING_PARAMS = %w(asset_proxy_enabled asset_proxy_url asset_proxy_secret_key asset_proxy_whitelist).freeze
+
     def execute
       validate_classification_label(application_setting, :external_authorization_service_default_label)
 
@@ -23,7 +25,13 @@ module ApplicationSettings
         params[:usage_stats_set_by_user_id] = current_user.id
       end
 
-      @application_setting.update(@params)
+      @application_setting.assign_attributes(params)
+
+      if invalidate_markdown_cache?
+        @application_setting[:local_markdown_version] = @application_setting.local_markdown_version + 1
+      end
+
+      @application_setting.save
     end
 
     private
@@ -32,6 +40,11 @@ module ApplicationSettings
       params.key?(:usage_ping_enabled) || params.key?(:version_check_enabled)
     end
 
+    def invalidate_markdown_cache?
+      !params.key?(:local_markdown_version) &&
+        (@application_setting.changes.keys & MARKDOWN_CACHE_INVALIDATING_PARAMS).any?
+    end
+
     def update_terms(terms)
       return unless terms.present?