kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Add support for using a Camo proxy server

Browse source 
User images and videos will get proxied through
the Camo server in order to keep malicious
sites from collecting the IP address of users.
This commit is contained in:
Brett Walker 2019-02-20 17:51:55 -06:00
parent 892e4c0da8
commit ad05e48863
33 changed files with 591 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
app/helpers/application_settings_helper.rb
View file

@ -164,6 +164,10 @@ module ApplicationSettingsHelper
:allow_local_requests_from_system_hooks,
: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,

18
app/models/application_setting.rb
View file

@ -18,12 +18,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
@ -192,6 +199,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

27
app/models/application_setting_implementation.rb
View file

@ -23,8 +23,9 @@ module ApplicationSettingImplementation
akismet_enabled: false,
allow_local_requests_from_web_hooks_and_services: false,
allow_local_requests_from_system_hooks: true,
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'],
@ -33,7 +34,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,
@ -52,9 +55,11 @@ 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,
outbound_local_requests_whitelist: [],
password_authentication_enabled_for_git: true,
password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'],
performance_bar_allowed_group_id: nil,
@ -63,6 +68,8 @@ module ApplicationSettingImplementation
plantuml_url: nil,
polling_interval_multiplier: 1,
project_export_enabled: true,
protected_ci_variables: false,
raw_blob_request_limit: 300,
recaptcha_enabled: false,
repository_checks_enabled: true,
repository_storages: ['default'],
@ -95,16 +102,10 @@ module ApplicationSettingImplementation
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,
snowplow_collector_hostname: nil,
snowplow_cookie_domain: nil,
snowplow_enabled: false,
snowplow_site_id: nil,
protected_ci_variables: false,
local_markdown_version: 0,
outbound_local_requests_whitelist: [],
raw_blob_request_limit: 300
snowplow_site_id: nil
}
end
@ -198,6 +199,15 @@ module ApplicationSettingImplementation
end
end
def asset_proxy_whitelist=(values)
values = domain_strings_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 repository_storages
Array(read_attribute(:repository_storages))
end
@ -306,6 +316,7 @@ module ApplicationSettingImplementation
values
.split(DOMAIN_LIST_SEPARATOR)
.map(&:strip)
.reject(&:empty?)
.uniq
end

15
app/services/application_settings/update_service.rb
View file

@ -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)
@ -25,7 +27,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
@ -41,6 +49,11 @@ module ApplicationSettings
@application_setting.add_to_outbound_local_requests_whitelist(values_array)
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?

5
changelogs/unreleased/security-enable-image-proxy.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Added image proxy to mitigate potential stealing of IP addresses
merge_request:
author:
type: security

6
config/initializers/asset_proxy_settings.rb Normal file
View file

@ -0,0 +1,6 @@
#
# Asset proxy settings
#
ActiveSupport.on_load(:active_record) do
Banzai::Filter::AssetProxyFilter.initialize_settings
end

4
config/initializers/fill_shards.rb
View file

@ -1,3 +1,5 @@
if Shard.connected? && !Gitlab::Database.read_only?
# The `table_exists?` check is needed because during our migration rollback testing,
# `Shard.connected?` could be cached and return true even though the table doesn't exist
if Shard.connected? && Shard.table_exists? && !Gitlab::Database.read_only?
Shard.populate!
end

16
db/migrate/20190219201635_add_asset_proxy_settings.rb Normal file
View file

@ -0,0 +1,16 @@
# frozen_string_literal: true
class AddAssetProxySettings < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def change
add_column :application_settings, :asset_proxy_enabled, :boolean, default: false, null: false
add_column :application_settings, :asset_proxy_url, :string
add_column :application_settings, :asset_proxy_whitelist, :text
add_column :application_settings, :encrypted_asset_proxy_secret_key, :text
add_column :application_settings, :encrypted_asset_proxy_secret_key_iv, :string
end
end

5
db/schema.rb
View file

@ -278,6 +278,11 @@ ActiveRecord::Schema.define(version: 2019_08_15_093949) do
t.boolean "allow_local_requests_from_system_hooks", default: true, null: false
t.bigint "instance_administration_project_id"
t.string "snowplow_collector_hostname"
t.boolean "asset_proxy_enabled", default: false, null: false
t.string "asset_proxy_url"
t.text "asset_proxy_whitelist"
t.text "encrypted_asset_proxy_secret_key"
t.string "encrypted_asset_proxy_secret_key_iv"
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"

10
doc/api/settings.md
View file

@ -68,6 +68,9 @@ Example response:
"allow_local_requests_from_hooks_and_services": true,
"allow_local_requests_from_web_hooks_and_services": true,
"allow_local_requests_from_system_hooks": false
"asset_proxy_enabled": true,
"asset_proxy_url": "https://assets.example.com",
"asset_proxy_whitelist": ["example.com", "*.example.com", "your-instance.com"]
}
```
@ -141,6 +144,9 @@ Example response:
"user_show_add_ssh_key_message": true,
"file_template_project_id": 1,
"local_markdown_version": 0,
"asset_proxy_enabled": true,
"asset_proxy_url": "https://assets.example.com",
"asset_proxy_whitelist": ["example.com", "*.example.com", "your-instance.com"],
"geo_node_allowed_ips": "0.0.0.0/0, ::/0",
"allow_local_requests_from_hooks_and_services": true,
"allow_local_requests_from_web_hooks_and_services": true,
@ -186,6 +192,10 @@ are listed in the descriptions of the relevant settings.
| `allow_local_requests_from_hooks_and_services` | boolean | no | (Deprecated: Use `allow_local_requests_from_web_hooks_and_services` instead) Allow requests to the local network from hooks and services. |
| `allow_local_requests_from_web_hooks_and_services` | boolean | no | Allow requests to the local network from web hooks and services. |
| `allow_local_requests_from_system_hooks` | boolean | no | Allow requests to the local network from system hooks. |
| `asset_proxy_enabled` | boolean | no | (**If enabled, requires:** `asset_proxy_url`) Enable proxying of assets. GitLab restart is required to apply changes. |
| `asset_proxy_secret_key` | string | no | Shared secret with the asset proxy server. GitLab restart is required to apply changes. |
| `asset_proxy_url` | string | no | URL of the asset proxy server. GitLab restart is required to apply changes. |
| `asset_proxy_whitelist` | string or array of strings | no | Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. GitLab restart is required to apply changes. |
| `authorized_keys_enabled` | boolean | no | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. |
| `auto_devops_domain` | string | no | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. |
| `auto_devops_enabled` | boolean | no | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. |

1
doc/security/README.md
View file

@ -17,3 +17,4 @@ type: index
- [Enforce Two-factor authentication](two_factor_authentication.md)
- [Send email confirmation on sign-up](user_email_confirmation.md)
- [Security of running jobs](https://docs.gitlab.com/runner/security/)
- [Proxying images](asset_proxy.md)

28
doc/security/asset_proxy.md Normal file
View file

@ -0,0 +1,28 @@
A possible security concern when managing a public facing GitLab instance is
the ability to steal a users IP address by referencing images in issues, comments, etc.
For example, adding `![Example image](http://example.com/example.png)` to
an issue description will cause the image to be loaded from the external
server in order to be displayed. However this also allows the external server
to log the IP address of the user.
One way to mitigate this is by proxying any external images to a server you
control. GitLab handles this by allowing you to run the "Camo" server
[cactus/go-camo](https://github.com/cactus/go-camo#how-it-works).
The image request is sent to the Camo server, which then makes the request for
the original image. This way an attacker only ever seems the IP address
of your Camo server.
Once you have your Camo server up and running, you can configure GitLab to
proxy image requests to it. The following settings are supported:
| Attribute | Description |
| ------------------------ | ----------- |
| `asset_proxy_enabled` | (**If enabled, requires:** `asset_proxy_url`) Enable proxying of assets. |
| `asset_proxy_secret_key` | Shared secret with the asset proxy server. |
| `asset_proxy_url` | URL of the asset proxy server. |
| `asset_proxy_whitelist` | Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted. |
These can be set via the [Application setting API](../api/settings.md)
Note that a GitLab restart is required to apply any changes.

3
lib/api/entities.rb
View file

@ -1174,6 +1174,9 @@ module API
attributes.delete(:performance_bar_enabled)
attributes.delete(:allow_local_requests_from_hooks_and_services)
# let's not expose the secret key in a response
attributes.delete(:asset_proxy_secret_key)
attributes
end

6
lib/api/settings.rb
View file

@ -36,6 +36,10 @@ module API
given akismet_enabled: ->(val) { val } do
requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com'
end
optional :asset_proxy_enabled, type: Boolean, desc: 'Enable proxying of assets'
optional :asset_proxy_url, type: String, desc: 'URL of the asset proxy server'
optional :asset_proxy_secret_key, type: String, desc: 'Shared secret with the asset proxy server'
optional :asset_proxy_whitelist, type: Array[String], coerce_with: Validations::Types::CommaSeparatedToArray.coerce, desc: 'Assets that match these domain(s) will NOT be proxied. Wildcards allowed. Your GitLab installation URL is automatically whitelisted.'
optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)'
optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts"
optional :default_project_creation, type: Integer, values: ::Gitlab::Access.project_creation_values, desc: 'Determine if developers can create projects in the group'
@ -123,7 +127,7 @@ 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 :usage_ping_enabled, type: Boolean, desc: 'Every week GitLab will report license usage back to GitLab, Inc.'
optional :instance_statistics_visibility_private, type: Boolean, desc: 'When set to `true` Instance statistics will only be available to admins'
optional :local_markdown_version, type: Integer, desc: "Local markdown version, increase this value when any cached markdown should be invalidated"
optional :local_markdown_version, type: Integer, desc: 'Local markdown version, increase this value when any cached markdown should be invalidated'
optional :allow_local_requests_from_hooks_and_services, type: Boolean, desc: 'Deprecated: Use :allow_local_requests_from_web_hooks_and_services instead. Allow requests to the local network from hooks and services.' # support legacy names, can be removed in v5
optional :snowplow_enabled, type: Grape::API::Boolean, desc: 'Enable Snowplow tracking'
given snowplow_enabled: ->(val) { val } do

22
lib/api/validations/types/comma_separated_to_array.rb Normal file
View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
module API
module Validations
module Types
class CommaSeparatedToArray
def self.coerce
lambda do |value|
case value
when String
value.split(',').map(&:strip)
when Array
value.map { |v| v.to_s.split(',').map(&:strip) }.flatten
else
[]
end
end
end
end
end
end
end

62
lib/banzai/filter/asset_proxy_filter.rb Normal file
View file

@ -0,0 +1,62 @@
# frozen_string_literal: true
module Banzai
module Filter
# Proxy's images/assets to another server. Reduces mixed content warnings
# as well as hiding the customer's IP address when requesting images.
# Copies the original img `src` to `data-canonical-src` then replaces the
# `src` with a new url to the proxy server.
class AssetProxyFilter < HTML::Pipeline::CamoFilter
def initialize(text, context = nil, result = nil)
super
end
def validate
needs(:asset_proxy, :asset_proxy_secret_key) if asset_proxy_enabled?
end
def asset_host_whitelisted?(host)
context[:asset_proxy_domain_regexp] ? context[:asset_proxy_domain_regexp].match?(host) : false
end
def self.transform_context(context)
context[:disable_asset_proxy] = !Gitlab.config.asset_proxy.enabled
unless context[:disable_asset_proxy]
context[:asset_proxy_enabled] = !context[:disable_asset_proxy]
context[:asset_proxy] = Gitlab.config.asset_proxy.url
context[:asset_proxy_secret_key] = Gitlab.config.asset_proxy.secret_key
context[:asset_proxy_domain_regexp] = Gitlab.config.asset_proxy.domain_regexp
end
context
end
# called during an initializer. Caching the values in Gitlab.config significantly increased
# performance, rather than querying Gitlab::CurrentSettings.current_application_settings
# over and over. However, this does mean that the Rails servers need to get restarted
# whenever the application settings are changed
def self.initialize_settings
application_settings = Gitlab::CurrentSettings.current_application_settings
Gitlab.config['asset_proxy'] ||= Settingslogic.new({})
if application_settings.respond_to?(:asset_proxy_enabled)
Gitlab.config.asset_proxy['enabled'] = application_settings.asset_proxy_enabled
Gitlab.config.asset_proxy['url'] = application_settings.asset_proxy_url
Gitlab.config.asset_proxy['secret_key'] = application_settings.asset_proxy_secret_key
Gitlab.config.asset_proxy['whitelist'] = application_settings.asset_proxy_whitelist || [Gitlab.config.gitlab.host]
Gitlab.config.asset_proxy['domain_regexp'] = compile_whitelist(Gitlab.config.asset_proxy.whitelist)
else
Gitlab.config.asset_proxy['enabled'] = ::ApplicationSetting.defaults[:asset_proxy_enabled]
end
end
def self.compile_whitelist(domain_list)
return if domain_list.empty?
escaped = domain_list.map { |domain| Regexp.escape(domain).gsub('\*', '.*?') }
Regexp.new("^(#{escaped.join('|')})$", Regexp::IGNORECASE)
end
end
end
end

18
lib/banzai/filter/external_link_filter.rb
View file

@ -14,10 +14,10 @@ module Banzai
# such as on `mailto:` links. Since we've been using it, do an
# initial parse for validity and then use Addressable
# for IDN support, etc
uri = uri_strict(node['href'].to_s)
uri = uri_strict(node_src(node))
if uri
node.set_attribute('href', uri.to_s)
addressable_uri = addressable_uri(node['href'])
node.set_attribute(node_src_attribute(node), uri.to_s)
addressable_uri = addressable_uri(node_src(node))
else
addressable_uri = nil
end
@ -35,6 +35,16 @@ module Banzai
private
# if this is a link to a proxied image, then `src` is already the correct
# proxied url, so work with the `data-canonical-src`
def node_src_attribute(node)
node['data-canonical-src'] ? 'data-canonical-src' : 'href'
end
def node_src(node)
node[node_src_attribute(node)]
end
def uri_strict(href)
URI.parse(href)
rescue URI::Error
@ -72,7 +82,7 @@ module Banzai
return unless uri
return unless context[:emailable_links]
unencoded_uri_str = Addressable::URI.unencode(node['href'])
unencoded_uri_str = Addressable::URI.unencode(node_src(node))
if unencoded_uri_str == node.content && idn?(uri)
node.content = uri.normalize

3
lib/banzai/filter/image_link_filter.rb
View file

@ -18,6 +18,9 @@ module Banzai
rel: 'noopener noreferrer'
)
# make sure the original non-proxied src carries over to the link
link['data-canonical-src'] = img['data-canonical-src'] if img['data-canonical-src']
link.children = img.clone
img.replace(link)

15
lib/banzai/filter/video_link_filter.rb
View file

@ -23,6 +23,14 @@ module Banzai
"'.#{ext}' = substring(@src, string-length(@src) - #{ext.size})"
end
if context[:asset_proxy_enabled].present?
src_query.concat(
UploaderHelper::VIDEO_EXT.map do |ext|
"'.#{ext}' = substring(@data-canonical-src, string-length(@data-canonical-src) - #{ext.size})"
end
)
end
"descendant-or-self::img[not(ancestor::a) and (#{src_query.join(' or ')})]"
end
end
@ -48,6 +56,13 @@ module Banzai
target: '_blank',
rel: 'noopener noreferrer',
title: "Download '#{element['title'] || element['alt']}'")
# make sure the original non-proxied src carries over
if element['data-canonical-src']
video['data-canonical-src'] = element['data-canonical-src']
link['data-canonical-src'] = element['data-canonical-src']
end
download_paragraph = doc.document.create_element('p')
download_paragraph.children = link

5
lib/banzai/pipeline/ascii_doc_pipeline.rb
View file

@ -6,12 +6,17 @@ module Banzai
def self.filters
FilterArray[
Filter::AsciiDocSanitizationFilter,
Filter::AssetProxyFilter,
Filter::SyntaxHighlightFilter,
Filter::ExternalLinkFilter,
Filter::PlantumlFilter,
Filter::AsciiDocPostProcessingFilter
]
end
def self.transform_context(context)
Filter::AssetProxyFilter.transform_context(context)
end
end
end
end

3
lib/banzai/pipeline/gfm_pipeline.rb
View file

@ -17,6 +17,7 @@ module Banzai
Filter::SpacedLinkFilter,
Filter::SanitizationFilter,
Filter::AssetProxyFilter,
Filter::SyntaxHighlightFilter,
Filter::MathFilter,
@ -60,7 +61,7 @@ module Banzai
def self.transform_context(context)
context[:only_path] = true unless context.key?(:only_path)
context
Filter::AssetProxyFilter.transform_context(context)
end
end
end

5
lib/banzai/pipeline/markup_pipeline.rb
View file

@ -6,11 +6,16 @@ module Banzai
def self.filters
@filters ||= FilterArray[
Filter::SanitizationFilter,
Filter::AssetProxyFilter,
Filter::ExternalLinkFilter,
Filter::PlantumlFilter,
Filter::SyntaxHighlightFilter
]
end
def self.transform_context(context)
Filter::AssetProxyFilter.transform_context(context)
end
end
end
end

3
lib/banzai/pipeline/single_line_pipeline.rb
View file

@ -7,6 +7,7 @@ module Banzai
@filters ||= FilterArray[
Filter::HtmlEntityFilter,
Filter::SanitizationFilter,
Filter::AssetProxyFilter,
Filter::EmojiFilter,
Filter::AutolinkFilter,
@ -29,6 +30,8 @@ module Banzai
end
def self.transform_context(context)
context = Filter::AssetProxyFilter.transform_context(context)
super(context).merge(
no_sourcepos: true
)

13
spec/initializers/asset_proxy_setting_spec.rb Normal file
View file

@ -0,0 +1,13 @@
require 'spec_helper'
describe 'Asset proxy settings initialization' do
describe '#asset_proxy' do
it 'defaults to disabled' do
expect(Banzai::Filter::AssetProxyFilter).to receive(:initialize_settings)
require_relative '../../config/initializers/asset_proxy_settings'
expect(Gitlab.config.asset_proxy.enabled).to be_falsey
end
end
end

95
spec/lib/banzai/filter/asset_proxy_filter_spec.rb Normal file
View file

@ -0,0 +1,95 @@
require 'spec_helper'
describe Banzai::Filter::AssetProxyFilter do
include FilterSpecHelper
def image(path)
%(<img src="#{path}" />)
end
it 'does not replace if disabled' do
stub_asset_proxy_setting(enabled: false)
context = described_class.transform_context({})
src = 'http://example.com/test.png'
doc = filter(image(src), context)
expect(doc.at_css('img')['src']).to eq src
end
context 'during initialization' do
after do
Gitlab.config.asset_proxy['enabled'] = false
end
it '#initialize_settings' do
stub_application_setting(asset_proxy_enabled: true)
stub_application_setting(asset_proxy_secret_key: 'shared-secret')
stub_application_setting(asset_proxy_url: 'https://assets.example.com')
stub_application_setting(asset_proxy_whitelist: %w(gitlab.com *.mydomain.com))
described_class.initialize_settings
expect(Gitlab.config.asset_proxy.enabled).to be_truthy
expect(Gitlab.config.asset_proxy.secret_key).to eq 'shared-secret'
expect(Gitlab.config.asset_proxy.url).to eq 'https://assets.example.com'
expect(Gitlab.config.asset_proxy.whitelist).to eq %w(gitlab.com *.mydomain.com)
expect(Gitlab.config.asset_proxy.domain_regexp).to eq /^(gitlab\.com|.*?\.mydomain\.com)$/i
end
end
context 'when properly configured' do
before do
stub_asset_proxy_setting(enabled: true)
stub_asset_proxy_setting(secret_key: 'shared-secret')
stub_asset_proxy_setting(url: 'https://assets.example.com')
stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host}))
stub_asset_proxy_setting(domain_regexp: described_class.compile_whitelist(Gitlab.config.asset_proxy.whitelist))
@context = described_class.transform_context({})
end
it 'replaces img src' do
src = 'http://example.com/test.png'
new_src = 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67'
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq new_src
expect(doc.at_css('img')['data-canonical-src']).to eq src
end
it 'skips internal images' do
src = "#{Gitlab.config.gitlab.url}/test.png"
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq src
end
it 'skip relative urls' do
src = "/test.png"
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq src
end
it 'skips single domain' do
src = "http://gitlab.com/test.png"
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq src
end
it 'skips single domain and ignores url in query string' do
src = "http://gitlab.com/test.png?url=http://example.com/test.png"
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq src
end
it 'skips wildcarded domain' do
src = "http://images.mydomain.com/test.png"
doc = filter(image(src), @context)
expect(doc.at_css('img')['src']).to eq src
end
end
end

12
spec/lib/banzai/filter/external_link_filter_spec.rb
View file

@ -156,6 +156,18 @@ describe Banzai::Filter::ExternalLinkFilter do
expect(doc_email.to_html).to include('http://xn--example-6p25f.com/</a>')
end
end
context 'autolinked image' do
let(:html) { %q(<a href="https://assets.example.com/6d8b/634c" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"><img src="http://exa%F0%9F%98%84mple.com/test.png" data-canonical-src="http://exa%F0%9F%98%84mple.com/test.png"></a>) }
let(:doc) { filter(html) }
it_behaves_like 'an external link with rel attribute'
it 'adds a toolip with punycode' do
expect(doc.to_html).to include('class="has-tooltip"')
expect(doc.to_html).to include('title="http://xn--example-6p25f.com/test.png"')
end
end
end
context 'for links that look malicious' do

7
spec/lib/banzai/filter/image_link_filter_spec.rb
View file

@ -28,4 +28,11 @@ describe Banzai::Filter::ImageLinkFilter do
doc = filter(%Q(<p>test #{image('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg')} inline</p>))
expect(doc.to_html).to match %r{^<p>test <a[^>]*><img[^>]*></a> inline</p>$}
end
it 'keep the data-canonical-src' do
doc = filter(%q(<img src="http://assets.example.com/6cd/4d7" data-canonical-src="http://example.com/test.png" />))
expect(doc.at_css('img')['src']).to eq doc.at_css('a')['href']
expect(doc.at_css('img')['data-canonical-src']).to eq doc.at_css('a')['data-canonical-src']
end
end

22
spec/lib/banzai/filter/video_link_filter_spec.rb
View file

@ -49,4 +49,26 @@ describe Banzai::Filter::VideoLinkFilter do
expect(element['src']).to eq '/path/my_image.jpg'
end
end
context 'when asset proxy is enabled' do
it 'uses the correct src' do
stub_asset_proxy_setting(enabled: true)
proxy_src = 'https://assets.example.com/6d8b63'
canonical_src = 'http://example.com/test.mp4'
image = %(<img src="#{proxy_src}" data-canonical-src="#{canonical_src}" />)
container = filter(image, asset_proxy_enabled: true).children.first
expect(container['class']).to eq 'video-container'
video, paragraph = container.children
expect(video['src']).to eq proxy_src
expect(video['data-canonical-src']).to eq canonical_src
link = paragraph.children.first
expect(link['href']).to eq proxy_src
end
end
end

44
spec/lib/banzai/pipeline/gfm_pipeline_spec.rb
View file

@ -142,4 +142,48 @@ describe Banzai::Pipeline::GfmPipeline do
expect(output).to include(Gitlab::Routing.url_helpers.milestone_path(milestone))
end
end
describe 'asset proxy' do
let(:project) { create(:project, :public) }
let(:image) { '![proxy](http://example.com/test.png)' }
let(:proxy) { 'https://assets.example.com/08df250eeeef1a8cf2c761475ac74c5065105612/687474703a2f2f6578616d706c652e636f6d2f746573742e706e67' }
let(:version) { Gitlab::CurrentSettings.current_application_settings.local_markdown_version }
before do
stub_asset_proxy_setting(enabled: true)
stub_asset_proxy_setting(secret_key: 'shared-secret')
stub_asset_proxy_setting(url: 'https://assets.example.com')
stub_asset_proxy_setting(whitelist: %W(gitlab.com *.mydomain.com #{Gitlab.config.gitlab.host}))
stub_asset_proxy_setting(domain_regexp: Banzai::Filter::AssetProxyFilter.compile_whitelist(Gitlab.config.asset_proxy.whitelist))
end
it 'replaces a lazy loaded img src' do
output = described_class.to_html(image, project: project)
doc = Nokogiri::HTML.fragment(output)
result = doc.css('img').first
expect(result['data-src']).to eq(proxy)
end
it 'autolinks images to the proxy' do
output = described_class.to_html(image, project: project)
doc = Nokogiri::HTML.fragment(output)
result = doc.css('a').first
expect(result['href']).to eq(proxy)
expect(result['data-canonical-src']).to eq('http://example.com/test.png')
end
it 'properly adds tooltips to link for IDN images' do
image = '![proxy](http://exa😄mple.com/test.png)'
proxy = 'https://assets.example.com/6d8b634c412a23c6bfe1b2963f174febf5635ddd/687474703a2f2f6578612546302539462539382538346d706c652e636f6d2f746573742e706e67'
output = described_class.to_html(image, project: project)
doc = Nokogiri::HTML.fragment(output)
result = doc.css('a').first
expect(result['href']).to eq(proxy)
expect(result['data-canonical-src']).to eq('http://exa%F0%9F%98%84mple.com/test.png')
expect(result['title']).to eq 'http://xn--example-6p25f.com/test.png'
end
end
end

65
spec/models/application_setting_spec.rb
View file

@ -355,6 +355,71 @@ describe ApplicationSetting do
end
end
end
context 'asset proxy settings' do
before do
subject.asset_proxy_enabled = true
end
describe '#asset_proxy_url' do
it { is_expected.not_to allow_value('').for(:asset_proxy_url) }
it { is_expected.to allow_value(http).for(:asset_proxy_url) }
it { is_expected.to allow_value(https).for(:asset_proxy_url) }
it { is_expected.not_to allow_value(ftp).for(:asset_proxy_url) }
it 'is not required when asset proxy is disabled' do
subject.asset_proxy_enabled = false
subject.asset_proxy_url = ''
expect(subject).to be_valid
end
end
describe '#asset_proxy_secret_key' do
it { is_expected.not_to allow_value('').for(:asset_proxy_secret_key) }
it { is_expected.to allow_value('anything').for(:asset_proxy_secret_key) }
it 'is not required when asset proxy is disabled' do
subject.asset_proxy_enabled = false
subject.asset_proxy_secret_key = ''
expect(subject).to be_valid
end
it 'is encrypted' do
subject.asset_proxy_secret_key = 'shared secret'
expect(subject.encrypted_asset_proxy_secret_key).to be_present
expect(subject.encrypted_asset_proxy_secret_key).not_to eq(subject.asset_proxy_secret_key)
end
end
describe '#asset_proxy_whitelist' do
context 'when given an Array' do
it 'sets the domains and adds current running host' do
setting.asset_proxy_whitelist = ['example.com', 'assets.example.com']
expect(setting.asset_proxy_whitelist).to eq(['example.com', 'assets.example.com', 'localhost'])
end
end
context 'when given a String' do
it 'sets multiple domains with spaces' do
setting.asset_proxy_whitelist = 'example.com *.example.com'
expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
end
it 'sets multiple domains with newlines and a space' do
setting.asset_proxy_whitelist = "example.com\n *.example.com"
expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
end
it 'sets multiple domains with commas' do
setting.asset_proxy_whitelist = "example.com, *.example.com"
expect(setting.asset_proxy_whitelist).to eq(['example.com', '*.example.com', 'localhost'])
end
end
end
end
end
context 'restrict creating duplicates' do

28
spec/requests/api/settings_spec.rb
View file

@ -224,5 +224,33 @@ describe API::Settings, 'Settings' do
expect(json_response['error']).to eq('plantuml_url is missing')
end
end
context 'asset_proxy settings' do
it 'updates application settings' do
put api('/application/settings', admin),
params: {
asset_proxy_enabled: true,
asset_proxy_url: 'http://assets.example.com',
asset_proxy_secret_key: 'shared secret',
asset_proxy_whitelist: ['example.com', '*.example.com']
}
expect(response).to have_gitlab_http_status(200)
expect(json_response['asset_proxy_enabled']).to be(true)
expect(json_response['asset_proxy_url']).to eq('http://assets.example.com')
expect(json_response['asset_proxy_secret_key']).to be_nil
expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost'])
end
it 'allows a string for asset_proxy_whitelist' do
put api('/application/settings', admin),
params: {
asset_proxy_whitelist: 'example.com, *.example.com'
}
expect(response).to have_gitlab_http_status(200)
expect(json_response['asset_proxy_whitelist']).to eq(['example.com', '*.example.com', 'localhost'])
end
end
end
end

33
spec/services/application_settings/update_service_spec.rb
View file

@ -110,6 +110,39 @@ describe ApplicationSettings::UpdateService do
end
end
describe 'markdown cache invalidators' do
shared_examples 'invalidates markdown cache' do |attribute|
let(:params) { attribute }
it 'increments cache' do
expect { subject.execute }.to change(application_settings, :local_markdown_version).by(1)
end
end
it_behaves_like 'invalidates markdown cache', { asset_proxy_enabled: true }
it_behaves_like 'invalidates markdown cache', { asset_proxy_url: 'http://test.com' }
it_behaves_like 'invalidates markdown cache', { asset_proxy_secret_key: 'another secret' }
it_behaves_like 'invalidates markdown cache', { asset_proxy_whitelist: ['domain.com'] }
context 'when also setting the local_markdown_version' do
let(:params) { { asset_proxy_enabled: true, local_markdown_version: 12 } }
it 'does not increment' do
expect { subject.execute }.to change(application_settings, :local_markdown_version).to(12)
end
end
context 'do not invalidate if value does not change' do
let(:params) { { asset_proxy_enabled: true, asset_proxy_secret_key: 'secret', asset_proxy_url: 'http://test.com' } }
it 'does not increment' do
described_class.new(application_settings, admin, params).execute
expect { described_class.new(application_settings, admin, params).execute }.not_to change(application_settings, :local_markdown_version)
end
end
end
describe 'performance bar settings' do
using RSpec::Parameterized::TableSyntax

4
spec/support/helpers/stub_configuration.rb
View file

@ -105,6 +105,10 @@ module StubConfiguration
allow(Gitlab.config.gitlab_shell).to receive_messages(to_settings(messages))
end
def stub_asset_proxy_setting(messages)
allow(Gitlab.config.asset_proxy).to receive_messages(to_settings(messages))
end
def stub_rack_attack_setting(messages)
allow(Gitlab.config.rack_attack).to receive(:git_basic_auth).and_return(messages)
allow(Gitlab.config.rack_attack.git_basic_auth).to receive_messages(to_settings(messages))