openstack/swift
Code Issues Proposed changes

Check if REST API version is valid

Browse Source 
Swift doesn't check if the used API version is valid. Currently there
is only one valid REST API version, but that might change in the
future.

This patch enforces "v1" or "v1.0" as the version string when accessing
account, containers and objects.

The list of accepted version strings can be manually overridden using a
comma-separated list in swift.conf to make this backward-compatible.
The constraint loader has been modified slightly to accept strings as
well as integers.

Any request to an account, container, and object which does not
provide the correct version string will get a 400 BadRequest response.

The allowed api versions are by default excluded from /info.

Co-Authored-By: Christian Schwede <christian.schwede@enovance.com>
Co-Authored-By: John Dickinson <me@not.mn>

Closes Bug #1437442

Change-Id: I5ab6e236544378abf2eab562ab759513d09bc256
This commit is contained in:
Clay Gerrard 2015-03-27 15:50:38 -07:00 committed by John Dickinson
parent e16df14a73
commit 4aba2fbb25
7 changed files with 110 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
etc/proxy-server.conf-sample
View File

@ -17,9 +17,10 @@ bind_port = 8080
# to /info. You can withhold subsections by separating the dict level with a
# ".". The following would cause the sections 'container_quotas' and 'tempurl'
# to not be listed, and the key max_failed_deletes would be removed from
# bulk_delete. Default is empty, allowing all registered features to be listed
# via HTTP GET /info.
# disallowed_sections = container_quotas, tempurl, bulk_delete.max_failed_deletes
# bulk_delete. Default value is 'swift.valid_api_versions' which allows all
# registered features to be listed via HTTP GET /info except
# swift.valid_api_versions information
# disallowed_sections = swift.valid_api_versions, container_quotas, tempurl
# Use an integer to override the number of pre-forked processes that will
# accept connections. Should default to the number of effective cpu

11
etc/swift.conf-sample
View File

@ -156,3 +156,14 @@ default = yes
# of a container name
#max_container_name_length = 256
# By default all REST API calls should use "v1" or "v1.0" as the version string,
# for example "/v1/account". This can be manually overridden to make this
# backward-compatible, in case a different version string has been used before.
# Use a comma-separated list in case of multiple allowed versions, for example
# valid_api_versions = v0,v1,v2
# This is only enforced for account, container and object requests. The allowed
# api versions are by default excluded from /info.
# valid_api_versions = v1,v1.0

18
swift/common/constraints.py
View File

@ -35,6 +35,7 @@ CONTAINER_LISTING_LIMIT = 10000
ACCOUNT_LISTING_LIMIT = 10000
MAX_ACCOUNT_NAME_LENGTH = 256
MAX_CONTAINER_NAME_LENGTH = 256
VALID_API_VERSIONS = ["v1", "v1.0"]
# If adding an entry to DEFAULT_CONSTRAINTS, note that
# these constraints are automatically published by the
@ -52,6 +53,7 @@ DEFAULT_CONSTRAINTS = {
'account_listing_limit': ACCOUNT_LISTING_LIMIT,
'max_account_name_length': MAX_ACCOUNT_NAME_LENGTH,
'max_container_name_length': MAX_CONTAINER_NAME_LENGTH,
'valid_api_versions': VALID_API_VERSIONS,
}
SWIFT_CONSTRAINTS_LOADED = False
@ -72,13 +74,17 @@ def reload_constraints():
SWIFT_CONSTRAINTS_LOADED = True
for name in DEFAULT_CONSTRAINTS:
try:
value = int(constraints_conf.get('swift-constraints', name))
value = constraints_conf.get('swift-constraints', name)
except NoOptionError:
pass
except NoSectionError:
# We are never going to find the section for another option
break
else:
try:
value = int(value)
except ValueError:
value = utils.list_from_csv(value)
OVERRIDE_CONSTRAINTS[name] = value
for name, default in DEFAULT_CONSTRAINTS.items():
value = OVERRIDE_CONSTRAINTS.get(name, default)
@ -412,3 +418,13 @@ def check_account_format(req, account):
request=req,
body='Account name cannot contain slashes')
return account
def valid_api_version(version):
""" Checks if the requested version is valid.
Currently Swift only supports "v1" and "v1.0". """
global VALID_API_VERSIONS
if not isinstance(VALID_API_VERSIONS, list):
VALID_API_VERSIONS = [str(VALID_API_VERSIONS)]
return version in VALID_API_VERSIONS

4
swift/common/exceptions.py
View File

@ -199,6 +199,10 @@ class MimeInvalid(SwiftException):
pass
class APIVersionError(SwiftException):
pass
class ClientException(Exception):
def __init__(self, msg, http_scheme='', http_host='', http_port='',

10
swift/proxy/server.py
View File

@ -33,13 +33,14 @@ from swift.common.utils import cache_from_env, get_logger, \
get_remote_client, split_path, config_true_value, generate_trans_id, \
affinity_key_function, affinity_locality_predicate, list_from_csv, \
register_swift_info
from swift.common.constraints import check_utf8
from swift.common.constraints import check_utf8, valid_api_version
from swift.proxy.controllers import AccountController, ContainerController, \
ObjectControllerRouter, InfoController
from swift.proxy.controllers.base import get_container_info
from swift.common.swob import HTTPBadRequest, HTTPForbidden, \
HTTPMethodNotAllowed, HTTPNotFound, HTTPPreconditionFailed, \
HTTPServerError, HTTPException, Request, HTTPServiceUnavailable
from swift.common.exceptions import APIVersionError
# List of entry points for mandatory middlewares.
@ -210,7 +211,7 @@ class Application(object):
self.expose_info = config_true_value(
conf.get('expose_info', 'yes'))
self.disallowed_sections = list_from_csv(
conf.get('disallowed_sections'))
conf.get('disallowed_sections', 'swift.valid_api_versions'))
self.admin_key = conf.get('admin_key', None)
register_swift_info(
version=swift_version,
@ -260,6 +261,8 @@ class Application(object):
account_name=account,
container_name=container,
object_name=obj)
if account and not valid_api_version(version):
raise APIVersionError('Invalid path')
if obj and container and account:
info = get_container_info(req.environ, self)
policy_index = req.headers.get('X-Backend-Storage-Policy-Index',
@ -340,6 +343,9 @@ class Application(object):
p = req.path_info
if isinstance(p, unicode):
p = p.encode('utf-8')
except APIVersionError:
self.logger.increment('errors')
return HTTPBadRequest(request=req)
except ValueError:
self.logger.increment('errors')
return HTTPNotFound(request=req)

66
test/unit/proxy/test_server.py
View File

@ -30,7 +30,7 @@ from textwrap import dedent
from urllib import quote
from hashlib import md5
from pyeclib.ec_iface import ECDriverError
from tempfile import mkdtemp
from tempfile import mkdtemp, NamedTemporaryFile
import weakref
import operator
import functools
@ -53,7 +53,8 @@ from swift.container import server as container_server
from swift.obj import server as object_server
from swift.common.middleware import proxy_logging
from swift.common.middleware.acl import parse_acl, format_acl
from swift.common.exceptions import ChunkReadTimeout, DiskFileNotExist
from swift.common.exceptions import ChunkReadTimeout, DiskFileNotExist, \
APIVersionError
from swift.common import utils, constraints
from swift.common.ring import RingData
from swift.common.utils import mkdirs, normalize_timestamp, NullLogger
@ -881,7 +882,9 @@ class TestProxyServer(unittest.TestCase):
self.assertTrue(app.expose_info)
self.assertTrue(isinstance(app.disallowed_sections, list))
self.assertEqual(0, len(app.disallowed_sections))
self.assertEqual(1, len(app.disallowed_sections))
self.assertEqual(['swift.valid_api_versions'],
app.disallowed_sections)
self.assertTrue(app.admin_key is None)
def test_get_info_controller(self):
@ -959,6 +962,58 @@ class TestProxyServer(unittest.TestCase):
self.assertEqual(log_kwargs['exc_info'][1], e3)
self.assertEqual(4, node_error_count(app, node))
def test_valid_api_version(self):
app = proxy_server.Application({}, FakeMemcache(),
account_ring=FakeRing(),
container_ring=FakeRing())
# The version string is only checked for account, container and object
# requests; the raised APIVersionError returns a 404 to the client
for path in [
'/v2/a',
'/v2/a/c',
'/v2/a/c/o']:
req = Request.blank(path)
self.assertRaises(APIVersionError, app.get_controller, req)
# Default valid API versions are ok
for path in [
'/v1/a',
'/v1/a/c',
'/v1/a/c/o',
'/v1.0/a',
'/v1.0/a/c',
'/v1.0/a/c/o']:
req = Request.blank(path)
controller, path_parts = app.get_controller(req)
self.assertTrue(controller is not None)
# Ensure settings valid API version constraint works
for version in ["42", 42]:
try:
with NamedTemporaryFile() as f:
f.write('[swift-constraints]\n')
f.write('valid_api_versions = %s\n' % version)
f.flush()
with mock.patch.object(utils, 'SWIFT_CONF_FILE', f.name):
constraints.reload_constraints()
req = Request.blank('/%s/a' % version)
controller, _ = app.get_controller(req)
self.assertTrue(controller is not None)
# In this case v1 is invalid
req = Request.blank('/v1/a')
self.assertRaises(APIVersionError, app.get_controller, req)
finally:
constraints.reload_constraints()
# Check that the valid_api_versions is not exposed by default
req = Request.blank('/info')
controller, path_parts = app.get_controller(req)
self.assertTrue('swift.valid_api_versions' in
path_parts.get('disallowed_sections'))
@patch_policies([
StoragePolicy(0, 'zero', is_default=True),
@ -8580,9 +8635,12 @@ class TestSwiftInfo(unittest.TestCase):
self.assertTrue('strict_cors_mode' in si)
self.assertEqual(si['allow_account_management'], False)
self.assertEqual(si['account_autocreate'], False)
# This setting is by default excluded by disallowed_sections
self.assertEqual(si['valid_api_versions'],
constraints.VALID_API_VERSIONS)
# this next test is deliberately brittle in order to alert if
# other items are added to swift info
self.assertEqual(len(si), 16)
self.assertEqual(len(si), 17)
self.assertTrue('policies' in si)
sorted_pols = sorted(si['policies'], key=operator.itemgetter('name'))

4
test/unit/proxy/test_sysmeta.py
View File

@ -144,11 +144,15 @@ class TestObjectSysmeta(unittest.TestCase):
fake_http_connect(200),
FakeServerConnection(self.obj_ctlr))
self.orig_base_http_connect = swift.proxy.controllers.base.http_connect
self.orig_obj_http_connect = swift.proxy.controllers.obj.http_connect
swift.proxy.controllers.base.http_connect = http_connect
swift.proxy.controllers.obj.http_connect = http_connect
def tearDown(self):
shutil.rmtree(self.tmpdir)
swift.proxy.controllers.base.http_connect = self.orig_base_http_connect
swift.proxy.controllers.obj.http_connect = self.orig_obj_http_connect
original_sysmeta_headers_1 = {'x-object-sysmeta-test0': 'val0',
'x-object-sysmeta-test1': 'val1'}