From 67bee14a9b2cb9bfc15cbf94d6db09d6155c9fc9 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 18 Oct 2018 13:44:07 +0200 Subject: [PATCH 01/19] Update open_api to store a copy of SpecificationHandler in the kb --- w3af/core/data/parsers/doc/open_api/main.py | 14 +++++++-- .../parsers/doc/open_api/specification.py | 29 +++++++++++++++---- w3af/plugins/crawl/open_api.py | 5 ++++ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/w3af/core/data/parsers/doc/open_api/main.py b/w3af/core/data/parsers/doc/open_api/main.py index d8a0785fae..14ddb20867 100644 --- a/w3af/core/data/parsers/doc/open_api/main.py +++ b/w3af/core/data/parsers/doc/open_api/main.py @@ -67,6 +67,7 @@ def __init__(self, http_response, no_validation=False, discover_fuzzable_headers self.api_calls = [] self.no_validation = no_validation self.discover_fuzzable_headers = discover_fuzzable_headers + self.specification_handler = None @staticmethod def content_type_match(http_resp): @@ -136,6 +137,13 @@ def can_parse(http_resp): # sure until we really parse it in OpenAPI.parse() return True + def get_specification_handler(self): + """ + TODO + :return: + """ + return self.specification_handler + def parse(self): """ Extract all the API endpoints using the bravado Open API parser. @@ -143,10 +151,10 @@ def parse(self): The method also looks for all parameters which are passed to endpoints via headers, and stores them in to the fuzzable request """ - specification_handler = SpecificationHandler(self.get_http_response(), - self.no_validation) + self.specification_handler = SpecificationHandler(self.get_http_response(), + self.no_validation) - for data in specification_handler.get_api_information(): + for data in self.specification_handler.get_api_information(): try: request_factory = RequestFactory(*data) fuzzable_request = request_factory.get_fuzzable_request(self.discover_fuzzable_headers) diff --git a/w3af/core/data/parsers/doc/open_api/specification.py b/w3af/core/data/parsers/doc/open_api/specification.py index eea58db091..35eaeb5cdd 100644 --- a/w3af/core/data/parsers/doc/open_api/specification.py +++ b/w3af/core/data/parsers/doc/open_api/specification.py @@ -58,6 +58,26 @@ def __init__(self, http_response, no_validation=False): def get_http_response(self): return self.http_response + def shallow_copy(self): + """ + TODO + :return: + """ + return SpecificationHandler(self.http_response, self.no_validation) + + def parse(self): + """ + TODO + :return: + """ + spec_dict = self._load_spec_dict() + if spec_dict is None: + return None + + self.spec = self._parse_spec_from_dict(spec_dict) + + return self.spec + def get_api_information(self): """ This is the main method. @@ -65,13 +85,10 @@ def get_api_information(self): :yield: All the information we were able to collect about each API endpoint This includes the specification, parameters, URL, etc. """ - spec_dict = self._load_spec_dict() + if not self.spec: + self.parse() - if spec_dict is None: - return - - self.spec = self._parse_spec_from_dict(spec_dict) - if self.spec is None: + if not self.spec: return for api_resource_name, resource in self.spec.resources.items(): diff --git a/w3af/plugins/crawl/open_api.py b/w3af/plugins/crawl/open_api.py index 85748789e7..3981d3a54b 100644 --- a/w3af/plugins/crawl/open_api.py +++ b/w3af/plugins/crawl/open_api.py @@ -230,6 +230,11 @@ def _report_to_kb_if_needed(self, http_response, parser): kb.kb.append(self, 'open_api', i) om.out.information(i.get_desc()) + # Save a shallow copy of the specification handler to the kb. + # It would be better to save the API spec + # but looks like pickling doesn't work well with Bravado. + kb.kb.raw_write('open_api', 'specification_handler', parser.get_specification_handler().shallow_copy()) + # Warn the user about missing credentials if self._query_string_auth or self._header_auth: return From d46c5ab8e35304b854b0009b4019ba091a2071de Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 18 Oct 2018 13:45:01 +0200 Subject: [PATCH 02/19] Added first draft of open_api_auth.py --- w3af/plugins/audit/open_api_auth.py | 113 ++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 w3af/plugins/audit/open_api_auth.py diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py new file mode 100644 index 0000000000..00b71e67ef --- /dev/null +++ b/w3af/plugins/audit/open_api_auth.py @@ -0,0 +1,113 @@ +""" +open_api_auth.py + +Copyright 2018 Andres Riancho + +This file is part of w3af, http://w3af.org/ . + +w3af is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation version 2 of the License. + +w3af is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with w3af; if not, write to the Free Software +Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + +""" +from w3af.core.controllers.plugins.audit_plugin import AuditPlugin + +import w3af.core.data.kb.knowledge_base as kb +import w3af.core.controllers.output_manager as om +from w3af.core.data.parsers.doc.open_api.specification import SpecificationHandler + + +class open_api_auth(AuditPlugin): + """ + TODO + + :author: Artem Smotrakov (artem.smotrakov@gmail.com) + :author: Andres Riancho (andres.riancho@gmail.com) + """ + + def __init__(self): + AuditPlugin.__init__(self) + self._spec = None + + def audit(self, freq, orig_response, debugging_id): + """ + TODO + + :param freq: A FuzzableRequest + :param orig_response: The HTTP response associated with the fuzzable request + :param debugging_id: A unique identifier for this call to audit() + """ + if self._has_no_api_spec(): + om.out.info("could not find an API specification, skip") + return + + if self._no_security_in_spec(): + om.out.info("the API spec defines no security, skip") + return + + + # TODO The check should only work when the open api specification requires authentication for a endpoint + # Check for security headers according to the spec + if self._should_skip(freq): + return + + # TODO The check should send a request for each authenticated endpoint using the provided authentication, + # then send it without the provided authentication and compare both.If they are similar (see fuzzy_equal) + # then a vulnerability should be reported. + + def _analyze_result(self, mutant, response): + """ + Analyze results of the _send_mutant method. + """ + pass + + # TODO The check should only work when the user configures authentication (api key, headers, etc.) + def _no_security_in_spec(self): + """ + TODO + :return: + """ + return True + + def _has_no_api_spec(self): + """ + TODO + :return: + """ + if self._spec: + return False + + specification_handler = kb.kb.raw_read('open_api', 'specification_handler') + if not specification_handler: + return True + + self._spec = specification_handler.parse() + + if not self._spec: + return True + + return False + + def _should_skip(self, freq): + """ + TODO + :return: + """ + return True + + def get_long_desc(self): + """ + :return: A DETAILED description of the plugin functions and features. + """ + return """ + TODO + """ From 1d010bec8f7c0a2ce1fc2dee87eae04ea0727ac1 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Fri, 19 Oct 2018 14:39:45 +0200 Subject: [PATCH 03/19] Updated open_api_auth plugin to check and remove auth info from requests --- w3af/plugins/audit/open_api_auth.py | 155 +++++++++++++++++++++++----- 1 file changed, 128 insertions(+), 27 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 00b71e67ef..d61416c08f 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -19,11 +19,12 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA """ -from w3af.core.controllers.plugins.audit_plugin import AuditPlugin +import copy import w3af.core.data.kb.knowledge_base as kb import w3af.core.controllers.output_manager as om -from w3af.core.data.parsers.doc.open_api.specification import SpecificationHandler + +from w3af.core.controllers.plugins.audit_plugin import AuditPlugin class open_api_auth(AuditPlugin): @@ -41,68 +42,168 @@ def __init__(self): def audit(self, freq, orig_response, debugging_id): """ TODO - :param freq: A FuzzableRequest :param orig_response: The HTTP response associated with the fuzzable request :param debugging_id: A unique identifier for this call to audit() """ - if self._has_no_api_spec(): - om.out.info("could not find an API specification, skip") + # The check works only with API endpoints. + # crawl.open_api plugin has to be called before. + if not self._has_api_spec(): + om.out.info("Could not find an API specification, skip") return - if self._no_security_in_spec(): - om.out.info("the API spec defines no security, skip") + # The API spec must define authentication mechanisms. + if not self._has_security_definitions_in_spec(): + om.out.info("The API spec has no security definitions, skip") return - - # TODO The check should only work when the open api specification requires authentication for a endpoint - # Check for security headers according to the spec - if self._should_skip(freq): + # The check only runs if the open api specification + # requires authentication for a endpoint. + # TODO: take into account `security` section for an operation (or there may be a global one) + # https://swagger.io/docs/specification/2-0/authentication/ + if not self._has_auth(freq): + om.out.info("Request doesn't have auth info, skip") return - # TODO The check should send a request for each authenticated endpoint using the provided authentication, - # then send it without the provided authentication and compare both.If they are similar (see fuzzy_equal) - # then a vulnerability should be reported. - - def _analyze_result(self, mutant, response): + # Remove auth info from the request + freq_with_no_auth = self._remove_auth(freq) + + # Send the request to the server, and check if access was denied. + self._uri_opener.send_mutant(freq_with_no_auth, + callback=self._check_access_denied, + debugging_id=debugging_id) + + # TODO consider sending requests with other HTTP methods, + # and check that 401 or 405 error code is returned + # On the one hand, it may not look too good if we just replace the method, + # and keep the rest of the request untouched (headers, payload, etc). + # But on the other hand, the application is supposed to check HTTP method in the very beginning, + # and reject requests if the method is not supported. + # If the API spec says a particular method is supported, then the check should expect 401 + # If the API spec says a particular method is not supported, + # then the check should expect 405 (preferred) or 401 (should it only expect 405 in this case?). + + # TODO Should we compare the response with orig_response (see fuzzy_equal), + # and report a vulnerability if they are similar? + # Or, just checking the response code is enough? + + def _check_access_denied(self, freq, response): """ - Analyze results of the _send_mutant method. + TODO """ pass - # TODO The check should only work when the user configures authentication (api key, headers, etc.) - def _no_security_in_spec(self): + def _has_security_definitions_in_spec(self): """ TODO :return: """ - return True + if self._spec.security_definitions: + return True + + return False - def _has_no_api_spec(self): + def _has_api_spec(self): """ TODO :return: """ if self._spec: - return False + return True specification_handler = kb.kb.raw_read('open_api', 'specification_handler') if not specification_handler: - return True + return False self._spec = specification_handler.parse() - - if not self._spec: + if self._spec: return True return False - def _should_skip(self, freq): + def _remove_auth(self, freq): + """ + TODO + :param freq: + :return: + """ + updated_freq = copy.deepcopy(freq) + for key, auth in self._spec.security_definitions.iteritems(): + if auth.type == 'basic' or auth.type == 'oauth2': + self._remove_header(updated_freq, 'Authorization') + + if auth.type == 'apiKey': + self._remove_api_key(updated_freq, auth) + + return updated_freq + + @staticmethod + def _remove_header(freq, name): """ TODO + :param freq: :return: """ - return True + headers = freq.get_headers() + if headers.icontains(name): + headers.idel(name) + + def _remove_api_key(self, freq, auth): + """ + TODO + :param freq: + :param auth: + :return: + """ + if auth['in'] == 'query': + params = freq.get_url().get_params() + del params[auth.name] + freq.get_url().set_params(params) + + if auth['in'] == 'header': + self._remove_header(freq, auth['name']) + + def _has_auth(self, freq): + """ + TODO + :return: + """ + for key, auth in self._spec.security_definitions.iteritems(): + if auth.type == 'basic' and self._has_basic_auth(freq): + return True + + if auth.type == 'apiKey' and self._has_api_key(freq, auth): + return True + + if auth.type == 'oauth2' and self._has_oauth2(freq): + return True + + return False + + @staticmethod + def _has_basic_auth(freq): + return freq.get_headers().iget('Authorization', '').startswith('basic') + + @staticmethod + def _has_api_key(freq, auth): + if not hasattr(auth, 'name'): + return False + + if not hasattr(auth, 'in'): + return False + + if auth['in'] == 'query': + params = freq.get_url().get_params() + return params and auth.name in params and params[auth.name] + + if auth['in'] == 'header' and freq.get_headers().iget(auth.name): + return True + + return False + + @staticmethod + def _has_oauth2(freq): + return freq.get_headers().iget('Authorization', '')[0].startswith('Bearer') def get_long_desc(self): """ From 47a4fde78ae258937ac63680404f843a1c4874ef Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Mon, 22 Oct 2018 17:17:15 +0200 Subject: [PATCH 04/19] Added test_open_api_auth.py and fixed issues in open_api_auth --- w3af/plugins/audit/open_api_auth.py | 20 +- .../audit/data/petstore_with_security.yaml | 142 +++++++++++++ .../plugins/tests/audit/test_open_api_auth.py | 199 ++++++++++++++++++ 3 files changed, 351 insertions(+), 10 deletions(-) create mode 100644 w3af/plugins/tests/audit/data/petstore_with_security.yaml create mode 100644 w3af/plugins/tests/audit/test_open_api_auth.py diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index d61416c08f..5eddf7a32b 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -49,12 +49,12 @@ def audit(self, freq, orig_response, debugging_id): # The check works only with API endpoints. # crawl.open_api plugin has to be called before. if not self._has_api_spec(): - om.out.info("Could not find an API specification, skip") + om.out.information("Could not find an API specification, skip") return # The API spec must define authentication mechanisms. if not self._has_security_definitions_in_spec(): - om.out.info("The API spec has no security definitions, skip") + om.out.information("The API spec has no security definitions, skip") return # The check only runs if the open api specification @@ -62,7 +62,7 @@ def audit(self, freq, orig_response, debugging_id): # TODO: take into account `security` section for an operation (or there may be a global one) # https://swagger.io/docs/specification/2-0/authentication/ if not self._has_auth(freq): - om.out.info("Request doesn't have auth info, skip") + om.out.information("Request doesn't have auth info, skip") return # Remove auth info from the request @@ -155,13 +155,13 @@ def _remove_api_key(self, freq, auth): :param auth: :return: """ - if auth['in'] == 'query': + if auth.location == 'query': params = freq.get_url().get_params() del params[auth.name] freq.get_url().set_params(params) - if auth['in'] == 'header': - self._remove_header(freq, auth['name']) + if auth.location == 'header': + self._remove_header(freq, auth.name) def _has_auth(self, freq): """ @@ -182,21 +182,21 @@ def _has_auth(self, freq): @staticmethod def _has_basic_auth(freq): - return freq.get_headers().iget('Authorization', '').startswith('basic') + return freq.get_headers().iget('Authorization', '')[0].startswith('basic') @staticmethod def _has_api_key(freq, auth): if not hasattr(auth, 'name'): return False - if not hasattr(auth, 'in'): + if not hasattr(auth, 'location'): return False - if auth['in'] == 'query': + if auth.location == 'query': params = freq.get_url().get_params() return params and auth.name in params and params[auth.name] - if auth['in'] == 'header' and freq.get_headers().iget(auth.name): + if auth.location == 'header' and freq.get_headers().iget(auth.name)[0]: return True return False diff --git a/w3af/plugins/tests/audit/data/petstore_with_security.yaml b/w3af/plugins/tests/audit/data/petstore_with_security.yaml new file mode 100644 index 0000000000..0d8dddf311 --- /dev/null +++ b/w3af/plugins/tests/audit/data/petstore_with_security.yaml @@ -0,0 +1,142 @@ +swagger: "2.0" +info: + version: 1.0.0 + title: Swagger Petstore + description: A sample API that uses a petstore as an example to demonstrate features in the swagger-2.0 specification + termsOfService: http://swagger.io/terms/ + contact: + name: Swagger API Team + email: foo@example.com + url: http://madskristensen.net + license: + name: MIT + url: http://github.com/gruntjs/grunt/blob/master/LICENSE-MIT +host: w3af.org +basePath: /api +schemes: + - http +consumes: + - application/json +produces: + - application/json +securityDefinitions: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key + OAuth2: + type: oauth2 + flow: accessCode + authorizationUrl: https://example.com/oauth/authorize + tokenUrl: https://example.com/oauth/token + scopes: + read: Grants read access + write: Grants write access +security: + - OAuth2: [read, write] +paths: + /ping: + get: + description: Checks if the server is running + operationId: ping + security: [] + responses: + "200": + description: Server is up and running + default: + description: Something is wrong + /pets: + get: + description: Returns all pets from the system that the user has access to + operationId: findPets + parameters: + - name: limit + in: query + description: How many items to return at one time (max 100) + required: false + type: integer + format: int32 + security: + - ApiKeyAuth: [] + responses: + "200": + description: pet response + schema: + type: array + items: + $ref: '#/definitions/Pet' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' + post: + description: Creates a new pet in the store. Duplicates are allowed + operationId: addPet + parameters: + - name: pet + in: body + description: Pet to add to the store + required: true + schema: + $ref: '#/definitions/NewPet' + security: + - ApiKeyAuth: [] + responses: + "200": + description: pet response + schema: + $ref: '#/definitions/Pet' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' + /pets/{id}: + get: + description: Returns a pet + operationId: find pet by id + parameters: + - name: id + in: path + description: ID of pet to fetch + required: true + type: integer + format: int64 + responses: + "200": + description: pet response + schema: + $ref: '#/definitions/Pet' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' +definitions: + Pet: + allOf: + - $ref: '#/definitions/NewPet' + - required: + - id + properties: + id: + type: integer + format: int64 + + NewPet: + required: + - name + properties: + name: + type: string + tag: + type: string + + Error: + required: + - code + - message + properties: + code: + type: integer + format: int32 + message: + type: string diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py new file mode 100644 index 0000000000..bd1581b30c --- /dev/null +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -0,0 +1,199 @@ +""" +test_open_api_auth.py + +Copyright 2018 Andres Riancho + +This file is part of w3af, http://w3af.org/ . + +w3af is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation version 2 of the License. + +w3af is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with w3af; if not, write to the Free Software +Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +""" +import os + +from w3af.core.data.dc.generic.kv_container import KeyValueContainer +from w3af.core.data.dc.headers import Headers + +from w3af.core.data.parsers.doc.open_api.specification import SpecificationHandler +from w3af.core.data.parsers.doc.url import URL +from w3af.core.data.request.fuzzable_request import FuzzableRequest +from w3af.core.data.url.HTTPResponse import HTTPResponse +from w3af.plugins.audit.open_api_auth import open_api_auth +from w3af.plugins.tests.helper import PluginTest, PluginConfig, MockResponse + + +CURRENT_PATH = os.path.split(__file__)[0] + + +def by_path(fra, frb): + return cmp(fra.get_url().url_string, frb.get_url().url_string) + + +def generate_response(specification_as_string): + url = URL('http://www.w3af.com/openapi.yaml') + headers = Headers([('content-type', 'application/yaml')]) + return HTTPResponse(200, specification_as_string, headers, + url, url, _id=1) + + +class AuthMockResponse(MockResponse): + + def get_response(self, http_request, uri, response_headers): + if http_request.headers.get('X-API-Key', '') != 'xxx': + return 401, response_headers, '' + + return super(AuthMockResponse, self).get_response(http_request, + uri, + response_headers) + + +class PetstoreWithSecurity(object): + + @staticmethod + def get_specification(): + return file('%s/data/petstore_with_security.yaml' % CURRENT_PATH).read() + + +class TestOpenAPIAuth(PluginTest): + + target_url = 'http://w3af.org/' + api_key_auth = ('header_auth', 'X-API-Key: xxx', PluginConfig.HEADER) + + _run_configs = { + 'cfg': { + 'target': target_url, + 'plugins': { + 'crawl': (PluginConfig('open_api', api_key_auth,),), + 'audit': (PluginConfig('open_api_auth'),) + } + } + } + + MOCK_RESPONSES = [MockResponse('http://w3af.org/openapi.yaml', + PetstoreWithSecurity().get_specification(), + content_type='application/yaml'), + MockResponse('http://w3af.org/api/ping', + '', + content_type='text/plain'), + MockResponse('http://w3af.org/api/pets', + '{ "id": 42 }', + content_type='application/json'), + AuthMockResponse('http://w3af.org/api/pets/*', + '{ "id": 42 }', + content_type='application/json') + ] + + def test_petstore_with_security(self): + cfg = self._run_configs['cfg'] + self._scan(cfg['target'], cfg['plugins']) + + specification_handler = self.kb.raw_read('open_api', 'specification_handler') + self.assertIsNotNone(specification_handler, "no specification handler") + self.assertTrue(isinstance(specification_handler, SpecificationHandler), "not a SpecificationHandler") + + infos = self.kb.get('open_api', 'open_api') + self.assertEqual(len(infos), 1, infos) + info_i = infos[0] + self.assertEqual(info_i.get_name(), 'Open API specification found') + + fuzzable_requests = self.kb.get_all_known_fuzzable_requests() + fuzzable_requests = [f for f in fuzzable_requests if f.get_url().get_path() not in ('/openapi.yaml', '/')] + fuzzable_requests.sort(by_path) + self.assertEqual(len(fuzzable_requests), 4) + + # TODO check for vulns + # vulns = self.kb.get('open_api_auth', 'open_api_auth') + # self.assertEquals(len(vulns), 1) + # vuln = vulns[0] + # self.assertEquals('TBD', vuln.get_name()) + # self.assertEquals('TBD', vuln.get_url().url_string) + + def test_open_api_auth(self): + api_http_response = generate_response(PetstoreWithSecurity.get_specification()) + specification_handler = SpecificationHandler(api_http_response) + self.kb.raw_write('open_api', 'specification_handler', specification_handler) + + spec = specification_handler.parse() + + plugin = open_api_auth() + self.assertTrue(plugin._has_api_spec()) + self.assertTrue(plugin._has_security_definitions_in_spec()) + + url = URL('http://w3af.org/api/pets') + + fr = FuzzableRequest(url, + headers=Headers([('Authorization', 'Bearer xxx')]), + post_data=KeyValueContainer(), + method='GET') + self.assertTrue(plugin._has_auth(fr)) + self.assertTrue(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_basic_auth(fr)) + + # Remove auth info from the request. + updated_fr = plugin._remove_auth(fr) + + # Make sure that the original request still has auth info. + self.assertTrue(plugin._has_auth(fr)) + self.assertTrue(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_basic_auth(fr)) + + # Check if the updated request doesn't have auth info. + self.assertFalse(plugin._has_auth(updated_fr)) + self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(updated_fr)) + self.assertFalse(plugin._has_basic_auth(updated_fr)) + + fr = FuzzableRequest(url, + headers=Headers([('X-API-Key', 'zzz')]), + post_data=KeyValueContainer(), + method='POST') + self.assertTrue(plugin._has_auth(fr)) + self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_basic_auth(fr)) + + # Remove auth info from the request. + updated_fr = plugin._remove_auth(fr) + + # Make sure that the original request still has auth info. + self.assertTrue(plugin._has_auth(fr)) + self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_basic_auth(fr)) + + # Check if the updated request doesn't have auth info. + self.assertFalse(plugin._has_auth(updated_fr)) + self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(updated_fr)) + self.assertFalse(plugin._has_basic_auth(updated_fr)) + + fr = FuzzableRequest(url, + headers=Headers([('X-Foo-Header', 'foo'), ('X-Bar-Header', 'bar')]), + post_data=KeyValueContainer(), + method='PUT') + self.assertFalse(plugin._has_auth(fr)) + self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_basic_auth(fr)) + + # Remove auth info from the request. + updated_fr = plugin._remove_auth(fr) + self.assertFalse(plugin._has_auth(updated_fr)) + self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_oauth2(updated_fr)) + self.assertFalse(plugin._has_basic_auth(updated_fr)) + + open_api_auth._remove_header(fr, 'X-Bar-Header') + self.assertFalse(fr.get_headers().icontains('X-Bar-Header')) + self.assertTrue(fr.get_headers().icontains('X-Foo-Header')) From 5dec3cdb2c949858e2482820ec396d062f3322e5 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Oct 2018 15:12:02 +0200 Subject: [PATCH 05/19] open_api_auth should take into account 'security' sections of operations --- w3af/core/data/parsers/doc/open_api/main.py | 15 +++- .../parsers/doc/open_api/specification.py | 7 ++ w3af/plugins/audit/open_api_auth.py | 82 +++++++++++++++++-- w3af/plugins/crawl/open_api.py | 7 +- .../audit/data/petstore_with_security.yaml | 16 ++-- .../plugins/tests/audit/test_open_api_auth.py | 52 ++++++++++-- 6 files changed, 156 insertions(+), 23 deletions(-) diff --git a/w3af/core/data/parsers/doc/open_api/main.py b/w3af/core/data/parsers/doc/open_api/main.py index 14ddb20867..80f7e4bc46 100644 --- a/w3af/core/data/parsers/doc/open_api/main.py +++ b/w3af/core/data/parsers/doc/open_api/main.py @@ -68,6 +68,7 @@ def __init__(self, http_response, no_validation=False, discover_fuzzable_headers self.no_validation = no_validation self.discover_fuzzable_headers = discover_fuzzable_headers self.specification_handler = None + self.request_to_operation_id = {} @staticmethod def content_type_match(http_resp): @@ -144,6 +145,13 @@ def get_specification_handler(self): """ return self.specification_handler + def get_request_to_operation_id(self): + """ + TODO + :return: + """ + return self.request_to_operation_id + def parse(self): """ Extract all the API endpoints using the bravado Open API parser. @@ -154,10 +162,14 @@ def parse(self): self.specification_handler = SpecificationHandler(self.get_http_response(), self.no_validation) + self.request_to_operation_id = {} for data in self.specification_handler.get_api_information(): try: request_factory = RequestFactory(*data) fuzzable_request = request_factory.get_fuzzable_request(self.discover_fuzzable_headers) + + key = '%s|%s' % (fuzzable_request.get_method(), fuzzable_request.get_url()) + self.request_to_operation_id[key] = data[4].operation_id except Exception, e: # # This is a strange situation because parsing of the OpenAPI @@ -188,7 +200,8 @@ def parse(self): self.api_calls.append(fuzzable_request) - def _should_audit(self, fuzzable_request): + @staticmethod + def _should_audit(fuzzable_request): """ We want to make sure that w3af doesn't delete all the items from the REST API, so we ignore DELETE calls. diff --git a/w3af/core/data/parsers/doc/open_api/specification.py b/w3af/core/data/parsers/doc/open_api/specification.py index 35eaeb5cdd..a6e8da5606 100644 --- a/w3af/core/data/parsers/doc/open_api/specification.py +++ b/w3af/core/data/parsers/doc/open_api/specification.py @@ -65,6 +65,13 @@ def shallow_copy(self): """ return SpecificationHandler(self.http_response, self.no_validation) + def get_spec(self): + """ + TODO + :return: + """ + return self.spec + def parse(self): """ TODO diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 5eddf7a32b..5d5cb2f2c5 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -57,15 +57,13 @@ def audit(self, freq, orig_response, debugging_id): om.out.information("The API spec has no security definitions, skip") return - # The check only runs if the open api specification + # The check only runs if the API specification # requires authentication for a endpoint. - # TODO: take into account `security` section for an operation (or there may be a global one) - # https://swagger.io/docs/specification/2-0/authentication/ if not self._has_auth(freq): om.out.information("Request doesn't have auth info, skip") return - # Remove auth info from the request + # Remove auth info from the request. freq_with_no_auth = self._remove_auth(freq) # Send the request to the server, and check if access was denied. @@ -163,6 +161,36 @@ def _remove_api_key(self, freq, auth): if auth.location == 'header': self._remove_header(freq, auth.name) + def _get_operation_by_id(self, operation_id): + """ + TODO + :param operation_id: + :return: + """ + for api_resource_name, resource in self._spec.resources.items(): + for operation_name, operation in resource.operations.items(): + if operation.operation_id == operation_id: + return operation + + return None + + @staticmethod + def _get_operation_id(freq): + """ + TODO + :param freq: + :return: + """ + request_to_operation_id = kb.kb.raw_read('open_api', 'request_to_operation_id') + if not request_to_operation_id: + return None + + key = '%s|%s' % (freq.get_method(), freq.get_url()) + if key not in request_to_operation_id: + return None + + return request_to_operation_id[key] + def _has_auth(self, freq): """ TODO @@ -182,10 +210,21 @@ def _has_auth(self, freq): @staticmethod def _has_basic_auth(freq): + """ + TODO + :param freq: + :return: + """ return freq.get_headers().iget('Authorization', '')[0].startswith('basic') @staticmethod def _has_api_key(freq, auth): + """ + TODO + :param freq: + :param auth: + :return: + """ if not hasattr(auth, 'name'): return False @@ -201,8 +240,39 @@ def _has_api_key(freq, auth): return False - @staticmethod - def _has_oauth2(freq): + def _is_acceptable_auth_type(self, freq, auth_type): + """ + TODO + :param freq: + :param auth_type: + :return: + """ + operation_id = self._get_operation_id(freq) + operation = self._get_operation_by_id(operation_id) + if not operation: + return False + + for security_spec in operation.security_specs: + for key, value in security_spec.iteritems(): + if key not in self._spec.security_definitions: + # Should not happen. + continue + + security_definition = self._spec.security_definitions[key] + if security_definition.type == auth_type: + return True + + return False + + def _has_oauth2(self, freq): + """ + TODO + :param freq: + :return: + """ + if not self._is_acceptable_auth_type(freq, 'oauth2'): + return False + return freq.get_headers().iget('Authorization', '')[0].startswith('Bearer') def get_long_desc(self): diff --git a/w3af/plugins/crawl/open_api.py b/w3af/plugins/crawl/open_api.py index 3981d3a54b..c0c83b13bd 100644 --- a/w3af/plugins/crawl/open_api.py +++ b/w3af/plugins/crawl/open_api.py @@ -233,7 +233,12 @@ def _report_to_kb_if_needed(self, http_response, parser): # Save a shallow copy of the specification handler to the kb. # It would be better to save the API spec # but looks like pickling doesn't work well with Bravado. - kb.kb.raw_write('open_api', 'specification_handler', parser.get_specification_handler().shallow_copy()) + kb.kb.raw_write('open_api', 'specification_handler', + parser.get_specification_handler().shallow_copy()) + + # Store the operation ids in the kb. + kb.kb.raw_write('open_api', 'request_to_operation_id', + parser.get_request_to_operation_id()) # Warn the user about missing credentials if self._query_string_auth or self._header_auth: diff --git a/w3af/plugins/tests/audit/data/petstore_with_security.yaml b/w3af/plugins/tests/audit/data/petstore_with_security.yaml index 0d8dddf311..8c02226909 100644 --- a/w3af/plugins/tests/audit/data/petstore_with_security.yaml +++ b/w3af/plugins/tests/audit/data/petstore_with_security.yaml @@ -58,6 +58,7 @@ paths: format: int32 security: - ApiKeyAuth: [] + - OAuth2: [read] responses: "200": description: pet response @@ -81,6 +82,7 @@ paths: $ref: '#/definitions/NewPet' security: - ApiKeyAuth: [] + - OAuth2: [write] responses: "200": description: pet response @@ -93,14 +95,14 @@ paths: /pets/{id}: get: description: Returns a pet - operationId: find pet by id + operationId: findPetById parameters: - - name: id - in: path - description: ID of pet to fetch - required: true - type: integer - format: int64 + - name: id + in: path + description: ID of pet to fetch + required: true + type: integer + format: int64 responses: "200": description: pet response diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index bd1581b30c..6aa8bd222c 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -22,6 +22,7 @@ from w3af.core.data.dc.generic.kv_container import KeyValueContainer from w3af.core.data.dc.headers import Headers +from w3af.core.data.parsers.doc.open_api import OpenAPI from w3af.core.data.parsers.doc.open_api.specification import SpecificationHandler from w3af.core.data.parsers.doc.url import URL @@ -119,21 +120,26 @@ def test_petstore_with_security(self): def test_open_api_auth(self): api_http_response = generate_response(PetstoreWithSecurity.get_specification()) - specification_handler = SpecificationHandler(api_http_response) - self.kb.raw_write('open_api', 'specification_handler', specification_handler) + parser = OpenAPI(api_http_response) + parser.parse() + self.kb.raw_write('open_api', 'specification_handler', parser.get_specification_handler().shallow_copy()) + self.kb.raw_write('open_api', 'request_to_operation_id', parser.get_request_to_operation_id()) - spec = specification_handler.parse() + spec = parser.get_specification_handler().get_spec() plugin = open_api_auth() self.assertTrue(plugin._has_api_spec()) self.assertTrue(plugin._has_security_definitions_in_spec()) - url = URL('http://w3af.org/api/pets') - - fr = FuzzableRequest(url, + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('Authorization', 'Bearer xxx')]), post_data=KeyValueContainer(), method='GET') + + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) self.assertTrue(plugin._has_auth(fr)) self.assertTrue(plugin._has_oauth2(fr)) self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) @@ -154,7 +160,7 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) - fr = FuzzableRequest(url, + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('X-API-Key', 'zzz')]), post_data=KeyValueContainer(), method='POST') @@ -178,7 +184,7 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) - fr = FuzzableRequest(url, + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('X-Foo-Header', 'foo'), ('X-Bar-Header', 'bar')]), post_data=KeyValueContainer(), method='PUT') @@ -197,3 +203,33 @@ def test_open_api_auth(self): open_api_auth._remove_header(fr, 'X-Bar-Header') self.assertFalse(fr.get_headers().icontains('X-Bar-Header')) self.assertTrue(fr.get_headers().icontains('X-Foo-Header')) + + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), + headers=Headers([('X-API-Key', 'zzz')]), + post_data=KeyValueContainer(), + method='POST') + + # TODO figure out why it doesn't have POST method for /api/pets + #self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) + #self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) + #self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + #self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + #self.assertTrue(plugin._has_auth(fr)) + #self.assertFalse(plugin._has_oauth2(fr)) + #self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + #self.assertFalse(plugin._has_basic_auth(fr)) + + # Check the endpoint which doesn't require auth. + fr = FuzzableRequest(URL('http://w3af.org/api/ping'), + headers=Headers(), + post_data=KeyValueContainer(), + method='GET') + + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'oauth2')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'apiKey')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + self.assertFalse(plugin._has_auth(fr)) + self.assertFalse(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_basic_auth(fr)) From f3910354976e6dc9af729899ac8ef49d00300007 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Oct 2018 15:58:15 +0200 Subject: [PATCH 06/19] Updated mock responses in TestOpenAPIAuth --- .../plugins/tests/audit/test_open_api_auth.py | 62 ++++++++++++------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index 6aa8bd222c..000c6ecf69 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -46,17 +46,6 @@ def generate_response(specification_as_string): url, url, _id=1) -class AuthMockResponse(MockResponse): - - def get_response(self, http_request, uri, response_headers): - if http_request.headers.get('X-API-Key', '') != 'xxx': - return 401, response_headers, '' - - return super(AuthMockResponse, self).get_response(http_request, - uri, - response_headers) - - class PetstoreWithSecurity(object): @staticmethod @@ -79,19 +68,44 @@ class TestOpenAPIAuth(PluginTest): } } - MOCK_RESPONSES = [MockResponse('http://w3af.org/openapi.yaml', - PetstoreWithSecurity().get_specification(), - content_type='application/yaml'), - MockResponse('http://w3af.org/api/ping', - '', - content_type='text/plain'), - MockResponse('http://w3af.org/api/pets', - '{ "id": 42 }', - content_type='application/json'), - AuthMockResponse('http://w3af.org/api/pets/*', - '{ "id": 42 }', - content_type='application/json') - ] + class AuthMockResponse(MockResponse): + + def get_response(self, http_request, uri, response_headers): + if http_request.headers.get('X-API-Key', '') != 'xxx': + return 401, response_headers, '' + + return self.status, response_headers, '{ "id": 42 }' + + MOCK_RESPONSES = [ + + # No auth required for the API spec. + MockResponse('http://w3af.org/openapi.yaml', + PetstoreWithSecurity().get_specification(), + content_type='application/yaml'), + + # No auth required for the health check endpoint. + MockResponse('http://w3af.org/api/ping', + '', + content_type='text/plain'), + + # Authenticate GET requests to /api/pets + AuthMockResponse('http://w3af.org/api/pets', + '{ "id": 42 }', + content_type='application/json', + method='GET'), + + # No auth for POST requests to /api/pets + # This should result to a vulnerability. + MockResponse('http://w3af.org/api/pets', + '{ "id": 42 }', + content_type='application/json', + method='POST'), + + # Authenticate requests to /api/pets/{id} + AuthMockResponse('http://w3af.org/api/pets/*', + '{ "id": 42 }', + content_type='application/json') + ] def test_petstore_with_security(self): cfg = self._run_configs['cfg'] From 846fdf7b52a61da5f5f6974430682c0abb941e03 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Oct 2018 16:06:38 +0200 Subject: [PATCH 07/19] NewPet and Error should be objects in petstore_with_security.yaml --- .../audit/data/petstore_with_security.yaml | 2 ++ .../plugins/tests/audit/test_open_api_auth.py | 19 +++++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/w3af/plugins/tests/audit/data/petstore_with_security.yaml b/w3af/plugins/tests/audit/data/petstore_with_security.yaml index 8c02226909..0e2188031c 100644 --- a/w3af/plugins/tests/audit/data/petstore_with_security.yaml +++ b/w3af/plugins/tests/audit/data/petstore_with_security.yaml @@ -124,6 +124,7 @@ definitions: format: int64 NewPet: + type: object required: - name properties: @@ -133,6 +134,7 @@ definitions: type: string Error: + type: object required: - code - message diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index 000c6ecf69..fe87dbeaef 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -123,7 +123,7 @@ def test_petstore_with_security(self): fuzzable_requests = self.kb.get_all_known_fuzzable_requests() fuzzable_requests = [f for f in fuzzable_requests if f.get_url().get_path() not in ('/openapi.yaml', '/')] fuzzable_requests.sort(by_path) - self.assertEqual(len(fuzzable_requests), 4) + self.assertEqual(len(fuzzable_requests), 6) # TODO check for vulns # vulns = self.kb.get('open_api_auth', 'open_api_auth') @@ -223,15 +223,14 @@ def test_open_api_auth(self): post_data=KeyValueContainer(), method='POST') - # TODO figure out why it doesn't have POST method for /api/pets - #self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) - #self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) - #self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) - #self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) - #self.assertTrue(plugin._has_auth(fr)) - #self.assertFalse(plugin._has_oauth2(fr)) - #self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) - #self.assertFalse(plugin._has_basic_auth(fr)) + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + self.assertTrue(plugin._has_auth(fr)) + self.assertFalse(plugin._has_oauth2(fr)) + self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_basic_auth(fr)) # Check the endpoint which doesn't require auth. fr = FuzzableRequest(URL('http://w3af.org/api/ping'), From 34302235d2e4b09a02e4eff2635e0b3d9a9c58fa Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Oct 2018 16:30:46 +0200 Subject: [PATCH 08/19] open_api_auth should take into account 'security' sections of operations for all auth schemes --- w3af/plugins/audit/open_api_auth.py | 34 +++++++++++-------- .../plugins/tests/audit/test_open_api_auth.py | 24 +++++++++++++ 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 5d5cb2f2c5..79d256ed67 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -208,23 +208,27 @@ def _has_auth(self, freq): return False - @staticmethod - def _has_basic_auth(freq): + def _has_basic_auth(self, freq): """ TODO :param freq: :return: """ + if not self._is_acceptable_auth_type(freq, 'basic'): + return False + return freq.get_headers().iget('Authorization', '')[0].startswith('basic') - @staticmethod - def _has_api_key(freq, auth): + def _has_api_key(self, freq, auth): """ TODO :param freq: :param auth: :return: """ + if not self._is_acceptable_auth_type(freq, 'apiKey'): + return False + if not hasattr(auth, 'name'): return False @@ -240,6 +244,17 @@ def _has_api_key(freq, auth): return False + def _has_oauth2(self, freq): + """ + TODO + :param freq: + :return: + """ + if not self._is_acceptable_auth_type(freq, 'oauth2'): + return False + + return freq.get_headers().iget('Authorization', '')[0].startswith('Bearer') + def _is_acceptable_auth_type(self, freq, auth_type): """ TODO @@ -264,17 +279,6 @@ def _is_acceptable_auth_type(self, freq, auth_type): return False - def _has_oauth2(self, freq): - """ - TODO - :param freq: - :return: - """ - if not self._is_acceptable_auth_type(freq, 'oauth2'): - return False - - return freq.get_headers().iget('Authorization', '')[0].startswith('Bearer') - def get_long_desc(self): """ :return: A DETAILED description of the plugin functions and features. diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index fe87dbeaef..41f3da55e3 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -154,6 +154,7 @@ def test_open_api_auth(self): self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + self.assertTrue(plugin._has_auth(fr)) self.assertTrue(plugin._has_oauth2(fr)) self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) @@ -178,6 +179,12 @@ def test_open_api_auth(self): headers=Headers([('X-API-Key', 'zzz')]), post_data=KeyValueContainer(), method='POST') + + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + self.assertTrue(plugin._has_auth(fr)) self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_oauth2(fr)) @@ -246,3 +253,20 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_oauth2(fr)) self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_basic_auth(fr)) + + # /api/pets/{id} doesn't have a 'security' section + # but global 'security' section should apply here (only OAuth2) + fr = FuzzableRequest(URL('http://w3af.org/api/pets/42'), + headers=Headers([('Authorization', 'Bearer xxx')]), + post_data=KeyValueContainer(), + method='GET') + + self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'apiKey')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) + self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) + + self.assertTrue(plugin._has_auth(fr)) + self.assertTrue(plugin._has_oauth2(fr)) + self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_basic_auth(fr)) From d613f4eeb1219adb74f035c81a5f4da81370cfbf Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Wed, 24 Oct 2018 17:51:33 +0200 Subject: [PATCH 09/19] Check response codes in open_api_auth plugin --- w3af/plugins/audit/open_api_auth.py | 20 +++++++- .../plugins/tests/audit/test_open_api_auth.py | 51 +++++++++++-------- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 79d256ed67..2820839c64 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -25,6 +25,8 @@ import w3af.core.controllers.output_manager as om from w3af.core.controllers.plugins.audit_plugin import AuditPlugin +from w3af.core.data.constants import severity +from w3af.core.data.kb.vuln import Vuln class open_api_auth(AuditPlugin): @@ -39,6 +41,9 @@ def __init__(self): AuditPlugin.__init__(self) self._spec = None + # Note: we can let a user set a list of expected codes. + self._expected_codes = [401] + def audit(self, freq, orig_response, debugging_id): """ TODO @@ -89,7 +94,20 @@ def _check_access_denied(self, freq, response): """ TODO """ - pass + if response.get_code() not in self._expected_codes: + desc = 'A %s request without auth info was send to %s. ' \ + 'The server replied with an unexpected code (%d) but expected %s' \ + % (freq.get_method(), freq.get_url(), response.get_code(), self._expected_codes) + + # TODO severity may depend on response codes + v = Vuln.from_fr('Broken authentication', desc, + severity.HIGH, response.id, + self.get_name(), freq) + + v['response_code'] = response.get_code() + v['method'] = freq.get_method() + + self.kb_append_uniq(self, 'open_api_auth', v) def _has_security_definitions_in_spec(self): """ diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index 41f3da55e3..8a84b71f54 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -19,6 +19,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA """ import os +import re from w3af.core.data.dc.generic.kv_container import KeyValueContainer from w3af.core.data.dc.headers import Headers @@ -89,20 +90,20 @@ def get_response(self, http_request, uri, response_headers): content_type='text/plain'), # Authenticate GET requests to /api/pets - AuthMockResponse('http://w3af.org/api/pets', + AuthMockResponse(re.compile('http://w3af.org/api/pets$'), '{ "id": 42 }', content_type='application/json', method='GET'), # No auth for POST requests to /api/pets # This should result to a vulnerability. - MockResponse('http://w3af.org/api/pets', + MockResponse(re.compile('http://w3af.org/api/pets$'), '{ "id": 42 }', content_type='application/json', method='POST'), # Authenticate requests to /api/pets/{id} - AuthMockResponse('http://w3af.org/api/pets/*', + AuthMockResponse(re.compile('http://w3af.org/api/pets.*'), '{ "id": 42 }', content_type='application/json') ] @@ -111,33 +112,40 @@ def test_petstore_with_security(self): cfg = self._run_configs['cfg'] self._scan(cfg['target'], cfg['plugins']) - specification_handler = self.kb.raw_read('open_api', 'specification_handler') + specification_handler = self.kb.raw_read('open_api', + 'specification_handler') self.assertIsNotNone(specification_handler, "no specification handler") - self.assertTrue(isinstance(specification_handler, SpecificationHandler), "not a SpecificationHandler") + self.assertTrue(isinstance(specification_handler, SpecificationHandler), + "not a SpecificationHandler") infos = self.kb.get('open_api', 'open_api') self.assertEqual(len(infos), 1, infos) info_i = infos[0] self.assertEqual(info_i.get_name(), 'Open API specification found') - fuzzable_requests = self.kb.get_all_known_fuzzable_requests() - fuzzable_requests = [f for f in fuzzable_requests if f.get_url().get_path() not in ('/openapi.yaml', '/')] - fuzzable_requests.sort(by_path) - self.assertEqual(len(fuzzable_requests), 6) + frs = self.kb.get_all_known_fuzzable_requests() + frs = [f for f in frs if f.get_url().get_path() not in ('/openapi.yaml', '/')] + frs.sort(by_path) + self.assertEqual(len(frs), 6) - # TODO check for vulns - # vulns = self.kb.get('open_api_auth', 'open_api_auth') - # self.assertEquals(len(vulns), 1) - # vuln = vulns[0] - # self.assertEquals('TBD', vuln.get_name()) - # self.assertEquals('TBD', vuln.get_url().url_string) + vulns = self.kb.get('open_api_auth', 'open_api_auth') + self.assertEquals(len(vulns), 1) + + vuln = vulns[0] + self.assertEquals('Broken authentication', vuln.get_name()) + self.assertEquals('High', vuln.get_severity()) + self.assertEquals('http://w3af.org/api/pets', vuln.get_url().url_string) + self.assertEquals('POST', vuln['method']) + self.assertEquals(200, vuln['response_code']) def test_open_api_auth(self): api_http_response = generate_response(PetstoreWithSecurity.get_specification()) parser = OpenAPI(api_http_response) parser.parse() - self.kb.raw_write('open_api', 'specification_handler', parser.get_specification_handler().shallow_copy()) - self.kb.raw_write('open_api', 'request_to_operation_id', parser.get_request_to_operation_id()) + self.kb.raw_write('open_api', 'specification_handler', + parser.get_specification_handler().shallow_copy()) + self.kb.raw_write('open_api', 'request_to_operation_id', + parser.get_request_to_operation_id()) spec = parser.get_specification_handler().get_spec() @@ -171,7 +179,8 @@ def test_open_api_auth(self): # Check if the updated request doesn't have auth info. self.assertFalse(plugin._has_auth(updated_fr)) - self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_api_key(updated_fr, + spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) @@ -201,7 +210,8 @@ def test_open_api_auth(self): # Check if the updated request doesn't have auth info. self.assertFalse(plugin._has_auth(updated_fr)) - self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_api_key(updated_fr, + spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) @@ -217,7 +227,8 @@ def test_open_api_auth(self): # Remove auth info from the request. updated_fr = plugin._remove_auth(fr) self.assertFalse(plugin._has_auth(updated_fr)) - self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) + self.assertFalse(plugin._has_api_key(updated_fr, + spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) From cd5b21f0ee41f5601fa3185976c18244c4e08a1a Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 25 Oct 2018 17:03:20 +0200 Subject: [PATCH 10/19] Split tests in test_open_api_auth.py --- .../plugins/tests/audit/test_open_api_auth.py | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index 8a84b71f54..da764a9188 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -54,7 +54,7 @@ def get_specification(): return file('%s/data/petstore_with_security.yaml' % CURRENT_PATH).read() -class TestOpenAPIAuth(PluginTest): +class TestOpenAPIAuthWithPetstore(PluginTest): target_url = 'http://w3af.org/' api_key_auth = ('header_auth', 'X-API-Key: xxx', PluginConfig.HEADER) @@ -138,7 +138,10 @@ def test_petstore_with_security(self): self.assertEquals('POST', vuln['method']) self.assertEquals(200, vuln['response_code']) - def test_open_api_auth(self): + +class TestOpenAPIAuth(PluginTest): + + def init_plugin(self): api_http_response = generate_response(PetstoreWithSecurity.get_specification()) parser = OpenAPI(api_http_response) parser.parse() @@ -153,6 +156,11 @@ def test_open_api_auth(self): self.assertTrue(plugin._has_api_spec()) self.assertTrue(plugin._has_security_definitions_in_spec()) + return plugin, spec + + def test_with_bearer(self): + plugin, spec = self.init_plugin() + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('Authorization', 'Bearer xxx')]), post_data=KeyValueContainer(), @@ -184,6 +192,9 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) + def test_with_api_key(self): + plugin, spec = self.init_plugin() + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('X-API-Key', 'zzz')]), post_data=KeyValueContainer(), @@ -215,6 +226,9 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_oauth2(updated_fr)) self.assertFalse(plugin._has_basic_auth(updated_fr)) + def test_with_no_auth(self): + plugin, spec = self.init_plugin() + fr = FuzzableRequest(URL('http://w3af.org/api/pets'), headers=Headers([('X-Foo-Header', 'foo'), ('X-Bar-Header', 'bar')]), post_data=KeyValueContainer(), @@ -226,6 +240,7 @@ def test_open_api_auth(self): # Remove auth info from the request. updated_fr = plugin._remove_auth(fr) + self.assertFalse(plugin._has_auth(updated_fr)) self.assertFalse(plugin._has_api_key(updated_fr, spec.security_definitions['ApiKeyAuth'])) @@ -236,19 +251,8 @@ def test_open_api_auth(self): self.assertFalse(fr.get_headers().icontains('X-Bar-Header')) self.assertTrue(fr.get_headers().icontains('X-Foo-Header')) - fr = FuzzableRequest(URL('http://w3af.org/api/pets'), - headers=Headers([('X-API-Key', 'zzz')]), - post_data=KeyValueContainer(), - method='POST') - - self.assertTrue(plugin._is_acceptable_auth_type(fr, 'oauth2')) - self.assertTrue(plugin._is_acceptable_auth_type(fr, 'apiKey')) - self.assertFalse(plugin._is_acceptable_auth_type(fr, 'basic')) - self.assertFalse(plugin._is_acceptable_auth_type(fr, 'unknown')) - self.assertTrue(plugin._has_auth(fr)) - self.assertFalse(plugin._has_oauth2(fr)) - self.assertTrue(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) - self.assertFalse(plugin._has_basic_auth(fr)) + def test_no_security(self): + plugin, spec = self.init_plugin() # Check the endpoint which doesn't require auth. fr = FuzzableRequest(URL('http://w3af.org/api/ping'), @@ -265,6 +269,9 @@ def test_open_api_auth(self): self.assertFalse(plugin._has_api_key(fr, spec.security_definitions['ApiKeyAuth'])) self.assertFalse(plugin._has_basic_auth(fr)) + def test_with_global_security(self): + plugin, spec = self.init_plugin() + # /api/pets/{id} doesn't have a 'security' section # but global 'security' section should apply here (only OAuth2) fr = FuzzableRequest(URL('http://w3af.org/api/pets/42'), From 1b9f54c32d22be09e1b4d9c3fd502d63e427eb8e Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Thu, 25 Oct 2018 18:03:43 +0200 Subject: [PATCH 11/19] Describe open_api_auth plugin --- w3af/plugins/audit/open_api_auth.py | 63 ++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 2820839c64..ef010fcf94 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -31,7 +31,27 @@ class open_api_auth(AuditPlugin): """ - TODO + Checks if REST API endpoints authenticate incoming requests + as defined in their API specification. + See details in the plugin description below. + + Here is a list of possible enhancements: + * Consider sending requests with other HTTP methods, + and check that 401 or 405 error code is returned + On the one hand, it may not look too good if we just replace the method, + and keep the rest of the request untouched (headers, payload, etc). + But on the other hand, the application is supposed to check HTTP method + in the very beginning, and reject requests if the method is not supported. + If the API spec says a particular method is supported, then the check should expect 401 + If the API spec says a particular method is not supported, + then the check should expect 405 (preferred) or 401 (should it only expect 405 in this case?). + * Allow a user to specify response codes that the plugin should expect. + Currently the plugin expects only 401. + We can introduce 'expected_code_regex' configuration parameter + which set a regex for expected response codes. + * If the plugin is updated to send other HTTP methods (see above), + we may want to introduce another configuration parameter + to set expected response codes in this case. :author: Artem Smotrakov (artem.smotrakov@gmail.com) :author: Andres Riancho (andres.riancho@gmail.com) @@ -73,24 +93,10 @@ def audit(self, freq, orig_response, debugging_id): # Send the request to the server, and check if access was denied. self._uri_opener.send_mutant(freq_with_no_auth, - callback=self._check_access_denied, + callback=self._check_response_code, debugging_id=debugging_id) - # TODO consider sending requests with other HTTP methods, - # and check that 401 or 405 error code is returned - # On the one hand, it may not look too good if we just replace the method, - # and keep the rest of the request untouched (headers, payload, etc). - # But on the other hand, the application is supposed to check HTTP method in the very beginning, - # and reject requests if the method is not supported. - # If the API spec says a particular method is supported, then the check should expect 401 - # If the API spec says a particular method is not supported, - # then the check should expect 405 (preferred) or 401 (should it only expect 405 in this case?). - - # TODO Should we compare the response with orig_response (see fuzzy_equal), - # and report a vulnerability if they are similar? - # Or, just checking the response code is enough? - - def _check_access_denied(self, freq, response): + def _check_response_code(self, freq, response): """ TODO """ @@ -302,5 +308,26 @@ def get_long_desc(self): :return: A DETAILED description of the plugin functions and features. """ return """ - TODO + This plugin checks that REST API endpoints require authentication + according to their API specification. + + OpenAPI specification offers the following structures + to define requirements for authentication: + * 'securityDefinitions' section describes all available authentication mechanisms. + It offers three methods: basic, apiKey and oauth2. + * 'security' section may be used in each operation to specify + which authentication mechanisms the operation accepts. + * Global 'security' section describes authentication mechanisms + for operations which don't have their own 'security' section + + Currently the check is pretty simple: + * Remove authentication info from the request (API key, header, etc). + * Send the modified request to the endpoint. + * Check if the response has 401 error code (access denied). + + A couple of important notes: + * The plugin requires the 'crawl.open_api' plugin to be enabled. + It works only with REST API endpoints. + * The check works only when API specification requires authentication for a endpoint. + * The check works only if a user provided authentication info (API key, header, etc). """ From c3c645436c0889810e5978082502e289b57c8985 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Fri, 26 Oct 2018 10:49:41 +0200 Subject: [PATCH 12/19] Describe methods in open_api_auth plugin --- w3af/plugins/audit/open_api_auth.py | 141 +++++++++++------- .../plugins/tests/audit/test_open_api_auth.py | 2 +- 2 files changed, 91 insertions(+), 52 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index ef010fcf94..dfd622e431 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -42,9 +42,11 @@ class open_api_auth(AuditPlugin): and keep the rest of the request untouched (headers, payload, etc). But on the other hand, the application is supposed to check HTTP method in the very beginning, and reject requests if the method is not supported. - If the API spec says a particular method is supported, then the check should expect 401 + If the API spec says a particular method is supported, + then the check should expect 401 If the API spec says a particular method is not supported, - then the check should expect 405 (preferred) or 401 (should it only expect 405 in this case?). + then the check should expect 405 (preferred) or 401 + (should it only expect 405 in this case?). * Allow a user to specify response codes that the plugin should expect. Currently the plugin expects only 401. We can introduce 'expected_code_regex' configuration parameter @@ -52,6 +54,8 @@ class open_api_auth(AuditPlugin): * If the plugin is updated to send other HTTP methods (see above), we may want to introduce another configuration parameter to set expected response codes in this case. + * Severity of identified issues may depend on response codes + returned by the server. :author: Artem Smotrakov (artem.smotrakov@gmail.com) :author: Andres Riancho (andres.riancho@gmail.com) @@ -60,20 +64,19 @@ class open_api_auth(AuditPlugin): def __init__(self): AuditPlugin.__init__(self) self._spec = None - - # Note: we can let a user set a list of expected codes. self._expected_codes = [401] def audit(self, freq, orig_response, debugging_id): """ - TODO - :param freq: A FuzzableRequest - :param orig_response: The HTTP response associated with the fuzzable request - :param debugging_id: A unique identifier for this call to audit() + Check if an API endpoint requires authentication according to its API spec. + + :param freq: A FuzzableRequest to the API endpoint. + :param orig_response: The HTTP response associated with the fuzzable request. + :param debugging_id: A unique identifier for this call to audit(). """ # The check works only with API endpoints. # crawl.open_api plugin has to be called before. - if not self._has_api_spec(): + if not self._is_api_spec_available(): om.out.information("Could not find an API specification, skip") return @@ -91,21 +94,26 @@ def audit(self, freq, orig_response, debugging_id): # Remove auth info from the request. freq_with_no_auth = self._remove_auth(freq) - # Send the request to the server, and check if access was denied. + # Send the request to the server, and check if access denied. self._uri_opener.send_mutant(freq_with_no_auth, callback=self._check_response_code, debugging_id=debugging_id) def _check_response_code(self, freq, response): """ - TODO + Check if an HTTP request contains an expected response code. + + :param freq: A FuzzableRequest to the API endpoint. + :param response: The HTTP response associated with the fuzzable request. """ if response.get_code() not in self._expected_codes: desc = 'A %s request without auth info was send to %s. ' \ - 'The server replied with an unexpected code (%d) but expected %s' \ - % (freq.get_method(), freq.get_url(), response.get_code(), self._expected_codes) + 'The server replied with an unexpected code (%d), expected %s' \ + % (freq.get_method(), freq.get_url(), + response.get_code(), self._expected_codes) - # TODO severity may depend on response codes + # Should severity depend on response codes? + # For example, 2xx codes results to HIGH, but 4xx may be MEDIUM/LOW v = Vuln.from_fr('Broken authentication', desc, severity.HIGH, response.id, self.get_name(), freq) @@ -117,18 +125,23 @@ def _check_response_code(self, freq, response): def _has_security_definitions_in_spec(self): """ - TODO - :return: + :return: True if the API spec contains 'securityDefinitions' section, + False otherwise. """ if self._spec.security_definitions: return True return False - def _has_api_spec(self): + def _is_api_spec_available(self): """ - TODO - :return: + Make sure that we have API specification. + The API spec has to be provided by crawl.open_api plugin + which should be called before. + + The plugins use the global knowledge base to share the API spec. + + :return: True if API specification is available, False otherwise. """ if self._spec: return True @@ -145,9 +158,15 @@ def _has_api_spec(self): def _remove_auth(self, freq): """ - TODO - :param freq: - :return: + Remove authentication info from a fuzzable request. + + The method looks for authentication info which is defined in the API spec. + For example, if 'securityDefinitions' section defines oauth2 and apiKey methods, + then the method will look for Authorization header and the API key + (may be passed in a query string or header). + + :param freq: The fuzzable request to be modified. + :return: A copy of the fuzzable request without auth info. """ updated_freq = copy.deepcopy(freq) for key, auth in self._spec.security_definitions.iteritems(): @@ -162,9 +181,10 @@ def _remove_auth(self, freq): @staticmethod def _remove_header(freq, name): """ - TODO - :param freq: - :return: + Remove a header from a fuzzable request. + + :param freq: The fuzzable request to be updated. + :param name: The header name. """ headers = freq.get_headers() if headers.icontains(name): @@ -172,10 +192,11 @@ def _remove_header(freq, name): def _remove_api_key(self, freq, auth): """ - TODO - :param freq: - :param auth: - :return: + Remove an API key from a fuzzable request. + + :param freq: The fuzzable request to be modified. + :param auth: An element of 'securityDefinitions' section + which describes the API key. """ if auth.location == 'query': params = freq.get_url().get_params() @@ -187,9 +208,10 @@ def _remove_api_key(self, freq, auth): def _get_operation_by_id(self, operation_id): """ - TODO - :param operation_id: - :return: + Look for an operation by its ID in the API specification. + + :param operation_id: ID of an operation. + :return: An instance of Operation (Bravado). """ for api_resource_name, resource in self._spec.resources.items(): for operation_name, operation in resource.operations.items(): @@ -201,9 +223,13 @@ def _get_operation_by_id(self, operation_id): @staticmethod def _get_operation_id(freq): """ - TODO - :param freq: - :return: + Get an operation ID which is associated with a fuzzable request. + + The method uses a mapping { method, url -> operation id } + which should be provided by crawl.open_api plugin. + + :param freq: The fuzzable request. + :return: An ID of the operation associated with the request. """ request_to_operation_id = kb.kb.raw_read('open_api', 'request_to_operation_id') if not request_to_operation_id: @@ -217,8 +243,11 @@ def _get_operation_id(freq): def _has_auth(self, freq): """ - TODO - :return: + Check if a fuzzable request contains authentication info + according to the API specification. + + :param freq: The fuzzable request to be checked. + :return: True if the request contains auth info, False otherwise. """ for key, auth in self._spec.security_definitions.iteritems(): if auth.type == 'basic' and self._has_basic_auth(freq): @@ -234,9 +263,11 @@ def _has_auth(self, freq): def _has_basic_auth(self, freq): """ - TODO - :param freq: - :return: + Check if a fuzzable request contains Basic auth info + if it's allowed by the API specification. + + :param freq: The fuzzable request to be checked. + :return: True if the request contains Basic auth info, False otherwise. """ if not self._is_acceptable_auth_type(freq, 'basic'): return False @@ -245,10 +276,13 @@ def _has_basic_auth(self, freq): def _has_api_key(self, freq, auth): """ - TODO - :param freq: - :param auth: - :return: + Check if a fuzzable request contains an API key + if it's allowed by the API specification. + + :param freq: The request to be checked. + :param auth: An element of 'securityDefinitions' section + which describes the API key. + :return: True if the request contains an API key, False otherwise. """ if not self._is_acceptable_auth_type(freq, 'apiKey'): return False @@ -270,9 +304,11 @@ def _has_api_key(self, freq, auth): def _has_oauth2(self, freq): """ - TODO - :param freq: - :return: + Check if a fuzzable request contains OAuth2 info + if it's allowed by the API specification. + + :param freq: The fuzzable request to be checked. + :return: True if the request contains OAuth2 info, False otherwise. """ if not self._is_acceptable_auth_type(freq, 'oauth2'): return False @@ -281,10 +317,13 @@ def _has_oauth2(self, freq): def _is_acceptable_auth_type(self, freq, auth_type): """ - TODO - :param freq: - :param auth_type: - :return: + Check if the API specification allows a fuzzable request + to have a specific auth info. + + :param freq: The fuzzable request to be checked. + :param auth_type: Auth method (oauth2, apiKey or basic). + :return: True if the API specification allows the request + to have the specified auth method, False otherwise. """ operation_id = self._get_operation_id(freq) operation = self._get_operation_by_id(operation_id) diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index da764a9188..a1e5f9dad2 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -153,7 +153,7 @@ def init_plugin(self): spec = parser.get_specification_handler().get_spec() plugin = open_api_auth() - self.assertTrue(plugin._has_api_spec()) + self.assertTrue(plugin._is_api_spec_available()) self.assertTrue(plugin._has_security_definitions_in_spec()) return plugin, spec From e154dafe83b8216968a9d64301ee6f5b852a3bcc Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Fri, 26 Oct 2018 11:08:58 +0200 Subject: [PATCH 13/19] Describe new methods in OpenAPI class --- w3af/core/data/parsers/doc/open_api/main.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/w3af/core/data/parsers/doc/open_api/main.py b/w3af/core/data/parsers/doc/open_api/main.py index 80f7e4bc46..907ad762cb 100644 --- a/w3af/core/data/parsers/doc/open_api/main.py +++ b/w3af/core/data/parsers/doc/open_api/main.py @@ -45,6 +45,11 @@ class OpenAPI(BaseParser): The parser only returns interesting results for get_forms(), where all FuzzableRequests associated with REST API calls are returned. + The parser stores a couple of things to the kb which then can be used by other plugins: + * An instance of SpecificationHandler which provides the API specification. + * A mapping { method, url -> operation id } which allows to find the operation + which is associated with a fuzzable request. + [0] https://www.openapis.org/ [1] https://github.com/Yelp/bravado-core @@ -140,15 +145,13 @@ def can_parse(http_resp): def get_specification_handler(self): """ - TODO - :return: + :return: An instance of SpecificationHandler which was used by the parser. """ return self.specification_handler def get_request_to_operation_id(self): """ - TODO - :return: + :return: A mapping { method, url -> operation id } """ return self.request_to_operation_id From 65dbf5d8cf523b1c1c5c5b996ede2fa7c3ffb061 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Fri, 26 Oct 2018 11:13:46 +0200 Subject: [PATCH 14/19] Describe new methods in SpecificationHandler --- .../data/parsers/doc/open_api/specification.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/w3af/core/data/parsers/doc/open_api/specification.py b/w3af/core/data/parsers/doc/open_api/specification.py index a6e8da5606..8eba788c7c 100644 --- a/w3af/core/data/parsers/doc/open_api/specification.py +++ b/w3af/core/data/parsers/doc/open_api/specification.py @@ -1,6 +1,6 @@ # -*- coding: UTF-8 -*- """ -requests.py +specification.py Copyright 2017 Andres Riancho @@ -50,32 +50,34 @@ class SpecificationHandler(object): + def __init__(self, http_response, no_validation=False): self.http_response = http_response - self.spec = None self.no_validation = no_validation + self.spec = None def get_http_response(self): return self.http_response def shallow_copy(self): """ - TODO - :return: + :return: A copy of the SpecificationHandler + which doesn't contain a Spec instance. """ return SpecificationHandler(self.http_response, self.no_validation) def get_spec(self): """ - TODO - :return: + :return: An instance of Spec (Bravado) + which was created by this SpecificationHandler. """ return self.spec def parse(self): """ - TODO - :return: + Parse an API specification provided in the HTTP response. + + :return: An instance of Spec (Bravado). """ spec_dict = self._load_spec_dict() if spec_dict is None: From f3352959c92b3268a71ce3f0e5ca50441b573985 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Mon, 12 Nov 2018 18:09:30 +0100 Subject: [PATCH 15/19] Better checks for API spec in audit.open_api_auth plugin --- w3af/plugins/audit/open_api_auth.py | 48 ++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index dfd622e431..2a71a7b9aa 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -64,6 +64,7 @@ class open_api_auth(AuditPlugin): def __init__(self): AuditPlugin.__init__(self) self._spec = None + self._spec_is_good = None self._expected_codes = [401] def audit(self, freq, orig_response, debugging_id): @@ -74,21 +75,13 @@ def audit(self, freq, orig_response, debugging_id): :param orig_response: The HTTP response associated with the fuzzable request. :param debugging_id: A unique identifier for this call to audit(). """ - # The check works only with API endpoints. - # crawl.open_api plugin has to be called before. - if not self._is_api_spec_available(): - om.out.information("Could not find an API specification, skip") - return - - # The API spec must define authentication mechanisms. - if not self._has_security_definitions_in_spec(): - om.out.information("The API spec has no security definitions, skip") + if not self._has_good_spec(): return # The check only runs if the API specification # requires authentication for a endpoint. if not self._has_auth(freq): - om.out.information("Request doesn't have auth info, skip") + om.out.debug("Fuzzable request doesn't contain authentication info, skipping open_api_auth.") return # Remove auth info from the request. @@ -99,6 +92,33 @@ def audit(self, freq, orig_response, debugging_id): callback=self._check_response_code, debugging_id=debugging_id) + def _has_good_spec(self): + """ + Checks if we have an API specification, and it contains all required data, + so that it makes sense to run the plugin. + + The method checks the API specification only once and cache the result. + + :return: True the API specification is available and good enough, False othersise. + """ + if self._spec_is_good is None: + # The check works only with API endpoints. + # crawl.open_api plugin has to be called before. + if not self._is_api_spec_available(): + om.out.debug("Could not find an API specification in the KB, skipping open_api_auth.") + self._spec_is_good = False + return False + + # The API spec must define authentication mechanisms. + if not self._has_security_definitions_in_spec(): + om.out.debug("The API specification has no security definitions, skipping open_api_auth.") + self._spec_is_good = False + return False + + self._spec_is_good = True + + return self._spec_is_good + def _check_response_code(self, freq, response): """ Check if an HTTP request contains an expected response code. @@ -107,10 +127,10 @@ def _check_response_code(self, freq, response): :param response: The HTTP response associated with the fuzzable request. """ if response.get_code() not in self._expected_codes: - desc = 'A %s request without auth info was send to %s. ' \ - 'The server replied with an unexpected code (%d), expected %s' \ - % (freq.get_method(), freq.get_url(), - response.get_code(), self._expected_codes) + desc = 'A %s request without authentication information was sent to %s. ' \ + 'The server replied with unexpected HTTP response code %d (expected one of %s). ' \ + 'This is a strong indicator of a REST API authentication bypass.' \ + % (freq.get_method(), freq.get_url(), response.get_code(), self._expected_codes) # Should severity depend on response codes? # For example, 2xx codes results to HIGH, but 4xx may be MEDIUM/LOW From 3edc480f23fb4f1d9cdb7f5fc0206bc3825b6071 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 13 Nov 2018 13:00:57 +0100 Subject: [PATCH 16/19] Don't use _spec_is_good flag in audit.open_api_auth plugin --- w3af/plugins/audit/open_api_auth.py | 40 ++++++++--------------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 2a71a7b9aa..7c4b73e774 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -64,7 +64,6 @@ class open_api_auth(AuditPlugin): def __init__(self): AuditPlugin.__init__(self) self._spec = None - self._spec_is_good = None self._expected_codes = [401] def audit(self, freq, orig_response, debugging_id): @@ -75,8 +74,16 @@ def audit(self, freq, orig_response, debugging_id): :param orig_response: The HTTP response associated with the fuzzable request. :param debugging_id: A unique identifier for this call to audit(). """ - if not self._has_good_spec(): - return + # The check works only with API endpoints. + # crawl.open_api plugin has to be called before. + if not self._is_api_spec_available(): + om.out.debug("Could not find an API specification in the KB, skipping open_api_auth.") + return False + + # The API spec must define authentication mechanisms. + if not self._has_security_definitions_in_spec(): + om.out.debug("The API specification has no security definitions, skipping open_api_auth.") + return False # The check only runs if the API specification # requires authentication for a endpoint. @@ -92,33 +99,6 @@ def audit(self, freq, orig_response, debugging_id): callback=self._check_response_code, debugging_id=debugging_id) - def _has_good_spec(self): - """ - Checks if we have an API specification, and it contains all required data, - so that it makes sense to run the plugin. - - The method checks the API specification only once and cache the result. - - :return: True the API specification is available and good enough, False othersise. - """ - if self._spec_is_good is None: - # The check works only with API endpoints. - # crawl.open_api plugin has to be called before. - if not self._is_api_spec_available(): - om.out.debug("Could not find an API specification in the KB, skipping open_api_auth.") - self._spec_is_good = False - return False - - # The API spec must define authentication mechanisms. - if not self._has_security_definitions_in_spec(): - om.out.debug("The API specification has no security definitions, skipping open_api_auth.") - self._spec_is_good = False - return False - - self._spec_is_good = True - - return self._spec_is_good - def _check_response_code(self, freq, response): """ Check if an HTTP request contains an expected response code. From ce56f14b51de2bd99b474fc641a74cf6340254a3 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 13 Nov 2018 13:01:52 +0100 Subject: [PATCH 17/19] audit.open_api_auth should depend on crawl.open_api plugin --- w3af/plugins/audit/open_api_auth.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 7c4b73e774..7d0112bb33 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -66,6 +66,13 @@ def __init__(self): self._spec = None self._expected_codes = [401] + def get_plugin_deps(self): + """ + :return: A list with the names of the plugins + that should be run before the current one. + """ + return ['crawl.open_api'] + def audit(self, freq, orig_response, debugging_id): """ Check if an API endpoint requires authentication according to its API spec. @@ -365,7 +372,7 @@ def get_long_desc(self): * Check if the response has 401 error code (access denied). A couple of important notes: - * The plugin requires the 'crawl.open_api' plugin to be enabled. + * The plugin enables the 'crawl.open_api' plugin. It works only with REST API endpoints. * The check works only when API specification requires authentication for a endpoint. * The check works only if a user provided authentication info (API key, header, etc). From b9283fb40359c09bc4534187a60b960908ebb517 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 13 Nov 2018 14:25:29 +0100 Subject: [PATCH 18/19] audit.open_api_auth plugin should expect multiple API specs --- w3af/plugins/audit/open_api_auth.py | 111 +++++++++--------- w3af/plugins/crawl/open_api.py | 15 ++- .../plugins/tests/audit/test_open_api_auth.py | 18 +-- 3 files changed, 77 insertions(+), 67 deletions(-) diff --git a/w3af/plugins/audit/open_api_auth.py b/w3af/plugins/audit/open_api_auth.py index 7d0112bb33..2beed78c61 100644 --- a/w3af/plugins/audit/open_api_auth.py +++ b/w3af/plugins/audit/open_api_auth.py @@ -63,7 +63,7 @@ class open_api_auth(AuditPlugin): def __init__(self): AuditPlugin.__init__(self) - self._spec = None + self._specs = [] self._expected_codes = [401] def get_plugin_deps(self): @@ -84,18 +84,15 @@ def audit(self, freq, orig_response, debugging_id): # The check works only with API endpoints. # crawl.open_api plugin has to be called before. if not self._is_api_spec_available(): - om.out.debug("Could not find an API specification in the KB, skipping open_api_auth.") - return False - - # The API spec must define authentication mechanisms. - if not self._has_security_definitions_in_spec(): - om.out.debug("The API specification has no security definitions, skipping open_api_auth.") - return False + om.out.debug('Could not find any acceptable API specification in the KB, ' + 'skipping open_api_auth.') + return # The check only runs if the API specification # requires authentication for a endpoint. if not self._has_auth(freq): - om.out.debug("Fuzzable request doesn't contain authentication info, skipping open_api_auth.") + om.out.debug('Fuzzable request does not contain authentication info, ' + 'skipping open_api_auth.') return # Remove auth info from the request. @@ -130,38 +127,41 @@ def _check_response_code(self, freq, response): self.kb_append_uniq(self, 'open_api_auth', v) - def _has_security_definitions_in_spec(self): - """ - :return: True if the API spec contains 'securityDefinitions' section, - False otherwise. - """ - if self._spec.security_definitions: - return True - - return False - def _is_api_spec_available(self): """ - Make sure that we have API specification. - The API spec has to be provided by crawl.open_api plugin + Make sure that we have at least one API specification + which contains 'securityDefinitions' section. + The API specs have to be provided by crawl.open_api plugin which should be called before. - The plugins use the global knowledge base to share the API spec. + The plugins use the global knowledge base to share the API specs. - :return: True if API specification is available, False otherwise. + :return: True if API specifications are available, False otherwise. """ - if self._spec: + if self._specs: return True - specification_handler = kb.kb.raw_read('open_api', 'specification_handler') - if not specification_handler: + specification_handlers = kb.kb.raw_read('open_api', 'specification_handlers') + if not specification_handlers: return False - self._spec = specification_handler.parse() - if self._spec: - return True + for specification_handler in specification_handlers: + spec = specification_handler.parse() + spec_url = specification_handler.get_http_response().get_url() - return False + if not spec: + om.out.debug('Could not parse the API specification from %s, ' + 'skipping it.' % spec_url) + continue + + if not spec.security_definitions: + om.out.debug('The API specification from %s ' + 'has no security definitions, skipping it.' % spec_url) + continue + + self._specs.append(spec) + + return len(self._specs) > 0 def _remove_auth(self, freq): """ @@ -176,12 +176,13 @@ def _remove_auth(self, freq): :return: A copy of the fuzzable request without auth info. """ updated_freq = copy.deepcopy(freq) - for key, auth in self._spec.security_definitions.iteritems(): - if auth.type == 'basic' or auth.type == 'oauth2': - self._remove_header(updated_freq, 'Authorization') + for spec in self._specs: + for key, auth in spec.security_definitions.iteritems(): + if auth.type == 'basic' or auth.type == 'oauth2': + self._remove_header(updated_freq, 'Authorization') - if auth.type == 'apiKey': - self._remove_api_key(updated_freq, auth) + if auth.type == 'apiKey': + self._remove_api_key(updated_freq, auth) return updated_freq @@ -215,17 +216,18 @@ def _remove_api_key(self, freq, auth): def _get_operation_by_id(self, operation_id): """ - Look for an operation by its ID in the API specification. + Look for an operation and it's specification. :param operation_id: ID of an operation. - :return: An instance of Operation (Bravado). + :return: An instance of Operation and Spec (Bravado). """ - for api_resource_name, resource in self._spec.resources.items(): - for operation_name, operation in resource.operations.items(): - if operation.operation_id == operation_id: - return operation + for spec in self._specs: + for api_resource_name, resource in spec.resources.items(): + for operation_name, operation in resource.operations.items(): + if operation.operation_id == operation_id: + return operation, spec - return None + return None, None @staticmethod def _get_operation_id(freq): @@ -256,15 +258,16 @@ def _has_auth(self, freq): :param freq: The fuzzable request to be checked. :return: True if the request contains auth info, False otherwise. """ - for key, auth in self._spec.security_definitions.iteritems(): - if auth.type == 'basic' and self._has_basic_auth(freq): - return True + for spec in self._specs: + for key, auth in spec.security_definitions.iteritems(): + if auth.type == 'basic' and self._has_basic_auth(freq): + return True - if auth.type == 'apiKey' and self._has_api_key(freq, auth): - return True + if auth.type == 'apiKey' and self._has_api_key(freq, auth): + return True - if auth.type == 'oauth2' and self._has_oauth2(freq): - return True + if auth.type == 'oauth2' and self._has_oauth2(freq): + return True return False @@ -333,17 +336,13 @@ def _is_acceptable_auth_type(self, freq, auth_type): to have the specified auth method, False otherwise. """ operation_id = self._get_operation_id(freq) - operation = self._get_operation_by_id(operation_id) - if not operation: + operation, spec = self._get_operation_by_id(operation_id) + if not operation or not spec: return False for security_spec in operation.security_specs: for key, value in security_spec.iteritems(): - if key not in self._spec.security_definitions: - # Should not happen. - continue - - security_definition = self._spec.security_definitions[key] + security_definition = spec.security_definitions[key] if security_definition.type == auth_type: return True diff --git a/w3af/plugins/crawl/open_api.py b/w3af/plugins/crawl/open_api.py index a73bcda7b3..f04f1d4ee3 100644 --- a/w3af/plugins/crawl/open_api.py +++ b/w3af/plugins/crawl/open_api.py @@ -237,12 +237,19 @@ def _report_to_kb_if_needed(self, http_response, parser): # Save a shallow copy of the specification handler to the kb. # It would be better to save the API spec # but looks like pickling doesn't work well with Bravado. - kb.kb.raw_write('open_api', 'specification_handler', - parser.get_specification_handler().shallow_copy()) + specification_handlers = kb.kb.raw_read('open_api', 'specification_handlers') + if not specification_handlers: + specification_handlers = [] + specification_handlers.append(parser.get_specification_handler().shallow_copy()) + kb.kb.raw_write('open_api', 'specification_handlers', specification_handlers) # Store the operation ids in the kb. - kb.kb.raw_write('open_api', 'request_to_operation_id', - parser.get_request_to_operation_id()) + request_to_operation_id = kb.kb.raw_read('open_api', 'request_to_operation_id') + if not request_to_operation_id: + request_to_operation_id = {} + for method_and_url, operation_id in parser.get_request_to_operation_id().iteritems(): + request_to_operation_id[method_and_url] = operation_id + kb.kb.raw_write('open_api', 'request_to_operation_id', request_to_operation_id) # Warn the user about missing credentials if self._query_string_auth or self._header_auth: diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index a1e5f9dad2..85c97e137b 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -112,11 +112,16 @@ def test_petstore_with_security(self): cfg = self._run_configs['cfg'] self._scan(cfg['target'], cfg['plugins']) - specification_handler = self.kb.raw_read('open_api', - 'specification_handler') - self.assertIsNotNone(specification_handler, "no specification handler") + specification_handlers = self.kb.raw_read('open_api', + 'specification_handlers') + self.assertIsNotNone(specification_handlers, 'no specification handlers (none)') + self.assertTrue(isinstance(specification_handlers, list), + 'not a list of SpecificationHandler') + self.assertEquals(len(specification_handlers), 1) + + specification_handler = specification_handlers[0] self.assertTrue(isinstance(specification_handler, SpecificationHandler), - "not a SpecificationHandler") + 'not a SpecificationHandler') infos = self.kb.get('open_api', 'open_api') self.assertEqual(len(infos), 1, infos) @@ -145,8 +150,8 @@ def init_plugin(self): api_http_response = generate_response(PetstoreWithSecurity.get_specification()) parser = OpenAPI(api_http_response) parser.parse() - self.kb.raw_write('open_api', 'specification_handler', - parser.get_specification_handler().shallow_copy()) + self.kb.raw_write('open_api', 'specification_handlers', + [parser.get_specification_handler().shallow_copy()]) self.kb.raw_write('open_api', 'request_to_operation_id', parser.get_request_to_operation_id()) @@ -154,7 +159,6 @@ def init_plugin(self): plugin = open_api_auth() self.assertTrue(plugin._is_api_spec_available()) - self.assertTrue(plugin._has_security_definitions_in_spec()) return plugin, spec From d1fef76027eb23c9101df86fb3eab54191c08509 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 13 Nov 2018 15:09:20 +0100 Subject: [PATCH 19/19] Added TestOpenAPIAuthMultipleSpecs --- .../tests/audit/data/users_with_security.yaml | 69 +++++++++++ .../plugins/tests/audit/test_open_api_auth.py | 117 ++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 w3af/plugins/tests/audit/data/users_with_security.yaml diff --git a/w3af/plugins/tests/audit/data/users_with_security.yaml b/w3af/plugins/tests/audit/data/users_with_security.yaml new file mode 100644 index 0000000000..5d897665f7 --- /dev/null +++ b/w3af/plugins/tests/audit/data/users_with_security.yaml @@ -0,0 +1,69 @@ +swagger: "2.0" +info: + version: 1.0.0 + title: Users + description: A sample API for a user directory +host: w3af.org +basePath: /api +schemes: + - http +consumes: + - application/json +produces: + - application/json +securityDefinitions: + ApiKeyAuth: + type: apiKey + in: header + name: X-API-Key + basicAuth: + type: basic +security: + - basicAuth: [] +paths: + /users: + get: + description: Returns all users + operationId: findUsers + parameters: + - name: limit + in: query + description: How many items to return at one time (max 100) + required: false + type: integer + format: int32 + security: + - ApiKeyAuth: [] + responses: + "200": + description: user + schema: + type: array + items: + $ref: '#/definitions/User' + default: + description: unexpected error + schema: + $ref: '#/definitions/Error' +definitions: + User: + type: object + required: + - name + properties: + name: + type: string + address: + type: string + + Error: + type: object + required: + - code + - message + properties: + code: + type: integer + format: int32 + message: + type: string diff --git a/w3af/plugins/tests/audit/test_open_api_auth.py b/w3af/plugins/tests/audit/test_open_api_auth.py index 85c97e137b..21a37ce741 100644 --- a/w3af/plugins/tests/audit/test_open_api_auth.py +++ b/w3af/plugins/tests/audit/test_open_api_auth.py @@ -54,6 +54,13 @@ def get_specification(): return file('%s/data/petstore_with_security.yaml' % CURRENT_PATH).read() +class UsersWithSecurity(object): + + @staticmethod + def get_specification(): + return file('%s/data/users_with_security.yaml' % CURRENT_PATH).read() + + class TestOpenAPIAuthWithPetstore(PluginTest): target_url = 'http://w3af.org/' @@ -144,6 +151,116 @@ def test_petstore_with_security(self): self.assertEquals(200, vuln['response_code']) +class TestOpenAPIAuthMultipleSpecs(PluginTest): + + target_url = 'http://w3af.org/' + api_key_auth = ('header_auth', 'X-API-Key: xxx', PluginConfig.HEADER) + + _run_configs = { + 'cfg': { + 'target': target_url, + 'plugins': { + 'crawl': (PluginConfig('open_api', api_key_auth,),), + 'audit': (PluginConfig('open_api_auth'),) + } + } + } + + class AuthMockResponse(MockResponse): + + def get_response(self, http_request, uri, response_headers): + if http_request.headers.get('X-API-Key', '') != 'xxx': + return 401, response_headers, '' + + return self.status, response_headers, '{ "id": 42 }' + + MOCK_RESPONSES = [ + + # No auth required for the API specs. + MockResponse('http://w3af.org/api/v1/openapi.yaml', + PetstoreWithSecurity().get_specification(), + content_type='application/yaml'), + + MockResponse('http://w3af.org/api/v2/openapi.yaml', + UsersWithSecurity().get_specification(), + content_type='application/yaml'), + + # No auth required for the health check endpoint. + MockResponse('http://w3af.org/api/ping', + 'ich bin gut', + content_type='text/plain'), + + # Authenticate GET requests to /api/pets + AuthMockResponse(re.compile('http://w3af.org/api/pets$'), + '{ "id": 42 }', + content_type='application/json', + method='GET'), + + # No auth for POST requests to /api/pets + # This should result to a vulnerability. + MockResponse(re.compile('http://w3af.org/api/pets$'), + '{ "id": 42 }', + content_type='application/json', + method='POST'), + + # Authenticate requests to /api/pets/{id} + AuthMockResponse(re.compile('http://w3af.org/api/pets.*'), + '{ "id": 42 }', + content_type='application/json'), + + # No auth requests to /api/users. + # This should result to a vulnerability. + MockResponse('http://w3af.org/api/users', + '{ "id": 123 }', + content_type='application/json') + ] + + def test_multiple_specs(self): + cfg = self._run_configs['cfg'] + self._scan(cfg['target'], cfg['plugins']) + + specification_handlers = self.kb.raw_read('open_api', + 'specification_handlers') + self.assertIsNotNone(specification_handlers, 'no specification handlers (none)') + self.assertTrue(isinstance(specification_handlers, list), + 'not a list of SpecificationHandler') + self.assertEquals(len(specification_handlers), 2) + + specification_handler = specification_handlers[0] + self.assertTrue(isinstance(specification_handler, SpecificationHandler), + 'not a SpecificationHandler') + + infos = self.kb.get('open_api', 'open_api') + self.assertEqual(len(infos), 2, infos) + info_i = infos[0] + self.assertEqual(info_i.get_name(), 'Open API specification found') + info_i = infos[1] + self.assertEqual(info_i.get_name(), 'Open API specification found') + + frs = self.kb.get_all_known_fuzzable_requests() + frs = [f for f in frs if f.get_url().get_path() not in ('/api/v1/openapi.yaml', '/api/v2/openapi.yaml', '/')] + frs.sort(by_path) + self.assertEqual(len(frs), 8) + + vulns = self.kb.get('open_api_auth', 'open_api_auth') + self.assertEquals(len(vulns), 2) + vulns.sort(by_path) + + vuln = vulns[0] + self.assertEquals('Broken authentication', vuln.get_name()) + self.assertEquals('High', vuln.get_severity()) + self.assertEquals('http://w3af.org/api/pets', vuln.get_url().url_string) + self.assertEquals('POST', vuln['method']) + self.assertEquals(200, vuln['response_code']) + + vuln = vulns[1] + self.assertEquals('Broken authentication', vuln.get_name()) + self.assertEquals('High', vuln.get_severity()) + self.assertEquals('http://w3af.org/api/users', vuln.get_url().url_string) + self.assertEquals('GET', vuln['method']) + self.assertEquals(200, vuln['response_code']) + + class TestOpenAPIAuth(PluginTest): def init_plugin(self):