commit 4c799735b5b86beea486ee3b1cd0ea8a4f5cb4b5 Author: Isis Lovecruft <isis@torproject.org> Date: Wed Dec 20 22:36:06 2017 +0000 Implement moat error handling when no bridges are available. * FIXES #24637: https://bugs.torproject.org/24637 --- bridgedb/distributors/moat/server.py | 73 +++++++++++++++++++++----- bridgedb/test/test_distributors_moat_server.py | 55 +++++++++++++++++-- 2 files changed, 112 insertions(+), 16 deletions(-) diff --git a/bridgedb/distributors/moat/server.py b/bridgedb/distributors/moat/server.py index eb356fe..a669ee5 100644 --- a/bridgedb/distributors/moat/server.py +++ b/bridgedb/distributors/moat/server.py @@ -209,6 +209,7 @@ class JsonAPIErrorResource(JsonAPIResource): resource403 = JsonAPIErrorResource(code=403, status="Forbidden") +resource404 = JsonAPIErrorResource(code=404, status="Not Found") resource406 = JsonAPIErrorResource(code=406, status="Not Acceptable") resource415 = JsonAPIErrorResource(code=415, status="Unsupported Media Type") resource419 = JsonAPIErrorResource(code=419, status="No You're A Teapot") @@ -497,21 +498,21 @@ class CaptchaCheckResource(CaptchaResource): self.nBridgesToGive = N self.useForwardedHeader = useForwardedHeader - def getBridgeLines(self, ip, data): - """Get bridge lines for a client's HTTP request. + def createBridgeRequest(self, ip, data): + """Create an appropriate :class:`MoatBridgeRequest` from the ``data`` + of a client's request. :param str ip: The client's IP address. :param dict data: The decoded JSON API data from the client's request. - :rtype: list or None - :returns: A list of bridge lines. + :rtype: :class:`MoatBridgeRequest` + :returns: An object which specifies the filters for retreiving + the type of bridges that the client requested. """ - bridgeLines = None - interval = self.schedule.intervalStart(time.time()) + logging.debug("Creating moat bridge request object for %s." % ip) - logging.debug("Replying to JSON API request from %s." % ip) + bridgeRequest = MoatBridgeRequest() if ip and data: - bridgeRequest = MoatBridgeRequest() bridgeRequest.client = IPAddress(ip) bridgeRequest.isValid(True) bridgeRequest.withIPversion() @@ -519,6 +520,23 @@ class CaptchaCheckResource(CaptchaResource): bridgeRequest.withoutBlockInCountry(data) bridgeRequest.generateFilters() + return bridgeRequest + + def getBridgeLines(self, bridgeRequest): + """Get bridge lines for a client's HTTP request. + + :type bridgeRequest: :class:`MoatBridgeRequest` + :param bridgeRequest: A valid bridge request object with pre-generated + filters (as returned by :meth:`createBridgeRequest`). + :rtype: list + :returns: A list of bridge lines. + """ + bridgeLines = list() + interval = self.schedule.intervalStart(time.time()) + + logging.debug("Replying to JSON API request from %s." % bridgeRequest.client) + + if bridgeRequest.isValid(): bridges = self.distributor.getBridges(bridgeRequest, interval) bridgeLines = [replaceControlChars(bridge.getBridgeLine(bridgeRequest)) for bridge in bridges] @@ -599,17 +617,34 @@ class CaptchaCheckResource(CaptchaResource): return valid - def failureResponse(self, id, request): - """Respond with status code "419 No You're A Teapot".""" - error_response = resource419 - error_response.type = 'moat-bridges' + def failureResponse(self, id, request, bridgeRequest=None): + """Respond with status code "419 No You're A Teapot" if the captcha + verification failed, or status code "404 Not Found" if there + were none of the type of bridges requested. + :param int id: The JSON API "id" field of the + ``JsonAPIErrorResource`` which should be returned. + :type request: :api:`twisted.web.http.Request` + :param request: The current request we're handling. + :type bridgeRequest: :class:`MoatBridgeRequest` + :param bridgeRequest: A valid bridge request object with pre-generated + filters (as returned by :meth:`createBridgeRequest`). + """ if id == 4: + error_response = resource419 error_response.id = 4 error_response.detail = "The CAPTCHA solution was incorrect." elif id == 5: + error_response = resource419 error_response.id = 5 error_response.detail = "The CAPTCHA challenge timed out." + elif id == 6: + error_response = resource404 + error_response.id = 6 + error_response.detail = ("No bridges available to fulfill " + "request: %s.") % bridgeRequest + + error_response.type = 'moat-bridges' return error_response.render(request) @@ -665,7 +700,19 @@ class CaptchaCheckResource(CaptchaResource): if valid: qrcode = None - bridgeLines = self.getBridgeLines(clientIP, client_data) + bridgeRequest = self.createBridgeRequest(clientIP, client_data) + bridgeLines = self.getBridgeLines(bridgeRequest) + + # If we can only return less than the configured + # MOAT_BRIDGES_PER_ANSWER then log a warning. + if len(bridgeLines) < self.nBridgesToGive: + logging.warn(("Not enough bridges of the type specified to " + "fulfill the following request: %s") % bridgeRequest) + + # If we have no bridges at all to give to the client, then + # return a JSON API 404 error. + if not bridgeLines: + return self.failureResponse(6, request) if include_qrcode: qrjpeg = generateQR(bridgeLines) diff --git a/bridgedb/test/test_distributors_moat_server.py b/bridgedb/test/test_distributors_moat_server.py index 95796d1..2f84557 100644 --- a/bridgedb/test/test_distributors_moat_server.py +++ b/bridgedb/test/test_distributors_moat_server.py @@ -581,6 +581,14 @@ class CaptchaFetchResourceTests(unittest.TestCase): self.assert_data_is_ok(decoded) +class MockCaptchaCheckResource(server.CaptchaCheckResource): + """A mocked :class:`server.CaptchaCheckResource` whose + ``getBridgeLines`` method returns no bridges. + """ + def getBridgeLines(self, bridgeRequest): + return list() + + class CaptchaCheckResourceTests(unittest.TestCase): """Tests for :class:`bridgedb.distributors.moat.server.CaptchaCheckResource`.""" @@ -611,6 +619,14 @@ class CaptchaCheckResourceTests(unittest.TestCase): "iz_PdOD2GIGPeclwiHAWM1pOS4cQVsTQR_z4ojZbpLiSp35n4Qbb11YOoreovZzlbS" "7W38rAsTirkdeugcNq82AxKP3phEkyRcw--CzV") + def mock_getBridgeLines(self): + self.resource = MockCaptchaCheckResource(self.distributor, + self.schedule, 3, + self.hmacKey, + self.publicKey, + self.secretKey, + useForwardedHeader=False) + def create_POST_with_data(self, data): request = DummyRequest([self.pagename]) request.requestHeaders.addRawHeader('Content-Type', 'application/vnd.api+json') @@ -826,13 +842,24 @@ class CaptchaCheckResourceTests(unittest.TestCase): self.assertEqual(error['type'], "moat-bridges") self.assertEqual(error['id'], 4) + def test_createBridgeRequest(self): + request = self.create_valid_POST_with_challenge(self.expiredChallenge) + request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443) + encoded_content = request.content.read() + content = json.loads(encoded_content)['data'][0] + + bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content) + + self.assertTrue(bridgeRequest.isValid()) + def test_getBridgeLines(self): request = self.create_valid_POST_with_challenge(self.expiredChallenge) request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443) encoded_content = request.content.read() content = json.loads(encoded_content)['data'][0] - bridgelines = self.resource.getBridgeLines('3.3.3.3', content) + bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content) + bridgelines = self.resource.getBridgeLines(bridgeRequest) self.assertTrue(bridgelines) @@ -840,9 +867,31 @@ class CaptchaCheckResourceTests(unittest.TestCase): request = self.create_valid_POST_with_challenge(self.expiredChallenge) request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443) - bridgelines = self.resource.getBridgeLines('3.3.3.3', None) + bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', None) + bridgelines = self.resource.getBridgeLines(bridgeRequest) + + self.assertFalse(bridgeRequest.isValid()) + self.assertEqual(len(bridgelines), 0) + + def test_failureResponse_no_bridges(self): + request = self.create_valid_POST_with_challenge(self.expiredChallenge) + request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443) + encoded_content = request.content.read() + content = json.loads(encoded_content)['data'][0] + + bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content) + + response = self.resource.failureResponse(6, request, bridgeRequest) + + self.assertIn("No bridges available", response) + + def test_render_POST_no_bridges(self): + self.mock_getBridgeLines() + + request = self.create_valid_POST_make_new_challenge() + response = self.resource.render(request) - self.assertIsNone(bridgelines) + self.assertIn("No bridges available", response) def test_render_POST_unexpired(self): request = self.create_valid_POST_make_new_challenge()