From cf1aa3c309a6fcb57755da6732226b494f0bf69b Mon Sep 17 00:00:00 2001 From: Eamonn O'Toole Date: Fri, 2 Mar 2012 11:23:17 +0000 Subject: [PATCH] Adds name_check filter Bug 926048. Filter checks path for user-defined forbidden characters, and for user-defined maximum length. Includes changes to reflect gholt's latest comments to Patch Set 4 Also includes a change to a unit-test, renames another unit-test, and removes one superfluous unit-test. Added section to the example proxy config Fixed-up unit test pep8 warnings Changed error response code to 400 (Bad Request) Change-Id: Iace719d6a3d00fb3dda1b9d0bc185b8c4cbc00ca --- etc/proxy-server.conf-sample | 6 + setup.py | 1 + swift/common/middleware/name_check.py | 109 ++++++++++++++++++ .../unit/common/middleware/test_name_check.py | 72 ++++++++++++ 4 files changed, 188 insertions(+) create mode 100644 swift/common/middleware/name_check.py create mode 100644 test/unit/common/middleware/test_name_check.py diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index e6afb91d57..e0138db210 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -220,3 +220,9 @@ use = egg:swift#tempurl # Note: Put formpost just before your auth filter(s) in the pipeline [filter:formpost] use = egg:swift#formpost + +# Note: Just needs to be placed before the proxy-server in the pipeline. +[filter:name_check] +use = egg:swift#name_check +# forbidden_chars = '"`<> +# maximum_length = 255 diff --git a/setup.py b/setup.py index 8f0e2321f1..06131565ab 100644 --- a/setup.py +++ b/setup.py @@ -93,6 +93,7 @@ setup( 'recon=swift.common.middleware.recon:filter_factory', 'tempurl=swift.common.middleware.tempurl:filter_factory', 'formpost=swift.common.middleware.formpost:filter_factory', + 'name_check=swift.common.middleware.name_check:filter_factory', ], }, ) diff --git a/swift/common/middleware/name_check.py b/swift/common/middleware/name_check.py new file mode 100644 index 0000000000..2f04966b3a --- /dev/null +++ b/swift/common/middleware/name_check.py @@ -0,0 +1,109 @@ +# Copyright (c) 2012 OpenStack, LLC. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +''' +Created on February 27, 2012 + +A filter that disallows any paths that contain defined forbidden characters +or that exceed a defined length. + +Place in proxy filter before proxy, e.g. + +[pipeline:main] +pipeline = catch_errors healthcheck name_check cache tempauth sos proxy-server + +[filter:name_check] +use = egg:swift#name_check +forbidden_chars = '"`<> +maximum_length = 255 + +There are default settings for forbidden_chars (FORBIDDEN_CHARS) and +maximum_length (MAX_LENGTH) + +The filter returns HTTPBadRequest if path is invalid. + +@author: eamonn-otoole +''' + +from swift.common.utils import get_logger +from webob import Request +from webob.exc import HTTPBadRequest +from urllib2 import unquote + +FORBIDDEN_CHARS = "\'\"`<>" +MAX_LENGTH = 255 + + +class NameCheckMiddleware(object): + + def __init__(self, app, conf): + self.app = app + self.conf = conf + self.forbidden_chars = self.conf.get('forbidden_chars', + FORBIDDEN_CHARS) + self.maximum_length = self.conf.get('maximum_length', MAX_LENGTH) + self.logger = get_logger(self.conf, log_route='name_check') + + def check_character(self, req): + ''' + Checks req.path for any forbidden characters + Returns True if there are any forbidden characters + Returns False if there aren't any forbidden characters + ''' + self.logger.debug("name_check: path %s" % req.path) + self.logger.debug("name_check: self.forbidden_chars %s" % + self.forbidden_chars) + + for c in unquote(req.path): + if c in self.forbidden_chars: + return True + else: + pass + return False + + def check_length(self, req): + ''' + Checks that req.path doesn't exceed the defined maximum length + Returns True if the length exceeds the maximum + Returns False if the length is <= the maximum + ''' + length = len(unquote(req.path)) + if length > self.maximum_length: + return True + else: + return False + + def __call__(self, env, start_response): + req = Request(env) + + if self.check_character(req): + return HTTPBadRequest(request=req, + body=("Object/Container name contains forbidden chars from %s" + % self.forbidden_chars))(env, start_response) + elif self.check_length(req): + return HTTPBadRequest(request=req, + body=("Object/Container name longer than the allowed maximum %s" + % self.maximum_length))(env, start_response) + else: + # Pass on to downstream WSGI component + return self.app(env, start_response) + + +def filter_factory(global_conf, **local_conf): + conf = global_conf.copy() + conf.update(local_conf) + + def name_check_filter(app): + return NameCheckMiddleware(app, conf) + return name_check_filter diff --git a/test/unit/common/middleware/test_name_check.py b/test/unit/common/middleware/test_name_check.py new file mode 100644 index 0000000000..792836322e --- /dev/null +++ b/test/unit/common/middleware/test_name_check.py @@ -0,0 +1,72 @@ +# Copyright (c) 2012 OpenStack, LLC. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +''' +Unit tests for Name_check filter + +Created on February 29, 2012 + +@author: eamonn-otoole +''' + +import unittest +from webob import Request, Response +from swift.common.middleware import name_check + +MAX_LENGTH = 255 +FORBIDDEN_CHARS = '\'\"<>`' + + +class FakeApp(object): + + def __call__(self, env, start_response): + return Response(body="OK")(env, start_response) + + +class TestNameCheckMiddleware(unittest.TestCase): + + def setUp(self): + self.conf = {'maximum_length': MAX_LENGTH, 'forbidden_chars': + FORBIDDEN_CHARS} + self.test_check = name_check.filter_factory(self.conf)(FakeApp()) + + def test_valid_length_and_character(self): + path = '/V1.0/' + 'c' * (MAX_LENGTH - 6) + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.body, 'OK') + + def test_invalid_character(self): + for c in self.conf['forbidden_chars']: + path = '/V1.0/1234' + c + '5' + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.body, + ("Object/Container name contains forbidden chars from %s" + % self.conf['forbidden_chars'])) + self.assertEquals(resp.status_int, 400) + + def test_invalid_length(self): + path = '/V1.0/' + 'c' * (MAX_LENGTH - 5) + resp = Request.blank(path, environ={'REQUEST_METHOD': 'PUT'} + ).get_response(self.test_check) + self.assertEquals(resp.body, + ("Object/Container name longer than the allowed maximum %s" + % self.conf['maximum_length'])) + self.assertEquals(resp.status_int, 400) + + +if __name__ == '__main__': + unittest.main()