Merge a529ba7 into bc8889f

docs/proposed/http-storage-node-protocol.rst

@@ -369,6 +369,19 @@ The authentication *type* used is ``Tahoe-LAFS``.
 The swissnum from the NURL used to locate the storage service is used as the *credentials*.
 If credentials are not presented or the swissnum is not associated with a storage service then no storage processing is performed and the request receives an ``401 UNAUTHORIZED`` response.
 
+There are also, for some endpoints, secrets sent via ``X-Tahoe-Authorization`` headers.
+If these are:
+
+1. Missing.
+2. The wrong length.
+3. Not the expected kind of secret.
+4. They are otherwise unparseable before they are actually semantically used.
+
+the server will respond with ``400 BAD REQUEST``.
+401 is not used because this isn't an authorization problem, this is a "you sent garbage and should know better" bug.
+
+If authorization using the secret fails, then a ``401 UNAUTHORIZED`` response should be sent.
+
 General
 ~~~~~~~
 

src/allmydata/storage/http_client.py

@@ -19,7 +19,7 @@
     from typing import Union
     from treq.testing import StubTreq
 
-import base64
+from base64 import b64encode
 
 # TODO Make sure to import Python version?
 from cbor2 import loads
@@ -44,7 +44,7 @@ def _decode_cbor(response):
 
 def swissnum_auth_header(swissnum):  # type: (bytes) -> bytes
     """Return value for ``Authentication`` header."""
-    return b"Tahoe-LAFS " + base64.b64encode(swissnum).strip()
+    return b"Tahoe-LAFS " + b64encode(swissnum).strip()
 
 
 class StorageClient(object):
@@ -68,12 +68,25 @@ def _get_headers(self):  # type: () -> Headers
         )
         return headers
 
+    def _request(self, method, url, secrets, **kwargs):
+        """
+        Like ``treq.request()``, but additional argument of secrets mapping
+        ``http_server.Secret`` to the bytes value of the secret.
+        """
+        headers = self._get_headers()
+        for key, value in secrets.items():
+            headers.addRawHeader(
+                "X-Tahoe-Authorization",
+                b"%s %s" % (key.value.encode("ascii"), b64encode(value).strip())
+            )
+        return self._treq.request(method, url, headers=headers, **kwargs)
+
     @inlineCallbacks
     def get_version(self):
         """
         Return the version metadata for the server.
         """
         url = self._base_url.click("/v1/version")
-        response = yield self._treq.get(url, headers=self._get_headers())
+        response = yield self._request("GET", url, {})
         decoded_response = yield _decode_cbor(response)
         returnValue(decoded_response)

src/allmydata/storage/http_server.py

@@ -13,8 +13,12 @@
     # fmt: off
     from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min  # noqa: F401
     # fmt: on
+else:
+    from typing import Dict, List, Set
 
 from functools import wraps
+from enum import Enum
+from base64 import b64decode
 
 from klein import Klein
 from twisted.web import http
@@ -26,38 +30,83 @@
 from .http_client import swissnum_auth_header
 
 
-def _authorization_decorator(f):
+class Secrets(Enum):
+    """Different kinds of secrets the client may send."""
+    LEASE_RENEW = "lease-renew-secret"
+    LEASE_CANCEL = "lease-cancel-secret"
+    UPLOAD = "upload-secret"
+
+
+class ClientSecretsException(Exception):
+    """The client did not send the appropriate secrets."""
+
+
+def _extract_secrets(header_values, required_secrets):  # type: (List[str], Set[Secrets]) -> Dict[Secrets, bytes]
+    """
+    Given list of values of ``X-Tahoe-Authorization`` headers, and required
+    secrets, return dictionary mapping secrets to decoded values.
+
+    If too few secrets were given, or too many, a ``ClientSecretsException`` is
+    raised.
+    """
+    string_key_to_enum = {e.value: e for e in Secrets}
+    result = {}
+    try:
+        for header_value in header_values:
+            string_key, string_value = header_value.strip().split(" ", 1)
+            key = string_key_to_enum[string_key]
+            value = b64decode(string_value)
+            if key in (Secrets.LEASE_CANCEL, Secrets.LEASE_RENEW) and len(value) != 32:
+                raise ClientSecretsException("Lease secrets must be 32 bytes long")
+            result[key] = value
+    except (ValueError, KeyError):
+        raise ClientSecretsException("Bad header value(s): {}".format(header_values))
+    if result.keys() != required_secrets:
+        raise ClientSecretsException(
+            "Expected {} secrets, got {}".format(required_secrets, result.keys())
+        )
+    return result
+
+
+def _authorization_decorator(required_secrets):
     """
     Check the ``Authorization`` header, and (TODO: in later revision of code)
     extract ``X-Tahoe-Authorization`` headers and pass them in.
     """
+    def decorator(f):
+        @wraps(f)
+        def route(self, request, *args, **kwargs):
+            if request.requestHeaders.getRawHeaders("Authorization", [None])[0] != str(
+                swissnum_auth_header(self._swissnum), "ascii"
+            ):
+                request.setResponseCode(http.UNAUTHORIZED)
+                return b""
+            authorization = request.requestHeaders.getRawHeaders("X-Tahoe-Authorization", [])
+            try:
+                secrets = _extract_secrets(authorization, required_secrets)
+            except ClientSecretsException:
+                request.setResponseCode(400)
+                return b""
+            return f(self, request, secrets, *args, **kwargs)
+
+        return route
 
-    @wraps(f)
-    def route(self, request, *args, **kwargs):
-        if request.requestHeaders.getRawHeaders("Authorization", [None])[0] != str(
-            swissnum_auth_header(self._swissnum), "ascii"
-        ):
-            request.setResponseCode(http.UNAUTHORIZED)
-            return b""
-        # authorization = request.requestHeaders.getRawHeaders("X-Tahoe-Authorization", [])
-        # For now, just a placeholder:
-        authorization = None
-        return f(self, request, authorization, *args, **kwargs)
-
-    return route
+    return decorator
 
 
-def _authorized_route(app, *route_args, **route_kwargs):
+def _authorized_route(app, required_secrets, *route_args, **route_kwargs):
     """
     Like Klein's @route, but with additional support for checking the
     ``Authorization`` header as well as ``X-Tahoe-Authorization`` headers.  The
-    latter will (TODO: in later revision of code) get passed in as second
-    argument to wrapped functions.
+    latter will get passed in as second argument to wrapped functions, a
+    dictionary mapping a ``Secret`` value to the uploaded secret.
+
+    :param required_secrets: Set of required ``Secret`` types.
     """
 
     def decorator(f):
         @app.route(*route_args, **route_kwargs)
-        @_authorization_decorator
+        @_authorization_decorator(required_secrets)
         def handle_route(*args, **kwargs):
             return f(*args, **kwargs)
 
@@ -89,6 +138,10 @@ def _cbor(self, request, data):
         # TODO if data is big, maybe want to use a temporary file eventually...
         return dumps(data)
 
-    @_authorized_route(_app, "/v1/version", methods=["GET"])
+
+    ##### Generic APIs #####
+
+    @_authorized_route(_app, set(), "/v1/version", methods=["GET"])
     def version(self, request, authorization):
+        """Return version information."""
         return self._cbor(request, self._storage_server.get_version())

src/allmydata/test/test_storage_http.py

@@ -15,34 +15,178 @@
     # fmt: on
 
 from unittest import SkipTest
+from base64 import b64encode
 
 from twisted.trial.unittest import TestCase
 from twisted.internet.defer import inlineCallbacks
 
 from treq.testing import StubTreq
+from klein import Klein
 from hyperlink import DecodedURL
 
 from ..storage.server import StorageServer
-from ..storage.http_server import HTTPServer
+from ..storage.http_server import (
+    HTTPServer, _extract_secrets, Secrets, ClientSecretsException,
+    _authorized_route,
+)
 from ..storage.http_client import StorageClient, ClientException
 
 
-class HTTPTests(TestCase):
+class ExtractSecretsTests(TestCase):
     """
-    Tests of HTTP client talking to the HTTP server.
+    Tests for ``_extract_secrets``.
     """
+    def setUp(self):
+        if PY2:
+            raise SkipTest("Not going to bother supporting Python 2")
+
+    def test_extract_secrets(self):
+        """
+        ``_extract_secrets()`` returns a dictionary with the extracted secrets
+        if the input secrets match the required secrets.
+        """
+        secret1 = b"\xFF" * 32
+        secret2 = b"\x34" * 32
+        lease_secret = "lease-renew-secret " + str(b64encode(secret1), "ascii").strip()
+        upload_secret = "upload-secret " + str(b64encode(secret2), "ascii").strip()
+
+        # No secrets needed, none given:
+        self.assertEqual(_extract_secrets([], set()), {})
+
+        # One secret:
+        self.assertEqual(
+            _extract_secrets([lease_secret],
+                             {Secrets.LEASE_RENEW}),
+            {Secrets.LEASE_RENEW: secret1}
+        )
+
+        # Two secrets:
+        self.assertEqual(
+            _extract_secrets([upload_secret, lease_secret],
+                             {Secrets.LEASE_RENEW, Secrets.UPLOAD}),
+            {Secrets.LEASE_RENEW: secret1, Secrets.UPLOAD: secret2}
+        )
+
+    def test_wrong_number_of_secrets(self):
+        """
+        If the wrong number of secrets are passed to ``_extract_secrets``, a
+        ``ClientSecretsException`` is raised.
+        """
+        secret1 = b"\xFF\x11ZEBRa"
+        lease_secret = "lease-renew-secret " + str(b64encode(secret1), "ascii").strip()
+
+        # Missing secret:
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets([], {Secrets.LEASE_RENEW})
+
+        # Wrong secret:
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets([lease_secret], {Secrets.UPLOAD})
+
+        # Extra secret:
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets([lease_secret], {})
+
+    def test_bad_secrets(self):
+        """
+        Bad inputs to ``_extract_secrets`` result in
+        ``ClientSecretsException``.
+        """
+
+        # Missing value.
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets(["lease-renew-secret"], {Secrets.LEASE_RENEW})
+
+        # Garbage prefix
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets(["FOO eA=="], {})
+
+        # Not base64.
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets(["lease-renew-secret x"], {Secrets.LEASE_RENEW})
+
+        # Wrong length lease secrets (must be 32 bytes long).
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets(["lease-renew-secret eA=="], {Secrets.LEASE_RENEW})
+        with self.assertRaises(ClientSecretsException):
+            _extract_secrets(["lease-upload-secret eA=="], {Secrets.LEASE_RENEW})
+
+
+SWISSNUM_FOR_TEST = b"abcd"
 
+
+class TestApp(object):
+    """HTTP API for testing purposes."""
+
+    _app = Klein()
+    _swissnum = SWISSNUM_FOR_TEST  # Match what the test client is using
+
+    @_authorized_route(_app, {Secrets.UPLOAD}, "/upload_secret", methods=["GET"])
+    def validate_upload_secret(self, request, authorization):
+        if authorization == {Secrets.UPLOAD: b"MAGIC"}:
+            return "GOOD SECRET"
+        else:
+            return "BAD: {}".format(authorization)
+
+
+class RoutingTests(TestCase):
+    """
+    Tests for the HTTP routing infrastructure.
+    """
     def setUp(self):
         if PY2:
             raise SkipTest("Not going to bother supporting Python 2")
-        self.storage_server = StorageServer(self.mktemp(), b"\x00" * 20)
-        # TODO what should the swissnum _actually_ be?
-        self._http_server = HTTPServer(self.storage_server, b"abcd")
+        self._http_server = TestApp()
         self.client = StorageClient(
-            DecodedURL.from_text("http://127.0.0.1"),
-            b"abcd",
-            treq=StubTreq(self._http_server.get_resource()),
+        DecodedURL.from_text("http://127.0.0.1"),
+        SWISSNUM_FOR_TEST,
+        treq=StubTreq(self._http_server._app.resource()),
+    )
+
+    @inlineCallbacks
+    def test_authorization_enforcement(self):
+        """
+        The requirement for secrets is enforced; if they are not given, a 400
+        response code is returned.
+        """
+        # Without secret, get a 400 error.
+        response = yield self.client._request(
+            "GET", "http://127.0.0.1/upload_secret", {}
         )
+        self.assertEqual(response.code, 400)
+
+        # With secret, we're good.
+        response = yield self.client._request(
+            "GET", "http://127.0.0.1/upload_secret", {Secrets.UPLOAD: b"MAGIC"}
+        )
+        self.assertEqual(response.code, 200)
+        self.assertEqual((yield response.content()), b"GOOD SECRET")
+
+
+def setup_http_test(self):
+    """
+    Setup HTTP tests; call from ``setUp``.
+    """
+    if PY2:
+        raise SkipTest("Not going to bother supporting Python 2")
+    self.storage_server = StorageServer(self.mktemp(), b"\x00" * 20)
+    # TODO what should the swissnum _actually_ be?
+    self._http_server = HTTPServer(self.storage_server, SWISSNUM_FOR_TEST)
+    self.client = StorageClient(
+        DecodedURL.from_text("http://127.0.0.1"),
+        SWISSNUM_FOR_TEST,
+        treq=StubTreq(self._http_server.get_resource()),
+    )
+
+
+class GenericHTTPAPITests(TestCase):
+    """
+    Tests of HTTP client talking to the HTTP server, for generic HTTP API
+    endpoints and concerns.
+    """
+
+    def setUp(self):
+        setup_http_test(self)
 
     @inlineCallbacks
     def test_bad_authentication(self):