x/swift3
Code Issues Proposed changes

Deny all access to controller instance method

Browse Source 
Current swift3 middleware can allow to access the controller instance
method via HTTP verb and it may have a risk to be attacked like brute
force.

likely:
  from boto.s3.connection import S3Connection
  conn = S3Connection(<snip>)
  # expected 405 Method Not Allowed but this results in 500
  # InternalError
  conn.make_request('_delete_segments_bucket', 'bucket')

Probably all instance method except public verb like ones don't work
well and nothing leaked but it will absolutely 500 InternalError without
any information. This is worse. Thus we should strict the method anyway.

This patch fixes it to set swift.common.utils.public decorator for all
public methods and then middleware will deny the accesses for non-public
methods.

Closes-Bug: #1592250

Change-Id: Ia5579011701eaff2bca555efe950b0c11a3ff5b9
This commit is contained in:
Kota Tsuyuzaki 2016-06-13 21:49:48 -07:00
parent a1cc181bd8
commit 4336ff4f99
13 changed files with 67 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
swift3/controllers/acl.py
View File

@ -15,6 +15,7 @@
from swift.common.http import HTTP_OK
from swift.common.middleware.acl import parse_acl, referrer_allowed
from swift.common.utils import public
from swift3.exception import ACLError
from swift3.controllers.base import Controller
@ -82,6 +83,7 @@ class AclController(Controller):
Those APIs are logged as ACL operations in the S3 server log.
"""
@public
def GET(self, req):
"""
Handles GET Bucket acl and GET Object acl.
@ -90,6 +92,7 @@ class AclController(Controller):
return get_acl(req.user_id, resp.headers)
@public
def PUT(self, req):
"""
Handles PUT Bucket acl and PUT Object acl.

7
swift3/controllers/bucket.py
View File

@ -16,7 +16,7 @@
import sys
from swift.common.http import HTTP_OK
from swift.common.utils import json
from swift.common.utils import json, public
from swift3.controllers.base import Controller
from swift3.etree import Element, SubElement, tostring, fromstring, \
@ -79,6 +79,7 @@ class BucketController(Controller):
except (BucketNotEmpty, InternalError):
raise ServiceUnavailable()
@public
def HEAD(self, req):
"""
Handle HEAD Bucket (Get Metadata) request
@ -87,6 +88,7 @@ class BucketController(Controller):
return HTTPOk(headers=resp.headers)
@public
def GET(self, req):
"""
Handle GET Bucket (List Objects) request
@ -168,6 +170,7 @@ class BucketController(Controller):
return HTTPOk(body=body, content_type='application/xml')
@public
def PUT(self, req):
"""
Handle PUT Bucket request
@ -196,6 +199,7 @@ class BucketController(Controller):
return resp
@public
def DELETE(self, req):
"""
Handle DELETE Bucket request
@ -205,6 +209,7 @@ class BucketController(Controller):
resp = req.get_response(self.app)
return resp
@public
def POST(self, req):
"""
Handle POST Bucket request

3
swift3/controllers/location.py
View File

@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from swift.common.utils import public
from swift3.controllers.base import Controller, bucket_operation
from swift3.etree import Element, tostring
from swift3.response import HTTPOk
@ -24,6 +26,7 @@ class LocationController(Controller):
Handles GET Bucket location, which is logged as a LOCATION operation in the
S3 server log.
"""
@public
@bucket_operation
def GET(self, req):
"""

4
swift3/controllers/logging.py
View File

@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from swift.common.utils import public
from swift3.controllers.base import Controller, bucket_operation
from swift3.etree import Element, tostring
from swift3.response import HTTPOk, S3NotImplemented, NoLoggingStatusForKey
@ -27,6 +29,7 @@ class LoggingStatusController(Controller):
Those APIs are logged as LOGGING_STATUS operations in the S3 server log.
"""
@public
@bucket_operation(err_resp=NoLoggingStatusForKey)
def GET(self, req):
"""
@ -40,6 +43,7 @@ class LoggingStatusController(Controller):
return HTTPOk(body=body, content_type='application/xml')
@public
@bucket_operation(err_resp=NoLoggingStatusForKey)
def PUT(self, req):
"""

3
swift3/controllers/multi_delete.py
View File

@ -15,6 +15,8 @@
import sys
from swift.common.utils import public
from swift3.controllers.base import Controller, bucket_operation
from swift3.etree import Element, SubElement, fromstring, tostring, \
XMLSyntaxError, DocumentInvalid
@ -44,6 +46,7 @@ class MultiObjectDeleteController(Controller):
return tostring(elem)
@public
@bucket_operation
def POST(self, req):
"""

8
swift3/controllers/multi_upload.py
View File

@ -47,7 +47,7 @@ import re
import sys
from swift.common.swob import Range
from swift.common.utils import json
from swift.common.utils import json, public
from swift.common.db import utf8encode
from six.moves.urllib.parse import urlparse # pylint: disable=F0401
@ -95,6 +95,7 @@ class PartController(Controller):
Those APIs are logged as PART operations in the S3 server log.
"""
@public
@object_operation
@check_container_existence
def PUT(self, req):
@ -171,6 +172,7 @@ class UploadsController(Controller):
Those APIs are logged as UPLOADS operations in the S3 server log.
"""
@public
@bucket_operation(err_resp=InvalidRequest,
err_msg="Key is not expected for the GET method "
"?uploads subresource")
@ -319,6 +321,7 @@ class UploadsController(Controller):
return HTTPOk(body=body, content_type='application/xml')
@public
@object_operation
@check_container_existence
def POST(self, req):
@ -359,6 +362,7 @@ class UploadController(Controller):
Those APIs are logged as UPLOAD operations in the S3 server log.
"""
@public
@object_operation
@check_container_existence
def GET(self, req):
@ -453,6 +457,7 @@ class UploadController(Controller):
return HTTPOk(body=body, content_type='application/xml')
@public
@object_operation
@check_container_existence
def DELETE(self, req):
@ -489,6 +494,7 @@ class UploadController(Controller):
return HTTPNoContent()
@public
@object_operation
@check_container_existence
def POST(self, req):

6
swift3/controllers/obj.py
View File

@ -17,6 +17,7 @@ import sys
from swift.common.http import HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_NO_CONTENT
from swift.common.swob import Range, content_range_header_value
from swift.common.utils import public
from swift3.utils import S3Timestamp
from swift3.controllers.base import Controller
@ -75,6 +76,7 @@ class ObjectController(Controller):
return resp
@public
def HEAD(self, req):
"""
Handle HEAD Object request
@ -87,12 +89,14 @@ class ObjectController(Controller):
return resp
@public
def GET(self, req):
"""
Handle GET Object request
"""
return self.GETorHEAD(req)
@public
def PUT(self, req):
"""
Handle PUT Object and PUT Object (Copy) request
@ -120,9 +124,11 @@ class ObjectController(Controller):
resp.status = HTTP_OK
return resp
@public
def POST(self, req):
raise S3NotImplemented()
@public
def DELETE(self, req):
"""
Handle DELETE Object request

7
swift3/controllers/s3_acl.py
View File

@ -14,6 +14,7 @@
# limitations under the License.
from urllib import quote
from swift.common.utils import public
from swift3.controllers.base import Controller
from swift3.response import HTTPOk
@ -31,19 +32,21 @@ class S3AclController(Controller):
Those APIs are logged as ACL operations in the S3 server log.
"""
@public
def GET(self, req):
"""
Handles GET Bucket acl and GET Object acl.
"""
resp = req.get_response(self.app)
acl = getattr(resp, '%s_acl' %
('object' if req.is_object_request else 'bucket'))
acl = resp.object_acl if req.is_object_request else resp.bucket_acl
resp = HTTPOk()
resp.body = tostring(acl.elem())
return resp
@public
def PUT(self, req):
"""
Handles PUT Bucket acl and PUT Object acl.

3
swift3/controllers/service.py
View File

@ -13,7 +13,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from swift.common.utils import json
from swift.common.utils import json, public
from swift3.controllers.base import Controller
from swift3.etree import Element, SubElement, tostring
@ -26,6 +26,7 @@ class ServiceController(Controller):
"""
Handles account level requests.
"""
@public
def GET(self, req):
"""
Handle GET Service request

4
swift3/controllers/versioning.py
View File

@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from swift.common.utils import public
from swift3.controllers.base import Controller, bucket_operation
from swift3.etree import Element, tostring
from swift3.response import HTTPOk, S3NotImplemented
@ -27,6 +29,7 @@ class VersioningController(Controller):
Those APIs are logged as VERSIONING operations in the S3 server log.
"""
@public
@bucket_operation
def GET(self, req):
"""
@ -40,6 +43,7 @@ class VersioningController(Controller):
return HTTPOk(body=body, content_type="text/plain")
@public
@bucket_operation
def PUT(self, req):
"""

6
swift3/middleware.py
View File

@ -100,7 +100,11 @@ class Swift3Middleware(object):
controller = req.controller(self.app)
if hasattr(controller, req.method):
res = getattr(controller, req.method)(req)
handler = getattr(controller, req.method)
if not getattr(handler, 'publicly_accessible', False):
raise MethodNotAllowed(req.method,
req.controller.resource_type())
res = handler(req)
else:
raise MethodNotAllowed(req.method,
req.controller.resource_type())

10
swift3/test/functional/test_bucket.py
View File

@ -314,6 +314,16 @@ class TestSwift3Bucket(Swift3FunctionalTestCase):
status, headers, body = self.conn.make_request('DELETE', 'bucket')
self.assertEquals(get_error_code(body), 'NoSuchBucket')
def test_bucket_invalid_method_error(self):
# non existed verb in the controller
status, headers, body = \
self.conn.make_request('GETPUT', 'bucket')
self.assertEquals(get_error_code(body), 'MethodNotAllowed')
# the method exists in the controller but deny as MethodNotAllowed
status, headers, body = \
self.conn.make_request('_delete_segments_bucket', 'bucket')
self.assertEquals(get_error_code(body), 'MethodNotAllowed')
@unittest.skipIf(os.environ['AUTH'] == 'tempauth',
'v4 is supported only in keystone')

9
swift3/test/unit/test_middleware.py
View File

@ -59,6 +59,15 @@ class TestSwift3Middleware(Swift3TestCase):
status, headers, body = self.call_swift3(req)
self.assertEquals(self._get_error_code(body), 'MethodNotAllowed')
def test_bad_method_but_method_exists_in_controller(self):
req = Request.blank(
'/bucket',
environ={'REQUEST_METHOD': '_delete_segments_bucket'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()})
status, headers, body = self.call_swift3(req)
self.assertEquals(self._get_error_code(body), 'MethodNotAllowed')
def test_path_info_encode(self):
bucket_name = 'b%75cket'
object_name = 'ob%6aect:1'