refactor permissions

Author: meejah

src/wormhole_mailbox_server/permission.py

@@ -0,0 +1,126 @@
+import os
+import base64
+import hashlib
+from zope.interface import (
+    Interface,
+    Attribute,
+    implementer,
+)
+
+
+class IPermission(Interface):
+    """
+    A server-side method of granting permission to a client.
+    """
+    name = Attribute("name")
+
+    def get_welcome_data():
+        """
+        return a dict of information to include under the name of this
+        Permission granter (under "permission-required" in the Welcome)
+        """
+
+    def verify_permission(submit_permission):
+        """
+        return a bool indicating if the submit_permission data is a valid
+        permission (or not)
+        """
+
+
+def create_permission_provider(kind):
+    """
+    returns a permissions-provider
+    """
+    if kind == "none":
+        return NoPermission
+    elif kind == "hashcash":
+        return HashcashPermission
+    raise ValueError(
+        "Unknown permission provider '{}'".format(kind)
+    )
+
+
+@implementer(IPermission)
+class NoPermission(object):
+    """
+    A no-op permission provider used to grant any client access (the
+    default).
+    """
+    name = "none"
+
+    def get_welcome_data(self):
+        return {}
+
+    def verify_permission(self, submit_permission):
+        return True
+
+
+@implementer(IPermission)
+class HashcashPermission(object):
+    """
+    A permission provider that generates a random 'resource' string
+    and checks a proof-of-work from the client.
+    """
+    name = "hashcash"
+
+    def __init__(self, bits=20):
+        self._bits = bits
+
+    def get_welcome_data(self):
+        """
+        Generate the data to include under this method's key in the
+        `permission-required` value of the welcome message.
+
+        Should be called at most once per connection.
+        """
+        self._hashcash_resource = base64.b64encode(os.urandom(8)).decode("utf8")
+        return {
+            "bits": self._bits,
+            "resource": self._hashcash_resource,
+        }
+
+    def verify_permission(self, perms):
+        """
+        :returns bool: an indication of whether the provided permissions
+            reply from a client is valid
+        """
+        # XXX THINK do we need this whole method to be constant-time?
+        # (basically impossible if it's not even syntactially valid?)
+        stamp = perms.get("stamp", "")
+        fields = stamp.split(":")
+        if len(fields) != 7:
+            return False
+        vers, claimed_bits, date, resource, ext, rand, counter = fields
+        vers = int(vers)
+        if vers != 1:
+            return False
+        if resource != self._hashcash_resource:
+            return False
+
+        claimed_bits = int(claimed_bits)
+        if claimed_bits < self._bits:
+            return False
+
+        h = hashlib.sha1()
+        h.update(stamp.encode("utf8"))
+        measured_hash = h.digest()
+        if leading_zero_bits(measured_hash) < claimed_bits:
+            return False
+        return True
+
+
+def leading_zero_bits(bytestring):
+    """
+    :returns int: the number of leading zeros in the given byte-string
+    """
+    measured_bits = 0
+    for byte in bytestring:
+        bit = 1 << 7
+        while bit:
+            if byte & bit:
+                return measured_bits
+            else:
+                measured_bits += 1
+            bit = bit >> 1
+
+

src/wormhole_mailbox_server/server.py

@@ -4,6 +4,7 @@
 from collections import namedtuple
 from twisted.python import log
 from twisted.application import service
+from .permission import create_permission_provider
 
 def generate_mailbox_id():
     return base64.b32encode(os.urandom(8)).lower().strip(b"=").decode("ascii")
@@ -552,102 +553,9 @@ def _shutdown(self):
             channel._shutdown()
 
 
-def leading_zero_bits(bytestring):
-    """
-    :returns int: the number of leading zeros in the given byte-string
-    """
-    measured_bits = 0
-    for byte in bytestring:
-        bit = 1 << 7
-        while bit:
-            if byte & bit:
-                return measured_bits
-            else:
-                measured_bits += 1
-            bit = bit >> 1
-
-
-class NoPermission(object):
-    """
-    A no-op permission provider used to grant any client access (the
-    default).
-    """
-    name = "none"
-
-    def get_welcome_data(self):
-        return {}
-
-    def verify_permission(self, submit_permission):
-        return True
-
-    def is_passed(self):
-        return True
-
-
-class HashcashPermission(object):
-    """
-    A permission provider that generates a random 'resource' string
-    and checks a proof-of-work from the client.
-    """
-    name = "hashcash"
-
-    def __init__(self, bits=20):
-        self._bits = bits
-        self._passed = False
-
-    def get_welcome_data(self):
-        """
-        Generate the data to include under this method's key in the
-        `permission-required` value of the welcome message.
-
-        Should be called at most once per connection.
-        """
-        self._hashcash_resource = base64.b64encode(os.urandom(8)).decode("utf8")
-        return {
-            "bits": self._bits,
-            "resource": self._hashcash_resource,
-        }
-
-    def is_passed(self):
-        """
-        :returns bool: True if verify_permission has been called successfully
-        """
-        return self._passed
-
-    def verify_permission(self, perms):
-        """
-        :returns bool: an indication of whether the provided permissions
-            reply from a client is valid
-        """
-        # XXX THINK do we need this whole method to be constant-time?
-        # (basically impossible if it's not even syntactially valid?)
-        stamp = perms.get("stamp", "")
-        fields = stamp.split(":")
-        if len(fields) != 7:
-            return False
-        vers, claimed_bits, date, resource, ext, rand, counter = fields
-        vers = int(vers)
-        if vers != 1:
-            return False
-        if resource != self._hashcash_resource:
-            return False
-
-        claimed_bits = int(claimed_bits)
-        if claimed_bits < self._bits:
-            return False
-
-        h = hashlib.sha1()
-        h.update(stamp.encode("utf8"))
-        measured_hash = h.digest()
-        if leading_zero_bits(measured_hash) < claimed_bits:
-            return False
-        self._passed = True
-        return True
-
-
 class Server(service.MultiService):
     def __init__(self, db, allow_list, welcome,
-                 blur_usage, usage_db=None, log_file=None, permissions="none"):
+                 blur_usage, usage_db=None, log_file=None, permission_provider=None):
         service.MultiService.__init__(self)
         self._db = db
         self._allow_list = allow_list
@@ -656,8 +564,8 @@ def __init__(self, db, allow_list, welcome,
         self._log_requests = blur_usage is None
         self._usage_db = usage_db
         self._log_file = log_file
-        self._permissions = permissions
-        assert self._permissions in ("none", "hashcash")
+        self._permission_provider = permission_provider
+        # XXX assert interface instead assert self._permissions in ("none", "hashcash")
         self._apps = {}
 
     def get_welcome(self):
@@ -678,14 +586,7 @@ def get_permission_method(self):
 
         :returns IPermissionGranter: a method of permission
         """
-        if self._permissions == "none":
-            return NoPermission()
-        elif self._permissions == "hashcash":
-            return HashcashPermission()
-        else:
-            raise ValueError(
-                'Unknown permission "{}"'.format(self._permissions)
-            )
+        return self._permission_provider()
 
     def get_log_requests(self):
         return self._log_requests
@@ -801,7 +702,7 @@ def make_server(db, allow_list=True,
                 advertise_version=None,
                 signal_error=None,
                 blur_usage=None,
-                permissions="none",
+                permission_provider=None,
                 usage_db=None,
                 log_file=None,
                 welcome_motd=None,
@@ -827,6 +728,9 @@ def make_server(db, allow_list=True,
     if signal_error:
         welcome["error"] = signal_error
 
+    if permission_provider is None:
+        permission_provider = create_permission_provider("none")
+
     return Server(db, allow_list=allow_list, welcome=welcome,
                   blur_usage=blur_usage, usage_db=usage_db, log_file=log_file,
-                  permissions=permissions)
+                  permission_provider=permission_provider)

src/wormhole_mailbox_server/server_tap.py

@@ -9,6 +9,7 @@
 from .server import make_server
 from .web import make_web_server
 from .database import create_or_upgrade_channel_db, create_or_upgrade_usage_db
+from .permission import create_permission_provider
 
 LONGDESC = """This plugin sets up a 'Mailbox' server for magic-wormhole.
 This service forwards short messages between clients, to perform key exchange
@@ -92,16 +93,18 @@ def makeService(config, channel_db="relay.sqlite", reactor=reactor):
     log_file = (os.fdopen(int(config["log-fd"]), "w")
                 if config["log-fd"] is not None
                 else None)
-    server = make_server(channel_db,
-                         allow_list=config["allow-list"],
-                         advertise_version=config["advertise-version"],
-                         signal_error=config["signal-error"],
-                         blur_usage=config["blur-usage"],
-                         permissions=config["permissions"],
-                         usage_db=usage_db,
-                         log_file=log_file,
-                         welcome_motd=config["motd"],
-                         )
+
+    server = make_server(
+        channel_db,
+        allow_list=config["allow-list"],
+        advertise_version=config["advertise-version"],
+        signal_error=config["signal-error"],
+        blur_usage=config["blur-usage"],
+        permission_provider=create_permission_provider(config.get("permissions", "none")),
+        usage_db=usage_db,
+        log_file=log_file,
+        welcome_motd=config["motd"],
+    )
     server.setServiceParent(parent)
     rebooted = time.time()
     def expire():

src/wormhole_mailbox_server/server_websocket.py

@@ -3,7 +3,8 @@
 from twisted.internet import reactor
 from twisted.python import log
 from autobahn.twisted import websocket
-from .server import CrowdedError, ReclaimedError, SidedMessage, NoPermission
+from .server import CrowdedError, ReclaimedError, SidedMessage
+from .permission import NoPermission
 from .util import dict_to_bytes, bytes_to_dict
 
 # The WebSocket allows the client to send "commands" to the server, and the
@@ -110,6 +111,7 @@ def __init__(self):
         self._mailbox_id = None
         self._did_close = False
         self._permission = None
+        self._permission_passed = False
 
     def onConnect(self, request):
         rv = self.factory.server
@@ -187,7 +189,7 @@ def handle_ping(self, msg):
 
     def handle_bind(self, msg, server_rx):
         # if demanding permission, but no permission yet .. error
-        if self._permission is not None and not self._permission.is_passed():
+        if not isinstance(self._permission, NoPermission) and not self._permission_passed:
             raise Error("must submit-permission first")
 
         if self._app or self._side:
@@ -205,7 +207,8 @@ def handle_bind(self, msg, server_rx):
     def handle_submit_permissions(self, msg, server_rx):
         if msg.get("method", None) != self._permission.name:
             raise Error("need permission method '{}'".format(self._permission.name))
-        if not self._permission.verify_permission(msg):
+        self._permission_passed = self._permission.verify_permission(msg)
+        if not self._permission_passed:
             raise Error("submit-permission failed")
 
     def handle_list(self):