Add new decorator to prevent reflected text attacks

Instead of disabling repo names with periods in them, we simply disallow calls to the API when they are GET requests, whose path ends in a dot, and that do not have a referrer from the frontend.
This commit is contained in:
Joseph Schorr 2018-01-29 14:52:50 -05:00
parent b342111edb
commit 188ea98441
8 changed files with 82 additions and 12 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

@ -786,14 +786,11 @@ class V2RegistryLoginMixin(object):
class RegistryTestsMixin(object):
def test_previously_bad_repo_name(self):
bad_names = ['logs', 'build', 'tokens', 'foo.bar', 'foo-bar', 'foo_bar']
# Push a new repository with two layers.
self.do_push('public', 'foo.bar', 'public', 'password')
for name in bad_names:
# Push a new repository with two layers.
self.do_push('public', name, 'public', 'password')
# Pull the repository to verify.
self.do_pull('public', name, '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.

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': {