From a848f218531bed10548b843e0fd624e12158c523 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 3 Oct 2019 14:07:37 -0400
Subject: [PATCH] Avoid the mutable announcement state on the plugin instance
We do so by introducing a limitation that the client must be configured with
an issuer and it must match the issuer announced by any storage server the
client is to talk to.
The client will decline to speak to any storage server without a matching
issuer.
---
docs/source/configuration.rst | 1 +
src/_zkapauthorizer/_plugin.py | 8 +----
src/_zkapauthorizer/controller.py | 46 +++++++++++++++++++++++-
src/_zkapauthorizer/tests/strategies.py | 4 ++-
src/_zkapauthorizer/tests/test_plugin.py | 38 ++++++++++++++++++++
5 files changed, 88 insertions(+), 9 deletions(-)
diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst
index 75bf5ef..c20c656 100644
--- a/docs/source/configuration.rst
+++ b/docs/source/configuration.rst
@@ -1,6 +1,7 @@
[storageclient.plugins.privatestorageio-zkapauthz-v1]
redeemer = < dummy | ristretto >
+ristretto-issuer-root-url = https://issuer.privatestorage.io/
[storageserver.plugins.privatestorageio-zkapauthz-v1]
ristretto-issuer-root-url = https://issuer.privatestorage.io/
diff --git a/src/_zkapauthorizer/_plugin.py b/src/_zkapauthorizer/_plugin.py
index ac0b87e..e5c59e2 100644
--- a/src/_zkapauthorizer/_plugin.py
+++ b/src/_zkapauthorizer/_plugin.py
@@ -77,7 +77,6 @@ class ZKAPAuthorizer(object):
"""
name = attr.ib(default=u"privatestorageio-zkapauthz-v1")
_stores = attr.ib(default=attr.Factory(WeakValueDictionary))
- _announcement = attr.ib(default=None)
def _get_store(self, node_config):
"""
@@ -127,9 +126,6 @@ class ZKAPAuthorizer(object):
managed by this plugin in the node directory that goes along with
``node_config``.
"""
- # XXXXXXXXXXXXxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx........................ :(
- self._announcement = announcement
-
from twisted.internet import reactor
redeemer = self._get_redeemer(node_config, announcement, reactor)
extract_unblinded_tokens = self._get_store(node_config).extract_unblinded_tokens
@@ -144,11 +140,9 @@ class ZKAPAuthorizer(object):
def get_client_resource(self, node_config):
- # XXXXXXXXXXXXxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx........................ :(
- announcement = self._announcement
from twisted.internet import reactor
return resource_from_configuration(
node_config,
store=self._get_store(node_config),
- redeemer=self._get_redeemer(node_config, announcement, reactor),
+ redeemer=self._get_redeemer(node_config, None, reactor),
)
diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py
index 457ed37..f6c38f2 100644
--- a/src/_zkapauthorizer/controller.py
+++ b/src/_zkapauthorizer/controller.py
@@ -178,6 +178,32 @@ class DummyRedeemer(object):
)
+class IssuerConfigurationMismatch(Exception):
+ """
+ The Ristretto issuer address in the local client configuration does not
+ match the Ristretto issuer address received in a storage server
+ announcement.
+
+ If these values do not match then there is no reason to expect that ZKAPs
+ will be accepted by the storage server because ZKAPs are bound to the
+ issuer's signing key.
+
+ This mismatch must be corrected before the storage server can be used.
+ Either the storage server needs to be reconfigured to respect the
+ authority of a different issuer (the same one the client is configured to
+ use), the client needs to select a different storage server to talk to, or
+ the client needs to be reconfigured to respect the authority of a
+ different issuer (the same one the storage server is announcing).
+
+ Note that issued ZKAPs cannot be exchanged between issues except through
+ some ad hoc, out-of-band means. That is, if the client already has some
+ ZKAPs and chooses to change its configured issuer address, those existing
+ ZKAPs will not be usable and new ones must be obtained.
+ """
+ def __str__(self):
+ return "Announced issuer ({}) disagrees with configured issuer ({}).".format(self.args)
+
+
@implementer(IRedeemer)
@attr.s
class RistrettoRedeemer(object):
@@ -188,9 +214,27 @@ class RistrettoRedeemer(object):
@classmethod
def make(cls, section_name, node_config, announcement, reactor):
+ configured_issuer = node_config.get_config(
+ section=section_name,
+ option=u"ristretto-issuer-root-url",
+ ).decode("ascii")
+ if announcement is not None:
+ # Don't let us talk to a storage server that has a different idea
+ # about who issues ZKAPs. We should lift this limitation (that is, we
+ # should support as many different issuers as the user likes) in the
+ # future but doing so requires changing how the web interface works
+ # and possibly also the interface for voucher submission.
+ #
+ # If we aren't given an announcement then we're not being used in
+ # the context of a specific storage server so the check is
+ # unnecessary and impossible.
+ announced_issuer = announcement[u"ristretto-issuer-root-url"]
+ if announced_issuer != configured_issuer:
+ raise IssuerConfigurationMismatch(announced_issuer, configured_issuer)
+
return cls(
HTTPClient(Agent(reactor)),
- URL.from_text(announcement[u"ristretto-issuer-root-url"]),
+ URL.from_text(configured_issuer),
)
def random_tokens_for_voucher(self, voucher, count):
diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py
index a57170d..0af3d1f 100644
--- a/src/_zkapauthorizer/tests/strategies.py
+++ b/src/_zkapauthorizer/tests/strategies.py
@@ -177,7 +177,9 @@ def client_configurations():
"""
Build configuration values for the client-side plugin.
"""
- return just({})
+ return just({
+ u"ristretto-issuer-root-url": u"https://issuer.example.invalid/",
+ })
def vouchers():
diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py
index b37cdbf..d497c72 100644
--- a/src/_zkapauthorizer/tests/test_plugin.py
+++ b/src/_zkapauthorizer/tests/test_plugin.py
@@ -16,6 +16,10 @@
Tests for the Tahoe-LAFS plugin.
"""
+from io import (
+ BytesIO,
+)
+
from zope.interface import (
implementer,
)
@@ -36,6 +40,9 @@ from testtools.matchers import (
from testtools.twistedsupport import (
succeeded,
)
+from testtools.content import (
+ text_content,
+)
from hypothesis import (
given,
)
@@ -76,6 +83,9 @@ from twisted.plugins.zkapauthorizer import (
from ..model import (
VoucherStore,
)
+from ..controller import (
+ IssuerConfigurationMismatch,
+)
from .strategies import (
minimal_tahoe_configs,
@@ -227,6 +237,9 @@ tahoe_configs_with_dummy_redeemer = minimal_tahoe_configs({
u"privatestorageio-zkapauthz-v1": just({u"redeemer": u"dummy"}),
})
+tahoe_configs_with_mismatched_issuer = minimal_tahoe_configs({
+ u"privatestorageio-zkapauthz-v1": just({u"ristretto-issuer-root-url": u"https://another-issuer.example.invalid/"}),
+})
class ClientPluginTests(TestCase):
"""
@@ -257,6 +270,31 @@ class ClientPluginTests(TestCase):
)
+ @given(tahoe_configs_with_mismatched_issuer, announcements())
+ def test_mismatched_ristretto_issuer(self, get_config, announcement):
+ """
+ ``get_storage_client`` raises an exception when called with an
+ announcement and local configuration which specify different issuers.
+ """
+ tempdir = self.useFixture(TempDir())
+ node_config = get_config(
+ tempdir.join(b"node"),
+ b"tub.port",
+ )
+ config_text = BytesIO()
+ node_config.config.write(config_text)
+ self.addDetail(u"config", text_content(config_text.getvalue()))
+ self.addDetail(u"announcement", text_content(unicode(announcement)))
+ try:
+ result = storage_server.get_storage_client(node_config, announcement, get_rref)
+ except IssuerConfigurationMismatch:
+ pass
+ except Exception as e:
+ self.fail("get_storage_client raised the wrong exception: {}".format(e))
+ else:
+ self.fail("get_storage_client didn't raise, returned: {}".format(result))
+
+
@given(
tahoe_configs_with_dummy_redeemer,
announcements(),
--
GitLab