diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index 8c80914aec6991d86d066731d470a56537b554dc..73753fe6c4d710af5e633c6ee683c3f195a034f0 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
# Copyright 2019 PrivateStorage.io, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
@@ -112,47 +113,31 @@ class MorePassesRequired(Exception):
ivar int required_count: The number of valid passes which must be
presented for the operation to be authorized.
+
+ :ivar list[int] signature_check_failed: Indices into the supplied list of
+ passes indicating passes which failed the signature check.
"""
valid_count = attr.ib()
required_count = attr.ib()
+ signature_check_failed = attr.ib()
-class LeaseRenewalRequired(Exception):
- """
- Mutable write operations fail with ``LeaseRenewalRequired`` when the slot
- which is the target of the write does not have an active lease and no
- passes are supplied to create one.
+@attr.s
+class _ValidationResult(object):
"""
+ The result of validating a list of passes.
+ :ivar list[int] valid: A list of indexes (into the validated list) of which
+ are acceptable.
-@implementer_only(RIPrivacyPassAuthorizedStorageServer, IReferenceable, IRemotelyCallable)
-# It would be great to use `frozen=True` (value-based hashing) instead of
-# `cmp=False` (identity based hashing) but Referenceable wants to set some
-# attributes on self and it's hard to avoid that.
-@attr.s(cmp=False)
-class ZKAPAuthorizerStorageServer(Referenceable):
- """
- A class which wraps an ``RIStorageServer`` to insert pass validity checks
- before allowing certain functionality.
+ :ivar list[int] signature_check_failed: A list of indexes (into the
+ validated list) of passes which did not have a correct signature.
"""
+ valid = attr.ib()
+ signature_check_failed = attr.ib()
- # This is the amount of time an added or renewed lease will last. We
- # duplicate the value used by the underlying anonymous-access storage
- # server which does not expose it via a Python API or allow it to be
- # configured or overridden. It would be great if the anonymous-access
- # storage server eventually made lease time a parameter so we could just
- # control it ourselves.
- LEASE_PERIOD = timedelta(days=31)
-
- _original = attr.ib(validator=provides(RIStorageServer))
- _pass_value = pass_value_attribute()
- _signing_key = attr.ib(validator=instance_of(SigningKey))
- _clock = attr.ib(
- validator=provides(IReactorTime),
- default=attr.Factory(partial(namedAny, "twisted.internet.reactor")),
- )
-
- def _is_invalid_pass(self, message, pass_):
+ @classmethod
+ def _is_invalid_pass(cls, message, pass_, signing_key):
"""
Cryptographically check the validity of a single pass.
@@ -168,7 +153,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
preimage_base64, signature_base64 = pass_.split(b" ")
preimage = TokenPreimage.decode_base64(preimage_base64)
proposed_signature = VerificationSignature.decode_base64(signature_base64)
- unblinded_token = self._signing_key.rederive_unblinded_token(preimage)
+ unblinded_token = signing_key.rederive_unblinded_token(preimage)
verification_key = unblinded_token.derive_verification_key_sha512()
invalid_pass = verification_key.invalid_sha512(proposed_signature, message.encode("utf-8"))
return invalid_pass
@@ -176,23 +161,76 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# It would be pretty nice to log something here, sometimes, I guess?
return True
- def _validate_passes(self, message, passes):
+ @classmethod
+ def validate_passes(cls, message, passes, signing_key):
"""
Check all of the given passes for validity.
:param unicode message: The shared message for pass validation.
:param list[bytes] passes: The encoded passes to validate.
+ :param SigningKey signing_key: The signing key to use to check the passes.
- :return list[bytes]: The passes which are found to be valid.
+ :return: An instance of this class describing the validation result
+ for all passes given.
"""
- result = list(
- pass_
- for pass_
- in passes
- if not self._is_invalid_pass(message, pass_)
+ valid = []
+ signature_check_failed = []
+ for idx, pass_ in enumerate(passes):
+ if cls._is_invalid_pass(message, pass_, signing_key):
+ signature_check_failed.append(idx)
+ else:
+ valid.append(idx)
+ return cls(
+ valid=valid,
+ signature_check_failed=signature_check_failed,
)
- # print("{}: {} passes, {} valid".format(message, len(passes), len(result)))
- return result
+
+ def raise_for(self, required_pass_count):
+ """
+ :raise MorePassesRequired: Always raised with fields populated from this
+ instance and the given ``required_pass_count``.
+ """
+ raise MorePassesRequired(
+ len(self.valid),
+ required_pass_count,
+ self.signature_check_failed,
+ )
+
+
+class LeaseRenewalRequired(Exception):
+ """
+ Mutable write operations fail with ``LeaseRenewalRequired`` when the slot
+ which is the target of the write does not have an active lease and no
+ passes are supplied to create one.
+ """
+
+
+@implementer_only(RIPrivacyPassAuthorizedStorageServer, IReferenceable, IRemotelyCallable)
+# It would be great to use `frozen=True` (value-based hashing) instead of
+# `cmp=False` (identity based hashing) but Referenceable wants to set some
+# attributes on self and it's hard to avoid that.
+@attr.s(cmp=False)
+class ZKAPAuthorizerStorageServer(Referenceable):
+ """
+ A class which wraps an ``RIStorageServer`` to insert pass validity checks
+ before allowing certain functionality.
+ """
+
+ # This is the amount of time an added or renewed lease will last. We
+ # duplicate the value used by the underlying anonymous-access storage
+ # server which does not expose it via a Python API or allow it to be
+ # configured or overridden. It would be great if the anonymous-access
+ # storage server eventually made lease time a parameter so we could just
+ # control it ourselves.
+ LEASE_PERIOD = timedelta(days=31)
+
+ _original = attr.ib(validator=provides(RIStorageServer))
+ _pass_value = pass_value_attribute()
+ _signing_key = attr.ib(validator=instance_of(SigningKey))
+ _clock = attr.ib(
+ validator=provides(IReactorTime),
+ default=attr.Factory(partial(namedAny, "twisted.internet.reactor")),
+ )
def remote_get_version(self):
"""
@@ -206,13 +244,14 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through after a pass check to ensure that clients can only allocate
storage for immutable shares if they present valid passes.
"""
- valid_passes = self._validate_passes(
+ validation = _ValidationResult.validate_passes(
allocate_buckets_message(storage_index),
passes,
+ self._signing_key,
)
check_pass_quantity_for_write(
self._pass_value,
- len(valid_passes),
+ validation,
sharenums,
allocated_size,
)
@@ -238,12 +277,15 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through after a pass check to ensure clients can only extend the
duration of share storage if they present valid passes.
"""
- # print("server add_lease({}, {!r})".format(len(passes), storage_index))
- valid_passes = self._validate_passes(add_lease_message(storage_index), passes)
+ validation = _ValidationResult.validate_passes(
+ add_lease_message(storage_index),
+ passes,
+ self._signing_key,
+ )
check_pass_quantity_for_lease(
self._pass_value,
storage_index,
- valid_passes,
+ validation,
self._original,
)
return self._original.remote_add_lease(storage_index, *a, **kw)
@@ -253,7 +295,11 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through after a pass check to ensure clients can only extend the
duration of share storage if they present valid passes.
"""
- valid_passes = self._validate_passes(renew_lease_message(storage_index), passes)
+ valid_passes = _ValidationResult.validate_passes(
+ renew_lease_message(storage_index),
+ passes,
+ self._signing_key,
+ )
check_pass_quantity_for_lease(
self._pass_value,
storage_index,
@@ -306,9 +352,10 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# necessary lease as part of the same operation. This must be
# supported because there is no separate protocol action to
# *create* a slot. Clients just begin writing to it.
- valid_passes = self._validate_passes(
+ validation = _ValidationResult.validate_passes(
slot_testv_and_readv_and_writev_message(storage_index),
passes,
+ self._signing_key,
)
if has_active_lease(self._original, storage_index, self._clock.seconds()):
# Some of the storage is paid for already.
@@ -328,8 +375,8 @@ class ZKAPAuthorizerStorageServer(Referenceable):
current_sizes,
tw_vectors,
)
- if required_new_passes > len(valid_passes):
- raise MorePassesRequired(len(valid_passes), required_new_passes)
+ if required_new_passes > len(validation.valid):
+ validation.raise_for(required_new_passes)
# Skip over the remotely exposed method and jump to the underlying
# implementation which accepts one additional parameter that we know
@@ -373,12 +420,15 @@ def has_active_lease(storage_server, storage_index, now):
)
-def check_pass_quantity(pass_value, valid_count, share_sizes):
+def check_pass_quantity(pass_value, validation, share_sizes):
"""
Check that the given number of passes is sufficient to cover leases for
one period for shares of the given sizes.
- :param int valid_count: The number of passes.
+ :param int pass_value: The value of a single pass in bytes × lease periods.
+
+ :param _ValidationResult validation: The validating results for a list of passes.
+
:param list[int] share_sizes: The sizes of the shares for which the lease
is being created.
@@ -388,17 +438,23 @@ def check_pass_quantity(pass_value, valid_count, share_sizes):
:return: ``None`` if the given number of passes is sufficient.
"""
required_pass_count = required_passes(pass_value, share_sizes)
- if valid_count < required_pass_count:
- raise MorePassesRequired(
- valid_count,
- required_pass_count,
- )
+ if len(validation.valid) < required_pass_count:
+ validation.raise_for(required_pass_count)
-def check_pass_quantity_for_lease(pass_value, storage_index, valid_passes, storage_server):
+def check_pass_quantity_for_lease(pass_value, storage_index, validation, storage_server):
"""
Check that the given number of passes is sufficient to add or renew a
lease for one period for the given storage index.
+
+ :param int pass_value: The value of a single pass in bytes × lease periods.
+
+ :param _ValidationResult validation: The validating results for a list of passes.
+
+ :raise MorePassesRequired: If the given number of passes is too few for
+ the share sizes at the given storage index.
+
+ :return: ``None`` if the given number of passes is sufficient.
"""
allocated_sizes = dict(
get_share_sizes(
@@ -407,16 +463,20 @@ def check_pass_quantity_for_lease(pass_value, storage_index, valid_passes, stora
list(get_all_share_numbers(storage_server, storage_index)),
),
).values()
- check_pass_quantity(pass_value, len(valid_passes), allocated_sizes)
+ check_pass_quantity(pass_value, validation, allocated_sizes)
-def check_pass_quantity_for_write(pass_value, valid_count, sharenums, allocated_size):
+def check_pass_quantity_for_write(pass_value, validation, sharenums, allocated_size):
"""
Determine if the given number of valid passes is sufficient for an
attempted write.
- :param int valid_count: The number of valid passes to consider.
+ :param int pass_value: The value of a single pass in bytes × lease periods.
+
+ :param _ValidationResult validation: The validating results for a list of passes.
+
:param set[int] sharenums: The shares being written to.
+
:param int allocated_size: The size of each share.
:raise MorePassedRequired: If the number of valid passes given is too
@@ -424,7 +484,7 @@ def check_pass_quantity_for_write(pass_value, valid_count, sharenums, allocated_
:return: ``None`` if the number of valid passes given is sufficient.
"""
- check_pass_quantity(pass_value, valid_count, [allocated_size] * len(sharenums))
+ check_pass_quantity(pass_value, validation, [allocated_size] * len(sharenums))
def get_all_share_paths(storage_server, storage_index):
diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py
index fad28cb7dd29e032da565921a97b43397e69df8d..1eddf1c2e2c173eda5ad4209c5a5397cf146dccb 100644
--- a/src/_zkapauthorizer/tests/test_storage_server.py
+++ b/src/_zkapauthorizer/tests/test_storage_server.py
@@ -33,7 +33,6 @@ from testtools import (
)
from testtools.matchers import (
Equals,
- AfterPreprocessing,
)
from hypothesis import (
given,
@@ -102,35 +101,24 @@ from ..storage_common import (
get_required_new_passes_for_mutable_write,
summarize,
)
+from .._storage_server import (
+ _ValidationResult,
+)
-class PassValidationTests(TestCase):
+
+class ValidationResultTests(TestCase):
"""
- Tests for pass validation performed by ``ZKAPAuthorizerStorageServer``.
+ Tests for ``_ValidationResult``.
"""
- pass_value = 128 * 1024
-
- @skipIf(platform.isWindows(), "Storage server is not supported on Windows")
def setUp(self):
- super(PassValidationTests, self).setUp()
- self.clock = Clock()
- # anonymous_storage_server uses time.time() so get our Clock close to
- # the same time so we can do lease expiration calculations more
- # easily.
- self.clock.advance(time())
- self.anonymous_storage_server = self.useFixture(AnonymousStorageServer()).storage_server
+ super(ValidationResultTests, self).setUp()
self.signing_key = random_signing_key()
- self.storage_server = ZKAPAuthorizerStorageServer(
- self.anonymous_storage_server,
- self.pass_value,
- self.signing_key,
- self.clock,
- )
@given(integers(min_value=0, max_value=64), lists(zkaps(), max_size=64))
def test_validation_result(self, valid_count, invalid_passes):
"""
- ``_get_valid_passes`` returns the number of cryptographically valid passes
- in the list passed to it.
+ ``validate_passes`` returns a ``_ValidationResult`` instance which
+ describes the valid and invalid passes.
"""
message = u"hello world"
valid_passes = make_passes(
@@ -146,13 +134,53 @@ class PassValidationTests(TestCase):
shuffle(all_passes)
self.assertThat(
- self.storage_server._validate_passes(message, all_passes),
- AfterPreprocessing(
- set,
- Equals(set(valid_passes)),
+ _ValidationResult.validate_passes(
+ message,
+ all_passes,
+ self.signing_key,
+ ),
+ Equals(
+ _ValidationResult(
+ valid=list(
+ idx
+ for (idx, pass_)
+ in enumerate(all_passes)
+ if pass_ in valid_passes
+ ),
+ signature_check_failed=list(
+ idx
+ for (idx, pass_)
+ in enumerate(all_passes)
+ if pass_ not in valid_passes
+ ),
+ ),
),
)
+
+class PassValidationTests(TestCase):
+ """
+ Tests for pass validation performed by ``ZKAPAuthorizerStorageServer``.
+ """
+ pass_value = 128 * 1024
+
+ @skipIf(platform.isWindows(), "Storage server is not supported on Windows")
+ def setUp(self):
+ super(PassValidationTests, self).setUp()
+ self.clock = Clock()
+ # anonymous_storage_server uses time.time() so get our Clock close to
+ # the same time so we can do lease expiration calculations more
+ # easily.
+ self.clock.advance(time())
+ self.anonymous_storage_server = self.useFixture(AnonymousStorageServer()).storage_server
+ self.signing_key = random_signing_key()
+ self.storage_server = ZKAPAuthorizerStorageServer(
+ self.anonymous_storage_server,
+ self.pass_value,
+ self.signing_key,
+ self.clock,
+ )
+
def test_allocate_buckets_fails_without_enough_passes(self):
"""
``remote_allocate_buckets`` fails with ``MorePassesRequired`` if it is
@@ -237,6 +265,7 @@ class PassValidationTests(TestCase):
MorePassesRequired(
valid_count=0,
required_count=1,
+ signature_check_failed=[],
),
),
)
@@ -339,6 +368,7 @@ class PassValidationTests(TestCase):
MorePassesRequired(
valid_count=0,
required_count=1,
+ signature_check_failed=[],
),
),
)
@@ -435,6 +465,7 @@ class PassValidationTests(TestCase):
MorePassesRequired(
valid_count=len(passes),
required_count=required_count,
+ signature_check_failed=[],
),
),
)