cleanup, more tests

src/allmydata/grid_manager.py

@@ -70,9 +70,12 @@ def load_grid_manager(config_path, config_location):
     with config_file:
         config = json.load(config_file)
 
-    if not config:
+    gm_version = config.get(u'grid_manager_config_version', None)
+    if gm_version != 0:
         raise ValueError(
-            "Invalid Grid Manager config in '{}'".format(config_location)
+            "Missing or unknown version '{}' of Grid Manager config".format(
+                gm_version
+            )
         )
     if 'private_key' not in config:
         raise ValueError(
@@ -101,13 +104,6 @@ def load_grid_manager(config_path, config_location):
             None,
         )
 
-    gm_version = config.get(u'grid_manager_config_version', None)
-    if gm_version != 0:
-        raise ValueError(
-            "Missing or unknown version '{}' of Grid Manager config".format(
-                gm_version
-            )
-        )
     return _GridManager(private_key_bytes, storage_servers)
 
 
@@ -153,6 +149,7 @@ class _GridManager(object):
         vk = ed25519.verifying_key_from_signing_key(self._private_key)
         ed25519.verify_signature(vk, sig, cert_data)
 
+        srv.add_certificate(certificate)
         return certificate
 
     def add_storage_server(self, name, public_key):
@@ -215,3 +212,137 @@ def save_grid_manager(file_path, grid_manager):
         fileutil.make_dirs(file_path.path, mode=0o700)
         with file_path.child("config.json").open("w") as f:
             f.write("{}\n".format(data))
+
+
+def parse_grid_manager_certificate(gm_data):
+    """
+    :param gm_data: some data that might be JSON that might be a valid
+       Grid Manager Certificate
+
+    :returns: json data of a valid Grid Manager certificate, or an
+        exception if the data is not valid.
+    """
+
+    required_keys = {
+        'certificate',
+        'signature',
+    }
+
+    js = json.loads(gm_data)
+
+    if not isinstance(js, dict):
+        raise ValueError(
+            "Grid Manager certificate must be a dict"
+        )
+    if set(js.keys()) != required_keys:
+            raise ValueError(
+                "Grid Manager certificate must contain: {}".format(
+                    ", ".join("'{}'".format(k) for k in js.keys()),
+                )
+            )
+    return js
+
+
+def validate_grid_manager_certificate(gm_key, alleged_cert):
+    """
+    :param gm_key: a VerifyingKey instance, a Grid Manager's public
+        key.
+
+    :param alleged_cert: dict with "certificate" and "signature" keys, where
+        "certificate" contains a JSON-serialized certificate for a Storage
+        Server (comes from a Grid Manager).
+
+    :return: a dict consisting of the deserialized certificate data or
+        None if the signature is invalid. Note we do NOT check the
+        expiry time in this function.
+    """
+    try:
+        ed25519.verify_signature(
+            gm_key,
+            base32.a2b(alleged_cert['signature'].encode('ascii')),
+            alleged_cert['certificate'].encode('ascii'),
+        )
+    except ed25519.BadSignature:
+        return None
+    # signature is valid; now we can load the actual data
+    cert = json.loads(alleged_cert['certificate'])
+    return cert
+
+
+def create_grid_manager_verifier(keys, certs, now_fn=None, bad_cert=None):
+    """
+    Creates a predicate for confirming some Grid Manager-issued
+    certificates against Grid Manager keys. A predicate is used
+    (instead of just returning True/False here) so that the
+    expiry-time can be tested on each call.
+
+    :param list keys: 0 or more `VerifyingKey` instances
+
+    :param list certs: 1 or more Grid Manager certificates each of
+        which is a `dict` containing 'signature' and 'certificate' keys.
+
+    :param callable now_fn: a callable which returns the current UTC
+        timestamp (or datetime.utcnow if None).
+
+    :param callable bad_cert: a two-argument callable which is invoked
+        when a certificate verification fails. The first argument is
+        the verifying key and the second is the certificate. If None
+        (the default) errors are print()-ed. Note that we may have
+        several certificates and only one must be valid, so this may
+        be called (multiple times) even if the function ultimately
+        returns successfully.
+
+    :returns: a callable which will return True only-if there is at
+        least one valid certificate (that has not at this moment
+        expired) in `certs` signed by one of the keys in `keys`.
+    """
+
+    now_fn = datetime.utcnow if now_fn is None else now_fn
+    valid_certs = []
+
+    # if we have zero grid-manager keys then everything is valid
+    if not keys:
+        return lambda: True
+
+    if bad_cert is None:
+
+        def bad_cert(key, alleged_cert):
+            """
+            We might want to let the user know about this failed-to-verify
+            certificate .. but also if you have multiple grid-managers
+            then a bunch of these messages would appear. Better would
+            be to bubble this up to some sort of status API (or maybe
+            on the Welcome page?)
+
+            The only thing that might actually be interesting, though,
+            is whether this whole function returns false or not..
+            """
+            print(
+                "Grid Manager certificate signature failed. Certificate: "
+                "\"{cert}\" for key \"{key}\".".format(
+                    cert=alleged_cert,
+                    key=ed25519.string_from_verifying_key(key),
+                )
+            )
+
+    # validate the signatures on any certificates we have (not yet the expiry dates)
+    for alleged_cert in certs:
+        for key in keys:
+            cert = _validate_grid_manager_certificate(key, alleged_cert)
+            if cert is not None:
+                valid_certs.append(cert)
+            else:
+                bad_cert(key, alleged_cert)
+
+    def validate():
+        now = now_fn()
+        # if *any* certificate is still valid then we consider the server valid
+        for cert in valid_certs:
+            expires = datetime.utcfromtimestamp(cert['expires'])
+            # cert_pubkey = keyutil.parse_pubkey(cert['public_key'].encode('ascii'))
+            if expires > now:
+                # not-expired
+                return True
+        return False
+
+    return validate

src/allmydata/scripts/admin.py

@@ -7,8 +7,8 @@ from os.path import exists, join
 from twisted.python import usage
 #from allmydata.node import read_config
 from allmydata.client import read_config
-from allmydata.storage_client import (
+from allmydata.grid_manager import (
-    parse_grid_manager_data,
+    parse_grid_manager_certificate,
 )
 from allmydata.scripts.cli import _default_nodedir
 from allmydata.scripts.common import BaseOptions
@@ -90,7 +90,7 @@ class AddGridManagerCertOptions(BaseOptions):
                 data = f.read()
 
         try:
-            self.certificate_data = parse_grid_manager_data(data)
+            self.certificate_data = parse_grid_manager_certificate(data)
         except ValueError as e:
             raise usage.UsageError(
                 "Error parsing certificate: {}".format(e)
@@ -142,11 +142,9 @@ def add_grid_manager_cert(options):
     # write all the data out
 
     fileutil.write(cert_path, cert_bytes)
-    # print("created {}: {} bytes".format(cert_fname, len(cert_bytes)))
     with open(config_path, "w") as f:
-        # XXX probably want a _Config.write_tahoe_cfg() or something? or just set_config() does that automagically
+        # XXX probably want a _Config.write_tahoe_cfg() or something?
         config.config.write(f)
-    # print("wrote {}".format(config_fname))
 
     cert_count = len(config.enumerate_section("grid_manager_certificates"))
     print("There are now {} certificates".format(cert_count), file=options.parent.parent.stderr)

src/allmydata/scripts/runner.py

@@ -11,7 +11,7 @@ from allmydata.version_checks import get_package_versions_string
 from allmydata.scripts.common import get_default_nodedir
 from allmydata.scripts import debug, create_node, cli, \
     stats_gatherer, admin, tahoe_daemonize, tahoe_start, \
-    tahoe_stop, tahoe_restart, tahoe_run, tahoe_invite, tahoe_grid_manager
+    tahoe_stop, tahoe_restart, tahoe_run, tahoe_invite
 from allmydata.util.encodingutil import quote_output, quote_local_unicode_path, get_io_encoding
 from allmydata.util.eliotutil import (
     opt_eliot_destination,
@@ -63,7 +63,6 @@ class Options(usage.Options):
                     +   debug.subCommands
                     +   cli.subCommands
                     +   tahoe_invite.subCommands
-                    +   tahoe_grid_manager.subCommands
                     )
 
     optFlags = [
@@ -155,8 +154,6 @@ def dispatch(config,
         # these are blocking, and must be run in a thread
         f0 = cli.dispatch[command]
         f = lambda so: threads.deferToThread(f0, so)
-    elif command in tahoe_grid_manager.dispatch:
-        f = tahoe_grid_manager.dispatch[command]
     elif command in tahoe_invite.dispatch:
         f = tahoe_invite.dispatch[command]
     else:

src/allmydata/storage_client.py

@@ -431,124 +431,6 @@ class StubServer(object):
         return "?"
 
 
-def parse_grid_manager_data(gm_data):
-    """
-    :param gm_data: some data that might be JSON that might be a valid
-       Grid Manager Certificate
-
-    :returns: json data of a valid Grid Manager certificate, or an
-        exception if the data is not valid.
-    """
-
-    required_keys = allowed_keys = [
-        'certificate',
-        'signature',
-    ]
-
-    js = json.loads(gm_data)
-    for k in js.keys():
-        if k not in allowed_keys:
-            raise ValueError(
-                "Grid Manager certificate JSON may not contain '{}'".format(
-                    k,
-                )
-            )
-    for k in required_keys:
-        if k not in js:
-            raise ValueError(
-                "Grid Manager certificate JSON must contain '{}'".format(
-                    k,
-                )
-            )
-    return js
-
-
-def _validate_grid_manager_certificate(gm_key, alleged_cert):
-    """
-    :param gm_key: a VerifyingKey instance, a Grid Manager's public
-        key.
-
-    :param alleged_cert: dict with "certificate" and "signature" keys, where
-        "certificate" contains a JSON-serialized certificate for a Storage
-        Server (comes from a Grid Manager).
-
-    :return: a dict consisting of the deserialized certificate data or
-        None if the signature is invalid. Note we do NOT check the
-        expiry time in this function.
-    """
-    try:
-        ed25519.verify_signature(
-            gm_key,
-            base32.a2b(alleged_cert['signature'].encode('ascii')),
-            alleged_cert['certificate'].encode('ascii'),
-        )
-    except ed25519.BadSignature:
-        return None
-    # signature is valid; now we can load the actual data
-    cert = json.loads(alleged_cert['certificate'])
-    return cert
-
-
-def create_grid_manager_verifier(keys, certs, now_fn=None):
-    """
-    Creates a predicate for confirming some Grid Manager-issued
-    certificates against Grid Manager keys. A predicate is used
-    (instead of just returning True/False here) so that the
-    expiry-time can be tested on each call.
-
-    :param list keys: 0 or more `VerifyingKey` instances
-
-    :param list certs: 1 or more Grid Manager certificates each of
-        which is a `dict` containing 'signature' and 'certificate' keys.
-
-    :param callable now_fn: a callable which returns the current UTC
-        timestamp (or datetime.utcnow if None).
-
-    :returns: a callable which will return True only-if there is at
-        least one valid certificate in `certs` signed by one of the keys
-        in `keys`.
-    """
-
-    now_fn = datetime.utcnow if now_fn is None else now_fn
-    valid_certs = []
-
-    # if we have zero grid-manager keys then everything is valid
-    if not keys:
-        return lambda: True
-
-    # validate the signatures on any certificates we have (not yet the expiry dates)
-    for alleged_cert in certs:
-        for key in keys:
-            cert = _validate_grid_manager_certificate(key, alleged_cert)
-            if cert is not None:
-                valid_certs.append(cert)
-            else:
-                # we might want to let the user know about this
-                # failed-to-verify certificate .. but also if you have
-                # multiple grid-managers then a bunch of these
-                # messages would appear
-                print(
-                    "Grid Manager certificate signature failed. Certificate: "
-                    "\"{cert}\" for key \"{key}\".".format(
-                        key=key,
-                        cert=alleged_cert,
-                    )
-                )
-
-    def validate():
-        now = now_fn()
-        # if *any* certificate is still valid then we consider the server valid
-        for cert in valid_certs:
-            expires = datetime.utcfromtimestamp(cert['expires'])
-            # cert_pubkey = keyutil.parse_pubkey(cert['public_key'].encode('ascii'))
-            if expires > now:
-                # not-expired
-                return True
-        return False
-
-    return validate
-
-
 class IFoolscapStorageServer(Interface):
     """
     An internal interface that mediates between ``NativeStorageServer`` and

src/allmydata/test/test_grid_manager.py

@@ -1,6 +1,10 @@
 
 import json
 
+from twisted.python.filepath import (
+    FilePath,
+)
+
 from allmydata.client import (
     create_storage_farm_broker,
 )
@@ -10,6 +14,17 @@ from allmydata.node import (
 from allmydata.client import (
     _valid_config as client_valid_config,
 )
+from allmydata.crypto import (
+    ed25519,
+)
+from allmydata.util import (
+    base32,
+)
+from allmydata.grid_manager import (
+    load_grid_manager,
+    save_grid_manager,
+    create_grid_manager,
+)
 
 from .common import SyncTestCase
 
@@ -78,3 +93,182 @@ class GridManagerUtilities(SyncTestCase):
             1,
             len(config.enumerate_section("grid_managers"))
         )
+
+
+class GridManagerVerifier(SyncTestCase):
+    """
+    Tests related to rejecting or accepting Grid Manager certificates.
+    """
+
+    def setUp(self):
+        self.gm = create_grid_manager()
+        return super(GridManagerVerifier, self).setUp()
+
+    def test_sign_cert(self):
+        """
+        Add a storage-server and sign a certificate for it
+        """
+        priv, pub = ed25519.create_signing_keypair()
+        self.gm.add_storage_server("test", pub)
+        cert = self.gm.sign("test", 86400)
+
+        self.assertEqual(
+            set(cert.keys()),
+            {"certificate", "signature"},
+        )
+        gm_key = ed25519.verifying_key_from_string(self.gm.public_identity())
+        self.assertEqual(
+            ed25519.verify_signature(
+                gm_key,
+                base32.a2b(cert["signature"]),
+                cert["certificate"],
+            ),
+            None
+        )
+
+    def test_sign_cert_wrong_name(self):
+        """
+        Try to sign a storage-server that doesn't exist
+        """
+        with self.assertRaises(KeyError):
+            self.gm.sign("doesn't exist", 86400)
+
+    def test_add_cert(self):
+        """
+        Add a storage-server and serialize it
+        """
+        priv, pub = ed25519.create_signing_keypair()
+        self.gm.add_storage_server("test", pub)
+
+        data = self.gm.marshal()
+        self.assertEqual(
+            data["storage_servers"],
+            {
+                "test": {
+                    "public_key": ed25519.string_from_verifying_key(pub),
+                }
+            }
+        )
+
+    def test_remove(self):
+        """
+        Add then remove a storage-server
+        """
+        priv, pub = ed25519.create_signing_keypair()
+        self.gm.add_storage_server("test", pub)
+        self.gm.remove_storage_server("test")
+        self.assertEqual(len(self.gm.storage_servers), 0)
+
+    def test_serialize(self):
+        """
+        Write and then read a Grid Manager config
+        """
+        priv0, pub0 = ed25519.create_signing_keypair()
+        priv1, pub1 = ed25519.create_signing_keypair()
+        self.gm.add_storage_server("test0", pub0)
+        self.gm.add_storage_server("test1", pub1)
+
+        tempdir = self.mktemp()
+        fp = FilePath(tempdir)
+
+        save_grid_manager(fp, self.gm)
+        gm2 = load_grid_manager(fp, tempdir)
+        self.assertEqual(
+            self.gm.public_identity(),
+            gm2.public_identity(),
+        )
+        self.assertEqual(
+            len(self.gm.storage_servers),
+            len(gm2.storage_servers),
+        )
+        for name, ss0 in self.gm.storage_servers.items():
+            ss1 = gm2.storage_servers[name]
+            self.assertEqual(ss0.name, ss1.name)
+            self.assertEqual(ss0.public_key(), ss1.public_key())
+        self.assertEqual(self.gm.marshal(), gm2.marshal())
+
+    def test_invalid_no_version(self):
+        """
+        Invalid Grid Manager config with no version
+        """
+        tempdir = self.mktemp()
+        fp = FilePath(tempdir)
+        bad_config = {
+            "private_key": "at least we have one",
+        }
+        fp.makedirs()
+        with fp.child("config.json").open("w") as f:
+            json.dump(bad_config, f)
+
+        with self.assertRaises(ValueError) as ctx:
+            load_grid_manager(fp, tempdir)
+        self.assertIn(
+            "unknown version",
+            str(ctx.exception),
+        )
+
+    def test_invalid_no_private_key(self):
+        """
+        Invalid Grid Manager config with no private key
+        """
+        tempdir = self.mktemp()
+        fp = FilePath(tempdir)
+        bad_config = {
+            "grid_manager_config_version": 0,
+        }
+        fp.makedirs()
+        with fp.child("config.json").open("w") as f:
+            json.dump(bad_config, f)
+
+        with self.assertRaises(ValueError) as ctx:
+            load_grid_manager(fp, tempdir)
+        self.assertIn(
+            "requires a 'private_key'",
+            str(ctx.exception),
+        )
+
+    def test_invalid_bad_private_key(self):
+        """
+        Invalid Grid Manager config with bad private-key
+        """
+        tempdir = self.mktemp()
+        fp = FilePath(tempdir)
+        bad_config = {
+            "grid_manager_config_version": 0,
+            "private_key": "not actually encoded key",
+        }
+        fp.makedirs()
+        with fp.child("config.json").open("w") as f:
+            json.dump(bad_config, f)
+
+        with self.assertRaises(ValueError) as ctx:
+            load_grid_manager(fp, tempdir)
+        self.assertIn(
+            "Invalid Grid Manager private_key",
+            str(ctx.exception),
+        )
+
+    def test_invalid_storage_server(self):
+        """
+        Invalid Grid Manager config with missing public-key for
+        storage-server
+        """
+        tempdir = self.mktemp()
+        fp = FilePath(tempdir)
+        bad_config = {
+            "grid_manager_config_version": 0,
+            "private_key": "priv-v0-ub7knkkmkptqbsax4tznymwzc4nk5lynskwjsiubmnhcpd7lvlqa",
+            "storage_servers": {
+                "bad": {}
+            }
+        }
+        fp.makedirs()
+        with fp.child("config.json").open("w") as f:
+            json.dump(bad_config, f)
+
+        with self.assertRaises(ValueError) as ctx:
+            load_grid_manager(fp, tempdir)
+        self.assertIn(
+            "No 'public_key' for storage server",
+            str(ctx.exception),
+        )