Merge pull request #2987 from coreos-inc/joseph.schorr/QUAY-805/dot-fix

Add decorator to prevent reflected text attacks
This commit is contained in:
josephschorr 2018-02-20 12:02:22 -05:00 committed by GitHub
commit 4857cd9c48
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 86 additions and 6 deletions

View file

@ -503,3 +503,7 @@ class DefaultConfig(ImmutableConfig):
# The size of pages returned by the Docker V2 API.
V2_PAGINATION_SIZE = 50
# If enabled, ensures that API calls are made with the X-Requested-With header
# when called from a browser.
BROWSER_API_CALLS_XHR_ONLY = True

View file

@ -21,7 +21,7 @@ from auth.decorators import process_oauth
from endpoints.csrf import csrf_protect
from endpoints.exception import (Unauthorized, InvalidRequest, InvalidResponse,
FreshLoginRequired, NotFound)
from endpoints.decorators import check_anon_protection
from endpoints.decorators import check_anon_protection, require_xhr_from_browser
from util.metrics.metricqueue import time_decorator
from util.names import parse_namespace_repository
from util.pagination import encrypt_page_token, decrypt_page_token
@ -42,7 +42,8 @@ api = ApiExceptionHandlingApi()
api.init_app(api_bp)
api.decorators = [csrf_protect(),
crossdomain(origin='*', headers=['Authorization', 'Content-Type']),
process_oauth, time_decorator(api_bp.name, metric_queue)]
process_oauth, time_decorator(api_bp.name, metric_queue),
require_xhr_from_browser]
def resource(*urls, **kwargs):

View file

@ -1,5 +1,7 @@
""" Various decorators for endpoint and API handlers. """
import logging
from functools import wraps
from flask import abort, request, make_response
@ -8,6 +10,9 @@ import features
from app import app
from auth.auth_context import get_authenticated_context
from util.names import parse_namespace_repository
from util.http import abort
logger = logging.getLogger(__name__)
def parse_repository_name(include_tag=False,
@ -92,3 +97,22 @@ def route_show_if(value):
return f(*args, **kwargs)
return decorated_function
return decorator
def require_xhr_from_browser(func):
""" Requires that API GET calls made from browsers are made via XHR, in order to prevent
reflected text attacks.
"""
@wraps(func)
def wrapper(*args, **kwargs):
if app.config.get('BROWSER_API_CALLS_XHR_ONLY', False):
if request.method == 'GET' and request.user_agent.browser:
has_xhr_header = request.headers.get('X-Requested-With') == 'XMLHttpRequest'
if not has_xhr_header:
logger.warning('Disallowed possible RTA to URL %s with user agent %s',
request.path, request.user_agent)
abort(400)
return func(*args, **kwargs)
return wrapper

View file

@ -0,0 +1,35 @@
from data import model
from endpoints.api import api
from endpoints.api.repository import Repository
from endpoints.test.shared import conduct_call
from test.fixtures import *
@pytest.mark.parametrize('user_agent, include_header, expected_code', [
('curl/whatever', True, 200),
('curl/whatever', False, 200),
('Mozilla/whatever', True, 200),
('Mozilla/5.0', True, 200),
('Mozilla/5.0 (Windows NT 5.1; Win64; x64)', False, 400),
])
def test_require_xhr_from_browser(user_agent, include_header, expected_code, app, client):
# Create a public repo with a dot in its name.
user = model.user.get_user('devtable')
model.repository.create_repository('devtable', 'somerepo.bat', user, 'public')
# Retrieve the repository and ensure we either allow it through or fail, depending on the
# user agent and header.
params = {
'repository': 'devtable/somerepo.bat'
}
headers = {
'User-Agent': user_agent,
}
if include_header:
headers['X-Requested-With'] = 'XMLHttpRequest'
conduct_call(client, Repository, api.url_for, 'GET', params, headers=headers,
expected_code=expected_code)

View file

@ -11,6 +11,7 @@ provideRun.$inject = [
'PlanService',
'$http',
'CookieService',
'UserService',
'Features',
'$anchorScroll',
'MetaService',
@ -20,6 +21,7 @@ export function provideRun($rootScope: QuayRunScope,
planService: any,
$http: ng.IHttpService,
cookieService: any,
userService: any,
features: any,
$anchorScroll: ng.IAnchorScrollService,
metaService: any): void {
@ -29,6 +31,8 @@ export function provideRun($rootScope: QuayRunScope,
restangular.setDefaultRequestParams(['post', 'put', 'remove', 'delete'],
{'_csrf_token': (<any>window).__token || ''});
restangular.setDefaultHeaders({'X-Requested-With': 'XMLHttpRequest'});
// Handle session expiration.
restangular.setErrorInterceptor(function(response) {
if (response !== undefined && response.status == 503) {
@ -120,6 +124,9 @@ export function provideRun($rootScope: QuayRunScope,
}
return $http.pendingRequests.length > 0;
};
// Load the inital user information.
userService.load();
}

View file

@ -251,8 +251,5 @@ function(ApiService, CookieService, $rootScope, Config, $location, $timeout) {
// Update the user in the root scope.
userService.updateUserIn($rootScope);
// Load the user the first time.
userService.load();
return userService;
}]);

View file

@ -785,6 +785,13 @@ class V2RegistryLoginMixin(object):
class RegistryTestsMixin(object):
def test_previously_bad_repo_name(self):
# Push a new repository with two layers.
self.do_push('public', 'foo.bar', 'public', 'password')
# Pull the repository to verify.
self.do_pull('public', 'foo.bar', 'public', 'password')
def test_application_repo(self):
# Create an application repository via the API.
self.conduct_api_login('devtable', 'password')

View file

@ -2241,7 +2241,7 @@ class TestGetRepository(ApiTestCase):
def test_getrepo_badnames(self):
self.login(ADMIN_ACCESS_USER)
bad_names = ['logs', 'build', 'tokens', 'foo-bar', 'foo_bar']
bad_names = ['logs', 'build', 'tokens', 'foo.bar', 'foo-bar', 'foo_bar']
# For each bad name, create the repo.
for bad_name in bad_names:

View file

@ -591,6 +591,11 @@ CONFIG_SCHEMA = {
'not authenticated as a superuser',
'x-example': 'somesecrethere',
},
'BROWSER_API_CALLS_XHR_ONLY': {
'type': 'boolean',
'description': 'If enabled, only API calls marked as being made by an XHR will be allowed from browsers. Defaults to True.',
'x-example': False,
},
# Time machine and tag expiration settings.
'FEATURE_CHANGE_TAG_EXPIRATION': {