Commit 8ae3112b authored by Marin Jankovski's avatar Marin Jankovski

Merge branch 'upload-xss-access-control' into 'master'

Fix note attachments XSS and access control

Replaces the reverted #1528, as proposed in https://gitlab.com/gitlab-org/omnibus-gitlab/issues/434, as discussed with @dzaporozhets and as summarized in #2032.

@marin Could you take a look at the nginx config and apply it to Omnibus once this gets merged?

See merge request !1553
parents acc312fc 26d57a64
...@@ -3,6 +3,7 @@ v 7.9.0 (unreleased) ...@@ -3,6 +3,7 @@ v 7.9.0 (unreleased)
- Improve UI for commits, issues and merge request lists - Improve UI for commits, issues and merge request lists
v 7.8.0 (unreleased) v 7.8.0 (unreleased)
- Fix access control and protection against XSS for note attachments and other uploads.
- Replace highlight.js with rouge-fork rugments (Stefan Tatschner) - Replace highlight.js with rouge-fork rugments (Stefan Tatschner)
- Make project search case insensitive (Hannes Rosenögger) - Make project search case insensitive (Hannes Rosenögger)
- Include issue/mr participants in list of recipients for reassign/close/reopen emails - Include issue/mr participants in list of recipients for reassign/close/reopen emails
......
class Projects::UploadsController < Projects::ApplicationController
layout "project"
before_filter :project
def show
path = File.join(project.path_with_namespace, params[:secret])
uploader = FileUploader.new('uploads', path)
uploader.retrieve_from_store!(params[:filename])
if uploader.file.exists?
# Right now, these are always images, so we can safely render them inline.
send_file uploader.file.path, disposition: 'inline'
else
not_found!
end
end
end
class UploadsController < ApplicationController
def show
model = params[:model].camelize.constantize.find(params[:id])
uploader = model.send(params[:mounted_as])
if uploader.file_storage?
if !model.respond_to?(:project) || can?(current_user, :read_project, model.project)
disposition = uploader.image? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition
else
not_found!
end
else
redirect_to uploader.url
end
end
end
begin
app = Rails.application
# The `ActionDispatch::Static` middleware intercepts requests for static files
# by checking if they exist in the `/public` directory.
# We're replacing it with our `Gitlab::Middleware::Static` that does the same,
# except ignoring `/uploads`, letting those go through to the GitLab Rails app.
app.config.middleware.swap(
ActionDispatch::Static,
Gitlab::Middleware::Static,
app.paths["public"].first,
app.config.static_cache_control
)
rescue
# If ActionDispatch::Static wasn't loaded onto the stack (like in production),
# an exception is raised.
end
...@@ -75,7 +75,21 @@ Gitlab::Application.routes.draw do ...@@ -75,7 +75,21 @@ Gitlab::Application.routes.draw do
end end
end end
#
# Uploads
#
scope path: :uploads do
# Note attachments and User/Group/Project avatars
get ":model/:mounted_as/:id/:filename",
to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /.+/ }
# Project markdown uploads
get ":id/:secret/:filename",
to: "projects/uploads#show",
constraints: { id: /[a-zA-Z.0-9_\-]+\/[a-zA-Z.0-9_\-]+/, filename: /.+/ }
end
# #
# Explore area # Explore area
......
...@@ -164,8 +164,6 @@ git diff 6-0-stable:config/gitlab.yml.example 7-8-stable:config/gitlab.yml.examp ...@@ -164,8 +164,6 @@ git diff 6-0-stable:config/gitlab.yml.example 7-8-stable:config/gitlab.yml.examp
* Make `/home/git/gitlab/config/gitlab.yml` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/config/gitlab.yml.example but with your settings. * Make `/home/git/gitlab/config/gitlab.yml` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/config/gitlab.yml.example but with your settings.
* Make `/home/git/gitlab/config/unicorn.rb` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/config/unicorn.rb.example but with your settings. * Make `/home/git/gitlab/config/unicorn.rb` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/config/unicorn.rb.example but with your settings.
* Make `/home/git/gitlab-shell/config.yml` the same as https://gitlab.com/gitlab-org/gitlab-shell/blob/v2.4.3/config.yml.example but with your settings. * Make `/home/git/gitlab-shell/config.yml` the same as https://gitlab.com/gitlab-org/gitlab-shell/blob/v2.4.3/config.yml.example but with your settings.
* HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/lib/support/nginx/gitlab but with your settings.
* HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stablef/lib/support/nginx/gitlab-ssl but with your settings.
* Copy rack attack middleware config * Copy rack attack middleware config
```bash ```bash
...@@ -178,6 +176,12 @@ sudo -u git -H cp config/initializers/rack_attack.rb.example config/initializers ...@@ -178,6 +176,12 @@ sudo -u git -H cp config/initializers/rack_attack.rb.example config/initializers
sudo cp lib/support/logrotate/gitlab /etc/logrotate.d/gitlab sudo cp lib/support/logrotate/gitlab /etc/logrotate.d/gitlab
``` ```
### Change Nginx settings
* HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stable/lib/support/nginx/gitlab but with your settings.
* HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as https://gitlab.com/gitlab-org/gitlab-ce/blob/7-8-stablef/lib/support/nginx/gitlab-ssl but with your settings.
* A new `location /uploads/` section has been added that needs to have the same content as the existing `location @gitlab` section.
## 9. Start application ## 9. Start application
sudo service gitlab start sudo service gitlab start
......
...@@ -75,8 +75,9 @@ git diff origin/7-6-stable:config/gitlab.yml.example origin/7-8-stable:config/gi ...@@ -75,8 +75,9 @@ git diff origin/7-6-stable:config/gitlab.yml.example origin/7-8-stable:config/gi
#### Change Nginx settings #### Change Nginx settings
* HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as [`lib/support/nginx/gitlab`](/lib/support/nginx/gitlab) but with your settings * HTTP setups: Make `/etc/nginx/sites-available/gitlab` the same as [`lib/support/nginx/gitlab`](/lib/support/nginx/gitlab) but with your settings.
* HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as [`lib/support/nginx/gitlab-ssl`](/lib/support/nginx/gitlab-ssl) but with your setting * HTTPS setups: Make `/etc/nginx/sites-available/gitlab-ssl` the same as [`lib/support/nginx/gitlab-ssl`](/lib/support/nginx/gitlab-ssl) but with your settings.
* A new `location /uploads/` section has been added that needs to have the same content as the existing `location @gitlab` section.
#### Setup time zone (optional) #### Setup time zone (optional)
......
module Gitlab
module Middleware
class Static < ActionDispatch::Static
UPLOADS_REGEX = /\A\/uploads(\/|\z)/.freeze
def call(env)
return @app.call(env) if env['PATH_INFO'] =~ UPLOADS_REGEX
super
end
end
end
end
## GitLab ## GitLab
## Contributors: randx, yin8086, sashkab, orkoden, axilleas, bbodenmiller ## Contributors: randx, yin8086, sashkab, orkoden, axilleas, bbodenmiller, DouweM
## ##
## Lines starting with two hashes (##) are comments with information. ## Lines starting with two hashes (##) are comments with information.
## Lines starting with one hash (#) are configuration parameters that can be uncommented. ## Lines starting with one hash (#) are configuration parameters that can be uncommented.
...@@ -56,6 +56,27 @@ server { ...@@ -56,6 +56,27 @@ server {
try_files $uri $uri/index.html $uri.html @gitlab; try_files $uri $uri/index.html $uri.html @gitlab;
} }
## We route uploads through GitLab to prevent XSS and enforce access control.
location /uploads/ {
## If you use HTTPS make sure you disable gzip compression
## to be safe against BREACH attack.
# gzip off;
## https://github.com/gitlabhq/gitlabhq/issues/694
## Some requests take more than 30 seconds.
proxy_read_timeout 300;
proxy_connect_timeout 300;
proxy_redirect off;
proxy_set_header Host $http_host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Frame-Options SAMEORIGIN;
proxy_pass http://gitlab;
}
## If a file, which is not found in the root folder is requested, ## If a file, which is not found in the root folder is requested,
## then the proxy passes the request to the upsteam (gitlab unicorn). ## then the proxy passes the request to the upsteam (gitlab unicorn).
location @gitlab { location @gitlab {
......
## GitLab ## GitLab
## Contributors: randx, yin8086, sashkab, orkoden, axilleas, bbodenmiller ## Contributors: randx, yin8086, sashkab, orkoden, axilleas, bbodenmiller, DouweM
## ##
## Modified from nginx http version ## Modified from nginx http version
## Modified from http://blog.phusion.nl/2012/04/21/tutorial-setting-up-gitlab-on-debian-6/ ## Modified from http://blog.phusion.nl/2012/04/21/tutorial-setting-up-gitlab-on-debian-6/
...@@ -101,6 +101,28 @@ server { ...@@ -101,6 +101,28 @@ server {
try_files $uri $uri/index.html $uri.html @gitlab; try_files $uri $uri/index.html $uri.html @gitlab;
} }
## We route uploads through GitLab to prevent XSS and enforce access control.
location /uploads/ {
## If you use HTTPS make sure you disable gzip compression
## to be safe against BREACH attack.
gzip off;
## https://github.com/gitlabhq/gitlabhq/issues/694
## Some requests take more than 30 seconds.
proxy_read_timeout 300;
proxy_connect_timeout 300;
proxy_redirect off;
proxy_set_header Host $http_host;
proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-Ssl on;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Frame-Options SAMEORIGIN;
proxy_pass http://gitlab;
}
## If a file, which is not found in the root folder is requested, ## If a file, which is not found in the root folder is requested,
## then the proxy passes the request to the upsteam (gitlab unicorn). ## then the proxy passes the request to the upsteam (gitlab unicorn).
location @gitlab { location @gitlab {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment