diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-08-30 00:34:29 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-08-30 00:34:29 +0300 |
commit | 090956259c47d839b136f9391c3f74255764da81 (patch) | |
tree | 5a8c5fcd9d90f0ae4ca7e8cefe1261d2d31b504f /spec/lib | |
parent | 21b5239a0016796f1e2b60955f47c7daea318208 (diff) | |
parent | ce07df77cc4d96d06dacc4319e4e42c23afe850e (diff) |
Merge branch 'security-enable-image-proxy' into 'master'
Use image proxy to mitigate stealing ip addresses
Closes #2812
See merge request gitlab/gitlabhq!2926
Diffstat (limited to 'spec/lib')
6 files changed, 181 insertions, 1 deletions
diff --git a/spec/lib/banzai/filter/asset_proxy_filter_spec.rb b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb new file mode 100644 index 00000000000..b7f45421b2a --- /dev/null +++ b/spec/lib/banzai/filter/asset_proxy_filter_spec.rb @@ -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 diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index 59fea5766ee..4b2500b31f7 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -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 diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb index 7b0cb675551..011e3a1e2da 100644 --- a/spec/lib/banzai/filter/image_link_filter_spec.rb +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -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 diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb index 483e806624c..cd932f502f3 100644 --- a/spec/lib/banzai/filter/video_link_filter_spec.rb +++ b/spec/lib/banzai/filter/video_link_filter_spec.rb @@ -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 diff --git a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb index 0a3e0962452..3a9ecd2fb81 100644 --- a/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/gfm_pipeline_spec.rb @@ -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 diff --git a/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb b/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb index 9bd0d800086..00033c5ba1c 100644 --- a/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb +++ b/spec/lib/gitlab/database_importers/self_monitoring/project/create_service_spec.rb @@ -130,7 +130,7 @@ describe Gitlab::DatabaseImporters::SelfMonitoring::Project::CreateService do end it 'returns error when saving project ID fails' do - allow(application_setting).to receive(:update) { false } + allow(application_setting).to receive(:save) { false } expect { result }.to raise_error(StandardError, 'Could not save project ID') end |