Merge branch 'master' of github.com:tahoe-lafs/tahoe-lafs into 3816.improve-release-checklist

.github/workflows/ci.yml

@@ -6,6 +6,23 @@ on:
       - "master"
   pull_request:
 
+# Control to what degree jobs in this workflow will run concurrently with
+# other instances of themselves.
+#
+# https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#concurrency
+concurrency:
+  # We want every revision on master to run the workflow completely.
+  # "head_ref" is not set for the "push" event but it is set for the
+  # "pull_request" event.  If it is set then it is the name of the branch and
+  # we can use it to make sure each branch has only one active workflow at a
+  # time.  If it is not set then we can compute a unique string that gives
+  # every master/push workflow its own group.
+  group: "${{ github.head_ref || format('{0}-{1}', github.run_number, github.run_attempt) }}"
+
+  # Then, we say that if a new workflow wants to start in the same group as a
+  # running workflow, the running workflow should be cancelled.
+  cancel-in-progress: true
+
 env:
   # Tell Hypothesis which configuration we want it to use.
   TAHOE_LAFS_HYPOTHESIS_PROFILE: "ci"

.gitignore

@@ -29,7 +29,7 @@ zope.interface-*.egg
 .pc
 
 /src/allmydata/test/plugins/dropin.cache
-/_trial_temp*
+**/_trial_temp*
 /tmp*
 /*.patch
 /dist/

.readthedocs.yaml

@@ -0,0 +1,5 @@
+version: 2
+
+python:
+   install:
+   - requirements: docs/requirements.txt

CREDITS

@@ -260,3 +260,7 @@ D: Community-manager and documentation improvements
 N: Yash Nayani
 E: yashaswi.nram@gmail.com
 D: Installation Guide improvements
+
+N: Florian Sesser
+E: florian@private.storage
+D: OpenMetrics support

NEWS.rst

@@ -6,6 +6,83 @@ User-Visible Changes in Tahoe-LAFS
 
 .. towncrier start line
 
+
+Release 1.17.0 (2021-12-06)
+'''''''''''''''''''''''''''
+
+Security-related Changes
+------------------------
+
+- The introducer server no longer writes the sensitive introducer fURL value to its log at startup time.  Instead it writes the well-known path of the file from which this value can be read. (`#3819 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3819>`_)
+- The storage protocol operation ``add_lease`` now safely rejects an attempt to add a 4,294,967,296th lease to an immutable share.
+  Previously this failed with an error after recording the new lease in the share file, resulting in the share file losing track of a one previous lease. (`#3821 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3821>`_)
+- The storage protocol operation ``readv`` now safely rejects attempts to read negative lengths.
+  Previously these read requests were satisfied with the complete contents of the share file (including trailing metadata) starting from the specified offset. (`#3822 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3822>`_)
+- The storage server implementation now respects the ``reserved_space`` configuration value when writing lease information and recording corruption advisories.
+  Previously, new leases could be created and written to disk even when the storage server had less remaining space than the configured reserve space value.
+  Now this operation will fail with an exception and the lease will not be created.
+  Similarly, if there is no space available, corruption advisories will be logged but not written to disk. (`#3823 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3823>`_)
+- The storage server implementation no longer records corruption advisories about storage indexes for which it holds no shares. (`#3824 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3824>`_)
+- The lease-checker now uses JSON instead of pickle to serialize its state.
+
+  tahoe will now refuse to run until you either delete all pickle files or
+  migrate them using the new command::
+
+      tahoe admin migrate-crawler
+
+  This will migrate all crawler-related pickle files. (`#3825 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3825>`_)
+- The SFTP server no longer accepts password-based credentials for authentication.
+  Public/private key-based credentials are now the only supported authentication type.
+  This removes plaintext password storage from the SFTP credentials file.
+  It also removes a possible timing side-channel vulnerability which might have allowed attackers to discover an account's plaintext password. (`#3827 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3827>`_)
+- The storage server now keeps hashes of lease renew and cancel secrets for immutable share files instead of keeping the original secrets. (`#3839 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3839>`_)
+- The storage server now keeps hashes of lease renew and cancel secrets for mutable share files instead of keeping the original secrets. (`#3841 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3841>`_)
+
+
+Features
+--------
+
+- Tahoe-LAFS releases now have just a .tar.gz source release and a (universal) wheel (`#3735 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3735>`_)
+- tahoe-lafs now provides its statistics also in OpenMetrics format (for Prometheus et. al.) at `/statistics?t=openmetrics`. (`#3786 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3786>`_)
+- If uploading an immutable hasn't had a write for 30 minutes, the storage server will abort the upload. (`#3807 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3807>`_)
+
+
+Bug Fixes
+---------
+
+- When uploading an immutable, overlapping writes that include conflicting data are rejected. In practice, this likely didn't happen in real-world usage. (`#3801 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3801>`_)
+
+
+Dependency/Installation Changes
+-------------------------------
+
+- Tahoe-LAFS now supports running on NixOS 21.05 with Python 3. (`#3808 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3808>`_)
+
+
+Documentation Changes
+---------------------
+
+- The news file for future releases will include a section for changes with a security impact. (`#3815 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3815>`_)
+
+
+Removed Features
+----------------
+
+- The little-used "control port" has been removed from all node types. (`#3814 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3814>`_)
+
+
+Other Changes
+-------------
+
+- Tahoe-LAFS no longer runs its Tor integration test suite on Python 2 due to the increased complexity of obtaining compatible versions of necessary dependencies. (`#3837 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3837>`_)
+
+
+Misc/Other
+----------
+
+- `#3525 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3525>`_, `#3527 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3527>`_, `#3754 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3754>`_, `#3758 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3758>`_, `#3784 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3784>`_, `#3792 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3792>`_, `#3793 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3793>`_, `#3795 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3795>`_, `#3797 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3797>`_, `#3798 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3798>`_, `#3799 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3799>`_, `#3800 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3800>`_, `#3805 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3805>`_, `#3806 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3806>`_, `#3810 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3810>`_, `#3812 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3812>`_, `#3820 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3820>`_, `#3829 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3829>`_, `#3830 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3830>`_, `#3831 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3831>`_, `#3832 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3832>`_, `#3833 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3833>`_, `#3834 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3834>`_, `#3835 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3835>`_, `#3836 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3836>`_, `#3838 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3838>`_, `#3842 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3842>`_, `#3843 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3843>`_, `#3847 <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3847>`_
+
+
 Release 1.16.0 (2021-09-17)
 '''''''''''''''''''''''''''
 

docs/frontends/FTP-and-SFTP.rst

@@ -47,8 +47,8 @@ servers must be configured with a way to first authenticate a user (confirm
 that a prospective client has a legitimate claim to whatever authorities we
 might grant a particular user), and second to decide what directory cap
 should be used as the root directory for a log-in by the authenticated user.
-A username and password can be used; as of Tahoe-LAFS v1.11, RSA or DSA
+As of Tahoe-LAFS v1.17,
-public key authentication is also supported.
+RSA/DSA public key authentication is the only supported mechanism.
 
 Tahoe-LAFS provides two mechanisms to perform this user-to-cap mapping.
 The first (recommended) is a simple flat file with one account per line.
@@ -59,20 +59,14 @@ Creating an Account File
 
 To use the first form, create a file (for example ``BASEDIR/private/accounts``)
 in which each non-comment/non-blank line is a space-separated line of
-(USERNAME, PASSWORD, ROOTCAP), like so::
+(USERNAME, KEY-TYPE, PUBLIC-KEY, ROOTCAP), like so::
 
  % cat BASEDIR/private/accounts
- # This is a password line: username password cap
- alice password URI:DIR2:ioej8xmzrwilg772gzj4fhdg7a:wtiizszzz2rgmczv4wl6bqvbv33ag4kvbr6prz3u6w3geixa6m6a
- bob sekrit URI:DIR2:6bdmeitystckbl9yqlw7g56f4e:serp5ioqxnh34mlbmzwvkp3odehsyrr7eytt5f64we3k9hhcrcja
-
  # This is a public key line: username keytype pubkey cap
  # (Tahoe-LAFS v1.11 or later)
  carol ssh-rsa AAAA... URI:DIR2:ovjy4yhylqlfoqg2vcze36dhde:4d4f47qko2xm5g7osgo2yyidi5m4muyo2vjjy53q4vjju2u55mfa
 
-For public key authentication, the keytype may be either "ssh-rsa" or "ssh-dsa".
+The key type may be either "ssh-rsa" or "ssh-dsa".
-To avoid ambiguity between passwords and public key types, a password cannot
-start with "ssh-".
 
 Now add an ``accounts.file`` directive to your ``tahoe.cfg`` file, as described in
 the next sections.

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

@@ -363,11 +363,11 @@ one branch contains all of the share data;
 another branch contains all of the lease data;
 etc.
 
-Authorization is required for all endpoints.
+An ``Authorization`` header in requests is required for all endpoints.
 The standard HTTP authorization protocol is used.
 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 ``UNAUTHORIZED`` response.
+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.
 
 General
 ~~~~~~~
@@ -396,17 +396,19 @@ For example::
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 Either renew or create a new lease on the bucket addressed by ``storage_index``.
-The details of the lease are encoded in the request body.
+
+The renew secret and cancellation secret should be included as ``X-Tahoe-Authorization`` headers.
 For example::
 
-  {"renew-secret": "abcd", "cancel-secret": "efgh"}
+    X-Tahoe-Authorization: lease-renew-secret <base64-lease-renew-secret>
+    X-Tahoe-Authorization: lease-cancel-secret <base64-lease-cancel-secret>
 
-If the ``renew-secret`` value matches an existing lease
+If the ``lease-renew-secret`` value matches an existing lease
 then the expiration time of that lease will be changed to 31 days after the time of this operation.
 If it does not match an existing lease
-then a new lease will be created with this ``renew-secret`` which expires 31 days after the time of this operation.
+then a new lease will be created with this ``lease-renew-secret`` which expires 31 days after the time of this operation.
 
-``renew-secret`` and ``cancel-secret`` values must be 32 bytes long.
+``lease-renew-secret`` and ``lease-cancel-secret`` values must be 32 bytes long.
 The server treats them as opaque values.
 :ref:`Share Leases` gives details about how the Tahoe-LAFS storage client constructs these values.
 
@@ -423,8 +425,10 @@ In these cases the server takes no action and returns ``NOT FOUND``.
 Discussion
 ``````````
 
-We considered an alternative where ``renew-secret`` and ``cancel-secret`` are placed in query arguments on the request path.
+We considered an alternative where ``lease-renew-secret`` and ``lease-cancel-secret`` are placed in query arguments on the request path.
-We chose to put these values into the request body to make the URL simpler.
+This increases chances of leaking secrets in logs.
+Putting the secrets in the body reduces the chances of leaking secrets,
+but eventually we chose headers as the least likely information to be logged.
 
 Several behaviors here are blindly copied from the Foolscap-based storage server protocol.
 
@@ -450,14 +454,22 @@ A lease is also created for the shares.
 Details of the buckets to create are encoded in the request body.
 For example::
 
-  {"renew-secret": "efgh", "cancel-secret": "ijkl",
+  {"share-numbers": [1, 7, ...], "allocated-size": 12345}
-   "share-numbers": [1, 7, ...], "allocated-size": 12345}
+
+The request must include ``X-Tahoe-Authorization`` HTTP headers that set the various secrets—upload, lease renewal, lease cancellation—that will be later used to authorize various operations.
+For example::
+
+   X-Tahoe-Authorization: lease-renew-secret <base64-lease-renew-secret>
+   X-Tahoe-Authorization: lease-cancel-secret <base64-lease-cancel-secret>
+   X-Tahoe-Authorization: upload-secret <base64-upload-secret>
 
 The response body includes encoded information about the created buckets.
 For example::
 
   {"already-have": [1, ...], "allocated": [7, ...]}
 
+The upload secret is an opaque _byte_ string.
+
 Discussion
 ``````````
 
@@ -482,6 +494,20 @@ The response includes ``already-have`` and ``allocated`` for two reasons:
   This might be because a server has become unavailable and a remaining server needs to store more shares for the upload.
   It could also just be that the client's preferred servers have changed.
 
+Regarding upload secrets,
+the goal is for uploading and aborting (see next sections) to be authenticated by more than just the storage index.
+In the future, we may want to generate them in a way that allows resuming/canceling when the client has issues.
+In the short term, they can just be a random byte string.
+The primary security constraint is that each upload to each server has its own unique upload key,
+tied to uploading that particular storage index to this particular server.
+
+Rejected designs for upload secrets:
+
+* Upload secret per share number.
+  In order to make the secret unguessable by attackers, which includes other servers,
+  it must contain randomness.
+  Randomness means there is no need to have a secret per share, since adding share-specific content to randomness doesn't actually make the secret any better.
+
 ``PATCH /v1/immutable/:storage_index/:share_number``
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
@@ -498,6 +524,12 @@ If any one of these requests fails then at most 128KiB of upload work needs to b
 The server must recognize when all of the data has been received and mark the share as complete
 (which it can do because it was informed of the size when the storage index was initialized).
 
+The request must include a ``X-Tahoe-Authorization`` header that includes the upload secret::
+
+    X-Tahoe-Authorization: upload-secret <base64-upload-secret>
+
+Responses:
+
 * When a chunk that does not complete the share is successfully uploaded the response is ``OK``.
   The response body indicates the range of share data that has yet to be uploaded.
   That is::
@@ -522,6 +554,10 @@ The server must recognize when all of the data has been received and mark the sh
 
 This cancels an *in-progress* upload.
 
+The request must include a ``X-Tahoe-Authorization`` header that includes the upload secret::
+
+    X-Tahoe-Authorization: upload-secret <base64-upload-secret>
+
 The response code:
 
 * When the upload is still in progress and therefore the abort has succeeded,
@@ -619,16 +655,16 @@ The first write operation on a mutable storage index creates it
 (that is,
 there is no separate "create this storage index" operation as there is for the immutable storage index type).
 
-The request body includes the secrets necessary to rewrite to the shares
+The request must include ``X-Tahoe-Authorization`` headers with write enabler and lease secrets::
-along with test, read, and write vectors for the operation.
+
+    X-Tahoe-Authorization: write-enabler <base64-write-enabler-secret>
+    X-Tahoe-Authorization: lease-cancel-secret <base64-lease-cancel-secret>
+    X-Tahoe-Authorization: lease-renew-secret <base64-lease-renew-secret>
+
+The request body includes test, read, and write vectors for the operation.
 For example::
 
    {
-       "secrets": {
-           "write-enabler": "abcd",
-           "lease-renew": "efgh",
-           "lease-cancel": "ijkl"
-       },
        "test-write-vectors": {
            0: {
                "test": [{
@@ -694,8 +730,12 @@ Immutable Data
 1. Create a bucket for storage index ``AAAAAAAAAAAAAAAA`` to hold two immutable shares, discovering that share ``1`` was already uploaded::
 
      POST /v1/immutable/AAAAAAAAAAAAAAAA
-     {"renew-secret": "efgh", "cancel-secret": "ijkl",
+     Authorization: Tahoe-LAFS nurl-swissnum
-      "share-numbers": [1, 7], "allocated-size": 48}
+     X-Tahoe-Authorization: lease-renew-secret efgh
+     X-Tahoe-Authorization: lease-cancel-secret jjkl
+     X-Tahoe-Authorization: upload-secret xyzf
+
+     {"share-numbers": [1, 7], "allocated-size": 48}
 
      200 OK
      {"already-have": [1], "allocated": [7]}
@@ -703,26 +743,34 @@ Immutable Data
 #. Upload the content for immutable share ``7``::
 
      PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
+     Authorization: Tahoe-LAFS nurl-swissnum
      Content-Range: bytes 0-15/48
+     X-Tahoe-Authorization: upload-secret xyzf
      <first 16 bytes of share data>
 
      200 OK
 
      PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
+     Authorization: Tahoe-LAFS nurl-swissnum
      Content-Range: bytes 16-31/48
+     X-Tahoe-Authorization: upload-secret xyzf
      <second 16 bytes of share data>
 
      200 OK
 
      PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
+     Authorization: Tahoe-LAFS nurl-swissnum
      Content-Range: bytes 32-47/48
+     X-Tahoe-Authorization: upload-secret xyzf
      <final 16 bytes of share data>
 
      201 CREATED
 
 #. Download the content of the previously uploaded immutable share ``7``::
 
-     GET /v1/immutable/AAAAAAAAAAAAAAAA?share=7&offset=0&size=48
+     GET /v1/immutable/AAAAAAAAAAAAAAAA?share=7
+     Authorization: Tahoe-LAFS nurl-swissnum
+     Range: bytes=0-47
 
      200 OK
      <complete 48 bytes of previously uploaded data>
@@ -730,7 +778,9 @@ Immutable Data
 #. Renew the lease on all immutable shares in bucket ``AAAAAAAAAAAAAAAA``::
 
      PUT /v1/lease/AAAAAAAAAAAAAAAA
-     {"renew-secret": "efgh", "cancel-secret": "ijkl"}
+     Authorization: Tahoe-LAFS nurl-swissnum
+     X-Tahoe-Authorization: lease-cancel-secret jjkl
+     X-Tahoe-Authorization: lease-renew-secret efgh
 
      204 NO CONTENT
 
@@ -743,12 +793,12 @@ if there is no existing share,
 otherwise it will read a byte which won't match `b""`::
 
      POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
+     Authorization: Tahoe-LAFS nurl-swissnum
+     X-Tahoe-Authorization: write-enabler abcd
+     X-Tahoe-Authorization: lease-cancel-secret efgh
+     X-Tahoe-Authorization: lease-renew-secret ijkl
+
      {
-         "secrets": {
-             "write-enabler": "abcd",
-             "lease-renew": "efgh",
-             "lease-cancel": "ijkl"
-         },
          "test-write-vectors": {
              3: {
                  "test": [{
@@ -775,12 +825,12 @@ otherwise it will read a byte which won't match `b""`::
 #. Safely rewrite the contents of a known version of mutable share number ``3`` (or fail)::
 
      POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
+     Authorization: Tahoe-LAFS nurl-swissnum
+     X-Tahoe-Authorization: write-enabler abcd
+     X-Tahoe-Authorization: lease-cancel-secret efgh
+     X-Tahoe-Authorization: lease-renew-secret ijkl
+
      {
-         "secrets": {
-             "write-enabler": "abcd",
-             "lease-renew": "efgh",
-             "lease-cancel": "ijkl"
-         },
          "test-write-vectors": {
              3: {
                  "test": [{
@@ -807,12 +857,16 @@ otherwise it will read a byte which won't match `b""`::
 #. Download the contents of share number ``3``::
 
      GET /v1/mutable/BBBBBBBBBBBBBBBB?share=3&offset=0&size=10
+     Authorization: Tahoe-LAFS nurl-swissnum
+
      <complete 16 bytes of previously uploaded data>
 
 #. Renew the lease on previously uploaded mutable share in slot ``BBBBBBBBBBBBBBBB``::
 
      PUT /v1/lease/BBBBBBBBBBBBBBBB
-     {"renew-secret": "efgh", "cancel-secret": "ijkl"}
+     Authorization: Tahoe-LAFS nurl-swissnum
+     X-Tahoe-Authorization: lease-cancel-secret efgh
+     X-Tahoe-Authorization: lease-renew-secret ijkl
 
      204 NO CONTENT
 

docs/requirements.txt

@@ -0,0 +1,4 @@
+sphinx
+docutils<0.18  # https://github.com/sphinx-doc/sphinx/issues/9788
+recommonmark
+sphinx_rtd_theme

integration/conftest.py

@@ -353,10 +353,23 @@ def storage_nodes(reactor, temp_dir, introducer, introducer_furl, flog_gatherer,
         nodes.append(process)
     return nodes
 
+@pytest.fixture(scope="session")
+def alice_sftp_client_key_path(temp_dir):
+    # The client SSH key path is typically going to be somewhere else (~/.ssh,
+    # typically), but for convenience sake for testing we'll put it inside node.
+    return join(temp_dir, "alice", "private", "ssh_client_rsa_key")
 
 @pytest.fixture(scope='session')
 @log_call(action_type=u"integration:alice", include_args=[], include_result=False)
-def alice(reactor, temp_dir, introducer_furl, flog_gatherer, storage_nodes, request):
+def alice(
+        reactor,
+        temp_dir,
+        introducer_furl,
+        flog_gatherer,
+        storage_nodes,
+        alice_sftp_client_key_path,
+        request,
+):
     process = pytest_twisted.blockon(
         _create_node(
             reactor, request, temp_dir, introducer_furl, flog_gatherer, "alice",
@@ -387,19 +400,13 @@ accounts.file = {accounts_path}
 """.format(ssh_key_path=host_ssh_key_path, accounts_path=accounts_path))
     generate_ssh_key(host_ssh_key_path)
 
-    # 3. Add a SFTP access file with username/password and SSH key auth.
+    # 3. Add a SFTP access file with an SSH key for auth.
-
+    generate_ssh_key(alice_sftp_client_key_path)
-    # The client SSH key path is typically going to be somewhere else (~/.ssh,
-    # typically), but for convenience sake for testing we'll put it inside node.
-    client_ssh_key_path = join(process.node_dir, "private", "ssh_client_rsa_key")
-    generate_ssh_key(client_ssh_key_path)
     # Pub key format is "ssh-rsa <thekey> <username>". We want the key.
-    ssh_public_key = open(client_ssh_key_path + ".pub").read().strip().split()[1]
+    ssh_public_key = open(alice_sftp_client_key_path + ".pub").read().strip().split()[1]
     with open(accounts_path, "w") as f:
         f.write("""\
-alice password {rwcap}
+alice-key ssh-rsa {ssh_public_key} {rwcap}
-
-alice2 ssh-rsa {ssh_public_key} {rwcap}
 """.format(rwcap=rwcap, ssh_public_key=ssh_public_key))
 
     # 4. Restart the node with new SFTP config.

integration/test_sftp.py

@@ -19,6 +19,7 @@ from future.utils import PY2
 if PY2:
     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
 
+import os.path
 from posixpath import join
 from stat import S_ISDIR
 
@@ -33,7 +34,7 @@ import pytest
 from .util import generate_ssh_key, run_in_thread
 
 
-def connect_sftp(connect_args={"username": "alice", "password": "password"}):
+def connect_sftp(connect_args):
     """Create an SFTP client."""
     client = SSHClient()
     client.set_missing_host_key_policy(AutoAddPolicy)
@@ -60,24 +61,24 @@ def connect_sftp(connect_args={"username": "alice", "password": "password"}):
 @run_in_thread
 def test_bad_account_password_ssh_key(alice, tmpdir):
     """
-    Can't login with unknown username, wrong password, or wrong SSH pub key.
+    Can't login with unknown username, any password, or wrong SSH pub key.
     """
-    # Wrong password, wrong username:
+    # Any password, wrong username:
-    for u, p in [("alice", "wrong"), ("someuser", "password")]:
+    for u, p in [("alice-key", "wrong"), ("someuser", "password")]:
         with pytest.raises(AuthenticationException):
             connect_sftp(connect_args={
                 "username": u, "password": p,
             })
 
-    another_key = join(str(tmpdir), "ssh_key")
+    another_key = os.path.join(str(tmpdir), "ssh_key")
     generate_ssh_key(another_key)
-    good_key = RSAKey(filename=join(alice.node_dir, "private", "ssh_client_rsa_key"))
+    good_key = RSAKey(filename=os.path.join(alice.node_dir, "private", "ssh_client_rsa_key"))
     bad_key = RSAKey(filename=another_key)
 
     # Wrong key:
     with pytest.raises(AuthenticationException):
         connect_sftp(connect_args={
-            "username": "alice2", "pkey": bad_key,
+            "username": "alice-key", "pkey": bad_key,
         })
 
     # Wrong username:
@@ -86,13 +87,24 @@ def test_bad_account_password_ssh_key(alice, tmpdir):
             "username": "someoneelse", "pkey": good_key,
         })
 
+def sftp_client_key(node):
+    return RSAKey(
+        filename=os.path.join(node.node_dir, "private", "ssh_client_rsa_key"),
+    )
+
+def test_sftp_client_key_exists(alice, alice_sftp_client_key_path):
+    """
+    Weakly validate the sftp client key fixture by asserting that *something*
+    exists at the supposed key path.
+    """
+    assert os.path.exists(alice_sftp_client_key_path)
 
 @run_in_thread
 def test_ssh_key_auth(alice):
     """It's possible to login authenticating with SSH public key."""
-    key = RSAKey(filename=join(alice.node_dir, "private", "ssh_client_rsa_key"))
+    key = sftp_client_key(alice)
     sftp = connect_sftp(connect_args={
-        "username": "alice2", "pkey": key
+        "username": "alice-key", "pkey": key
     })
     assert sftp.listdir() == []
 
@@ -100,7 +112,10 @@ def test_ssh_key_auth(alice):
 @run_in_thread
 def test_read_write_files(alice):
     """It's possible to upload and download files."""
-    sftp = connect_sftp()
+    sftp = connect_sftp(connect_args={
+        "username": "alice-key",
+        "pkey": sftp_client_key(alice),
+    })
     with sftp.file("myfile", "wb") as f:
         f.write(b"abc")
         f.write(b"def")
@@ -117,7 +132,10 @@ def test_directories(alice):
     It's possible to create, list directories, and create and remove files in
     them.
     """
-    sftp = connect_sftp()
+    sftp = connect_sftp(connect_args={
+        "username": "alice-key",
+        "pkey": sftp_client_key(alice),
+    })
     assert sftp.listdir() == []
 
     sftp.mkdir("childdir")
@@ -148,7 +166,10 @@ def test_directories(alice):
 @run_in_thread
 def test_rename(alice):
     """Directories and files can be renamed."""
-    sftp = connect_sftp()
+    sftp = connect_sftp(connect_args={
+        "username": "alice-key",
+        "pkey": sftp_client_key(alice),
+    })
     sftp.mkdir("dir")
 
     filepath = join("dir", "file")

integration/test_tor.py

@@ -35,6 +35,9 @@ from allmydata.test.common import (
 if sys.platform.startswith('win'):
     pytest.skip('Skipping Tor tests on Windows', allow_module_level=True)
 
+if PY2:
+    pytest.skip('Skipping Tor tests on Python 2 because dependencies are hard to come by', allow_module_level=True)
+
 @pytest_twisted.inlineCallbacks
 def test_onion_service_storage(reactor, request, temp_dir, flog_gatherer, tor_network, tor_introducer_furl):
     yield _create_anonymous_node(reactor, 'carol', 8008, request, temp_dir, flog_gatherer, tor_network, tor_introducer_furl)

newsfragments/3735.feature

@@ -1 +0,0 @@
-Tahoe-LAFS releases now have just a .tar.gz source release and a (universal) wheel

newsfragments/3786.feature

@@ -1 +0,0 @@
-tahoe-lafs now provides its statistics also in OpenMetrics format (for Prometheus et. al.) at `/statistics?t=openmetrics`.

newsfragments/3801.bugfix

@@ -1 +0,0 @@
-When uploading an immutable, overlapping writes that include conflicting data are rejected. In practice, this likely didn't happen in real-world usage.

newsfragments/3808.installation

@@ -1 +0,0 @@
-Tahoe-LAFS now supports running on NixOS 21.05 with Python 3.

newsfragments/3814.removed

@@ -1 +0,0 @@
-The little-used "control port" has been removed from all node types.

newsfragments/3815.documentation

@@ -1 +0,0 @@
-The news file for future releases will include a section for changes with a security impact.

nix/cbor2.nix

@@ -0,0 +1,20 @@
+{ lib, buildPythonPackage, fetchPypi, setuptools_scm }:
+buildPythonPackage rec {
+  pname = "cbor2";
+  version = "5.2.0";
+
+  src = fetchPypi {
+    sha256 = "1gwlgjl70vlv35cgkcw3cg7b5qsmws36hs4mmh0l9msgagjs4fm3";
+    inherit pname version;
+  };
+
+  doCheck = false;
+
+  propagatedBuildInputs = [ setuptools_scm ];
+
+  meta = with lib; {
+    homepage = https://github.com/agronholm/cbor2;
+    description = "CBOR encoder/decoder";
+    license = licenses.mit;
+  };
+}

nix/overlays.nix

@@ -21,6 +21,9 @@ self: super: {
 
       # collections-extended is not part of nixpkgs at this time.
       collections-extended = python-super.pythonPackages.callPackage ./collections-extended.nix { };
+
+      # cbor2 is not part of nixpkgs at this time.
+      cbor2 = python-super.pythonPackages.callPackage ./cbor2.nix { };
     };
   };
 

nix/tahoe-lafs.nix

@@ -4,10 +4,10 @@
 , setuptools, setuptoolsTrial, pyasn1, zope_interface
 , service-identity, pyyaml, magic-wormhole, treq, appdirs
 , beautifulsoup4, eliot, autobahn, cryptography, netifaces
-, html5lib, pyutil, distro, configparser
+, html5lib, pyutil, distro, configparser, klein, cbor2
 }:
 python.pkgs.buildPythonPackage rec {
-  # Most of the time this is not exactly the release version (eg 1.16.0).
+  # Most of the time this is not exactly the release version (eg 1.17.0).
   # Give it a `post` component to make it look newer than the release version
   # and we'll bump this up at the time of each release.
   #
@@ -20,7 +20,7 @@ python.pkgs.buildPythonPackage rec {
   # is not a reproducable artifact (in the sense of "reproducable builds") so
   # it is excluded from the source tree by default.  When it is included, the
   # package tends to be frequently spuriously rebuilt.
-  version = "1.16.0.post1";
+  version = "1.17.0.post1";
   name = "tahoe-lafs-${version}";
   src = lib.cleanSourceWith {
     src = ../.;
@@ -95,9 +95,10 @@ EOF
   propagatedBuildInputs = with python.pkgs; [
     twisted foolscap zfec appdirs
     setuptoolsTrial pyasn1 zope_interface
-    service-identity pyyaml magic-wormhole treq
+    service-identity pyyaml magic-wormhole
     eliot autobahn cryptography netifaces setuptools
     future pyutil distro configparser collections-extended
+    klein cbor2 treq
   ];
 
   checkInputs = with python.pkgs; [

relnotes.txt

@@ -1,6 +1,6 @@
-ANNOUNCING Tahoe, the Least-Authority File Store, v1.16.0
+ANNOUNCING Tahoe, the Least-Authority File Store, v1.17.0
 
-The Tahoe-LAFS team is pleased to announce version 1.16.0 of
+The Tahoe-LAFS team is pleased to announce version 1.17.0 of
 Tahoe-LAFS, an extremely reliable decentralized storage
 system. Get it with "pip install tahoe-lafs", or download a
 tarball here:
@@ -15,24 +15,17 @@ unique security and fault-tolerance properties:
 
   https://tahoe-lafs.readthedocs.org/en/latest/about.html
 
-The previous stable release of Tahoe-LAFS was v1.15.1, released on
+The previous stable release of Tahoe-LAFS was v1.16.0, released on
-March 23rd, 2021.
+October 19, 2021.
 
-The major change in this release is the completion of the Python 3
+This release fixes several security issues raised as part of an audit
-port -- while maintaining support for Python 2. A future release will
+by Cure53. We developed fixes for these issues in a private
-remove Python 2 support.
+repository. Shortly after this release, public tickets will be updated
+with further information (along with, of course, all the code).
 
-The previously deprecated subcommands "start", "stop", "restart" and
+There is also OpenMetrics support now and several bug fixes.
-"daemonize" have been removed. You must now use "tahoe run" (possibly
-along with your favourite daemonization software).
 
-Several features are now removed: the Account Server, stats-gatherer
+In all, 46 issues have been fixed since the last release.
-and FTP support.
-
-There are several dependency changes that will be interesting for
-distribution maintainers.
-
-In all, 240 issues have been fixed since the last release.
 
 Please see ``NEWS.rst`` for a more complete list of changes.
 
@@ -151,19 +144,19 @@ solely as a labor of love by volunteers. Thank you very much
 to the team of "hackers in the public interest" who make
 Tahoe-LAFS possible.
 
-fenn-cs + meejah
+meejah
 on behalf of the Tahoe-LAFS team
 
-October 19, 2021
+December 6, 2021
 Planet Earth
 
 
-[1] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.16.0/NEWS.rst
+[1] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.0/NEWS.rst
 [2] https://github.com/tahoe-lafs/tahoe-lafs/blob/master/docs/known_issues.rst
 [3] https://tahoe-lafs.org/trac/tahoe-lafs/wiki/RelatedProjects
-[4] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.16.0/COPYING.GPL
+[4] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.0/COPYING.GPL
-[5] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.16.0/COPYING.TGPPL.rst
+[5] https://github.com/tahoe-lafs/tahoe-lafs/blob/tahoe-lafs-1.17.0/COPYING.TGPPL.rst
-[6] https://tahoe-lafs.readthedocs.org/en/tahoe-lafs-1.16.0/INSTALL.html
+[6] https://tahoe-lafs.readthedocs.org/en/tahoe-lafs-1.17.0/INSTALL.html
 [7] https://lists.tahoe-lafs.org/mailman/listinfo/tahoe-dev
 [8] https://tahoe-lafs.org/trac/tahoe-lafs/roadmap
 [9] https://github.com/tahoe-lafs/tahoe-lafs/blob/master/CREDITS

setup.py

@@ -140,6 +140,11 @@ install_requires = [
 
     # For the RangeMap datastructure.
     "collections-extended",
+
+    # HTTP server and client
+    "klein",
+    "treq",
+    "cbor2"
 ]
 
 setup_requires = [
@@ -397,7 +402,6 @@ setup(name="tahoe-lafs", # also set in __init__.py
               # Python 2.7.
               "decorator < 5",
               "hypothesis >= 3.6.1",
-              "treq",
               "towncrier",
               "testtools",
               "fixtures",

src/allmydata/frontends/auth.py

@@ -12,7 +12,7 @@ if PY2:
 
 from zope.interface import implementer
 from twisted.internet import defer
-from twisted.cred import error, checkers, credentials
+from twisted.cred import checkers, credentials
 from twisted.conch.ssh import keys
 from twisted.conch.checkers import SSHPublicKeyChecker, InMemorySSHKeyDB
 
@@ -32,65 +32,93 @@ class FTPAvatarID(object):
 
 @implementer(checkers.ICredentialsChecker)
 class AccountFileChecker(object):
-    credentialInterfaces = (credentials.IUsernamePassword,
+    credentialInterfaces = (credentials.ISSHPrivateKey,)
-                            credentials.IUsernameHashedPassword,
+
-                            credentials.ISSHPrivateKey)
     def __init__(self, client, accountfile):
         self.client = client
-        self.passwords = BytesKeyDict()
+        path = abspath_expanduser_unicode(accountfile)
-        pubkeys = BytesKeyDict()
+        with open_account_file(path) as f:
-        self.rootcaps = BytesKeyDict()
+            self.rootcaps, pubkeys = load_account_file(f)
-        with open(abspath_expanduser_unicode(accountfile), "rb") as f:
-            for line in f:
-                line = line.strip()
-                if line.startswith(b"#") or not line:
-                    continue
-                name, passwd, rest = line.split(None, 2)
-                if passwd.startswith(b"ssh-"):
-                    bits = rest.split()
-                    keystring = b" ".join([passwd] + bits[:-1])
-                    key = keys.Key.fromString(keystring)
-                    rootcap = bits[-1]
-                    pubkeys[name] = [key]
-                else:
-                    self.passwords[name] = passwd
-                    rootcap = rest
-                self.rootcaps[name] = rootcap
         self._pubkeychecker = SSHPublicKeyChecker(InMemorySSHKeyDB(pubkeys))
 
     def _avatarId(self, username):
         return FTPAvatarID(username, self.rootcaps[username])
 
-    def _cbPasswordMatch(self, matched, username):
-        if matched:
-            return self._avatarId(username)
-        raise error.UnauthorizedLogin
-
     def requestAvatarId(self, creds):
         if credentials.ISSHPrivateKey.providedBy(creds):
             d = defer.maybeDeferred(self._pubkeychecker.requestAvatarId, creds)
             d.addCallback(self._avatarId)
             return d
-        elif credentials.IUsernameHashedPassword.providedBy(creds):
-            return self._checkPassword(creds)
-        elif credentials.IUsernamePassword.providedBy(creds):
-            return self._checkPassword(creds)
-        else:
         raise NotImplementedError()
 
-    def _checkPassword(self, creds):
+def open_account_file(path):
     """
-        Determine whether the password in the given credentials matches the
+    Open and return the accounts file at the given path.
-        password in the account file.
-
-        Returns a Deferred that fires with the username if the password matches
-        or with an UnauthorizedLogin failure otherwise.
     """
-        try:
+    return open(path, "rt", encoding="utf-8")
-            correct = self.passwords[creds.username]
-        except KeyError:
-            return defer.fail(error.UnauthorizedLogin())
 
-        d = defer.maybeDeferred(creds.checkPassword, correct)
+def load_account_file(lines):
-        d.addCallback(self._cbPasswordMatch, creds.username)
+    """
-        return d
+    Load credentials from an account file.
+
+    :param lines: An iterable of account lines to load.
+
+    :return: See ``create_account_maps``.
+    """
+    return create_account_maps(
+        parse_accounts(
+            content_lines(
+                lines,
+            ),
+        ),
+    )
+
+def content_lines(lines):
+    """
+    Drop empty and commented-out lines (``#``-prefixed) from an iterator of
+    lines.
+
+    :param lines: An iterator of lines to process.
+
+    :return: An iterator of lines including only those from ``lines`` that
+        include content intended to be loaded.
+    """
+    for line in lines:
+        line = line.strip()
+        if line and not line.startswith("#"):
+            yield line
+
+def parse_accounts(lines):
+    """
+    Parse account lines into their components (name, key, rootcap).
+    """
+    for line in lines:
+        name, passwd, rest = line.split(None, 2)
+        if not passwd.startswith("ssh-"):
+            raise ValueError(
+                "Password-based authentication is not supported; "
+                "configure key-based authentication instead."
+            )
+
+        bits = rest.split()
+        keystring = " ".join([passwd] + bits[:-1])
+        key = keys.Key.fromString(keystring)
+        rootcap = bits[-1]
+        yield (name, key, rootcap)
+
+def create_account_maps(accounts):
+    """
+    Build mappings from account names to keys and rootcaps.
+
+    :param accounts: An iterator if (name, key, rootcap) tuples.
+
+    :return: A tuple of two dicts.  The first maps account names to rootcaps.
+        The second maps account names to public keys.
+    """
+    rootcaps = BytesKeyDict()
+    pubkeys = BytesKeyDict()
+    for (name, key, rootcap) in accounts:
+        name_bytes = name.encode("utf-8")
+        rootcaps[name_bytes] = rootcap.encode("utf-8")
+        pubkeys[name_bytes] = [key]
+    return rootcaps, pubkeys

src/allmydata/interfaces.py

@@ -52,6 +52,8 @@ WriteEnablerSecret = Hash # used to protect mutable share modifications
 LeaseRenewSecret = Hash # used to protect lease renewal requests
 LeaseCancelSecret = Hash # was used to protect lease cancellation requests
 
+class NoSpace(Exception):
+    """Storage space was not available for a space-allocating operation."""
 
 class DataTooLargeError(Exception):
     """The write went past the expected size of the bucket."""

src/allmydata/introducer/server.py

@@ -133,7 +133,7 @@ class _IntroducerNode(node.Node):
             os.rename(old_public_fn, private_fn)
         furl = self.tub.registerReference(introducerservice,
                                           furlFile=private_fn)
-        self.log(" introducer is at %s" % furl, umid="qF2L9A")
+        self.log(" introducer can be found in {!r}".format(private_fn), umid="qF2L9A")
         self.introducer_url = furl # for tests
 
     def init_web(self, webport):

src/allmydata/scripts/admin.py

@@ -18,7 +18,17 @@ except ImportError:
     pass
 
 from twisted.python import usage
-from allmydata.scripts.common import BaseOptions
+from twisted.python.filepath import (
+    FilePath,
+)
+from allmydata.scripts.common import (
+    BaseOptions,
+    BasedirOptions,
+)
+from allmydata.storage import (
+    crawler,
+    expirer,
+)
 
 class GenerateKeypairOptions(BaseOptions):
 
@@ -65,12 +75,55 @@ def derive_pubkey(options):
     print("public:", str(ed25519.string_from_verifying_key(public_key), "ascii"), file=out)
     return 0
 
+class MigrateCrawlerOptions(BasedirOptions):
+
+    def getSynopsis(self):
+        return "Usage: tahoe [global-options] admin migrate-crawler"
+
+    def getUsage(self, width=None):
+        t = BasedirOptions.getUsage(self, width)
+        t += (
+            "The crawler data is now stored as JSON to avoid"
+            " potential security issues with pickle files.\n\nIf"
+            " you are confident the state files in the 'storage/'"
+            " subdirectory of your node are trustworthy, run this"
+            " command to upgrade them to JSON.\n\nThe files are:"
+            " lease_checker.history, lease_checker.state, and"
+            " bucket_counter.state"
+        )
+        return t
+
+
+def migrate_crawler(options):
+    out = options.stdout
+    storage = FilePath(options['basedir']).child("storage")
+
+    conversions = [
+        (storage.child("lease_checker.state"), crawler._convert_pickle_state_to_json),
+        (storage.child("bucket_counter.state"), crawler._convert_pickle_state_to_json),
+        (storage.child("lease_checker.history"), expirer._convert_pickle_state_to_json),
+    ]
+
+    for fp, converter in conversions:
+        existed = fp.exists()
+        newfp = crawler._upgrade_pickle_to_json(fp, converter)
+        if existed:
+            print("Converted '{}' to '{}'".format(fp.path, newfp.path), file=out)
+        else:
+            if newfp.exists():
+                print("Already converted: '{}'".format(newfp.path), file=out)
+            else:
+                print("Not found: '{}'".format(fp.path), file=out)
+
+
 class AdminCommand(BaseOptions):
     subCommands = [
         ("generate-keypair", None, GenerateKeypairOptions,
          "Generate a public/private keypair, write to stdout."),
         ("derive-pubkey", None, DerivePubkeyOptions,
          "Derive a public key from a private key."),
+        ("migrate-crawler", None, MigrateCrawlerOptions,
+         "Write the crawler-history data as JSON."),
         ]
     def postOptions(self):
         if not hasattr(self, 'subOptions'):
@@ -88,6 +141,7 @@ each subcommand.
 subDispatch = {
     "generate-keypair": print_keypair,
     "derive-pubkey": derive_pubkey,
+    "migrate-crawler": migrate_crawler,
     }
 
 def do_admin(options):

src/allmydata/scripts/common.py

@@ -141,7 +141,9 @@ def write_introducer(basedir, petname, furl):
     """
     if isinstance(furl, bytes):
         furl = furl.decode("utf-8")
-    basedir.child(b"private").child(b"introducers.yaml").setContent(
+    private = basedir.child(b"private")
+    private.makedirs(ignoreExistingDirectory=True)
+    private.child(b"introducers.yaml").setContent(
         safe_dump({
             "introducers": {
                 petname: {

src/allmydata/scripts/debug.py

@@ -15,15 +15,22 @@ try:
 except ImportError:
     pass
 
-
-# do not import any allmydata modules at this level. Do that from inside
-# individual functions instead.
 import struct, time, os, sys
+
 from twisted.python import usage, failure
 from twisted.internet import defer
 from foolscap.logging import cli as foolscap_cli
-from allmydata.scripts.common import BaseOptions
 
+from allmydata.scripts.common import BaseOptions
+from allmydata import uri
+from allmydata.storage.mutable import MutableShareFile
+from allmydata.storage.immutable import ShareFile
+from allmydata.mutable.layout import unpack_share
+from allmydata.mutable.layout import MDMFSlotReadProxy
+from allmydata.mutable.common import NeedMoreDataError
+from allmydata.immutable.layout import ReadBucketProxy
+from allmydata.util import base32
+from allmydata.util.encodingutil import quote_output
 
 class DumpOptions(BaseOptions):
     def getSynopsis(self):
@@ -56,10 +63,8 @@ def dump_share(options):
     # check the version, to see if we have a mutable or immutable share
     print("share filename: %s" % quote_output(options['filename']), file=out)
 
-    f = open(options['filename'], "rb")
+    with open(options['filename'], "rb") as f:
-    prefix = f.read(32)
+        if MutableShareFile.is_valid_header(f.read(32)):
-    f.close()
-    if prefix == MutableShareFile.MAGIC:
             return dump_mutable_share(options)
         # otherwise assume it's immutable
         return dump_immutable_share(options)
@@ -170,7 +175,7 @@ def dump_immutable_lease_info(f, out):
     leases = list(f.get_leases())
     if leases:
         for i,lease in enumerate(leases):
-            when = format_expiration_time(lease.expiration_time)
+            when = format_expiration_time(lease.get_expiration_time())
             print(" Lease #%d: owner=%d, expire in %s" \
                   % (i, lease.owner_num, when), file=out)
     else:
@@ -223,10 +228,10 @@ def dump_mutable_share(options):
             print(file=out)
             print(" Lease #%d:" % leasenum, file=out)
             print("  ownerid: %d" % lease.owner_num, file=out)
-            when = format_expiration_time(lease.expiration_time)
+            when = format_expiration_time(lease.get_expiration_time())
             print("  expires in %s" % when, file=out)
-            print("  renew_secret: %s" % str(base32.b2a(lease.renew_secret), "utf-8"), file=out)
+            print("  renew_secret: %s" % lease.present_renew_secret(), file=out)
-            print("  cancel_secret: %s" % str(base32.b2a(lease.cancel_secret), "utf-8"), file=out)
+            print("  cancel_secret: %s" % lease.present_cancel_secret(), file=out)
             print("  secrets are for nodeid: %s" % idlib.nodeid_b2a(lease.nodeid), file=out)
     else:
         print("No leases.", file=out)
@@ -712,25 +717,21 @@ def call(c, *args, **kwargs):
     return results[0]
 
 def describe_share(abs_sharefile, si_s, shnum_s, now, out):
-    from allmydata import uri
+    with open(abs_sharefile, "rb") as f:
-    from allmydata.storage.mutable import MutableShareFile
-    from allmydata.storage.immutable import ShareFile
-    from allmydata.mutable.layout import unpack_share
-    from allmydata.mutable.common import NeedMoreDataError
-    from allmydata.immutable.layout import ReadBucketProxy
-    from allmydata.util import base32
-    from allmydata.util.encodingutil import quote_output
-    import struct
-
-    f = open(abs_sharefile, "rb")
         prefix = f.read(32)
+        if MutableShareFile.is_valid_header(prefix):
+            _describe_mutable_share(abs_sharefile, f, now, si_s, out)
+        elif ShareFile.is_valid_header(prefix):
+            _describe_immutable_share(abs_sharefile, now, si_s, out)
+        else:
+            print("UNKNOWN really-unknown %s" % quote_output(abs_sharefile), file=out)
 
-    if prefix == MutableShareFile.MAGIC:
+def _describe_mutable_share(abs_sharefile, f, now, si_s, out):
     # mutable share
     m = MutableShareFile(abs_sharefile)
     WE, nodeid = m._read_write_enabler_and_nodeid(f)
     data_length = m._read_data_length(f)
-        expiration_time = min( [lease.expiration_time
+    expiration_time = min( [lease.get_expiration_time()
                             for (i,lease) in m._enumerate_leases(f)] )
     expiration = max(0, expiration_time - now)
 
@@ -745,6 +746,13 @@ def describe_share(abs_sharefile, si_s, shnum_s, now, out):
 
     if share_type == "SDMF":
         f.seek(m.DATA_OFFSET)
+
+        # Read at least the mutable header length, if possible.  If there's
+        # less data than that in the share, don't try to read more (we won't
+        # be able to unpack the header in this case but we surely don't want
+        # to try to unpack bytes *following* the data section as if they were
+        # header data).  Rather than 2000 we could use HEADER_LENGTH from
+        # allmydata/mutable/layout.py, probably.
         data = f.read(min(data_length, 2000))
 
         try:
@@ -764,7 +772,6 @@ def describe_share(abs_sharefile, si_s, shnum_s, now, out):
                seqnum, str(base32.b2a(root_hash), "utf-8"),
                expiration, quote_output(abs_sharefile)), file=out)
     elif share_type == "MDMF":
-            from allmydata.mutable.layout import MDMFSlotReadProxy
         fake_shnum = 0
         # TODO: factor this out with dump_MDMF_share()
         class ShareDumper(MDMFSlotReadProxy):
@@ -795,9 +802,8 @@ def describe_share(abs_sharefile, si_s, shnum_s, now, out):
     else:
         print("UNKNOWN mutable %s" % quote_output(abs_sharefile), file=out)
 
-    elif struct.unpack(">L", prefix[:4]) == (1,):
-        # immutable
 
+def _describe_immutable_share(abs_sharefile, now, si_s, out):
     class ImmediateReadBucketProxy(ReadBucketProxy):
         def __init__(self, sf):
             self.sf = sf
@@ -811,8 +817,8 @@ def describe_share(abs_sharefile, si_s, shnum_s, now, out):
     sf = ShareFile(abs_sharefile)
     bp = ImmediateReadBucketProxy(sf)
 
-        expiration_time = min( [lease.expiration_time
+    expiration_time = min(lease.get_expiration_time()
-                                for lease in sf.get_leases()] )
+                          for lease in sf.get_leases())
     expiration = max(0, expiration_time - now)
 
     UEB_data = call(bp.get_uri_extension)
@@ -827,10 +833,6 @@ def describe_share(abs_sharefile, si_s, shnum_s, now, out):
                                         str(ueb_hash, "utf-8"), expiration,
                                         quote_output(abs_sharefile)), file=out)
 
-    else:
-        print("UNKNOWN really-unknown %s" % quote_output(abs_sharefile), file=out)
-
-    f.close()
 
 def catalog_shares(options):
     from allmydata.util.encodingutil import listdir_unicode, quote_output
@@ -933,14 +935,15 @@ def corrupt_share(options):
         f.write(d)
         f.close()
 
-    f = open(fn, "rb")
+    with open(fn, "rb") as f:
         prefix = f.read(32)
-    f.close()
+
-    if prefix == MutableShareFile.MAGIC:
+        if MutableShareFile.is_valid_header(prefix):
             # mutable
             m = MutableShareFile(fn)
-        f = open(fn, "rb")
+            with open(fn, "rb") as f:
                 f.seek(m.DATA_OFFSET)
+                # Read enough data to get a mutable header to unpack.
                 data = f.read(2000)
             # make sure this slot contains an SMDF share
             assert data[0:1] == b"\x00", "non-SDMF mutable shares not supported"

src/allmydata/scripts/tahoe_run.py

@@ -27,7 +27,9 @@ from allmydata.scripts.default_nodedir import _default_nodedir
 from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path
 from allmydata.util.configutil import UnknownConfigError
 from allmydata.util.deferredutil import HookMixin
-
+from allmydata.storage.crawler import (
+    MigratePickleFileError,
+)
 from allmydata.node import (
     PortAssignmentRequired,
     PrivacyError,
@@ -164,6 +166,18 @@ class DaemonizeTheRealService(Service, HookMixin):
                     self.stderr.write("\ntub.port cannot be 0: you must choose.\n\n")
                 elif reason.check(PrivacyError):
                     self.stderr.write("\n{}\n\n".format(reason.value))
+                elif reason.check(MigratePickleFileError):
+                    self.stderr.write(
+                        "Error\nAt least one 'pickle' format file exists.\n"
+                        "The file is {}\n"
+                        "You must either delete the pickle-format files"
+                        " or migrate them using the command:\n"
+                        "    tahoe admin migrate-crawler --basedir {}\n\n"
+                        .format(
+                            reason.value.args[0].path,
+                            self.basedir,
+                        )
+                    )
                 else:
                     self.stderr.write("\nUnknown error\n")
                     reason.printTraceback(self.stderr)

src/allmydata/storage/common.py

@@ -16,11 +16,22 @@ from allmydata.util import base32
 # Backwards compatibility.
 from allmydata.interfaces import DataTooLargeError  # noqa: F401
 
-class UnknownMutableContainerVersionError(Exception):
+class UnknownContainerVersionError(Exception):
-    pass
+    def __init__(self, filename, version):
-class UnknownImmutableContainerVersionError(Exception):
+        self.filename = filename
+        self.version = version
+
+    def __str__(self):
+        return "sharefile {!r} had unexpected version {!r}".format(
+            self.filename,
+            self.version,
+        )
+
+class UnknownMutableContainerVersionError(UnknownContainerVersionError):
     pass
 
+class UnknownImmutableContainerVersionError(UnknownContainerVersionError):
+    pass
 
 def si_b2a(storageindex):
     return base32.b2a(storageindex)

src/allmydata/storage/crawler.py

@@ -11,23 +11,185 @@ from __future__ import print_function
 
 from future.utils import PY2, PY3
 if PY2:
-    # We don't import bytes, object, dict, and list just in case they're used,
+    from 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
-    # so as not to create brittle pickles with random magic objects.
-    from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, range, str, max, min  # noqa: F401
 
-import os, time, struct
+import os
-try:
+import time
-    import cPickle as pickle
+import json
-except ImportError:
+import struct
-    import pickle  # type: ignore
 from twisted.internet import reactor
 from twisted.application import service
+from twisted.python.filepath import FilePath
 from allmydata.storage.common import si_b2a
 from allmydata.util import fileutil
 
 class TimeSliceExceeded(Exception):
     pass
 
+
+class MigratePickleFileError(Exception):
+    """
+    A pickle-format file exists (the FilePath to the file will be the
+    single arg).
+    """
+    pass
+
+
+def _convert_cycle_data(state):
+    """
+    :param dict state: cycle-to-date or history-item state
+
+    :return dict: the state in the JSON form
+    """
+
+    def _convert_expiration_mode(value):
+        # original is a 4-tuple, with the last element being a 2-tuple
+        # .. convert both to lists
+        return [
+            value[0],
+            value[1],
+            value[2],
+            list(value[3]),
+        ]
+
+    def _convert_lease_age(value):
+        # if we're in cycle-to-date, this is a dict
+        if isinstance(value, dict):
+            return {
+                "{},{}".format(k[0], k[1]): v
+                for k, v in value.items()
+            }
+        # otherwise, it's a history-item and they're 3-tuples
+        return [
+            list(v)
+            for v in value
+        ]
+
+    converters = {
+        "configured-expiration-mode": _convert_expiration_mode,
+        "cycle-start-finish-times": list,
+        "lease-age-histogram": _convert_lease_age,
+        "corrupt-shares": lambda value: [
+            list(x)
+            for x in value
+        ],
+        "leases-per-share-histogram": lambda value: {
+            str(k): v
+            for k, v in value.items()
+        },
+    }
+    return {
+            k: converters.get(k, lambda z: z)(v)
+            for k, v in state.items()
+    }
+
+
+def _convert_pickle_state_to_json(state):
+    """
+    :param dict state: the pickled state
+
+    :return dict: the state in the JSON form
+    """
+    assert state["version"] == 1, "Only known version is 1"
+
+    converters = {
+        "cycle-to-date": _convert_cycle_data,
+    }
+    return {
+        k: converters.get(k, lambda x: x)(v)
+        for k, v in state.items()
+    }
+
+
+def _upgrade_pickle_to_json(state_path, convert_pickle):
+    """
+    :param FilePath state_path: the filepath to ensure is json
+
+    :param Callable[dict] convert_pickle: function to change
+        pickle-style state into JSON-style state
+
+    :returns FilePath: the local path where the state is stored
+
+    If this state is pickle, convert to the JSON format and return the
+    JSON path.
+    """
+    json_state_path = state_path.siblingExtension(".json")
+
+    # if there's no file there at all, we're done because there's
+    # nothing to upgrade
+    if not state_path.exists():
+        return json_state_path
+
+    # upgrade the pickle data to JSON
+    import pickle
+    with state_path.open("rb") as f:
+        state = pickle.load(f)
+    new_state = convert_pickle(state)
+    _dump_json_to_file(new_state, json_state_path)
+
+    # we've written the JSON, delete the pickle
+    state_path.remove()
+    return json_state_path
+
+
+def _confirm_json_format(fp):
+    """
+    :param FilePath fp: the original (pickle) name of a state file
+
+    This confirms that we do _not_ have the pickle-version of a
+    state-file and _do_ either have nothing, or the JSON version. If
+    the pickle-version exists, an exception is raised.
+
+    :returns FilePath: the JSON name of a state file
+    """
+    if fp.path.endswith(".json"):
+        return fp
+    jsonfp = fp.siblingExtension(".json")
+    if fp.exists():
+        raise MigratePickleFileError(fp)
+    return jsonfp
+
+
+def _dump_json_to_file(js, afile):
+    """
+    Dump the JSON object `js` to the FilePath `afile`
+    """
+    with afile.open("wb") as f:
+        data = json.dumps(js)
+        if PY2:
+            f.write(data)
+        else:
+            f.write(data.encode("utf8"))
+
+
+class _LeaseStateSerializer(object):
+    """
+    Read and write state for LeaseCheckingCrawler. This understands
+    how to read the legacy pickle format files and upgrade them to the
+    new JSON format (which will occur automatically).
+    """
+
+    def __init__(self, state_path):
+        self._path = _confirm_json_format(FilePath(state_path))
+
+    def load(self):
+        """
+        :returns: deserialized JSON state
+        """
+        with self._path.open("rb") as f:
+            return json.load(f)
+
+    def save(self, data):
+        """
+        Serialize the given data as JSON into the state-path
+        :returns: None
+        """
+        tmpfile = self._path.siblingExtension(".tmp")
+        _dump_json_to_file(data, tmpfile)
+        fileutil.move_into_place(tmpfile.path, self._path.path)
+        return None
+
+
 class ShareCrawler(service.MultiService):
     """A ShareCrawler subclass is attached to a StorageServer, and
     periodically walks all of its shares, processing each one in some
@@ -90,7 +252,7 @@ class ShareCrawler(service.MultiService):
             self.allowed_cpu_percentage = allowed_cpu_percentage
         self.server = server
         self.sharedir = server.sharedir
-        self.statefile = statefile
+        self._state_serializer = _LeaseStateSerializer(statefile)
         self.prefixes = [si_b2a(struct.pack(">H", i << (16-10)))[:2]
                          for i in range(2**10)]
         if PY3:
@@ -213,8 +375,7 @@ class ShareCrawler(service.MultiService):
         #                            of the last bucket to be processed, or
         #                            None if we are sleeping between cycles
         try:
-            with open(self.statefile, "rb") as f:
+            state = self._state_serializer.load()
-                state = pickle.load(f)
         except Exception:
             state = {"version": 1,
                      "last-cycle-finished": None,
@@ -250,12 +411,7 @@ class ShareCrawler(service.MultiService):
         else:
             last_complete_prefix = self.prefixes[lcpi]
         self.state["last-complete-prefix"] = last_complete_prefix
-        tmpfile = self.statefile + ".tmp"
+        self._state_serializer.save(self.get_state())
-        with open(tmpfile, "wb") as f:
-            # Newer protocols won't work in Python 2; when it is dropped,
-            # protocol v4 can be used (added in Python 3.4).
-            pickle.dump(self.state, f, protocol=2)
-        fileutil.move_into_place(tmpfile, self.statefile)
 
     def startService(self):
         # arrange things to look like we were just sleeping, so

src/allmydata/storage/expirer.py

@@ -5,15 +5,69 @@ from __future__ import unicode_literals
 
 from future.utils import PY2
 if PY2:
-    # We omit anything that might end up in pickle, just in case.
+    from 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
-    from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, range, str, max, min  # noqa: F401
+import json
-
+import time
-import time, os, pickle, struct
+import os
-from allmydata.storage.crawler import ShareCrawler
+import struct
+from allmydata.storage.crawler import (
+    ShareCrawler,
+    _confirm_json_format,
+    _convert_cycle_data,
+    _dump_json_to_file,
+)
 from allmydata.storage.shares import get_share_file
 from allmydata.storage.common import UnknownMutableContainerVersionError, \
      UnknownImmutableContainerVersionError
 from twisted.python import log as twlog
+from twisted.python.filepath import FilePath
+
+
+def _convert_pickle_state_to_json(state):
+    """
+    Convert a pickle-serialized crawler-history state to the new JSON
+    format.
+
+    :param dict state: the pickled state
+
+    :return dict: the state in the JSON form
+    """
+    return {
+        str(k): _convert_cycle_data(v)
+        for k, v in state.items()
+    }
+
+
+class _HistorySerializer(object):
+    """
+    Serialize the 'history' file of the lease-crawler state. This is
+    "storage/lease_checker.history" for the pickle or
+    "storage/lease_checker.history.json" for the new JSON format.
+    """
+
+    def __init__(self, history_path):
+        self._path = _confirm_json_format(FilePath(history_path))
+
+        if not self._path.exists():
+            _dump_json_to_file({}, self._path)
+
+    def load(self):
+        """
+        Deserialize the existing data.
+
+        :return dict: the existing history state
+        """
+        with self._path.open("rb") as f:
+            history = json.load(f)
+        return history
+
+    def save(self, new_history):
+        """
+        Serialize the existing data as JSON.
+        """
+        _dump_json_to_file(new_history, self._path)
+        return None
+
 
 class LeaseCheckingCrawler(ShareCrawler):
     """I examine the leases on all shares, determining which are still valid
@@ -63,7 +117,7 @@ class LeaseCheckingCrawler(ShareCrawler):
                  override_lease_duration, # used if expiration_mode=="age"
                  cutoff_date, # used if expiration_mode=="cutoff-date"
                  sharetypes):
-        self.historyfile = historyfile
+        self._history_serializer = _HistorySerializer(historyfile)
         self.expiration_enabled = expiration_enabled
         self.mode = mode
         self.override_lease_duration = None
@@ -91,14 +145,6 @@ class LeaseCheckingCrawler(ShareCrawler):
         for k in so_far:
             self.state["cycle-to-date"].setdefault(k, so_far[k])
 
-        # initialize history
-        if not os.path.exists(self.historyfile):
-            history = {} # cyclenum -> dict
-            with open(self.historyfile, "wb") as f:
-                # Newer protocols won't work in Python 2; when it is dropped,
-                # protocol v4 can be used (added in Python 3.4).
-                pickle.dump(history, f, protocol=2)
-
     def create_empty_cycle_dict(self):
         recovered = self.create_empty_recovered_dict()
         so_far = {"corrupt-shares": [],
@@ -142,7 +188,7 @@ class LeaseCheckingCrawler(ShareCrawler):
                     struct.error):
                 twlog.msg("lease-checker error processing %s" % sharefile)
                 twlog.err()
-                which = (storage_index_b32, shnum)
+                which = [storage_index_b32, shnum]
                 self.state["cycle-to-date"]["corrupt-shares"].append(which)
                 wks = (1, 1, 1, "unknown")
             would_keep_shares.append(wks)
@@ -212,7 +258,7 @@ class LeaseCheckingCrawler(ShareCrawler):
                 num_valid_leases_configured += 1
 
         so_far = self.state["cycle-to-date"]
-        self.increment(so_far["leases-per-share-histogram"], num_leases, 1)
+        self.increment(so_far["leases-per-share-histogram"], str(num_leases), 1)
         self.increment_space("examined", s, sharetype)
 
         would_keep_share = [1, 1, 1, sharetype]
@@ -291,12 +337,14 @@ class LeaseCheckingCrawler(ShareCrawler):
 
         start = self.state["current-cycle-start-time"]
         now = time.time()
-        h["cycle-start-finish-times"] = (start, now)
+        h["cycle-start-finish-times"] = [start, now]
         h["expiration-enabled"] = self.expiration_enabled
-        h["configured-expiration-mode"] = (self.mode,
+        h["configured-expiration-mode"] = [
+            self.mode,
             self.override_lease_duration,
             self.cutoff_date,
-                                           self.sharetypes_to_expire)
+            self.sharetypes_to_expire,
+        ]
 
         s = self.state["cycle-to-date"]
 
@@ -314,16 +362,12 @@ class LeaseCheckingCrawler(ShareCrawler):
         # copy() needs to become a deepcopy
         h["space-recovered"] = s["space-recovered"].copy()
 
-        with open(self.historyfile, "rb") as f:
+        history = self._history_serializer.load()
-            history = pickle.load(f)
+        history[str(cycle)] = h
-        history[cycle] = h
         while len(history) > 10:
-            oldcycles = sorted(history.keys())
+            oldcycles = sorted(int(k) for k in history.keys())
-            del history[oldcycles[0]]
+            del history[str(oldcycles[0])]
-        with open(self.historyfile, "wb") as f:
+        self._history_serializer.save(history)
-            # Newer protocols won't work in Python 2; when it is dropped,
-            # protocol v4 can be used (added in Python 3.4).
-            pickle.dump(history, f, protocol=2)
 
     def get_state(self):
         """In addition to the crawler state described in
@@ -392,9 +436,7 @@ class LeaseCheckingCrawler(ShareCrawler):
         progress = self.get_progress()
 
         state = ShareCrawler.get_state(self) # does a shallow copy
-        with open(self.historyfile, "rb") as f:
+        state["history"] = self._history_serializer.load()
-            history = pickle.load(f)
-        state["history"] = history
 
         if not progress["cycle-in-progress"]:
             del state["cycle-to-date"]
@@ -406,10 +448,12 @@ class LeaseCheckingCrawler(ShareCrawler):
         lah = so_far["lease-age-histogram"]
         so_far["lease-age-histogram"] = self.convert_lease_age_histogram(lah)
         so_far["expiration-enabled"] = self.expiration_enabled
-        so_far["configured-expiration-mode"] = (self.mode,
+        so_far["configured-expiration-mode"] = [
+            self.mode,
             self.override_lease_duration,
             self.cutoff_date,
-                                                self.sharetypes_to_expire)
+            self.sharetypes_to_expire,
+        ]
 
         so_far_sr = so_far["space-recovered"]
         remaining_sr = {}

src/allmydata/storage/http_client.py

@@ -0,0 +1,79 @@
+"""
+HTTP client that talks to the HTTP storage server.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+
+if PY2:
+    # 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:
+    # typing module not available in Python 2, and we only do type checking in
+    # Python 3 anyway.
+    from typing import Union
+    from treq.testing import StubTreq
+
+import base64
+
+# TODO Make sure to import Python version?
+from cbor2 import loads
+
+
+from twisted.web.http_headers import Headers
+from twisted.internet.defer import inlineCallbacks, returnValue, fail
+from hyperlink import DecodedURL
+import treq
+
+
+class ClientException(Exception):
+    """An unexpected error."""
+
+
+def _decode_cbor(response):
+    """Given HTTP response, return decoded CBOR body."""
+    if response.code > 199 and response.code < 300:
+        return treq.content(response).addCallback(loads)
+    return fail(ClientException(response.code, response.phrase))
+
+
+def swissnum_auth_header(swissnum):  # type: (bytes) -> bytes
+    """Return value for ``Authentication`` header."""
+    return b"Tahoe-LAFS " + base64.b64encode(swissnum).strip()
+
+
+class StorageClient(object):
+    """
+    HTTP client that talks to the HTTP storage server.
+    """
+
+    def __init__(
+        self, url, swissnum, treq=treq
+    ):  # type: (DecodedURL, bytes, Union[treq,StubTreq]) -> None
+        self._base_url = url
+        self._swissnum = swissnum
+        self._treq = treq
+
+    def _get_headers(self):  # type: () -> Headers
+        """Return the basic headers to be used by default."""
+        headers = Headers()
+        headers.addRawHeader(
+            "Authorization",
+            swissnum_auth_header(self._swissnum),
+        )
+        return headers
+
+    @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())
+        decoded_response = yield _decode_cbor(response)
+        returnValue(decoded_response)

src/allmydata/storage/http_server.py

@@ -0,0 +1,94 @@
+"""
+HTTP server for storage.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+
+if PY2:
+    # 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
+
+from functools import wraps
+
+from klein import Klein
+from twisted.web import http
+
+# TODO Make sure to use pure Python versions?
+from cbor2 import dumps
+
+from .server import StorageServer
+from .http_client import swissnum_auth_header
+
+
+def _authorization_decorator(f):
+    """
+    Check the ``Authorization`` header, and (TODO: in later revision of code)
+    extract ``X-Tahoe-Authorization`` headers and pass them in.
+    """
+
+    @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
+
+
+def _authorized_route(app, *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.
+    """
+
+    def decorator(f):
+        @app.route(*route_args, **route_kwargs)
+        @_authorization_decorator
+        def handle_route(*args, **kwargs):
+            return f(*args, **kwargs)
+
+        return handle_route
+
+    return decorator
+
+
+class HTTPServer(object):
+    """
+    A HTTP interface to the storage server.
+    """
+
+    _app = Klein()
+
+    def __init__(
+        self, storage_server, swissnum
+    ):  # type: (StorageServer, bytes) -> None
+        self._storage_server = storage_server
+        self._swissnum = swissnum
+
+    def get_resource(self):
+        """Return twisted.web ``Resource`` for this object."""
+        return self._app.resource()
+
+    def _cbor(self, request, data):
+        """Return CBOR-encoded data."""
+        request.setHeader("Content-Type", "application/cbor")
+        # TODO if data is big, maybe want to use a temporary file eventually...
+        return dumps(data)
+
+    @_authorized_route(_app, "/v1/version", methods=["GET"])
+    def version(self, request, authorization):
+        return self._cbor(request, self._storage_server.remote_get_version())

src/allmydata/storage/immutable.py

@@ -21,27 +21,32 @@ from zope.interface import implementer
 from allmydata.interfaces import (
     RIBucketWriter, RIBucketReader, ConflictingWriteError,
     DataTooLargeError,
+    NoSpace,
 )
 from allmydata.util import base32, fileutil, log
 from allmydata.util.assertutil import precondition
-from allmydata.util.hashutil import timing_safe_compare
-from allmydata.storage.lease import LeaseInfo
 from allmydata.storage.common import UnknownImmutableContainerVersionError
 
+from .immutable_schema import (
+    NEWEST_SCHEMA_VERSION,
+    schema_from_version,
+)
+
+
 # each share file (in storage/shares/$SI/$SHNUM) contains lease information
 # and share data. The share data is accessed by RIBucketWriter.write and
 # RIBucketReader.read . The lease information is not accessible through these
 # interfaces.
 
 # The share file has the following layout:
-#  0x00: share file version number, four bytes, current version is 1
+#  0x00: share file version number, four bytes, current version is 2
 #  0x04: share data length, four bytes big-endian = A # See Footnote 1 below.
 #  0x08: number of leases, four bytes big-endian
 #  0x0c: beginning of share data (see immutable.layout.WriteBucketProxy)
 #  A+0x0c = B: first lease. Lease format is:
 #   B+0x00: owner number, 4 bytes big-endian, 0 is reserved for no-owner
-#   B+0x04: renew secret, 32 bytes (SHA256)
+#   B+0x04: renew secret, 32 bytes (SHA256 + blake2b) # See Footnote 2 below.
-#   B+0x24: cancel secret, 32 bytes (SHA256)
+#   B+0x24: cancel secret, 32 bytes (SHA256 + blake2b)
 #   B+0x44: expiration time, 4 bytes big-endian seconds-since-epoch
 #   B+0x48: next lease, or end of record
 
@@ -53,13 +58,126 @@ from allmydata.storage.common import UnknownImmutableContainerVersionError
 # then the value stored in this field will be the actual share data length
 # modulo 2**32.
 
+# Footnote 2: The change between share file version number 1 and 2 is that
+# storage of lease secrets is changed from plaintext to hashed.  This change
+# protects the secrets from compromises of local storage on the server: if a
+# plaintext cancel secret is somehow exfiltrated from the storage server, an
+# attacker could use it to cancel that lease and potentially cause user data
+# to be discarded before intended by the real owner.  As of this comment,
+# lease cancellation is disabled because there have been at least two bugs
+# which leak the persisted value of the cancellation secret.  If lease secrets
+# were stored hashed instead of plaintext then neither of these bugs would
+# have allowed an attacker to learn a usable cancel secret.
+#
+# Clients are free to construct these secrets however they like.  The
+# Tahoe-LAFS client uses a SHA256-based construction.  The server then uses
+# blake2b to hash these values for storage so that it retains no persistent
+# copy of the original secret.
+#
+
+def _fix_lease_count_format(lease_count_format):
+    """
+    Turn a single character struct format string into a format string suitable
+    for use in encoding and decoding the lease count value inside a share
+    file, if possible.
+
+    :param str lease_count_format: A single character format string like
+        ``"B"`` or ``"L"``.
+
+    :raise ValueError: If the given format string is not suitable for use
+        encoding and decoding a lease count.
+
+    :return str: A complete format string which can safely be used to encode
+        and decode lease counts in a share file.
+    """
+    if len(lease_count_format) != 1:
+        raise ValueError(
+            "Cannot construct ShareFile with lease_count_format={!r}; "
+            "format must accept a single value".format(
+                lease_count_format,
+            ),
+        )
+    # Make it big-endian with standard size so all platforms agree on the
+    # result.
+    fixed = ">" + lease_count_format
+    if struct.calcsize(fixed) > 4:
+        # There is only room for at most 4 bytes in the share file format so
+        # we can't allow any larger formats.
+        raise ValueError(
+            "Cannot construct ShareFile with lease_count_format={!r}; "
+            "size must be smaller than size of '>L'".format(
+                lease_count_format,
+            ),
+        )
+    return fixed
+
+
 class ShareFile(object):
+    """
+    Support interaction with persistent storage of a share.
+
+    :ivar str _lease_count_format: The format string which is used to encode
+        and decode the lease count inside the share file.  As stated in the
+        comment in this module there is room for at most 4 bytes in this part
+        of the file.  A format string that works on fewer bytes is allowed to
+        restrict the number of leases allowed in the share file to a smaller
+        number than could be supported by using the full 4 bytes.  This is
+        mostly of interest for testing.
+    """
     LEASE_SIZE = struct.calcsize(">L32s32sL")
     sharetype = "immutable"
 
-    def __init__(self, filename, max_size=None, create=False):
+    @classmethod
-        """ If max_size is not None then I won't allow more than max_size to be written to me. If create=True and max_size must not be None. """
+    def is_valid_header(cls, header):
+        # type: (bytes) -> bool
+        """
+        Determine if the given bytes constitute a valid header for this type of
+        container.
+
+        :param header: Some bytes from the beginning of a container.
+
+        :return: ``True`` if the bytes could belong to this container,
+            ``False`` otherwise.
+        """
+        (version,) = struct.unpack(">L", header[:4])
+        return schema_from_version(version) is not None
+
+    def __init__(
+            self,
+            filename,
+            max_size=None,
+            create=False,
+            lease_count_format="L",
+            schema=NEWEST_SCHEMA_VERSION,
+    ):
+        """
+        Initialize a ``ShareFile``.
+
+        :param Optional[int] max_size: If given, the maximum number of bytes
+           that this ``ShareFile`` will accept to be stored.
+
+        :param bool create: If ``True``, create the file (and fail if it
+            exists already).  ``max_size`` must not be ``None`` in this case.
+            If ``False``, open an existing file for reading.
+
+        :param str lease_count_format: A format character to use to encode and
+            decode the number of leases in the share file.  There are only 4
+            bytes available in the file so the format must be 4 bytes or
+            smaller.  If different formats are used at different times with
+            the same share file, the result will likely be nonsense.
+
+            This parameter is intended for the test suite to use to be able to
+            exercise values near the maximum encodeable value without having
+            to create billions of leases.
+
+        :raise ValueError: If the encoding of ``lease_count_format`` is too
+            large or if it is not a single format character.
+        """
+
         precondition((max_size is not None) or (not create), max_size, create)
+
+        self._lease_count_format = _fix_lease_count_format(lease_count_format)
+        self._lease_count_size = struct.calcsize(self._lease_count_format)
         self.home = filename
         self._max_size = max_size
         if create:
@@ -67,27 +185,18 @@ class ShareFile(object):
             # it. Also construct the metadata.
             assert not os.path.exists(self.home)
             fileutil.make_dirs(os.path.dirname(self.home))
-            # The second field -- the four-byte share data length -- is no
+            self._schema = schema
-            # longer used as of Tahoe v1.3.0, but we continue to write it in
-            # there in case someone downgrades a storage server from >=
-            # Tahoe-1.3.0 to < Tahoe-1.3.0, or moves a share file from one
-            # server to another, etc. We do saturation -- a share data length
-            # larger than 2**32-1 (what can fit into the field) is marked as
-            # the largest length that can fit into the field. That way, even
-            # if this does happen, the old < v1.3.0 server will still allow
-            # clients to read the first part of the share.
             with open(self.home, 'wb') as f:
-                f.write(struct.pack(">LLL", 1, min(2**32-1, max_size), 0))
+                f.write(self._schema.header(max_size))
             self._lease_offset = max_size + 0x0c
             self._num_leases = 0
         else:
             with open(self.home, 'rb') as f:
                 filesize = os.path.getsize(self.home)
                 (version, unused, num_leases) = struct.unpack(">LLL", f.read(0xc))
-            if version != 1:
+            self._schema = schema_from_version(version)
-                msg = "sharefile %s had version %d but we wanted 1" % \
+            if self._schema is None:
-                      (filename, version)
+                raise UnknownImmutableContainerVersionError(filename, version)
-                raise UnknownImmutableContainerVersionError(msg)
             self._num_leases = num_leases
             self._lease_offset = filesize - (num_leases * self.LEASE_SIZE)
         self._data_offset = 0xc
@@ -122,16 +231,25 @@ class ShareFile(object):
         offset = self._lease_offset + lease_number * self.LEASE_SIZE
         f.seek(offset)
         assert f.tell() == offset
-        f.write(lease_info.to_immutable_data())
+        f.write(self._schema.lease_serializer.serialize(lease_info))
 
     def _read_num_leases(self, f):
         f.seek(0x08)
-        (num_leases,) = struct.unpack(">L", f.read(4))
+        (num_leases,) = struct.unpack(
+            self._lease_count_format,
+            f.read(self._lease_count_size),
+        )
         return num_leases
 
     def _write_num_leases(self, f, num_leases):
+        self._write_encoded_num_leases(
+            f,
+            struct.pack(self._lease_count_format, num_leases),
+        )
+
+    def _write_encoded_num_leases(self, f, encoded_num_leases):
         f.seek(0x08)
-        f.write(struct.pack(">L", num_leases))
+        f.write(encoded_num_leases)
 
     def _truncate_leases(self, f, num_leases):
         f.truncate(self._lease_offset + num_leases * self.LEASE_SIZE)
@@ -144,34 +262,63 @@ class ShareFile(object):
             for i in range(num_leases):
                 data = f.read(self.LEASE_SIZE)
                 if data:
-                    yield LeaseInfo().from_immutable_data(data)
+                    yield self._schema.lease_serializer.unserialize(data)
 
     def add_lease(self, lease_info):
         with open(self.home, 'rb+') as f:
             num_leases = self._read_num_leases(f)
+            # Before we write the new lease record, make sure we can encode
+            # the new lease count.
+            new_lease_count = struct.pack(self._lease_count_format, num_leases + 1)
             self._write_lease_record(f, num_leases, lease_info)
-            self._write_num_leases(f, num_leases+1)
+            self._write_encoded_num_leases(f, new_lease_count)
 
-    def renew_lease(self, renew_secret, new_expire_time):
+    def renew_lease(self, renew_secret, new_expire_time, allow_backdate=False):
+        # type: (bytes, int, bool) -> None
+        """
+        Update the expiration time on an existing lease.
+
+        :param allow_backdate: If ``True`` then allow the new expiration time
+            to be before the current expiration time.  Otherwise, make no
+            change when this is the case.
+
+        :raise IndexError: If there is no lease matching the given renew
+            secret.
+        """
         for i,lease in enumerate(self.get_leases()):
-            if timing_safe_compare(lease.renew_secret, renew_secret):
+            if lease.is_renew_secret(renew_secret):
                 # yup. See if we need to update the owner time.
-                if new_expire_time > lease.expiration_time:
+                if allow_backdate or new_expire_time > lease.get_expiration_time():
                     # yes
-                    lease.expiration_time = new_expire_time
+                    lease = lease.renew(new_expire_time)
                     with open(self.home, 'rb+') as f:
                         self._write_lease_record(f, i, lease)
                 return
         raise IndexError("unable to renew non-existent lease")
 
-    def add_or_renew_lease(self, lease_info):
+    def add_or_renew_lease(self, available_space, lease_info):
+        """
+        Renew an existing lease if possible, otherwise allocate a new one.
+
+        :param int available_space: The maximum number of bytes of storage to
+            commit in this operation.  If more than this number of bytes is
+            required, raise ``NoSpace`` instead.
+
+        :param LeaseInfo lease_info: The details of the lease to renew or add.
+
+        :raise NoSpace: If more than ``available_space`` bytes is required to
+            complete the operation.  In this case, no lease is added.
+
+        :return: ``None``
+        """
         try:
             self.renew_lease(lease_info.renew_secret,
-                             lease_info.expiration_time)
+                             lease_info.get_expiration_time())
         except IndexError:
+            if lease_info.immutable_size() > available_space:
+                raise NoSpace()
             self.add_lease(lease_info)
 
-
     def cancel_lease(self, cancel_secret):
         """Remove a lease with the given cancel_secret. If the last lease is
         cancelled, the file will be removed. Return the number of bytes that
@@ -183,7 +330,7 @@ class ShareFile(object):
         leases = list(self.get_leases())
         num_leases_removed = 0
         for i,lease in enumerate(leases):
-            if timing_safe_compare(lease.cancel_secret, cancel_secret):
+            if lease.is_cancel_secret(cancel_secret):
                 leases[i] = None
                 num_leases_removed += 1
         if not num_leases_removed:
@@ -208,7 +355,7 @@ class ShareFile(object):
 @implementer(RIBucketWriter)
 class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
 
-    def __init__(self, ss, incominghome, finalhome, max_size, lease_info):
+    def __init__(self, ss, incominghome, finalhome, max_size, lease_info, clock):
         self.ss = ss
         self.incominghome = incominghome
         self.finalhome = finalhome
@@ -220,12 +367,16 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
         # added by simultaneous uploaders
         self._sharefile.add_lease(lease_info)
         self._already_written = RangeMap()
+        self._clock = clock
+        self._timeout = clock.callLater(30 * 60, self._abort_due_to_timeout)
 
     def allocated_size(self):
         return self._max_size
 
     def remote_write(self, offset, data):
-        start = time.time()
+        # Delay the timeout, since we received data:
+        self._timeout.reset(30 * 60)
+        start = self._clock.seconds()
         precondition(not self.closed)
         if self.throw_out_all_data:
             return
@@ -243,12 +394,16 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
         self._sharefile.write_share_data(offset, data)
 
         self._already_written.set(True, offset, end)
-        self.ss.add_latency("write", time.time() - start)
+        self.ss.add_latency("write", self._clock.seconds() - start)
         self.ss.count("write")
 
     def remote_close(self):
+        self.close()
+
+    def close(self):
         precondition(not self.closed)
-        start = time.time()
+        self._timeout.cancel()
+        start = self._clock.seconds()
 
         fileutil.make_dirs(os.path.dirname(self.finalhome))
         fileutil.rename(self.incominghome, self.finalhome)
@@ -281,20 +436,28 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
 
         filelen = os.stat(self.finalhome)[stat.ST_SIZE]
         self.ss.bucket_writer_closed(self, filelen)
-        self.ss.add_latency("close", time.time() - start)
+        self.ss.add_latency("close", self._clock.seconds() - start)
         self.ss.count("close")
 
     def disconnected(self):
         if not self.closed:
-            self._abort()
+            self.abort()
+
+    def _abort_due_to_timeout(self):
+        """
+        Called if we run out of time.
+        """
+        log.msg("storage: aborting sharefile %s due to timeout" % self.incominghome,
+                facility="tahoe.storage", level=log.UNUSUAL)
+        self.abort()
 
     def remote_abort(self):
         log.msg("storage: aborting sharefile %s" % self.incominghome,
                 facility="tahoe.storage", level=log.UNUSUAL)
-        self._abort()
+        self.abort()
         self.ss.count("abort")
 
-    def _abort(self):
+    def abort(self):
         if self.closed:
             return
 
@@ -312,6 +475,10 @@ class BucketWriter(Referenceable):  # type: ignore # warner/foolscap#78
         self.closed = True
         self.ss.bucket_writer_closed(self, 0)
 
+        # Cancel timeout if it wasn't already cancelled.
+        if self._timeout.active():
+            self._timeout.cancel()
+
 
 @implementer(RIBucketReader)
 class BucketReader(Referenceable):  # type: ignore # warner/foolscap#78

src/allmydata/storage/immutable_schema.py

@@ -0,0 +1,72 @@
+"""
+Ported to Python 3.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    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
+
+import struct
+
+import attr
+
+from .lease_schema import (
+    v1_immutable,
+    v2_immutable,
+)
+
+@attr.s(frozen=True)
+class _Schema(object):
+    """
+    Implement encoding and decoding for multiple versions of the immutable
+    container schema.
+
+    :ivar int version: the version number of the schema this object supports
+
+    :ivar lease_serializer: an object that is responsible for lease
+        serialization and unserialization
+    """
+    version = attr.ib()
+    lease_serializer = attr.ib()
+
+    def header(self, max_size):
+        # type: (int) -> bytes
+        """
+        Construct a container header.
+
+        :param max_size: the maximum size the container can hold
+
+        :return: the header bytes
+        """
+        # The second field -- the four-byte share data length -- is no longer
+        # used as of Tahoe v1.3.0, but we continue to write it in there in
+        # case someone downgrades a storage server from >= Tahoe-1.3.0 to <
+        # Tahoe-1.3.0, or moves a share file from one server to another,
+        # etc. We do saturation -- a share data length larger than 2**32-1
+        # (what can fit into the field) is marked as the largest length that
+        # can fit into the field. That way, even if this does happen, the old
+        # < v1.3.0 server will still allow clients to read the first part of
+        # the share.
+        return struct.pack(">LLL", self.version, min(2**32 - 1, max_size), 0)
+
+ALL_SCHEMAS = {
+    _Schema(version=2, lease_serializer=v2_immutable),
+    _Schema(version=1, lease_serializer=v1_immutable),
+}
+ALL_SCHEMA_VERSIONS = {schema.version for schema in ALL_SCHEMAS}
+NEWEST_SCHEMA_VERSION = max(ALL_SCHEMAS, key=lambda schema: schema.version)
+
+def schema_from_version(version):
+    # (int) -> Optional[type]
+    """
+    Find the schema object that corresponds to a certain version number.
+    """
+    for schema in ALL_SCHEMAS:
+        if schema.version == version:
+            return schema
+    return None

src/allmydata/storage/lease.py

@@ -13,52 +13,375 @@ if PY2:
 
 import struct, time
 
+import attr
+
+from zope.interface import (
+    Interface,
+    implementer,
+)
+
+from twisted.python.components import (
+    proxyForInterface,
+)
+
+from allmydata.util.hashutil import timing_safe_compare
+from allmydata.util import base32
+
+# struct format for representation of a lease in an immutable share
+IMMUTABLE_FORMAT = ">L32s32sL"
+
+# struct format for representation of a lease in a mutable share
+MUTABLE_FORMAT = ">LL32s32s20s"
+
+
+class ILeaseInfo(Interface):
+    """
+    Represent a marker attached to a share that indicates that share should be
+    retained for some amount of time.
+
+    Typically clients will create and renew leases on their shares as a way to
+    inform storage servers that there is still interest in those shares.  A
+    share may have more than one lease.  If all leases on a share have
+    expiration times in the past then the storage server may take this as a
+    strong hint that no one is interested in the share anymore and therefore
+    the share may be deleted to reclaim the space.
+    """
+    def renew(new_expire_time):
+        """
+        Create a new ``ILeaseInfo`` with the given expiration time.
+
+        :param Union[int, float] new_expire_time: The expiration time the new
+            ``ILeaseInfo`` will have.
+
+        :return: The new ``ILeaseInfo`` provider with the new expiration time.
+        """
+
+    def get_expiration_time():
+        """
+        :return Union[int, float]: this lease's expiration time
+        """
+
+    def get_grant_renew_time_time():
+        """
+        :return Union[int, float]: a guess about the last time this lease was
+            renewed
+        """
+
+    def get_age():
+        """
+        :return Union[int, float]: a guess about how long it has been since this
+            lease was renewed
+        """
+
+    def to_immutable_data():
+        """
+        :return bytes: a serialized representation of this lease suitable for
+            inclusion in an immutable container
+        """
+
+    def to_mutable_data():
+        """
+        :return bytes: a serialized representation of this lease suitable for
+            inclusion in a mutable container
+        """
+
+    def immutable_size():
+        """
+        :return int: the size of the serialized representation of this lease in an
+            immutable container
+        """
+
+    def mutable_size():
+        """
+        :return int: the size of the serialized representation of this lease in a
+            mutable container
+        """
+
+    def is_renew_secret(candidate_secret):
+        """
+        :return bool: ``True`` if the given byte string is this lease's renew
+            secret, ``False`` otherwise
+        """
+
+    def present_renew_secret():
+        """
+        :return str: Text which could reasonably be shown to a person representing
+            this lease's renew secret.
+        """
+
+    def is_cancel_secret(candidate_secret):
+        """
+        :return bool: ``True`` if the given byte string is this lease's cancel
+            secret, ``False`` otherwise
+        """
+
+    def present_cancel_secret():
+        """
+        :return str: Text which could reasonably be shown to a person representing
+            this lease's cancel secret.
+        """
+
+
+@implementer(ILeaseInfo)
+@attr.s(frozen=True)
 class LeaseInfo(object):
-    def __init__(self, owner_num=None, renew_secret=None, cancel_secret=None,
+    """
-                 expiration_time=None, nodeid=None):
+    Represent the details of one lease, a marker which is intended to inform
-        self.owner_num = owner_num
+    the storage server how long to store a particular share.
-        self.renew_secret = renew_secret
+    """
-        self.cancel_secret = cancel_secret
+    owner_num = attr.ib(default=None)
-        self.expiration_time = expiration_time
+
-        if nodeid is not None:
+    # Don't put secrets into the default string representation.  This makes it
-            assert isinstance(nodeid, bytes)
+    # slightly less likely the secrets will accidentally be leaked to
-            assert len(nodeid) == 20
+    # someplace they're not meant to be.
-        self.nodeid = nodeid
+    renew_secret = attr.ib(default=None, repr=False)
+    cancel_secret = attr.ib(default=None, repr=False)
+
+    _expiration_time = attr.ib(default=None)
+
+    nodeid = attr.ib(default=None)
+
+    @nodeid.validator
+    def _validate_nodeid(self, attribute, value):
+        if value is not None:
+            if not isinstance(value, bytes):
+                raise ValueError(
+                    "nodeid value must be bytes, not {!r}".format(value),
+                )
+            if len(value) != 20:
+                raise ValueError(
+                    "nodeid value must be 20 bytes long, not {!r}".format(value),
+                )
+        return None
 
     def get_expiration_time(self):
-        return self.expiration_time
+        # type: () -> float
+        """
+        Retrieve a POSIX timestamp representing the time at which this lease is
+        set to expire.
+        """
+        return self._expiration_time
+
+    def renew(self, new_expire_time):
+        # type: (float) -> LeaseInfo
+        """
+        Create a new lease the same as this one but with a new expiration time.
+
+        :param new_expire_time: The new expiration time.
+
+        :return: The new lease info.
+        """
+        return attr.assoc(
+            self,
+            _expiration_time=new_expire_time,
+        )
+
+    def is_renew_secret(self, candidate_secret):
+        # type: (bytes) -> bool
+        """
+        Check a string to see if it is the correct renew secret.
+
+        :return: ``True`` if it is the correct renew secret, ``False``
+            otherwise.
+        """
+        return timing_safe_compare(self.renew_secret, candidate_secret)
+
+    def present_renew_secret(self):
+        # type: () -> str
+        """
+        Return the renew secret, base32-encoded.
+        """
+        return str(base32.b2a(self.renew_secret), "utf-8")
+
+    def is_cancel_secret(self, candidate_secret):
+        # type: (bytes) -> bool
+        """
+        Check a string to see if it is the correct cancel secret.
+
+        :return: ``True`` if it is the correct cancel secret, ``False``
+            otherwise.
+        """
+        return timing_safe_compare(self.cancel_secret, candidate_secret)
+
+    def present_cancel_secret(self):
+        # type: () -> str
+        """
+        Return the cancel secret, base32-encoded.
+        """
+        return str(base32.b2a(self.cancel_secret), "utf-8")
 
     def get_grant_renew_time_time(self):
         # hack, based upon fixed 31day expiration period
-        return self.expiration_time - 31*24*60*60
+        return self._expiration_time - 31*24*60*60
 
     def get_age(self):
         return time.time() - self.get_grant_renew_time_time()
 
-    def from_immutable_data(self, data):
+    @classmethod
-        (self.owner_num,
+    def from_immutable_data(cls, data):
-         self.renew_secret,
+        """
-         self.cancel_secret,
+        Create a new instance from the encoded data given.
-         self.expiration_time) = struct.unpack(">L32s32sL", data)
+
-        self.nodeid = None
+        :param data: A lease serialized using the immutable-share-file format.
-        return self
+        """
+        names = [
+            "owner_num",
+            "renew_secret",
+            "cancel_secret",
+            "expiration_time",
+        ]
+        values = struct.unpack(IMMUTABLE_FORMAT, data)
+        return cls(nodeid=None, **dict(zip(names, values)))
+
+    def immutable_size(self):
+        """
+        :return int: The size, in bytes, of the representation of this lease in an
+            immutable share file.
+        """
+        return struct.calcsize(IMMUTABLE_FORMAT)
+
+    def mutable_size(self):
+        """
+        :return int: The size, in bytes, of the representation of this lease in a
+            mutable share file.
+        """
+        return struct.calcsize(MUTABLE_FORMAT)
 
     def to_immutable_data(self):
-        return struct.pack(">L32s32sL",
+        return struct.pack(IMMUTABLE_FORMAT,
                            self.owner_num,
                            self.renew_secret, self.cancel_secret,
-                           int(self.expiration_time))
+                           int(self._expiration_time))
 
     def to_mutable_data(self):
-        return struct.pack(">LL32s32s20s",
+        return struct.pack(MUTABLE_FORMAT,
                            self.owner_num,
-                           int(self.expiration_time),
+                           int(self._expiration_time),
                            self.renew_secret, self.cancel_secret,
                            self.nodeid)
 
-    def from_mutable_data(self, data):
+    @classmethod
-        (self.owner_num,
+    def from_mutable_data(cls, data):
-         self.expiration_time,
+        """
-         self.renew_secret, self.cancel_secret,
+        Create a new instance from the encoded data given.
-         self.nodeid) = struct.unpack(">LL32s32s20s", data)
+
-        return self
+        :param data: A lease serialized using the mutable-share-file format.
+        """
+        names = [
+            "owner_num",
+            "expiration_time",
+            "renew_secret",
+            "cancel_secret",
+            "nodeid",
+        ]
+        values = struct.unpack(MUTABLE_FORMAT, data)
+        return cls(**dict(zip(names, values)))
+
+
+@attr.s(frozen=True)
+class HashedLeaseInfo(proxyForInterface(ILeaseInfo, "_lease_info")): # type: ignore # unsupported dynamic base class
+    """
+    A ``HashedLeaseInfo`` wraps lease information in which the secrets have
+    been hashed.
+    """
+    _lease_info = attr.ib()
+    _hash = attr.ib()
+
+    # proxyForInterface will take care of forwarding all methods on ILeaseInfo
+    # to `_lease_info`.  Here we override a few of those methods to adjust
+    # their behavior to make them suitable for use with hashed secrets.
+
+    def renew(self, new_expire_time):
+        # Preserve the HashedLeaseInfo wrapper around the renewed LeaseInfo.
+        return attr.assoc(
+            self,
+            _lease_info=super(HashedLeaseInfo, self).renew(new_expire_time),
+        )
+
+    def is_renew_secret(self, candidate_secret):
+        # type: (bytes) -> bool
+        """
+        Hash the candidate secret and compare the result to the stored hashed
+        secret.
+        """
+        return super(HashedLeaseInfo, self).is_renew_secret(self._hash(candidate_secret))
+
+    def present_renew_secret(self):
+        # type: () -> str
+        """
+        Present the hash of the secret with a marker indicating it is a hash.
+        """
+        return u"hash:" + super(HashedLeaseInfo, self).present_renew_secret()
+
+    def is_cancel_secret(self, candidate_secret):
+        # type: (bytes) -> bool
+        """
+        Hash the candidate secret and compare the result to the stored hashed
+        secret.
+        """
+        if isinstance(candidate_secret, _HashedCancelSecret):
+            # Someone read it off of this object in this project - probably
+            # the lease crawler - and is just trying to use it to identify
+            # which lease it wants to operate on.  Avoid re-hashing the value.
+            #
+            # It is important that this codepath is only availably internally
+            # for this process to talk to itself.  If it were to be exposed to
+            # clients over the network, they could just provide the hashed
+            # value to avoid having to ever learn the original value.
+            hashed_candidate = candidate_secret.hashed_value
+        else:
+            # It is not yet hashed so hash it.
+            hashed_candidate = self._hash(candidate_secret)
+
+        return super(HashedLeaseInfo, self).is_cancel_secret(hashed_candidate)
+
+    def present_cancel_secret(self):
+        # type: () -> str
+        """
+        Present the hash of the secret with a marker indicating it is a hash.
+        """
+        return u"hash:" + super(HashedLeaseInfo, self).present_cancel_secret()
+
+    @property
+    def owner_num(self):
+        return self._lease_info.owner_num
+
+    @property
+    def nodeid(self):
+        return self._lease_info.nodeid
+
+    @property
+    def cancel_secret(self):
+        """
+        Give back an opaque wrapper around the hashed cancel secret which can
+        later be presented for a succesful equality comparison.
+        """
+        # We don't *have* the cancel secret.  We hashed it and threw away the
+        # original.  That's good.  It does mean that some code that runs
+        # in-process with the storage service (LeaseCheckingCrawler) runs into
+        # some difficulty.  That code wants to cancel leases and does so using
+        # the same interface that faces storage clients (or would face them,
+        # if lease cancellation were exposed).
+        #
+        # Since it can't use the hashed secret to cancel a lease (that's the
+        # point of the hashing) and we don't have the unhashed secret to give
+        # it, instead we give it a marker that `cancel_lease` will recognize.
+        # On recognizing it, if the hashed value given matches the hashed
+        # value stored it is considered a match and the lease can be
+        # cancelled.
+        #
+        # This isn't great.  Maybe the internal and external consumers of
+        # cancellation should use different interfaces.
+        return _HashedCancelSecret(self._lease_info.cancel_secret)
+
+
+@attr.s(frozen=True)
+class _HashedCancelSecret(object):
+    """
+    ``_HashedCancelSecret`` is a marker type for an already-hashed lease
+    cancel secret that lets internal lease cancellers bypass the hash-based
+    protection that's imposed on external lease cancellers.
+
+    :ivar bytes hashed_value: The already-hashed secret.
+    """
+    hashed_value = attr.ib()

src/allmydata/storage/lease_schema.py

@@ -0,0 +1,138 @@
+"""
+Ported to Python 3.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    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
+
+try:
+    from typing import Union
+except ImportError:
+    pass
+
+import attr
+
+from nacl.hash import blake2b
+from nacl.encoding import RawEncoder
+
+from .lease import (
+    LeaseInfo,
+    HashedLeaseInfo,
+)
+
+@attr.s(frozen=True)
+class CleartextLeaseSerializer(object):
+    """
+    Serialize and unserialize leases with cleartext secrets.
+    """
+    _to_data = attr.ib()
+    _from_data = attr.ib()
+
+    def serialize(self, lease):
+        # type: (LeaseInfo) -> bytes
+        """
+        Represent the given lease as bytes with cleartext secrets.
+        """
+        if isinstance(lease, LeaseInfo):
+            return self._to_data(lease)
+        raise ValueError(
+            "ShareFile v1 schema only supports LeaseInfo, not {!r}".format(
+                lease,
+            ),
+        )
+
+    def unserialize(self, data):
+        # type: (bytes) -> LeaseInfo
+        """
+        Load a lease with cleartext secrets from the given bytes representation.
+        """
+        # In v1 of the immutable schema lease secrets are stored plaintext.
+        # So load the data into a plain LeaseInfo which works on plaintext
+        # secrets.
+        return self._from_data(data)
+
+@attr.s(frozen=True)
+class HashedLeaseSerializer(object):
+    _to_data = attr.ib()
+    _from_data = attr.ib()
+
+    @classmethod
+    def _hash_secret(cls, secret):
+        # type: (bytes) -> bytes
+        """
+        Hash a lease secret for storage.
+        """
+        return blake2b(secret, digest_size=32, encoder=RawEncoder())
+
+    @classmethod
+    def _hash_lease_info(cls, lease_info):
+        # type: (LeaseInfo) -> HashedLeaseInfo
+        """
+        Hash the cleartext lease info secrets into a ``HashedLeaseInfo``.
+        """
+        if not isinstance(lease_info, LeaseInfo):
+            # Provide a little safety against misuse, especially an attempt to
+            # re-hash an already-hashed lease info which is represented as a
+            # different type.
+            raise TypeError(
+                "Can only hash LeaseInfo, not {!r}".format(lease_info),
+            )
+
+        # Hash the cleartext secrets in the lease info and wrap the result in
+        # a new type.
+        return HashedLeaseInfo(
+            attr.assoc(
+                lease_info,
+                renew_secret=cls._hash_secret(lease_info.renew_secret),
+                cancel_secret=cls._hash_secret(lease_info.cancel_secret),
+            ),
+            cls._hash_secret,
+        )
+
+    def serialize(self, lease):
+        # type: (Union[LeaseInfo, HashedLeaseInfo]) -> bytes
+        if isinstance(lease, LeaseInfo):
+            # v2 of the immutable schema stores lease secrets hashed.  If
+            # we're given a LeaseInfo then it holds plaintext secrets.  Hash
+            # them before trying to serialize.
+            lease = self._hash_lease_info(lease)
+        if isinstance(lease, HashedLeaseInfo):
+            return self._to_data(lease)
+        raise ValueError(
+            "ShareFile v2 schema cannot represent lease {!r}".format(
+                lease,
+            ),
+        )
+
+    def unserialize(self, data):
+        # type: (bytes) -> HashedLeaseInfo
+        # In v2 of the immutable schema lease secrets are stored hashed.  Wrap
+        # a LeaseInfo in a HashedLeaseInfo so it can supply the correct
+        # interpretation for those values.
+        return HashedLeaseInfo(self._from_data(data), self._hash_secret)
+
+v1_immutable = CleartextLeaseSerializer(
+    LeaseInfo.to_immutable_data,
+    LeaseInfo.from_immutable_data,
+)
+
+v2_immutable = HashedLeaseSerializer(
+    HashedLeaseInfo.to_immutable_data,
+    LeaseInfo.from_immutable_data,
+)
+
+v1_mutable = CleartextLeaseSerializer(
+    LeaseInfo.to_mutable_data,
+    LeaseInfo.from_mutable_data,
+)
+
+v2_mutable = HashedLeaseSerializer(
+    HashedLeaseInfo.to_mutable_data,
+    LeaseInfo.from_mutable_data,
+)

src/allmydata/storage/mutable.py

@@ -13,7 +13,10 @@ if PY2:
 
 import os, stat, struct
 
-from allmydata.interfaces import BadWriteEnablerError
+from allmydata.interfaces import (
+    BadWriteEnablerError,
+    NoSpace,
+)
 from allmydata.util import idlib, log
 from allmydata.util.assertutil import precondition
 from allmydata.util.hashutil import timing_safe_compare
@@ -21,7 +24,10 @@ from allmydata.storage.lease import LeaseInfo
 from allmydata.storage.common import UnknownMutableContainerVersionError, \
      DataTooLargeError
 from allmydata.mutable.layout import MAX_MUTABLE_SHARE_SIZE
-
+from .mutable_schema import (
+    NEWEST_SCHEMA_VERSION,
+    schema_from_header,
+)
 
 # the MutableShareFile is like the ShareFile, but used for mutable data. It
 # has a different layout. See docs/mutable.txt for more details.
@@ -61,26 +67,34 @@ class MutableShareFile(object):
     # our sharefiles share with a recognizable string, plus some random
     # binary data to reduce the chance that a regular text file will look
     # like a sharefile.
-    MAGIC = b"Tahoe mutable container v1\n" + b"\x75\x09\x44\x03\x8e"
-    assert len(MAGIC) == 32
-    assert isinstance(MAGIC, bytes)
     MAX_SIZE = MAX_MUTABLE_SHARE_SIZE
     # TODO: decide upon a policy for max share size
 
-    def __init__(self, filename, parent=None):
+    @classmethod
+    def is_valid_header(cls, header):
+        # type: (bytes) -> bool
+        """
+        Determine if the given bytes constitute a valid header for this type of
+        container.
+
+        :param header: Some bytes from the beginning of a container.
+
+        :return: ``True`` if the bytes could belong to this container,
+            ``False`` otherwise.
+        """
+        return schema_from_header(header) is not None
+
+    def __init__(self, filename, parent=None, schema=NEWEST_SCHEMA_VERSION):
         self.home = filename
         if os.path.exists(self.home):
             # we don't cache anything, just check the magic
             with open(self.home, 'rb') as f:
-                data = f.read(self.HEADER_SIZE)
+                header = f.read(self.HEADER_SIZE)
-            (magic,
+            self._schema = schema_from_header(header)
-             write_enabler_nodeid, write_enabler,
+            if self._schema is None:
-             data_length, extra_least_offset) = \
+                raise UnknownMutableContainerVersionError(filename, header)
-             struct.unpack(">32s20s32sQQ", data)
+        else:
-            if magic != self.MAGIC:
+            self._schema = schema
-                msg = "sharefile %s had magic '%r' but we wanted '%r'" % \
-                      (filename, magic, self.MAGIC)
-                raise UnknownMutableContainerVersionError(msg)
         self.parent = parent # for logging
 
     def log(self, *args, **kwargs):
@@ -88,23 +102,8 @@ class MutableShareFile(object):
 
     def create(self, my_nodeid, write_enabler):
         assert not os.path.exists(self.home)
-        data_length = 0
-        extra_lease_offset = (self.HEADER_SIZE
-                              + 4 * self.LEASE_SIZE
-                              + data_length)
-        assert extra_lease_offset == self.DATA_OFFSET # true at creation
-        num_extra_leases = 0
         with open(self.home, 'wb') as f:
-            header = struct.pack(
+            f.write(self._schema.header(my_nodeid, write_enabler))
-                ">32s20s32sQQ",
-                self.MAGIC, my_nodeid, write_enabler,
-                data_length, extra_lease_offset,
-            )
-            leases = (b"\x00" * self.LEASE_SIZE) * 4
-            f.write(header + leases)
-            # data goes here, empty after creation
-            f.write(struct.pack(">L", num_extra_leases))
-            # extra leases go here, none at creation
 
     def unlink(self):
         os.unlink(self.home)
@@ -120,6 +119,7 @@ class MutableShareFile(object):
 
     def _read_share_data(self, f, offset, length):
         precondition(offset >= 0)
+        precondition(length >= 0)
         data_length = self._read_data_length(f)
         if offset+length > data_length:
             # reads beyond the end of the data are truncated. Reads that
@@ -236,7 +236,7 @@ class MutableShareFile(object):
                       + (lease_number-4)*self.LEASE_SIZE)
         f.seek(offset)
         assert f.tell() == offset
-        f.write(lease_info.to_mutable_data())
+        f.write(self._schema.lease_serializer.serialize(lease_info))
 
     def _read_lease_record(self, f, lease_number):
         # returns a LeaseInfo instance, or None
@@ -253,7 +253,7 @@ class MutableShareFile(object):
         f.seek(offset)
         assert f.tell() == offset
         data = f.read(self.LEASE_SIZE)
-        lease_info = LeaseInfo().from_mutable_data(data)
+        lease_info = self._schema.lease_serializer.unserialize(data)
         if lease_info.owner_num == 0:
             return None
         return lease_info
@@ -288,7 +288,19 @@ class MutableShareFile(object):
             except IndexError:
                 return
 
-    def add_lease(self, lease_info):
+    def add_lease(self, available_space, lease_info):
+        """
+        Add a new lease to this share.
+
+        :param int available_space: The maximum number of bytes of storage to
+            commit in this operation.  If more than this number of bytes is
+            required, raise ``NoSpace`` instead.
+
+        :raise NoSpace: If more than ``available_space`` bytes is required to
+            complete the operation.  In this case, no lease is added.
+
+        :return: ``None``
+        """
         precondition(lease_info.owner_num != 0) # 0 means "no lease here"
         with open(self.home, 'rb+') as f:
             num_lease_slots = self._get_num_lease_slots(f)
@@ -296,17 +308,30 @@ class MutableShareFile(object):
             if empty_slot is not None:
                 self._write_lease_record(f, empty_slot, lease_info)
             else:
+                if lease_info.mutable_size() > available_space:
+                    raise NoSpace()
                 self._write_lease_record(f, num_lease_slots, lease_info)
 
-    def renew_lease(self, renew_secret, new_expire_time):
+    def renew_lease(self, renew_secret, new_expire_time, allow_backdate=False):
+        # type: (bytes, int, bool) -> None
+        """
+        Update the expiration time on an existing lease.
+
+        :param allow_backdate: If ``True`` then allow the new expiration time
+            to be before the current expiration time.  Otherwise, make no
+            change when this is the case.
+
+        :raise IndexError: If there is no lease matching the given renew
+            secret.
+        """
         accepting_nodeids = set()
         with open(self.home, 'rb+') as f:
             for (leasenum,lease) in self._enumerate_leases(f):
-                if timing_safe_compare(lease.renew_secret, renew_secret):
+                if lease.is_renew_secret(renew_secret):
                     # yup. See if we need to update the owner time.
-                    if new_expire_time > lease.expiration_time:
+                    if allow_backdate or new_expire_time > lease.get_expiration_time():
                         # yes
-                        lease.expiration_time = new_expire_time
+                        lease = lease.renew(new_expire_time)
                         self._write_lease_record(f, leasenum, lease)
                     return
                 accepting_nodeids.add(lease.nodeid)
@@ -320,13 +345,13 @@ class MutableShareFile(object):
         msg += " ."
         raise IndexError(msg)
 
-    def add_or_renew_lease(self, lease_info):
+    def add_or_renew_lease(self, available_space, lease_info):
         precondition(lease_info.owner_num != 0) # 0 means "no lease here"
         try:
             self.renew_lease(lease_info.renew_secret,
-                             lease_info.expiration_time)
+                             lease_info.get_expiration_time())
         except IndexError:
-            self.add_lease(lease_info)
+            self.add_lease(available_space, lease_info)
 
     def cancel_lease(self, cancel_secret):
         """Remove any leases with the given cancel_secret. If the last lease
@@ -346,7 +371,7 @@ class MutableShareFile(object):
         with open(self.home, 'rb+') as f:
             for (leasenum,lease) in self._enumerate_leases(f):
                 accepting_nodeids.add(lease.nodeid)
-                if timing_safe_compare(lease.cancel_secret, cancel_secret):
+                if lease.is_cancel_secret(cancel_secret):
                     self._write_lease_record(f, leasenum, blank_lease)
                     modified += 1
                 else:
@@ -377,7 +402,7 @@ class MutableShareFile(object):
          write_enabler_nodeid, write_enabler,
          data_length, extra_least_offset) = \
          struct.unpack(">32s20s32sQQ", data)
-        assert magic == self.MAGIC
+        assert self.is_valid_header(data)
         return (write_enabler, write_enabler_nodeid)
 
     def readv(self, readv):
@@ -454,4 +479,3 @@ def create_mutable_sharefile(filename, my_nodeid, write_enabler, parent):
     ms.create(my_nodeid, write_enabler)
     del ms
     return MutableShareFile(filename, parent)
-

src/allmydata/storage/mutable_schema.py

@@ -0,0 +1,144 @@
+"""
+Ported to Python 3.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    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
+
+import struct
+
+import attr
+
+from ..util.hashutil import (
+    tagged_hash,
+)
+from .lease import (
+    LeaseInfo,
+)
+from .lease_schema import (
+    v1_mutable,
+    v2_mutable,
+)
+
+def _magic(version):
+    # type: (int) -> bytes
+    """
+    Compute a "magic" header string for a container of the given version.
+
+    :param version: The version number of the container.
+    """
+    # Make it easy for people to recognize
+    human_readable = u"Tahoe mutable container v{:d}\n".format(version).encode("ascii")
+    # But also keep the chance of accidental collision low
+    if version == 1:
+        # It's unclear where this byte sequence came from.  It may have just
+        # been random.  In any case, preserve it since it is the magic marker
+        # in all v1 share files.
+        random_bytes = b"\x75\x09\x44\x03\x8e"
+    else:
+        # For future versions, use a reproducable scheme.
+        random_bytes = tagged_hash(
+            b"allmydata_mutable_container_header",
+            human_readable,
+            truncate_to=5,
+        )
+    magic = human_readable + random_bytes
+    assert len(magic) == 32
+    if version > 1:
+        # The chance of collision is pretty low but let's just be sure about
+        # it.
+        assert magic != _magic(version - 1)
+
+    return magic
+
+def _header(magic, extra_lease_offset, nodeid, write_enabler):
+    # type: (bytes, int, bytes, bytes) -> bytes
+    """
+    Construct a container header.
+
+    :param nodeid: A unique identifier for the node holding this
+        container.
+
+    :param write_enabler: A secret shared with the client used to
+        authorize changes to the contents of this container.
+    """
+    fixed_header = struct.pack(
+        ">32s20s32sQQ",
+        magic,
+        nodeid,
+        write_enabler,
+        # data length, initially the container is empty
+        0,
+        extra_lease_offset,
+    )
+    blank_leases = b"\x00" * LeaseInfo().mutable_size() * 4
+    extra_lease_count = struct.pack(">L", 0)
+
+    return b"".join([
+        fixed_header,
+        # share data will go in between the next two items eventually but
+        # for now there is none.
+        blank_leases,
+        extra_lease_count,
+    ])
+
+
+_HEADER_FORMAT = ">32s20s32sQQ"
+
+# This size excludes leases
+_HEADER_SIZE = struct.calcsize(_HEADER_FORMAT)
+
+_EXTRA_LEASE_OFFSET = _HEADER_SIZE + 4 * LeaseInfo().mutable_size()
+
+
+@attr.s(frozen=True)
+class _Schema(object):
+    """
+    Implement encoding and decoding for the mutable container.
+
+    :ivar int version: the version number of the schema this object supports
+
+    :ivar lease_serializer: an object that is responsible for lease
+        serialization and unserialization
+    """
+    version = attr.ib()
+    lease_serializer = attr.ib()
+    _magic = attr.ib()
+
+    @classmethod
+    def for_version(cls, version, lease_serializer):
+        return cls(version, lease_serializer, magic=_magic(version))
+
+    def magic_matches(self, candidate_magic):
+        # type: (bytes) -> bool
+        """
+        Return ``True`` if a candidate string matches the expected magic string
+        from a mutable container header, ``False`` otherwise.
+        """
+        return candidate_magic[:len(self._magic)] == self._magic
+
+    def header(self, nodeid, write_enabler):
+        return _header(self._magic, _EXTRA_LEASE_OFFSET, nodeid, write_enabler)
+
+ALL_SCHEMAS = {
+    _Schema.for_version(version=2, lease_serializer=v2_mutable),
+    _Schema.for_version(version=1, lease_serializer=v1_mutable),
+}
+ALL_SCHEMA_VERSIONS = {schema.version for schema in ALL_SCHEMAS}
+NEWEST_SCHEMA_VERSION = max(ALL_SCHEMAS, key=lambda schema: schema.version)
+
+def schema_from_header(header):
+    # (int) -> Optional[type]
+    """
+    Find the schema object that corresponds to a certain version number.
+    """
+    for schema in ALL_SCHEMAS:
+        if schema.magic_matches(header):
+            return schema
+    return None

src/allmydata/storage/server.py

@@ -14,12 +14,12 @@ if PY2:
 else:
     from typing import Dict
 
-import os, re, struct, time
+import os, re
-import six
 
 from foolscap.api import Referenceable
 from foolscap.ipb import IRemoteReference
 from twisted.application import service
+from twisted.internet import reactor
 
 from zope.interface import implementer
 from allmydata.interfaces import RIStorageServer, IStatsProducer
@@ -57,7 +57,11 @@ DEFAULT_RENEWAL_TIME = 31 * 24 * 60 * 60
 
 @implementer(RIStorageServer, IStatsProducer)
 class StorageServer(service.MultiService, Referenceable):
+    """
+    A filesystem-based implementation of ``RIStorageServer``.
+    """
     name = 'storage'
+    # only the tests change this to anything else
     LeaseCheckerClass = LeaseCheckingCrawler
 
     def __init__(self, storedir, nodeid, reserved_space=0,
@@ -68,7 +72,7 @@ class StorageServer(service.MultiService, Referenceable):
                  expiration_override_lease_duration=None,
                  expiration_cutoff_date=None,
                  expiration_sharetypes=("mutable", "immutable"),
-                 get_current_time=time.time):
+                 clock=reactor):
         service.MultiService.__init__(self)
         assert isinstance(nodeid, bytes)
         assert len(nodeid) == 20
@@ -78,9 +82,9 @@ class StorageServer(service.MultiService, Referenceable):
         sharedir = os.path.join(storedir, "shares")
         fileutil.make_dirs(sharedir)
         self.sharedir = sharedir
-        # we don't actually create the corruption-advisory dir until necessary
         self.corruption_advisory_dir = os.path.join(storedir,
                                                     "corruption-advisories")
+        fileutil.make_dirs(self.corruption_advisory_dir)
         self.reserved_space = int(reserved_space)
         self.no_storage = discard_storage
         self.readonly_storage = readonly_storage
@@ -119,7 +123,7 @@ class StorageServer(service.MultiService, Referenceable):
                                    expiration_cutoff_date,
                                    expiration_sharetypes)
         self.lease_checker.setServiceParent(self)
-        self._get_current_time = get_current_time
+        self._clock = clock
 
         # Currently being-written Bucketwriters. For Foolscap, lifetime is tied
         # to connection: when disconnection happens, the BucketWriters are
@@ -132,6 +136,12 @@ class StorageServer(service.MultiService, Referenceable):
         # Canaries and disconnect markers for BucketWriters created via Foolscap:
         self._bucket_writer_disconnect_markers = {}  # type: Dict[BucketWriter,(IRemoteReference, object)]
 
+    def stopService(self):
+        # Cancel any in-progress uploads:
+        for bw in list(self._bucket_writers.values()):
+            bw.disconnected()
+        return service.MultiService.stopService(self)
+
     def __repr__(self):
         return "<StorageServer %s>" % (idlib.shortnodeid_b2a(self.my_nodeid),)
 
@@ -277,16 +287,21 @@ class StorageServer(service.MultiService, Referenceable):
     def _allocate_buckets(self, storage_index,
                           renew_secret, cancel_secret,
                           sharenums, allocated_size,
-                          owner_num=0):
+                          owner_num=0, renew_leases=True):
         """
         Generic bucket allocation API.
+
+        :param bool renew_leases: If and only if this is ``True`` then renew a
+            secret-matching lease on (or, if none match, add a new lease to)
+            existing shares in this bucket.  Any *new* shares are given a new
+            lease regardless.
         """
         # owner_num is not for clients to set, but rather it should be
         # curried into the PersonalStorageServer instance that is dedicated
         # to a particular owner.
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("allocate")
-        alreadygot = set()
+        alreadygot = {}
         bucketwriters = {} # k: shnum, v: BucketWriter
         si_dir = storage_index_to_dir(storage_index)
         si_s = si_b2a(storage_index)
@@ -297,7 +312,7 @@ class StorageServer(service.MultiService, Referenceable):
         # goes into the share files themselves. It could also be put into a
         # separate database. Note that the lease should not be added until
         # the BucketWriter has been closed.
-        expire_time = self._get_current_time() + DEFAULT_RENEWAL_TIME
+        expire_time = self._clock.seconds() + DEFAULT_RENEWAL_TIME
         lease_info = LeaseInfo(owner_num,
                                renew_secret, cancel_secret,
                                expire_time, self.my_nodeid)
@@ -318,9 +333,9 @@ class StorageServer(service.MultiService, Referenceable):
         # leases for all of them: if they want us to hold shares for this
         # file, they'll want us to hold leases for this file.
         for (shnum, fn) in self._get_bucket_shares(storage_index):
-            alreadygot.add(shnum)
+            alreadygot[shnum] = ShareFile(fn)
-            sf = ShareFile(fn)
+        if renew_leases:
-            sf.add_or_renew_lease(lease_info)
+            self._add_or_renew_leases(alreadygot.values(), lease_info)
 
         for shnum in sharenums:
             incominghome = os.path.join(self.incomingdir, si_dir, "%d" % shnum)
@@ -337,7 +352,8 @@ class StorageServer(service.MultiService, Referenceable):
             elif (not limited) or (remaining_space >= max_space_per_bucket):
                 # ok! we need to create the new share file.
                 bw = BucketWriter(self, incominghome, finalhome,
-                                  max_space_per_bucket, lease_info)
+                                  max_space_per_bucket, lease_info,
+                                  clock=self._clock)
                 if self.no_storage:
                     bw.throw_out_all_data = True
                 bucketwriters[shnum] = bw
@@ -351,8 +367,8 @@ class StorageServer(service.MultiService, Referenceable):
         if bucketwriters:
             fileutil.make_dirs(os.path.join(self.sharedir, si_dir))
 
-        self.add_latency("allocate", self._get_current_time() - start)
+        self.add_latency("allocate", self._clock.seconds() - start)
-        return alreadygot, bucketwriters
+        return set(alreadygot), bucketwriters
 
     def remote_allocate_buckets(self, storage_index,
                                 renew_secret, cancel_secret,
@@ -361,7 +377,7 @@ class StorageServer(service.MultiService, Referenceable):
         """Foolscap-specific ``allocate_buckets()`` API."""
         alreadygot, bucketwriters = self._allocate_buckets(
             storage_index, renew_secret, cancel_secret, sharenums, allocated_size,
-            owner_num=owner_num,
+            owner_num=owner_num, renew_leases=True,
         )
         # Abort BucketWriters if disconnection happens.
         for bw in bucketwriters.values():
@@ -373,12 +389,12 @@ class StorageServer(service.MultiService, Referenceable):
         for shnum, filename in self._get_bucket_shares(storage_index):
             with open(filename, 'rb') as f:
                 header = f.read(32)
-            if header[:32] == MutableShareFile.MAGIC:
+            if MutableShareFile.is_valid_header(header):
                 sf = MutableShareFile(filename, self)
                 # note: if the share has been migrated, the renew_lease()
                 # call will throw an exception, with information to help the
                 # client update the lease.
-            elif header[:4] == struct.pack(">L", 1):
+            elif ShareFile.is_valid_header(header):
                 sf = ShareFile(filename)
             else:
                 continue # non-sharefile
@@ -386,26 +402,28 @@ class StorageServer(service.MultiService, Referenceable):
 
     def remote_add_lease(self, storage_index, renew_secret, cancel_secret,
                          owner_num=1):
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("add-lease")
-        new_expire_time = self._get_current_time() + DEFAULT_RENEWAL_TIME
+        new_expire_time = self._clock.seconds() + DEFAULT_RENEWAL_TIME
         lease_info = LeaseInfo(owner_num,
                                renew_secret, cancel_secret,
                                new_expire_time, self.my_nodeid)
-        for sf in self._iter_share_files(storage_index):
+        self._add_or_renew_leases(
-            sf.add_or_renew_lease(lease_info)
+            self._iter_share_files(storage_index),
-        self.add_latency("add-lease", self._get_current_time() - start)
+            lease_info,
+        )
+        self.add_latency("add-lease", self._clock.seconds() - start)
         return None
 
     def remote_renew_lease(self, storage_index, renew_secret):
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("renew")
-        new_expire_time = self._get_current_time() + DEFAULT_RENEWAL_TIME
+        new_expire_time = self._clock.seconds() + DEFAULT_RENEWAL_TIME
         found_buckets = False
         for sf in self._iter_share_files(storage_index):
             found_buckets = True
             sf.renew_lease(renew_secret, new_expire_time)
-        self.add_latency("renew", self._get_current_time() - start)
+        self.add_latency("renew", self._clock.seconds() - start)
         if not found_buckets:
             raise IndexError("no such lease to renew")
 
@@ -432,7 +450,7 @@ class StorageServer(service.MultiService, Referenceable):
             pass
 
     def remote_get_buckets(self, storage_index):
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("get")
         si_s = si_b2a(storage_index)
         log.msg("storage: get_buckets %r" % si_s)
@@ -440,7 +458,7 @@ class StorageServer(service.MultiService, Referenceable):
         for shnum, filename in self._get_bucket_shares(storage_index):
             bucketreaders[shnum] = BucketReader(self, filename,
                                                 storage_index, shnum)
-        self.add_latency("get", self._get_current_time() - start)
+        self.add_latency("get", self._clock.seconds() - start)
         return bucketreaders
 
     def get_leases(self, storage_index):
@@ -579,10 +597,8 @@ class StorageServer(service.MultiService, Referenceable):
             else:
                 if sharenum not in shares:
                     # allocate a new share
-                    allocated_size = 2000 # arbitrary, really
                     share = self._allocate_slot_share(bucketdir, secrets,
                                                       sharenum,
-                                                      allocated_size,
                                                       owner_num=0)
                     shares[sharenum] = share
                 shares[sharenum].writev(datav, new_length)
@@ -601,7 +617,7 @@ class StorageServer(service.MultiService, Referenceable):
         :return LeaseInfo: Information for a new lease for a share.
         """
         ownerid = 1 # TODO
-        expire_time = self._get_current_time() + DEFAULT_RENEWAL_TIME
+        expire_time = self._clock.seconds() + DEFAULT_RENEWAL_TIME
         lease_info = LeaseInfo(ownerid,
                                renew_secret, cancel_secret,
                                expire_time, self.my_nodeid)
@@ -611,13 +627,13 @@ class StorageServer(service.MultiService, Referenceable):
         """
         Put the given lease onto the given shares.
 
-        :param dict[int, MutableShareFile] shares: The shares to put the lease
+        :param Iterable[Union[MutableShareFile, ShareFile]] shares: The shares
-            onto.
+            to put the lease onto.
 
         :param LeaseInfo lease_info: The lease to put on the shares.
         """
-        for share in six.viewvalues(shares):
+        for share in shares:
-            share.add_or_renew_lease(lease_info)
+            share.add_or_renew_lease(self.get_available_space(), lease_info)
 
     def slot_testv_and_readv_and_writev(  # type: ignore # warner/foolscap#78
             self,
@@ -631,13 +647,15 @@ class StorageServer(service.MultiService, Referenceable):
         Read data from shares and conditionally write some data to them.
 
         :param bool renew_leases: If and only if this is ``True`` and the test
-            vectors pass then shares in this slot will also have an updated
+            vectors pass then shares mentioned in ``test_and_write_vectors``
-            lease applied to them.
+            that still exist after the changes are made will also have a
+            secret-matching lease renewed (or, if none match, a new lease
+            added).
 
         See ``allmydata.interfaces.RIStorageServer`` for details about other
         parameters and return value.
         """
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("writev")
         si_s = si_b2a(storage_index)
         log.msg("storage: slot_writev %r" % si_s)
@@ -675,10 +693,10 @@ class StorageServer(service.MultiService, Referenceable):
             )
             if renew_leases:
                 lease_info = self._make_lease_info(renew_secret, cancel_secret)
-                self._add_or_renew_leases(remaining_shares, lease_info)
+                self._add_or_renew_leases(remaining_shares.values(), lease_info)
 
         # all done
-        self.add_latency("writev", self._get_current_time() - start)
+        self.add_latency("writev", self._clock.seconds() - start)
         return (testv_is_good, read_data)
 
     def remote_slot_testv_and_readv_and_writev(self, storage_index,
@@ -694,7 +712,7 @@ class StorageServer(service.MultiService, Referenceable):
         )
 
     def _allocate_slot_share(self, bucketdir, secrets, sharenum,
-                             allocated_size, owner_num=0):
+                             owner_num=0):
         (write_enabler, renew_secret, cancel_secret) = secrets
         my_nodeid = self.my_nodeid
         fileutil.make_dirs(bucketdir)
@@ -704,7 +722,7 @@ class StorageServer(service.MultiService, Referenceable):
         return share
 
     def remote_slot_readv(self, storage_index, shares, readv):
-        start = self._get_current_time()
+        start = self._clock.seconds()
         self.count("readv")
         si_s = si_b2a(storage_index)
         lp = log.msg("storage: slot_readv %r %r" % (si_s, shares),
@@ -713,7 +731,7 @@ class StorageServer(service.MultiService, Referenceable):
         # shares exist if there is a file for them
         bucketdir = os.path.join(self.sharedir, si_dir)
         if not os.path.isdir(bucketdir):
-            self.add_latency("readv", self._get_current_time() - start)
+            self.add_latency("readv", self._clock.seconds() - start)
             return {}
         datavs = {}
         for sharenum_s in os.listdir(bucketdir):
@@ -727,33 +745,113 @@ class StorageServer(service.MultiService, Referenceable):
                 datavs[sharenum] = msf.readv(readv)
         log.msg("returning shares %s" % (list(datavs.keys()),),
                 facility="tahoe.storage", level=log.NOISY, parent=lp)
-        self.add_latency("readv", self._get_current_time() - start)
+        self.add_latency("readv", self._clock.seconds() - start)
         return datavs
 
+    def _share_exists(self, storage_index, shnum):
+        """
+        Check local share storage to see if a matching share exists.
+
+        :param bytes storage_index: The storage index to inspect.
+        :param int shnum: The share number to check for.
+
+        :return bool: ``True`` if a share with the given number exists at the
+            given storage index, ``False`` otherwise.
+        """
+        for existing_sharenum, ignored in self._get_bucket_shares(storage_index):
+            if existing_sharenum == shnum:
+                return True
+        return False
+
     def remote_advise_corrupt_share(self, share_type, storage_index, shnum,
                                     reason):
         # This is a remote API, I believe, so this has to be bytes for legacy
         # protocol backwards compatibility reasons.
         assert isinstance(share_type, bytes)
         assert isinstance(reason, bytes), "%r is not bytes" % (reason,)
-        fileutil.make_dirs(self.corruption_advisory_dir)
+
-        now = time_format.iso_utc(sep="T")
         si_s = si_b2a(storage_index)
-        # windows can't handle colons in the filename
+
-        fn = os.path.join(
+        if not self._share_exists(storage_index, shnum):
-            self.corruption_advisory_dir,
+            log.msg(
-            ("%s--%s-%d" % (now, str(si_s, "utf-8"), shnum)).replace(":","")
+                format=(
+                    "discarding client corruption claim for %(si)s/%(shnum)d "
+                    "which I do not have"
+                ),
+                si=si_s,
+                shnum=shnum,
             )
-        with open(fn, "w") as f:
+            return
-            f.write("report: Share Corruption\n")
+
-            f.write("type: %s\n" % bytes_to_native_str(share_type))
-            f.write("storage_index: %s\n" % bytes_to_native_str(si_s))
-            f.write("share_number: %d\n" % shnum)
-            f.write("\n")
-            f.write(bytes_to_native_str(reason))
-            f.write("\n")
         log.msg(format=("client claims corruption in (%(share_type)s) " +
                         "%(si)s-%(shnum)d: %(reason)s"),
                 share_type=share_type, si=si_s, shnum=shnum, reason=reason,
                 level=log.SCARY, umid="SGx2fA")
+
+        report = render_corruption_report(share_type, si_s, shnum, reason)
+        if len(report) > self.get_available_space():
             return None
+
+        now = time_format.iso_utc(sep="T")
+        report_path = get_corruption_report_path(
+            self.corruption_advisory_dir,
+            now,
+            si_s,
+            shnum,
+        )
+        with open(report_path, "w") as f:
+            f.write(report)
+
+        return None
+
+CORRUPTION_REPORT_FORMAT = """\
+report: Share Corruption
+type: {type}
+storage_index: {storage_index}
+share_number: {share_number}
+
+{reason}
+
+"""
+
+def render_corruption_report(share_type, si_s, shnum, reason):
+    """
+    Create a string that explains a corruption report using freeform text.
+
+    :param bytes share_type: The type of the share which the report is about.
+
+    :param bytes si_s: The encoded representation of the storage index which
+        the report is about.
+
+    :param int shnum: The share number which the report is about.
+
+    :param bytes reason: The reason given by the client for the corruption
+        report.
+    """
+    return CORRUPTION_REPORT_FORMAT.format(
+        type=bytes_to_native_str(share_type),
+        storage_index=bytes_to_native_str(si_s),
+        share_number=shnum,
+        reason=bytes_to_native_str(reason),
+    )
+
+def get_corruption_report_path(base_dir, now, si_s, shnum):
+    """
+    Determine the path to which a certain corruption report should be written.
+
+    :param str base_dir: The directory beneath which to construct the path.
+
+    :param str now: The time of the report.
+
+    :param str si_s: The encoded representation of the storage index which the
+        report is about.
+
+    :param int shnum: The share number which the report is about.
+
+    :return str: A path to which the report can be written.
+    """
+    # windows can't handle colons in the filename
+    return os.path.join(
+        base_dir,
+        ("%s--%s-%d" % (now, str(si_s, "utf-8"), shnum)).replace(":","")
+    )

src/allmydata/storage/shares.py

@@ -17,8 +17,7 @@ from allmydata.storage.immutable import ShareFile
 def get_share_file(filename):
     with open(filename, "rb") as f:
         prefix = f.read(32)
-    if prefix == MutableShareFile.MAGIC:
+    if MutableShareFile.is_valid_header(prefix):
         return MutableShareFile(filename)
     # otherwise assume it's immutable
     return ShareFile(filename)
-

src/allmydata/test/__init__.py

@@ -125,5 +125,5 @@ if sys.platform == "win32":
     initialize()
 
 from eliot import to_file
-from allmydata.util.jsonbytes import AnyBytesJSONEncoder
+from allmydata.util.eliotutil import eliot_json_encoder
-to_file(open("eliot.log", "wb"), encoder=AnyBytesJSONEncoder)
+to_file(open("eliot.log", "wb"), encoder=eliot_json_encoder)

src/allmydata/test/cli/test_admin.py

@@ -0,0 +1,87 @@
+"""
+Ported to Python 3.
+"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    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
+
+from six.moves import StringIO
+
+from testtools.matchers import (
+    Contains,
+)
+
+from twisted.python.filepath import (
+    FilePath,
+)
+
+from allmydata.scripts.admin import (
+    migrate_crawler,
+)
+from allmydata.scripts.runner import (
+    Options,
+)
+from ..common import (
+    SyncTestCase,
+)
+
+class AdminMigrateCrawler(SyncTestCase):
+    """
+    Tests related to 'tahoe admin migrate-crawler'
+    """
+
+    def test_already(self):
+        """
+        We've already migrated; don't do it again.
+        """
+
+        root = FilePath(self.mktemp())
+        storage = root.child("storage")
+        storage.makedirs()
+        with storage.child("lease_checker.state.json").open("w") as f:
+            f.write(b"{}\n")
+
+        top = Options()
+        top.parseOptions([
+            "admin", "migrate-crawler",
+            "--basedir", storage.parent().path,
+        ])
+        options = top.subOptions
+        while hasattr(options, "subOptions"):
+            options = options.subOptions
+        options.stdout = StringIO()
+        migrate_crawler(options)
+
+        self.assertThat(
+            options.stdout.getvalue(),
+            Contains("Already converted:"),
+        )
+
+    def test_usage(self):
+        """
+        We've already migrated; don't do it again.
+        """
+
+        root = FilePath(self.mktemp())
+        storage = root.child("storage")
+        storage.makedirs()
+        with storage.child("lease_checker.state.json").open("w") as f:
+            f.write(b"{}\n")
+
+        top = Options()
+        top.parseOptions([
+            "admin", "migrate-crawler",
+            "--basedir", storage.parent().path,
+        ])
+        options = top.subOptions
+        while hasattr(options, "subOptions"):
+            options = options.subOptions
+        self.assertThat(
+            str(options),
+            Contains("security issues with pickle")
+        )

src/allmydata/test/cli/test_create.py

@@ -11,16 +11,24 @@ if PY2:
     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
 
 import os
-import mock
+
+try:
+    from typing import Any, List, Tuple
+except ImportError:
+    pass
+
 from twisted.trial import unittest
 from twisted.internet import defer, reactor
 from twisted.python import usage
 from allmydata.util import configutil
+from allmydata.util import tor_provider, i2p_provider
 from ..common_util import run_cli, parse_cli
+from ..common import (
+    disable_modules,
+)
 from ...scripts import create_node
 from ... import client
 
-
 def read_config(basedir):
     tahoe_cfg = os.path.join(basedir, "tahoe.cfg")
     config = configutil.get_config(tahoe_cfg)
@@ -105,11 +113,12 @@ class Config(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_client_hide_ip_no_i2p_txtorcon(self):
-        # hmm, I must be doing something weird, these don't work as
+        """
-        # @mock.patch decorators for some reason
+        The ``create-client`` sub-command tells the user to install the necessary
-        txi2p = mock.patch('allmydata.util.i2p_provider._import_txi2p', return_value=None)
+        dependencies if they have neither tor nor i2p support installed and
-        txtorcon = mock.patch('allmydata.util.tor_provider._import_txtorcon', return_value=None)
+        they request network location privacy with the ``--hide-ip`` flag.
-        with txi2p, txtorcon:
+        """
+        with disable_modules("txi2p", "txtorcon"):
             basedir = self.mktemp()
             rc, out, err = yield run_cli("create-client", "--hide-ip", basedir)
             self.assertTrue(rc != 0, out)
@@ -118,8 +127,7 @@ class Config(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_client_i2p_option_no_txi2p(self):
-        txi2p = mock.patch('allmydata.util.i2p_provider._import_txi2p', return_value=None)
+        with disable_modules("txi2p"):
-        with txi2p:
             basedir = self.mktemp()
             rc, out, err = yield run_cli("create-node", "--listen=i2p", "--i2p-launch", basedir)
             self.assertTrue(rc != 0)
@@ -127,8 +135,7 @@ class Config(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_client_tor_option_no_txtorcon(self):
-        txtorcon = mock.patch('allmydata.util.tor_provider._import_txtorcon', return_value=None)
+        with disable_modules("txtorcon"):
-        with txtorcon:
             basedir = self.mktemp()
             rc, out, err = yield run_cli("create-node", "--listen=tor", "--tor-launch", basedir)
             self.assertTrue(rc != 0)
@@ -145,9 +152,7 @@ class Config(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_client_hide_ip_no_txtorcon(self):
-        txtorcon = mock.patch('allmydata.util.tor_provider._import_txtorcon',
+        with disable_modules("txtorcon"):
-                              return_value=None)
-        with txtorcon:
             basedir = self.mktemp()
             rc, out, err = yield run_cli("create-client", "--hide-ip", basedir)
             self.assertEqual(0, rc)
@@ -295,8 +300,7 @@ class Config(unittest.TestCase):
     def test_node_slow_tor(self):
         basedir = self.mktemp()
         d = defer.Deferred()
-        with mock.patch("allmydata.util.tor_provider.create_config",
+        self.patch(tor_provider, "create_config", lambda *a, **kw: d)
-                        return_value=d):
         d2 = run_cli("create-node", "--listen=tor", basedir)
         d.callback(({}, "port", "location"))
         rc, out, err = yield d2
@@ -308,8 +312,7 @@ class Config(unittest.TestCase):
     def test_node_slow_i2p(self):
         basedir = self.mktemp()
         d = defer.Deferred()
-        with mock.patch("allmydata.util.i2p_provider.create_config",
+        self.patch(i2p_provider, "create_config", lambda *a, **kw: d)
-                        return_value=d):
         d2 = run_cli("create-node", "--listen=i2p", basedir)
         d.callback(({}, "port", "location"))
         rc, out, err = yield d2
@@ -353,6 +356,27 @@ class Config(unittest.TestCase):
         self.assertIn("is not empty", err)
         self.assertIn("To avoid clobbering anything, I am going to quit now", err)
 
+def fake_config(testcase, module, result):
+    # type: (unittest.TestCase, Any, Any) -> List[Tuple]
+    """
+    Monkey-patch a fake configuration function into the given module.
+
+    :param testcase: The test case to use to do the monkey-patching.
+
+    :param module: The module into which to patch the fake function.
+
+    :param result: The return value for the fake function.
+
+    :return: A list of tuples of the arguments the fake function was called
+        with.
+    """
+    calls = []
+    def fake_config(reactor, cli_config):
+        calls.append((reactor, cli_config))
+        return result
+    testcase.patch(module, "create_config", fake_config)
+    return calls
+
 class Tor(unittest.TestCase):
     def test_default(self):
         basedir = self.mktemp()
@@ -360,12 +384,14 @@ class Tor(unittest.TestCase):
         tor_port = "ghi"
         tor_location = "jkl"
         config_d = defer.succeed( (tor_config, tor_port, tor_location) )
-        with mock.patch("allmydata.util.tor_provider.create_config",
+
-                        return_value=config_d) as co:
+        calls = fake_config(self, tor_provider, config_d)
         rc, out, err = self.successResultOf(
-                run_cli("create-node", "--listen=tor", basedir))
+            run_cli("create-node", "--listen=tor", basedir),
-        self.assertEqual(len(co.mock_calls), 1)
+        )
-        args = co.mock_calls[0][1]
+
+        self.assertEqual(len(calls), 1)
+        args = calls[0]
         self.assertIdentical(args[0], reactor)
         self.assertIsInstance(args[1], create_node.CreateNodeOptions)
         self.assertEqual(args[1]["listen"], "tor")
@@ -380,12 +406,15 @@ class Tor(unittest.TestCase):
         tor_port = "ghi"
         tor_location = "jkl"
         config_d = defer.succeed( (tor_config, tor_port, tor_location) )
-        with mock.patch("allmydata.util.tor_provider.create_config",
+
-                        return_value=config_d) as co:
+        calls = fake_config(self, tor_provider, config_d)
         rc, out, err = self.successResultOf(
-                run_cli("create-node", "--listen=tor", "--tor-launch",
+            run_cli(
-                        basedir))
+                "create-node", "--listen=tor", "--tor-launch",
-        args = co.mock_calls[0][1]
+                basedir,
+            ),
+        )
+        args = calls[0]
         self.assertEqual(args[1]["listen"], "tor")
         self.assertEqual(args[1]["tor-launch"], True)
         self.assertEqual(args[1]["tor-control-port"], None)
@@ -396,12 +425,15 @@ class Tor(unittest.TestCase):
         tor_port = "ghi"
         tor_location = "jkl"
         config_d = defer.succeed( (tor_config, tor_port, tor_location) )
-        with mock.patch("allmydata.util.tor_provider.create_config",
+
-                        return_value=config_d) as co:
+        calls = fake_config(self, tor_provider, config_d)
         rc, out, err = self.successResultOf(
-                run_cli("create-node", "--listen=tor", "--tor-control-port=mno",
+            run_cli(
-                        basedir))
+                "create-node", "--listen=tor", "--tor-control-port=mno",
-        args = co.mock_calls[0][1]
+                basedir,
+            ),
+        )
+        args = calls[0]
         self.assertEqual(args[1]["listen"], "tor")
         self.assertEqual(args[1]["tor-launch"], False)
         self.assertEqual(args[1]["tor-control-port"], "mno")
@@ -434,12 +466,13 @@ class I2P(unittest.TestCase):
         i2p_port = "ghi"
         i2p_location = "jkl"
         dest_d = defer.succeed( (i2p_config, i2p_port, i2p_location) )
-        with mock.patch("allmydata.util.i2p_provider.create_config",
+
-                        return_value=dest_d) as co:
+        calls = fake_config(self, i2p_provider, dest_d)
         rc, out, err = self.successResultOf(
-                run_cli("create-node", "--listen=i2p", basedir))
+            run_cli("create-node", "--listen=i2p", basedir),
-        self.assertEqual(len(co.mock_calls), 1)
+        )
-        args = co.mock_calls[0][1]
+        self.assertEqual(len(calls), 1)
+        args = calls[0]
         self.assertIdentical(args[0], reactor)
         self.assertIsInstance(args[1], create_node.CreateNodeOptions)
         self.assertEqual(args[1]["listen"], "i2p")
@@ -461,12 +494,15 @@ class I2P(unittest.TestCase):
         i2p_port = "ghi"
         i2p_location = "jkl"
         dest_d = defer.succeed( (i2p_config, i2p_port, i2p_location) )
-        with mock.patch("allmydata.util.i2p_provider.create_config",
+
-                        return_value=dest_d) as co:
+        calls = fake_config(self, i2p_provider, dest_d)
         rc, out, err = self.successResultOf(
-                run_cli("create-node", "--listen=i2p", "--i2p-sam-port=mno",
+            run_cli(
-                        basedir))
+                "create-node", "--listen=i2p", "--i2p-sam-port=mno",
-        args = co.mock_calls[0][1]
+                basedir,
+            ),
+        )
+        args = calls[0]
         self.assertEqual(args[1]["listen"], "i2p")
         self.assertEqual(args[1]["i2p-launch"], False)
         self.assertEqual(args[1]["i2p-sam-port"], "mno")

src/allmydata/test/common.py

@@ -28,6 +28,7 @@ __all__ = [
 
 import sys
 import os, random, struct
+from contextlib import contextmanager
 import six
 import tempfile
 from tempfile import mktemp
@@ -87,6 +88,7 @@ from allmydata.interfaces import (
     SDMF_VERSION,
     MDMF_VERSION,
     IAddressFamily,
+    NoSpace,
 )
 from allmydata.check_results import CheckResults, CheckAndRepairResults, \
      DeepCheckResults, DeepCheckAndRepairResults
@@ -139,6 +141,42 @@ EMPTY_CLIENT_CONFIG = config_from_string(
     ""
 )
 
+@attr.s
+class FakeDisk(object):
+    """
+    Just enough of a disk to be able to report free / used information.
+    """
+    total = attr.ib()
+    used = attr.ib()
+
+    def use(self, num_bytes):
+        """
+        Mark some amount of available bytes as used (and no longer available).
+
+        :param int num_bytes: The number of bytes to use.
+
+        :raise NoSpace: If there are fewer bytes available than ``num_bytes``.
+
+        :return: ``None``
+        """
+        if num_bytes > self.total - self.used:
+            raise NoSpace()
+        self.used += num_bytes
+
+    @property
+    def available(self):
+        return self.total - self.used
+
+    def get_disk_stats(self, whichdir, reserved_space):
+        avail = self.available
+        return {
+            'total': self.total,
+            'free_for_root': avail,
+            'free_for_nonroot': avail,
+            'used': self.used,
+            'avail': avail - reserved_space,
+        }
+
 
 @attr.s
 class MemoryIntroducerClient(object):
@@ -267,8 +305,12 @@ class UseNode(object):
     node_config = attr.ib(default=attr.Factory(dict))
 
     config = attr.ib(default=None)
+    reactor = attr.ib(default=None)
 
     def setUp(self):
+        self.assigner = SameProcessStreamEndpointAssigner()
+        self.assigner.setUp()
+
         def format_config_items(config):
             return "\n".join(
                 " = ".join((key, value))
@@ -292,6 +334,23 @@ class UseNode(object):
             "default",
             self.introducer_furl,
         )
+
+        node_config = self.node_config.copy()
+        if "tub.port" not in node_config:
+            if "tub.location" in node_config:
+                raise ValueError(
+                    "UseNode fixture does not support specifying tub.location "
+                    "without tub.port"
+                )
+
+            # Don't use the normal port auto-assignment logic.  It produces
+            # collisions and makes tests fail spuriously.
+            tub_location, tub_endpoint = self.assigner.assign(self.reactor)
+            node_config.update({
+                "tub.port": tub_endpoint,
+                "tub.location": tub_location,
+            })
+
         self.config = config_from_string(
             self.basedir.asTextMode().path,
             "tub.port",
@@ -304,7 +363,7 @@ storage.plugins = {storage_plugin}
 {plugin_config_section}
 """.format(
     storage_plugin=self.storage_plugin,
-    node_config=format_config_items(self.node_config),
+    node_config=format_config_items(node_config),
     plugin_config_section=plugin_config_section,
 )
         )
@@ -316,7 +375,7 @@ storage.plugins = {storage_plugin}
         )
 
     def cleanUp(self):
-        pass
+        self.assigner.tearDown()
 
 
     def getDetails(self):
@@ -1068,7 +1127,7 @@ def _corrupt_offset_of_uri_extension_to_force_short_read(data, debug=False):
 
 def _corrupt_mutable_share_data(data, debug=False):
     prefix = data[:32]
-    assert prefix == MutableShareFile.MAGIC, "This function is designed to corrupt mutable shares of v1, and the magic number doesn't look right: %r vs %r" % (prefix, MutableShareFile.MAGIC)
+    assert MutableShareFile.is_valid_header(prefix), "This function is designed to corrupt mutable shares of v1, and the magic number doesn't look right: %r vs %r" % (prefix, MutableShareFile.MAGIC)
     data_offset = MutableShareFile.DATA_OFFSET
     sharetype = data[data_offset:data_offset+1]
     assert sharetype == b"\x00", "non-SDMF mutable shares not supported"
@@ -1213,6 +1272,29 @@ class ConstantAddresses(object):
             raise Exception("{!r} has no client endpoint.")
         return self._handler
 
+@contextmanager
+def disable_modules(*names):
+    """
+    A context manager which makes modules appear to be missing while it is
+    active.
+
+    :param *names: The names of the modules to disappear.  Only top-level
+        modules are supported (that is, "." is not allowed in any names).
+        This is an implementation shortcoming which could be lifted if
+        desired.
+    """
+    if any("." in name for name in names):
+        raise ValueError("Names containing '.' are not supported.")
+    missing = object()
+    modules = list(sys.modules.get(n, missing) for n in names)
+    for n in names:
+        sys.modules[n] = None
+    yield
+    for n, original in zip(names, modules):
+        if original is missing:
+            del sys.modules[n]
+        else:
+            sys.modules[n] = original
 
 class _TestCaseMixin(object):
     """

src/allmydata/test/common_storage.py

@@ -0,0 +1,65 @@
+
+from .common_util import (
+    FakeCanary,
+)
+
+def upload_immutable(storage_server, storage_index, renew_secret, cancel_secret, shares):
+    """
+    Synchronously upload some immutable shares to a ``StorageServer``.
+
+    :param allmydata.storage.server.StorageServer storage_server: The storage
+        server object to use to perform the upload.
+
+    :param bytes storage_index: The storage index for the immutable shares.
+
+    :param bytes renew_secret: The renew secret for the implicitly created lease.
+    :param bytes cancel_secret: The cancel secret for the implicitly created lease.
+
+    :param dict[int, bytes] shares: A mapping from share numbers to share data
+        to upload.  The data for all shares must be of the same length.
+
+    :return: ``None``
+    """
+    already, writers = storage_server.remote_allocate_buckets(
+        storage_index,
+        renew_secret,
+        cancel_secret,
+        shares.keys(),
+        len(next(iter(shares.values()))),
+        canary=FakeCanary(),
+    )
+    for shnum, writer in writers.items():
+        writer.remote_write(0, shares[shnum])
+        writer.remote_close()
+
+
+def upload_mutable(storage_server, storage_index, secrets, shares):
+    """
+    Synchronously upload some mutable shares to a ``StorageServer``.
+
+    :param allmydata.storage.server.StorageServer storage_server: The storage
+        server object to use to perform the upload.
+
+    :param bytes storage_index: The storage index for the immutable shares.
+
+    :param secrets: A three-tuple of a write enabler, renew secret, and cancel
+        secret.
+
+    :param dict[int, bytes] shares: A mapping from share numbers to share data
+        to upload.
+
+    :return: ``None``
+    """
+    test_and_write_vectors = {
+        sharenum: ([], [(0, data)], None)
+        for sharenum, data
+        in shares.items()
+    }
+    read_vector = []
+
+    storage_server.remote_slot_testv_and_readv_and_writev(
+        storage_index,
+        secrets,
+        test_and_write_vectors,
+        read_vector,
+    )

src/allmydata/test/common_system.py

@@ -672,11 +672,14 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         """
         iv_dir = self.getdir("introducer")
         if not os.path.isdir(iv_dir):
-            _, port_endpoint = self.port_assigner.assign(reactor)
+            _, web_port_endpoint = self.port_assigner.assign(reactor)
+            main_location_hint, main_port_endpoint = self.port_assigner.assign(reactor)
             introducer_config = (
                 u"[node]\n"
                 u"nickname = introducer \N{BLACK SMILING FACE}\n" +
-                u"web.port = {}\n".format(port_endpoint)
+                u"web.port = {}\n".format(web_port_endpoint) +
+                u"tub.port = {}\n".format(main_port_endpoint) +
+                u"tub.location = {}\n".format(main_location_hint)
             ).encode("utf-8")
 
             fileutil.make_dirs(iv_dir)
@@ -764,13 +767,15 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
     def _generate_config(self, which, basedir):
         config = {}
 
-        except1 = set(range(self.numclients)) - {1}
+        allclients = set(range(self.numclients))
+        except1 = allclients - {1}
         feature_matrix = {
             ("client", "nickname"): except1,
 
-            # client 1 has to auto-assign an address.
+            # Auto-assigning addresses is extremely failure prone and not
-            ("node", "tub.port"): except1,
+            # amenable to automated testing in _this_ manner.
-            ("node", "tub.location"): except1,
+            ("node", "tub.port"): allclients,
+            ("node", "tub.location"): allclients,
 
             # client 0 runs a webserver and a helper
             # client 3 runs a webserver but no helper
@@ -852,7 +857,13 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         # connection-lost code
         basedir = FilePath(self.getdir("client%d" % client_num))
         basedir.makedirs()
-        config = "[client]\n"
+        config = (
+            "[node]\n"
+            "tub.location = {}\n"
+            "tub.port = {}\n"
+            "[client]\n"
+        ).format(*self.port_assigner.assign(reactor))
+
         if helper_furl:
             config += "helper.furl = %s\n" % helper_furl
         basedir.child("tahoe.cfg").setContent(config.encode("utf-8"))

src/allmydata/test/data/lease_checker.history.txt

@@ -0,0 +1,501 @@
+(dp0
+I363
+(dp1
+Vconfigured-expiration-mode
+p2
+(S'age'
+p3
+NN(S'immutable'
+p4
+S'mutable'
+p5
+tp6
+tp7
+sVexpiration-enabled
+p8
+I00
+sVleases-per-share-histogram
+p9
+(dp10
+I1
+I39774
+ssVlease-age-histogram
+p11
+(lp12
+(I0
+I86400
+I3125
+tp13
+a(I345600
+I432000
+I4175
+tp14
+a(I950400
+I1036800
+I141
+tp15
+a(I1036800
+I1123200
+I345
+tp16
+a(I1123200
+I1209600
+I81
+tp17
+a(I1296000
+I1382400
+I1832
+tp18
+a(I1555200
+I1641600
+I390
+tp19
+a(I1728000
+I1814400
+I12
+tp20
+a(I2073600
+I2160000
+I84
+tp21
+a(I2160000
+I2246400
+I228
+tp22
+a(I2246400
+I2332800
+I75
+tp23
+a(I2592000
+I2678400
+I644
+tp24
+a(I2678400
+I2764800
+I273
+tp25
+a(I2764800
+I2851200
+I94
+tp26
+a(I2851200
+I2937600
+I97
+tp27
+a(I3196800
+I3283200
+I143
+tp28
+a(I3283200
+I3369600
+I48
+tp29
+a(I4147200
+I4233600
+I374
+tp30
+a(I4320000
+I4406400
+I534
+tp31
+a(I5270400
+I5356800
+I1005
+tp32
+a(I6739200
+I6825600
+I8704
+tp33
+a(I6825600
+I6912000
+I3986
+tp34
+a(I6912000
+I6998400
+I7592
+tp35
+a(I6998400
+I7084800
+I2607
+tp36
+a(I7689600
+I7776000
+I35
+tp37
+a(I8035200
+I8121600
+I33
+tp38
+a(I8294400
+I8380800
+I54
+tp39
+a(I8640000
+I8726400
+I45
+tp40
+a(I8726400
+I8812800
+I27
+tp41
+a(I8812800
+I8899200
+I12
+tp42
+a(I9763200
+I9849600
+I77
+tp43
+a(I9849600
+I9936000
+I91
+tp44
+a(I9936000
+I10022400
+I1210
+tp45
+a(I10022400
+I10108800
+I45
+tp46
+a(I10108800
+I10195200
+I186
+tp47
+a(I10368000
+I10454400
+I113
+tp48
+a(I10972800
+I11059200
+I21
+tp49
+a(I11232000
+I11318400
+I5
+tp50
+a(I11318400
+I11404800
+I19
+tp51
+a(I11404800
+I11491200
+I238
+tp52
+a(I11491200
+I11577600
+I159
+tp53
+a(I11750400
+I11836800
+I1
+tp54
+a(I11836800
+I11923200
+I32
+tp55
+a(I11923200
+I12009600
+I192
+tp56
+a(I12009600
+I12096000
+I222
+tp57
+a(I12096000
+I12182400
+I18
+tp58
+a(I12182400
+I12268800
+I224
+tp59
+a(I12268800
+I12355200
+I9
+tp60
+a(I12355200
+I12441600
+I9
+tp61
+a(I12441600
+I12528000
+I10
+tp62
+a(I12528000
+I12614400
+I6
+tp63
+a(I12614400
+I12700800
+I6
+tp64
+a(I12700800
+I12787200
+I18
+tp65
+a(I12787200
+I12873600
+I6
+tp66
+a(I12873600
+I12960000
+I62
+tp67
+asVcycle-start-finish-times
+p68
+(F1634446505.241972
+F1634446666.055401
+tp69
+sVspace-recovered
+p70
+(dp71
+Vexamined-buckets-immutable
+p72
+I17896
+sVconfigured-buckets-mutable
+p73
+I0
+sVexamined-shares-mutable
+p74
+I2473
+sVoriginal-shares-mutable
+p75
+I1185
+sVconfigured-buckets-immutable
+p76
+I0
+sVoriginal-shares-immutable
+p77
+I27457
+sVoriginal-diskbytes-immutable
+p78
+I2810982400
+sVexamined-shares-immutable
+p79
+I37301
+sVoriginal-buckets
+p80
+I14047
+sVactual-shares-immutable
+p81
+I0
+sVconfigured-shares
+p82
+I0
+sVoriginal-buckets-mutable
+p83
+I691
+sVactual-diskbytes
+p84
+I4096
+sVactual-shares-mutable
+p85
+I0
+sVconfigured-buckets
+p86
+I1
+sVexamined-buckets-unknown
+p87
+I14
+sVactual-sharebytes
+p88
+I0
+sVoriginal-shares
+p89
+I28642
+sVactual-buckets-immutable
+p90
+I0
+sVoriginal-sharebytes
+p91
+I2695552941
+sVexamined-sharebytes-immutable
+p92
+I2754798505
+sVactual-shares
+p93
+I0
+sVactual-sharebytes-immutable
+p94
+I0
+sVoriginal-diskbytes
+p95
+I2818981888
+sVconfigured-diskbytes-mutable
+p96
+I0
+sVconfigured-sharebytes-immutable
+p97
+I0
+sVconfigured-shares-mutable
+p98
+I0
+sVactual-diskbytes-immutable
+p99
+I0
+sVconfigured-diskbytes-immutable
+p100
+I0
+sVoriginal-diskbytes-mutable
+p101
+I7995392
+sVactual-sharebytes-mutable
+p102
+I0
+sVconfigured-sharebytes
+p103
+I0
+sVexamined-shares
+p104
+I39774
+sVactual-diskbytes-mutable
+p105
+I0
+sVactual-buckets
+p106
+I1
+sVoriginal-buckets-immutable
+p107
+I13355
+sVconfigured-sharebytes-mutable
+p108
+I0
+sVexamined-sharebytes
+p109
+I2763646972
+sVoriginal-sharebytes-immutable
+p110
+I2692076909
+sVoriginal-sharebytes-mutable
+p111
+I3476032
+sVactual-buckets-mutable
+p112
+I0
+sVexamined-buckets-mutable
+p113
+I1286
+sVconfigured-shares-immutable
+p114
+I0
+sVexamined-diskbytes
+p115
+I2854801408
+sVexamined-diskbytes-mutable
+p116
+I12161024
+sVexamined-sharebytes-mutable
+p117
+I8848467
+sVexamined-buckets
+p118
+I19197
+sVconfigured-diskbytes
+p119
+I4096
+sVexamined-diskbytes-immutable
+p120
+I2842640384
+ssVcorrupt-shares
+p121
+(lp122
+(V2dn6xnlnsqwtnapwxfdivpm3s4
+p123
+I3
+tp124
+a(g123
+I0
+tp125
+a(V2rrzthwsrrxolevmwdvbdy3rqi
+p126
+I3
+tp127
+a(g126
+I0
+tp128
+a(V2skfngcto6h7eqmn4uo7ntk3ne
+p129
+I3
+tp130
+a(g129
+I0
+tp131
+a(V32d5swqpqx2mwix7xmqzvhdwje
+p132
+I3
+tp133
+a(g132
+I0
+tp134
+a(V5mmayp66yflmpon3o6unsnbaca
+p135
+I3
+tp136
+a(g135
+I0
+tp137
+a(V6ixhpvbtre7fnrl6pehlrlflc4
+p138
+I3
+tp139
+a(g138
+I0
+tp140
+a(Vewzhvswjsz4vp2bqkb6mi3bz2u
+p141
+I3
+tp142
+a(g141
+I0
+tp143
+a(Vfu7pazf6ogavkqj6z4q5qqex3u
+p144
+I3
+tp145
+a(g144
+I0
+tp146
+a(Vhbyjtqvpcimwxiyqbcbbdn2i4a
+p147
+I3
+tp148
+a(g147
+I0
+tp149
+a(Vpmcjbdkbjdl26k3e6yja77femq
+p150
+I3
+tp151
+a(g150
+I0
+tp152
+a(Vr6swof4v2uttbiiqwj5pi32cm4
+p153
+I3
+tp154
+a(g153
+I0
+tp155
+a(Vt45v5akoktf53evc2fi6gwnv6y
+p156
+I3
+tp157
+a(g156
+I0
+tp158
+a(Vy6zb4faar3rdvn3e6pfg4wlotm
+p159
+I3
+tp160
+a(g159
+I0
+tp161
+a(Vz3yghutvqoqbchjao4lndnrh3a
+p162
+I3
+tp163
+a(g162
+I0
+tp164
+ass.

src/allmydata/test/data/lease_checker.state.txt

@@ -0,0 +1,545 @@
+(dp1
+S'last-complete-prefix'
+p2
+NsS'version'
+p3
+I1
+sS'current-cycle-start-time'
+p4
+F1635003106.611748
+sS'last-cycle-finished'
+p5
+I312
+sS'cycle-to-date'
+p6
+(dp7
+Vleases-per-share-histogram
+p8
+(dp9
+I1
+I36793
+sI2
+I1
+ssVspace-recovered
+p10
+(dp11
+Vexamined-buckets-immutable
+p12
+I17183
+sVconfigured-buckets-mutable
+p13
+I0
+sVexamined-shares-mutable
+p14
+I1796
+sVoriginal-shares-mutable
+p15
+I1563
+sVconfigured-buckets-immutable
+p16
+I0
+sVoriginal-shares-immutable
+p17
+I27926
+sVoriginal-diskbytes-immutable
+p18
+I431149056
+sVexamined-shares-immutable
+p19
+I34998
+sVoriginal-buckets
+p20
+I14661
+sVactual-shares-immutable
+p21
+I0
+sVconfigured-shares
+p22
+I0
+sVoriginal-buckets-immutable
+p23
+I13761
+sVactual-diskbytes
+p24
+I4096
+sVactual-shares-mutable
+p25
+I0
+sVconfigured-buckets
+p26
+I1
+sVexamined-buckets-unknown
+p27
+I14
+sVactual-sharebytes
+p28
+I0
+sVoriginal-shares
+p29
+I29489
+sVoriginal-sharebytes
+p30
+I312664812
+sVexamined-sharebytes-immutable
+p31
+I383801602
+sVactual-shares
+p32
+I0
+sVactual-sharebytes-immutable
+p33
+I0
+sVoriginal-diskbytes
+p34
+I441643008
+sVconfigured-diskbytes-mutable
+p35
+I0
+sVconfigured-sharebytes-immutable
+p36
+I0
+sVconfigured-shares-mutable
+p37
+I0
+sVactual-diskbytes-immutable
+p38
+I0
+sVconfigured-diskbytes-immutable
+p39
+I0
+sVoriginal-diskbytes-mutable
+p40
+I10489856
+sVactual-sharebytes-mutable
+p41
+I0
+sVconfigured-sharebytes
+p42
+I0
+sVexamined-shares
+p43
+I36794
+sVactual-diskbytes-mutable
+p44
+I0
+sVactual-buckets
+p45
+I1
+sVoriginal-buckets-mutable
+p46
+I899
+sVconfigured-sharebytes-mutable
+p47
+I0
+sVexamined-sharebytes
+p48
+I390369660
+sVoriginal-sharebytes-immutable
+p49
+I308125753
+sVoriginal-sharebytes-mutable
+p50
+I4539059
+sVactual-buckets-mutable
+p51
+I0
+sVexamined-diskbytes-mutable
+p52
+I9154560
+sVexamined-buckets-mutable
+p53
+I1043
+sVconfigured-shares-immutable
+p54
+I0
+sVexamined-diskbytes
+p55
+I476598272
+sVactual-buckets-immutable
+p56
+I0
+sVexamined-sharebytes-mutable
+p57
+I6568058
+sVexamined-buckets
+p58
+I18241
+sVconfigured-diskbytes
+p59
+I4096
+sVexamined-diskbytes-immutable
+p60
+I467443712
+ssVcorrupt-shares
+p61
+(lp62
+(V2dn6xnlnsqwtnapwxfdivpm3s4
+p63
+I4
+tp64
+a(g63
+I1
+tp65
+a(V2rrzthwsrrxolevmwdvbdy3rqi
+p66
+I4
+tp67
+a(g66
+I1
+tp68
+a(V2skfngcto6h7eqmn4uo7ntk3ne
+p69
+I4
+tp70
+a(g69
+I1
+tp71
+a(V32d5swqpqx2mwix7xmqzvhdwje
+p72
+I4
+tp73
+a(g72
+I1
+tp74
+a(V5mmayp66yflmpon3o6unsnbaca
+p75
+I4
+tp76
+a(g75
+I1
+tp77
+a(V6ixhpvbtre7fnrl6pehlrlflc4
+p78
+I4
+tp79
+a(g78
+I1
+tp80
+a(Vewzhvswjsz4vp2bqkb6mi3bz2u
+p81
+I4
+tp82
+a(g81
+I1
+tp83
+a(Vfu7pazf6ogavkqj6z4q5qqex3u
+p84
+I4
+tp85
+a(g84
+I1
+tp86
+a(Vhbyjtqvpcimwxiyqbcbbdn2i4a
+p87
+I4
+tp88
+a(g87
+I1
+tp89
+a(Vpmcjbdkbjdl26k3e6yja77femq
+p90
+I4
+tp91
+a(g90
+I1
+tp92
+a(Vr6swof4v2uttbiiqwj5pi32cm4
+p93
+I4
+tp94
+a(g93
+I1
+tp95
+a(Vt45v5akoktf53evc2fi6gwnv6y
+p96
+I4
+tp97
+a(g96
+I1
+tp98
+a(Vy6zb4faar3rdvn3e6pfg4wlotm
+p99
+I4
+tp100
+a(g99
+I1
+tp101
+a(Vz3yghutvqoqbchjao4lndnrh3a
+p102
+I4
+tp103
+a(g102
+I1
+tp104
+asVlease-age-histogram
+p105
+(dp106
+(I45619200
+I45705600
+tp107
+I4
+s(I12441600
+I12528000
+tp108
+I78
+s(I11923200
+I12009600
+tp109
+I89
+s(I33436800
+I33523200
+tp110
+I7
+s(I37411200
+I37497600
+tp111
+I4
+s(I38361600
+I38448000
+tp112
+I5
+s(I4665600
+I4752000
+tp113
+I256
+s(I11491200
+I11577600
+tp114
+I20
+s(I10713600
+I10800000
+tp115
+I183
+s(I42076800
+I42163200
+tp116
+I4
+s(I47865600
+I47952000
+tp117
+I7
+s(I3110400
+I3196800
+tp118
+I328
+s(I5788800
+I5875200
+tp119
+I954
+s(I9331200
+I9417600
+tp120
+I12
+s(I7430400
+I7516800
+tp121
+I7228
+s(I1555200
+I1641600
+tp122
+I492
+s(I37929600
+I38016000
+tp123
+I3
+s(I38880000
+I38966400
+tp124
+I3
+s(I12528000
+I12614400
+tp125
+I193
+s(I10454400
+I10540800
+tp126
+I1239
+s(I11750400
+I11836800
+tp127
+I7
+s(I950400
+I1036800
+tp128
+I4435
+s(I44409600
+I44496000
+tp129
+I13
+s(I12787200
+I12873600
+tp130
+I218
+s(I10368000
+I10454400
+tp131
+I117
+s(I3283200
+I3369600
+tp132
+I86
+s(I7516800
+I7603200
+tp133
+I993
+s(I42336000
+I42422400
+tp134
+I33
+s(I46310400
+I46396800
+tp135
+I1
+s(I39052800
+I39139200
+tp136
+I51
+s(I7603200
+I7689600
+tp137
+I2004
+s(I10540800
+I10627200
+tp138
+I16
+s(I36374400
+I36460800
+tp139
+I3
+s(I3369600
+I3456000
+tp140
+I79
+s(I12700800
+I12787200
+tp141
+I25
+s(I4838400
+I4924800
+tp142
+I386
+s(I10972800
+I11059200
+tp143
+I122
+s(I8812800
+I8899200
+tp144
+I57
+s(I38966400
+I39052800
+tp145
+I61
+s(I3196800
+I3283200
+tp146
+I628
+s(I9244800
+I9331200
+tp147
+I73
+s(I30499200
+I30585600
+tp148
+I5
+s(I12009600
+I12096000
+tp149
+I329
+s(I12960000
+I13046400
+tp150
+I8
+s(I12614400
+I12700800
+tp151
+I210
+s(I3801600
+I3888000
+tp152
+I32
+s(I10627200
+I10713600
+tp153
+I43
+s(I44928000
+I45014400
+tp154
+I2
+s(I8208000
+I8294400
+tp155
+I38
+s(I8640000
+I8726400
+tp156
+I32
+s(I7344000
+I7430400
+tp157
+I12689
+s(I49075200
+I49161600
+tp158
+I19
+s(I2764800
+I2851200
+tp159
+I76
+s(I2592000
+I2678400
+tp160
+I40
+s(I2073600
+I2160000
+tp161
+I388
+s(I37497600
+I37584000
+tp162
+I11
+s(I1641600
+I1728000
+tp163
+I78
+s(I12873600
+I12960000
+tp164
+I5
+s(I1814400
+I1900800
+tp165
+I1860
+s(I40176000
+I40262400
+tp166
+I1
+s(I3715200
+I3801600
+tp167
+I104
+s(I2332800
+I2419200
+tp168
+I12
+s(I2678400
+I2764800
+tp169
+I278
+s(I12268800
+I12355200
+tp170
+I2
+s(I28771200
+I28857600
+tp171
+I6
+s(I41990400
+I42076800
+tp172
+I10
+sssS'last-complete-bucket'
+p173
+NsS'current-cycle'
+p174
+Ns.

src/allmydata/test/eliotutil.py

@@ -42,7 +42,6 @@ from zope.interface import (
 from eliot import (
     ActionType,
     Field,
-    MemoryLogger,
     ILogger,
 )
 from eliot.testing import (
@@ -54,8 +53,9 @@ from twisted.python.monkey import (
     MonkeyPatcher,
 )
 
-from ..util.jsonbytes import AnyBytesJSONEncoder
+from ..util.eliotutil import (
-
+    MemoryLogger,
+)
 
 _NAME = Field.for_types(
     u"name",
@@ -71,14 +71,6 @@ RUN_TEST = ActionType(
 )
 
 
-# On Python 3, we want to use our custom JSON encoder when validating messages
-# can be encoded to JSON:
-if PY2:
-    _memory_logger = MemoryLogger
-else:
-    _memory_logger = lambda: MemoryLogger(encoder=AnyBytesJSONEncoder)
-
-
 @attr.s
 class EliotLoggedRunTest(object):
     """
@@ -170,7 +162,7 @@ def with_logging(
     """
     @wraps(test_method)
     def run_with_logging(*args, **kwargs):
-        validating_logger = _memory_logger()
+        validating_logger = MemoryLogger()
         original = swap_logger(None)
         try:
             swap_logger(_TwoLoggers(original, validating_logger))

src/allmydata/test/no_network.py

@@ -25,6 +25,11 @@ if PY2:
 from past.builtins import unicode
 from six import ensure_text
 
+try:
+    from typing import Dict, Callable
+except ImportError:
+    pass
+
 import os
 from base64 import b32encode
 from functools import (
@@ -479,6 +484,18 @@ class GridTestMixin(object):
 
     def set_up_grid(self, num_clients=1, num_servers=10,
                     client_config_hooks={}, oneshare=False):
+        """
+        Create a Tahoe-LAFS storage grid.
+
+        :param num_clients: See ``NoNetworkGrid``
+        :param num_servers: See `NoNetworkGrid``
+        :param client_config_hooks: See ``NoNetworkGrid``
+
+        :param bool oneshare: If ``True`` then the first client node is
+            configured with ``n == k == happy == 1``.
+
+        :return: ``None``
+        """
         # self.basedir must be set
         port_assigner = SameProcessStreamEndpointAssigner()
         port_assigner.setUp()
@@ -557,6 +574,15 @@ class GridTestMixin(object):
         return sorted(shares)
 
     def copy_shares(self, uri):
+        # type: (bytes) -> Dict[bytes, bytes]
+        """
+        Read all of the share files for the given capability from the storage area
+        of the storage servers created by ``set_up_grid``.
+
+        :param bytes uri: A Tahoe-LAFS data capability.
+
+        :return: A ``dict`` mapping share file names to share file contents.
+        """
         shares = {}
         for (shnum, serverid, sharefile) in self.find_uri_shares(uri):
             with open(sharefile, "rb") as f:
@@ -601,10 +627,15 @@ class GridTestMixin(object):
                     f.write(corruptdata)
 
     def corrupt_all_shares(self, uri, corruptor, debug=False):
+        # type: (bytes, Callable[[bytes, bool], bytes], bool) -> None
+        """
+        Apply ``corruptor`` to the contents of all share files associated with a
+        given capability and replace the share file contents with its result.
+        """
         for (i_shnum, i_serverid, i_sharefile) in self.find_uri_shares(uri):
             with open(i_sharefile, "rb") as f:
                 sharedata = f.read()
-            corruptdata = corruptor(sharedata, debug=debug)
+            corruptdata = corruptor(sharedata, debug)
             with open(i_sharefile, "wb") as f:
                 f.write(corruptdata)
 

src/allmydata/test/strategies.py

@@ -16,6 +16,7 @@ from hypothesis.strategies import (
     one_of,
     builds,
     binary,
+    integers,
 )
 
 from ..uri import (
@@ -119,3 +120,17 @@ def dir2_mdmf_capabilities():
         MDMFDirectoryURI,
         mdmf_capabilities(),
     )
+
+def offsets(min_value=0, max_value=2 ** 16):
+    """
+    Build ``int`` values that could be used as valid offsets into a sequence
+    (such as share data in a share file).
+    """
+    return integers(min_value, max_value)
+
+def lengths(min_value=1, max_value=2 ** 16):
+    """
+    Build ``int`` values that could be used as valid lengths of data (such as
+    share data in a share file).
+    """
+    return integers(min_value, max_value)

src/allmydata/test/test_auth.py

@@ -8,7 +8,16 @@ from __future__ import unicode_literals
 
 from future.utils import PY2
 if PY2:
-    from future.builtins import str  # noqa: F401
+    from future.builtins import str, open  # noqa: F401
+
+from hypothesis import (
+    given,
+)
+from hypothesis.strategies import (
+    text,
+    characters,
+    lists,
+)
 
 from twisted.trial import unittest
 from twisted.python import filepath
@@ -38,25 +47,184 @@ dBSD8940XU3YW+oeq8e+p3yQ2GinHfeJ3BYQyNQLuMAJ
 -----END RSA PRIVATE KEY-----
 """)
 
-DUMMY_ACCOUNTS = u"""\
+DUMMY_KEY_DSA = keys.Key.fromString("""\
-alice herpassword URI:DIR2:aaaaaaaaaaaaaaaaaaaaaaaaaa:1111111111111111111111111111111111111111111111111111
+-----BEGIN OPENSSH PRIVATE KEY-----
-bob sekrit URI:DIR2:bbbbbbbbbbbbbbbbbbbbbbbbbb:2222222222222222222222222222222222222222222222222222
+b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABsQAAAAdzc2gtZH
+NzAAAAgQDKMh/ELaiP21LYRBuPbUy7dUhv/XZwV7aS1LzxSP+KaJvtDOei8X76XEAfkqX+
+aGh9eup+BLkezrV6LlpO9uPzhY8ChlKpkvw5PZKv/2agSrVxZyG7yEzHNtSBQXE6qNMwIk
+N/ycXLGCqyAhQSzRhLz9ETNaslRDLo7YyVWkiuAQAAABUA5nTatFKux5EqZS4EarMWFRBU
+i1UAAACAFpkkK+JsPixSTPyn0DNMoGKA0Klqy8h61Ds6pws+4+aJQptUBshpwNw1ypo7MO
++goDZy3wwdWtURTPGMgesNdEfxp8L2/kqE4vpMK0myoczCqOiWMeNB/x1AStbSkBI8WmHW
+2htgsC01xbaix/FrA3edK8WEyv+oIxlbV1FkrPkAAACANb0EpCc8uoR4/32rO2JLsbcLBw
+H5wc2khe7AKkIa9kUknRIRvoCZUtXF5XuXXdRmnpVEm2KcsLdtZjip43asQcqgt0Kz3nuF
+kAf7bI98G1waFUimcCSPsal4kCmW2HC11sg/BWOt5qczX/0/3xVxpo6juUeBq9ncnFTvPX
+5fOlEAAAHoJkFqHiZBah4AAAAHc3NoLWRzcwAAAIEAyjIfxC2oj9tS2EQbj21Mu3VIb/12
+cFe2ktS88Uj/imib7QznovF++lxAH5Kl/mhofXrqfgS5Hs61ei5aTvbj84WPAoZSqZL8OT
+2Sr/9moEq1cWchu8hMxzbUgUFxOqjTMCJDf8nFyxgqsgIUEs0YS8/REzWrJUQy6O2MlVpI
+rgEAAAAVAOZ02rRSrseRKmUuBGqzFhUQVItVAAAAgBaZJCvibD4sUkz8p9AzTKBigNCpas
+vIetQ7OqcLPuPmiUKbVAbIacDcNcqaOzDvoKA2ct8MHVrVEUzxjIHrDXRH8afC9v5KhOL6
+TCtJsqHMwqjoljHjQf8dQErW0pASPFph1tobYLAtNcW2osfxawN3nSvFhMr/qCMZW1dRZK
+z5AAAAgDW9BKQnPLqEeP99qztiS7G3CwcB+cHNpIXuwCpCGvZFJJ0SEb6AmVLVxeV7l13U
+Zp6VRJtinLC3bWY4qeN2rEHKoLdCs957hZAH+2yPfBtcGhVIpnAkj7GpeJAplthwtdbIPw
+VjreanM1/9P98VcaaOo7lHgavZ3JxU7z1+XzpRAAAAFQC7360pZLbv7PFt4BPFJ8zAHxAe
+QwAAAA5leGFya3VuQGJhcnlvbgECAwQ=
+-----END OPENSSH PRIVATE KEY-----
+""")
 
-# dennis password URI:DIR2:aaaaaaaaaaaaaaaaaaaaaaaaaa:1111111111111111111111111111111111111111111111111111
+ACCOUNTS = u"""\
+# dennis {key} URI:DIR2:aaaaaaaaaaaaaaaaaaaaaaaaaa:1111111111111111111111111111111111111111111111111111
 carol {key} URI:DIR2:cccccccccccccccccccccccccc:3333333333333333333333333333333333333333333333333333
 """.format(key=str(DUMMY_KEY.public().toString("openssh"), "ascii")).encode("ascii")
 
+# Python str.splitlines considers NEXT LINE, LINE SEPARATOR, and PARAGRAPH
+# separator to be line separators, too.  However, file.readlines() does not...
+LINE_SEPARATORS = (
+    '\x0a', # line feed
+    '\x0b', # vertical tab
+    '\x0c', # form feed
+    '\x0d', # carriage return
+)
+
+class AccountFileParserTests(unittest.TestCase):
+    """
+    Tests for ``load_account_file`` and its helper functions.
+    """
+    @given(lists(
+        text(alphabet=characters(
+            blacklist_categories=(
+                # Surrogates are an encoding trick to help out UTF-16.
+                # They're not necessary to represent any non-surrogate code
+                # point in unicode.  They're also not legal individually but
+                # only in pairs.
+                'Cs',
+            ),
+            # Exclude all our line separators too.
+            blacklist_characters=("\n", "\r"),
+        )),
+    ))
+    def test_ignore_comments(self, lines):
+        """
+        ``auth.content_lines`` filters out lines beginning with `#` and empty
+        lines.
+        """
+        expected = set()
+
+        # It's not clear that real files and StringIO behave sufficiently
+        # similarly to use the latter instead of the former here.  In
+        # particular, they seem to have distinct and incompatible
+        # line-splitting rules.
+        bufpath = self.mktemp()
+        with open(bufpath, "wt", encoding="utf-8") as buf:
+            for line in lines:
+                stripped = line.strip()
+                is_content = stripped and not stripped.startswith("#")
+                if is_content:
+                    expected.add(stripped)
+                buf.write(line + "\n")
+
+        with auth.open_account_file(bufpath) as buf:
+            actual = set(auth.content_lines(buf))
+
+        self.assertEqual(expected, actual)
+
+    def test_parse_accounts(self):
+        """
+        ``auth.parse_accounts`` accepts an iterator of account lines and returns
+        an iterator of structured account data.
+        """
+        alice_key = DUMMY_KEY.public().toString("openssh").decode("utf-8")
+        alice_cap = "URI:DIR2:aaaa:1111"
+
+        bob_key = DUMMY_KEY_DSA.public().toString("openssh").decode("utf-8")
+        bob_cap = "URI:DIR2:aaaa:2222"
+        self.assertEqual(
+            list(auth.parse_accounts([
+                "alice {} {}".format(alice_key, alice_cap),
+                "bob {} {}".format(bob_key, bob_cap),
+            ])),
+            [
+                ("alice", DUMMY_KEY.public(), alice_cap),
+                ("bob", DUMMY_KEY_DSA.public(), bob_cap),
+            ],
+        )
+
+    def test_parse_accounts_rejects_passwords(self):
+        """
+        The iterator returned by ``auth.parse_accounts`` raises ``ValueError``
+        when processing reaches a line that has what looks like a password
+        instead of an ssh key.
+        """
+        with self.assertRaises(ValueError):
+            list(auth.parse_accounts(["alice apassword URI:DIR2:aaaa:1111"]))
+
+    def test_create_account_maps(self):
+        """
+        ``auth.create_account_maps`` accepts an iterator of structured account
+        data and returns two mappings: one from account name to rootcap, the
+        other from account name to public keys.
+        """
+        alice_cap = "URI:DIR2:aaaa:1111"
+        alice_key = DUMMY_KEY.public()
+        bob_cap = "URI:DIR2:aaaa:2222"
+        bob_key = DUMMY_KEY_DSA.public()
+        accounts = [
+            ("alice", alice_key, alice_cap),
+            ("bob", bob_key, bob_cap),
+        ]
+        self.assertEqual(
+            auth.create_account_maps(accounts),
+            ({
+                b"alice": alice_cap.encode("utf-8"),
+                b"bob": bob_cap.encode("utf-8"),
+            },
+             {
+                 b"alice": [alice_key],
+                 b"bob": [bob_key],
+             }),
+        )
+
+    def test_load_account_file(self):
+        """
+        ``auth.load_account_file`` accepts an iterator of serialized account lines
+        and returns two mappings: one from account name to rootcap, the other
+        from account name to public keys.
+        """
+        alice_key = DUMMY_KEY.public().toString("openssh").decode("utf-8")
+        alice_cap = "URI:DIR2:aaaa:1111"
+
+        bob_key = DUMMY_KEY_DSA.public().toString("openssh").decode("utf-8")
+        bob_cap = "URI:DIR2:aaaa:2222"
+
+        accounts = [
+            "alice {} {}".format(alice_key, alice_cap),
+            "bob {} {}".format(bob_key, bob_cap),
+            "# carol {} {}".format(alice_key, alice_cap),
+        ]
+
+        self.assertEqual(
+            auth.load_account_file(accounts),
+            ({
+                b"alice": alice_cap.encode("utf-8"),
+                b"bob": bob_cap.encode("utf-8"),
+            },
+             {
+                 b"alice": [DUMMY_KEY.public()],
+                 b"bob": [DUMMY_KEY_DSA.public()],
+             }),
+        )
+
+
 class AccountFileCheckerKeyTests(unittest.TestCase):
     """
     Tests for key handling done by allmydata.frontends.auth.AccountFileChecker.
     """
     def setUp(self):
         self.account_file = filepath.FilePath(self.mktemp())
-        self.account_file.setContent(DUMMY_ACCOUNTS)
+        self.account_file.setContent(ACCOUNTS)
         abspath = abspath_expanduser_unicode(str(self.account_file.path))
         self.checker = auth.AccountFileChecker(None, abspath)
 
-    def test_unknown_user_ssh(self):
+    def test_unknown_user(self):
         """
         AccountFileChecker.requestAvatarId returns a Deferred that fires with
         UnauthorizedLogin if called with an SSHPrivateKey object with a
@@ -67,67 +235,6 @@ class AccountFileCheckerKeyTests(unittest.TestCase):
         avatarId = self.checker.requestAvatarId(key_credentials)
         return self.assertFailure(avatarId, error.UnauthorizedLogin)
 
-    def test_unknown_user_password(self):
-        """
-        AccountFileChecker.requestAvatarId returns a Deferred that fires with
-        UnauthorizedLogin if called with an SSHPrivateKey object with a
-        username not present in the account file.
-
-        We use a commented out user, so we're also checking that comments are
-        skipped.
-        """
-        key_credentials = credentials.UsernamePassword(b"dennis", b"password")
-        d = self.checker.requestAvatarId(key_credentials)
-        return self.assertFailure(d, error.UnauthorizedLogin)
-
-    def test_password_auth_user_with_ssh_key(self):
-        """
-        AccountFileChecker.requestAvatarId returns a Deferred that fires with
-        UnauthorizedLogin if called with an SSHPrivateKey object for a username
-        only associated with a password in the account file.
-        """
-        key_credentials = credentials.SSHPrivateKey(
-            b"alice", b"md5", None, None, None)
-        avatarId = self.checker.requestAvatarId(key_credentials)
-        return self.assertFailure(avatarId, error.UnauthorizedLogin)
-
-    def test_password_auth_user_with_correct_password(self):
-        """
-        AccountFileChecker.requestAvatarId returns a Deferred that fires with
-        the user if the correct password is given.
-        """
-        key_credentials = credentials.UsernamePassword(b"alice", b"herpassword")
-        d = self.checker.requestAvatarId(key_credentials)
-        def authenticated(avatarId):
-            self.assertEqual(
-                (b"alice",
-                 b"URI:DIR2:aaaaaaaaaaaaaaaaaaaaaaaaaa:1111111111111111111111111111111111111111111111111111"),
-                (avatarId.username, avatarId.rootcap))
-        return d
-
-    def test_password_auth_user_with_correct_hashed_password(self):
-        """
-        AccountFileChecker.requestAvatarId returns a Deferred that fires with
-        the user if the correct password is given in hashed form.
-        """
-        key_credentials = credentials.UsernameHashedPassword(b"alice", b"herpassword")
-        d = self.checker.requestAvatarId(key_credentials)
-        def authenticated(avatarId):
-            self.assertEqual(
-                (b"alice",
-                 b"URI:DIR2:aaaaaaaaaaaaaaaaaaaaaaaaaa:1111111111111111111111111111111111111111111111111111"),
-                (avatarId.username, avatarId.rootcap))
-        return d
-
-    def test_password_auth_user_with_wrong_password(self):
-        """
-        AccountFileChecker.requestAvatarId returns a Deferred that fires with
-        UnauthorizedLogin if the wrong password is given.
-        """
-        key_credentials = credentials.UsernamePassword(b"alice", b"WRONG")
-        avatarId = self.checker.requestAvatarId(key_credentials)
-        return self.assertFailure(avatarId, error.UnauthorizedLogin)
-
     def test_unrecognized_key(self):
         """
         AccountFileChecker.requestAvatarId returns a Deferred that fires with

src/allmydata/test/test_client.py

@@ -89,6 +89,7 @@ from .common import (
     UseTestPlugins,
     MemoryIntroducerClient,
     get_published_announcements,
+    UseNode,
 )
 from .matchers import (
     MatchesSameElements,
@@ -953,13 +954,14 @@ class Run(unittest.TestCase, testutil.StallMixin):
 
     @defer.inlineCallbacks
     def test_reloadable(self):
-        basedir = FilePath("test_client.Run.test_reloadable")
+        from twisted.internet import reactor
-        private = basedir.child("private")
+
-        private.makedirs()
         dummy = "pb://wl74cyahejagspqgy4x5ukrvfnevlknt@127.0.0.1:58889/bogus"
-        write_introducer(basedir, "someintroducer", dummy)
+        fixture = UseNode(None, None, FilePath(self.mktemp()), dummy, reactor=reactor)
-        basedir.child("tahoe.cfg").setContent(BASECONFIG. encode("ascii"))
+        fixture.setUp()
-        c1 = yield client.create_client(basedir.path)
+        self.addCleanup(fixture.cleanUp)
+
+        c1 = yield fixture.create_node()
         c1.setServiceParent(self.sparent)
 
         # delay to let the service start up completely. I'm not entirely sure
@@ -981,7 +983,7 @@ class Run(unittest.TestCase, testutil.StallMixin):
         # also change _check_exit_trigger to use it instead of a raw
         # reactor.stop, also instrument the shutdown event in an
         # attribute that we can check.)
-        c2 = yield client.create_client(basedir.path)
+        c2 = yield fixture.create_node()
         c2.setServiceParent(self.sparent)
         yield c2.disownServiceParent()
 

src/allmydata/test/test_common_util.py

@@ -10,16 +10,30 @@ from future.utils import PY2
 if PY2:
     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
 
+import sys
 import random
-import unittest
 
+from hypothesis import given
+from hypothesis.strategies import lists, sampled_from
+from testtools.matchers import Equals
+from twisted.python.reflect import (
+    ModuleNotFound,
+    namedAny,
+)
+
+from .common import (
+    SyncTestCase,
+    disable_modules,
+)
 from allmydata.test.common_util import flip_one_bit
 
 
-class TestFlipOneBit(unittest.TestCase):
+class TestFlipOneBit(SyncTestCase):
 
     def setUp(self):
-        random.seed(42)  # I tried using version=1 on PY3 to avoid the if below, to no avail.
+        super(TestFlipOneBit, self).setUp()
+        # I tried using version=1 on PY3 to avoid the if below, to no avail.
+        random.seed(42)
 
     def test_accepts_byte_string(self):
         actual = flip_one_bit(b'foo')
@@ -27,3 +41,61 @@ class TestFlipOneBit(unittest.TestCase):
 
     def test_rejects_unicode_string(self):
         self.assertRaises(AssertionError, flip_one_bit, u'foo')
+
+
+
+def some_existing_modules():
+    """
+    Build the names of modules (as native strings) that exist and can be
+    imported.
+    """
+    candidates = sorted(
+        name
+        for name
+        in sys.modules
+        if "." not in name
+        and sys.modules[name] is not None
+    )
+    return sampled_from(candidates)
+
+class DisableModulesTests(SyncTestCase):
+    """
+    Tests for ``disable_modules``.
+    """
+    def setup_example(self):
+        return sys.modules.copy()
+
+    def teardown_example(self, safe_modules):
+        sys.modules.update(safe_modules)
+
+    @given(lists(some_existing_modules(), unique=True))
+    def test_importerror(self, module_names):
+        """
+        While the ``disable_modules`` context manager is active any import of the
+        modules identified by the names passed to it result in ``ImportError``
+        being raised.
+        """
+        def get_modules():
+            return list(
+                namedAny(name)
+                for name
+                in module_names
+            )
+        before_modules = get_modules()
+
+        with disable_modules(*module_names):
+            for name in module_names:
+                with self.assertRaises(ModuleNotFound):
+                    namedAny(name)
+
+        after_modules = get_modules()
+        self.assertThat(before_modules, Equals(after_modules))
+
+    def test_dotted_names_rejected(self):
+        """
+        If names with "." in them are passed to ``disable_modules`` then
+        ``ValueError`` is raised.
+        """
+        with self.assertRaises(ValueError):
+            with disable_modules("foo.bar"):
+                pass

src/allmydata/test/test_download.py

@@ -14,6 +14,11 @@ if PY2:
 # a previous run. This asserts that the current code is capable of decoding
 # shares from a previous version.
 
+try:
+    from typing import Any
+except ImportError:
+    pass
+
 import six
 import os
 from twisted.trial import unittest
@@ -493,7 +498,7 @@ class DownloadTest(_Base, unittest.TestCase):
         d.addCallback(_done)
         return d
 
-    def test_simultaneous_onefails_onecancelled(self):
+    def test_simul_1fail_1cancel(self):
         # This exercises an mplayer behavior in ticket #1154. I believe that
         # mplayer made two simultaneous webapi GET requests: first one for an
         # index region at the end of the (mp3/video) file, then one for the
@@ -951,12 +956,52 @@ class Corruption(_Base, unittest.TestCase):
         self.corrupt_shares_numbered(imm_uri, [2], _corruptor)
 
     def _corrupt_set(self, ign, imm_uri, which, newvalue):
+        # type: (Any, bytes, int, int) -> None
+        """
+        Replace a single byte share file number 2 for the given capability with a
+        new byte.
+
+        :param imm_uri: Corrupt share number 2 belonging to this capability.
+        :param which: The byte position to replace.
+        :param newvalue: The new byte value to set in the share.
+        """
         log.msg("corrupt %d" % which)
         def _corruptor(s, debug=False):
             return s[:which] + bchr(newvalue) + s[which+1:]
         self.corrupt_shares_numbered(imm_uri, [2], _corruptor)
 
     def test_each_byte(self):
+        """
+        Test share selection behavior of the downloader in the face of certain
+        kinds of data corruption.
+
+        1. upload a small share to the no-network grid
+        2. read all of the resulting share files out of the no-network storage servers
+        3. for each of
+
+           a. each byte of the share file version field
+           b. each byte of the immutable share version field
+           c. each byte of the immutable share data offset field
+           d. the most significant byte of the block_shares offset field
+           e. one of the bytes of one of the merkle trees
+           f. one of the bytes of the share hashes list
+
+           i. flip the least significant bit in all of the the share files
+           ii. perform the download/check/restore process
+
+        4. add 2 ** 24 to the share file version number
+        5. perform the download/check/restore process
+
+        6. add 2 ** 24 to the share version number
+        7. perform the download/check/restore process
+
+        The download/check/restore process is:
+
+        1. attempt to download the data
+        2. assert that the recovered plaintext is correct
+        3. assert that only the "correct" share numbers were used to reconstruct the plaintext
+        4. restore all of the share files to their pristine condition
+        """
         # Setting catalog_detection=True performs an exhaustive test of the
         # Downloader's response to corruption in the lsb of each byte of the
         # 2070-byte share, with two goals: make sure we tolerate all forms of
@@ -1068,8 +1113,16 @@ class Corruption(_Base, unittest.TestCase):
                 d.addCallback(_download, imm_uri, i, expected)
                 d.addCallback(lambda ign: self.restore_all_shares(self.shares))
                 d.addCallback(fireEventually)
-            corrupt_values = [(3, 2, "no-sh2"),
+            corrupt_values = [
-                              (15, 2, "need-4th"), # share looks v2
+                # Make the container version for share number 2 look
+                # unsupported.  If you add support for immutable share file
+                # version number much past 16 million then you will have to
+                # update this test.  Also maybe you have other problems.
+                (1, 255, "no-sh2"),
+                # Make the immutable share number 2 (not the container, the
+                # thing inside the container) look unsupported.  Ditto the
+                # above about version numbers in the ballpark of 16 million.
+                (13, 255, "need-4th"),
             ]
             for i,newvalue,expected in corrupt_values:
                 d.addCallback(self._corrupt_set, imm_uri, i, newvalue)
@@ -1145,8 +1198,18 @@ class Corruption(_Base, unittest.TestCase):
         return d
 
     def _corrupt_flip_all(self, ign, imm_uri, which):
+        # type: (Any, bytes, int) -> None
+        """
+        Flip the least significant bit at a given byte position in all share files
+        for the given capability.
+        """
         def _corruptor(s, debug=False):
-            return s[:which] + bchr(ord(s[which:which+1])^0x01) + s[which+1:]
+            # type: (bytes, bool) -> bytes
+            before_corruption = s[:which]
+            after_corruption = s[which+1:]
+            original_byte = s[which:which+1]
+            corrupt_byte = bchr(ord(original_byte) ^ 0x01)
+            return b"".join([before_corruption, corrupt_byte, after_corruption])
         self.corrupt_all_shares(imm_uri, _corruptor)
 
 class DownloadV2(_Base, unittest.TestCase):

src/allmydata/test/test_eliotutil.py

@@ -27,13 +27,12 @@ from fixtures import (
 )
 from testtools import (
     TestCase,
-)
-from testtools import (
     TestResult,
 )
 from testtools.matchers import (
     Is,
     IsInstance,
+    Not,
     MatchesStructure,
     Equals,
     HasLength,
@@ -65,11 +64,11 @@ from twisted.internet.task import deferLater
 from twisted.internet import reactor
 
 from ..util.eliotutil import (
+    eliot_json_encoder,
     log_call_deferred,
     _parse_destination_description,
     _EliotLogging,
 )
-from ..util.jsonbytes import AnyBytesJSONEncoder
 
 from .common import (
     SyncTestCase,
@@ -77,24 +76,105 @@ from .common import (
 )
 
 
-class EliotLoggedTestTests(AsyncTestCase):
+def passes():
+    """
+    Create a matcher that matches a ``TestCase`` that runs without failures or
+    errors.
+    """
+    def run(case):
+        result = TestResult()
+        case.run(result)
+        return result.wasSuccessful()
+    return AfterPreprocessing(run, Equals(True))
+
+
+class EliotLoggedTestTests(TestCase):
+    """
+    Tests for the automatic log-related provided by ``AsyncTestCase``.
+
+    This class uses ``testtools.TestCase`` because it is inconvenient to nest
+    ``AsyncTestCase`` inside ``AsyncTestCase`` (in particular, Eliot messages
+    emitted by the inner test case get observed by the outer test case and if
+    an inner case emits invalid messages they cause the outer test case to
+    fail).
+    """
+    def test_fails(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass can fail.
+        """
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
+                self.fail("make sure it can fail")
+
+        self.assertThat(UnderTest("test_it"), Not(passes()))
+
+    def test_unserializable_fails(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass that logs an unserializable
+        value with Eliot fails.
+        """
+        class world(object):
+            """
+            an unserializable object
+            """
+
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
+                Message.log(hello=world)
+
+        self.assertThat(UnderTest("test_it"), Not(passes()))
+
+    def test_logs_non_utf_8_byte(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass can log a message that
+        contains a non-UTF-8 byte string and return ``None`` and pass.
+        """
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
+                Message.log(hello=b"\xFF")
+
+        self.assertThat(UnderTest("test_it"), passes())
+
     def test_returns_none(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass can log a message and
+        return ``None`` and pass.
+        """
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
                 Message.log(hello="world")
 
+        self.assertThat(UnderTest("test_it"), passes())
+
     def test_returns_fired_deferred(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass can log a message and
+        return an already-fired ``Deferred`` and pass.
+        """
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
                 Message.log(hello="world")
                 return succeed(None)
 
+        self.assertThat(UnderTest("test_it"), passes())
+
     def test_returns_unfired_deferred(self):
+        """
+        A test method of an ``AsyncTestCase`` subclass can log a message and
+        return an unfired ``Deferred`` and pass when the ``Deferred`` fires.
+        """
+        class UnderTest(AsyncTestCase):
+            def test_it(self):
                 Message.log(hello="world")
-        # @eliot_logged_test automatically gives us an action context but it's
+                # @eliot_logged_test automatically gives us an action context
-        # still our responsibility to maintain it across stack-busting
+                # but it's still our responsibility to maintain it across
-        # operations.
+                # stack-busting operations.
                 d = DeferredContext(deferLater(reactor, 0.0, lambda: None))
                 d.addCallback(lambda ignored: Message.log(goodbye="world"))
                 # We didn't start an action.  We're not finishing an action.
                 return d.result
 
+        self.assertThat(UnderTest("test_it"), passes())
 
 
 class ParseDestinationDescriptionTests(SyncTestCase):
@@ -109,7 +189,7 @@ class ParseDestinationDescriptionTests(SyncTestCase):
         reactor = object()
         self.assertThat(
             _parse_destination_description("file:-")(reactor),
-            Equals(FileDestination(stdout, encoder=AnyBytesJSONEncoder)),
+            Equals(FileDestination(stdout, encoder=eliot_json_encoder)),
         )
 
 

src/allmydata/test/test_istorageserver.py

@@ -21,6 +21,7 @@ if PY2:
 from random import Random
 
 from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.task import Clock
 
 from foolscap.api import Referenceable, RemoteException
 
@@ -1017,16 +1018,17 @@ class _FoolscapMixin(SystemTestMixin):
                 self.server = s
                 break
         assert self.server is not None, "Couldn't find StorageServer"
-        self._current_time = 123456
+        self._clock = Clock()
-        self.server._get_current_time = self.fake_time
+        self._clock.advance(123456)
+        self.server._clock = self._clock
 
     def fake_time(self):
         """Return the current fake, test-controlled, time."""
-        return self._current_time
+        return self._clock.seconds()
 
     def fake_sleep(self, seconds):
         """Advance the fake time by the given number of seconds."""
-        self._current_time += seconds
+        self._clock.advance(seconds)
 
     @inlineCallbacks
     def tearDown(self):

src/allmydata/test/test_node.py

@@ -69,6 +69,8 @@ import allmydata.test.common_util as testutil
 
 from .common import (
     ConstantAddresses,
+    SameProcessStreamEndpointAssigner,
+    UseNode,
 )
 
 def port_numbers():
@@ -80,11 +82,10 @@ class LoggingMultiService(service.MultiService):
 
 
 # see https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2946
-def testing_tub(config_data=''):
+def testing_tub(reactor, config_data=''):
     """
     Creates a 'main' Tub for testing purposes, from config data
     """
-    from twisted.internet import reactor
     basedir = 'dummy_basedir'
     config = config_from_string(basedir, 'DEFAULT_PORTNUMFILE_BLANK', config_data)
     fileutil.make_dirs(os.path.join(basedir, 'private'))
@@ -112,6 +113,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         # try to bind the port.  We'll use a low-numbered one that's likely to
         # conflict with another service to prove it.
         self._available_port = 22
+        self.port_assigner = SameProcessStreamEndpointAssigner()
+        self.port_assigner.setUp()
+        self.addCleanup(self.port_assigner.tearDown)
 
     def _test_location(
             self,
@@ -137,11 +141,23 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         :param local_addresses: If not ``None`` then a list of addresses to
             supply to the system under test as local addresses.
         """
+        from twisted.internet import reactor
+
         basedir = self.mktemp()
         create_node_dir(basedir, "testing")
+        if tub_port is None:
+            # Always configure a usable tub.port address instead of relying on
+            # the automatic port assignment.  The automatic port assignment is
+            # prone to collisions and spurious test failures.
+            _, tub_port = self.port_assigner.assign(reactor)
+
         config_data = "[node]\n"
-        if tub_port:
         config_data += "tub.port = {}\n".format(tub_port)
+
+        # If they wanted a certain location, go for it.  This probably won't
+        # agree with the tub.port value we set but that only matters if
+        # anything tries to use this to establish a connection ... which
+        # nothing in this test suite will.
         if tub_location is not None:
             config_data += "tub.location = {}\n".format(tub_location)
 
@@ -149,7 +165,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             self.patch(iputil, 'get_local_addresses_sync',
                        lambda: local_addresses)
 
-        tub = testing_tub(config_data)
+        tub = testing_tub(reactor, config_data)
 
         class Foo(object):
             pass
@@ -431,7 +447,12 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_logdir_is_str(self):
-        basedir = "test_node/test_logdir_is_str"
+        from twisted.internet import reactor
+
+        basedir = FilePath(self.mktemp())
+        fixture = UseNode(None, None, basedir, "pb://introducer/furl", {}, reactor=reactor)
+        fixture.setUp()
+        self.addCleanup(fixture.cleanUp)
 
         ns = Namespace()
         ns.called = False
@@ -440,8 +461,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             self.failUnless(isinstance(logdir, str), logdir)
         self.patch(foolscap.logging.log, 'setLogDir', call_setLogDir)
 
-        create_node_dir(basedir, "nothing to see here")
+        yield fixture.create_node()
-        yield client.create_client(basedir)
         self.failUnless(ns.called)
 
     def test_set_config_unescaped_furl_hash(self):

src/allmydata/test/test_storage_http.py

@@ -0,0 +1,84 @@
+"""
+Tests for HTTP storage client + server.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+
+if PY2:
+    # 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
+
+from unittest import SkipTest
+
+from twisted.trial.unittest import TestCase
+from twisted.internet.defer import inlineCallbacks
+
+from treq.testing import StubTreq
+from hyperlink import DecodedURL
+
+from ..storage.server import StorageServer
+from ..storage.http_server import HTTPServer
+from ..storage.http_client import StorageClient, ClientException
+
+
+class HTTPTests(TestCase):
+    """
+    Tests of HTTP client talking to the HTTP server.
+    """
+
+    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.client = StorageClient(
+            DecodedURL.from_text("http://127.0.0.1"),
+            b"abcd",
+            treq=StubTreq(self._http_server.get_resource()),
+        )
+
+    @inlineCallbacks
+    def test_bad_authentication(self):
+        """
+        If the wrong swissnum is used, an ``Unauthorized`` response code is
+        returned.
+        """
+        client = StorageClient(
+            DecodedURL.from_text("http://127.0.0.1"),
+            b"something wrong",
+            treq=StubTreq(self._http_server.get_resource()),
+        )
+        with self.assertRaises(ClientException) as e:
+            yield client.get_version()
+        self.assertEqual(e.exception.args[0], 401)
+
+    @inlineCallbacks
+    def test_version(self):
+        """
+        The client can return the version.
+
+        We ignore available disk space and max immutable share size, since that
+        might change across calls.
+        """
+        version = yield self.client.get_version()
+        version[b"http://allmydata.org/tahoe/protocols/storage/v1"].pop(
+            b"available-space"
+        )
+        version[b"http://allmydata.org/tahoe/protocols/storage/v1"].pop(
+            b"maximum-immutable-share-size"
+        )
+        expected_version = self.storage_server.remote_get_version()
+        expected_version[b"http://allmydata.org/tahoe/protocols/storage/v1"].pop(
+            b"available-space"
+        )
+        expected_version[b"http://allmydata.org/tahoe/protocols/storage/v1"].pop(
+            b"maximum-immutable-share-size"
+        )
+        self.assertEqual(version, expected_version)

src/allmydata/test/test_storage_web.py

@@ -19,25 +19,40 @@ import time
 import os.path
 import re
 import json
+from unittest import skipIf
+from six.moves import StringIO
 
 from twisted.trial import unittest
-
 from twisted.internet import defer
 from twisted.application import service
 from twisted.web.template import flattenString
+from twisted.python.filepath import FilePath
+from twisted.python.runtime import platform
 
 from foolscap.api import fireEventually
 from allmydata.util import fileutil, hashutil, base32, pollmixin
 from allmydata.storage.common import storage_index_to_dir, \
      UnknownMutableContainerVersionError, UnknownImmutableContainerVersionError
 from allmydata.storage.server import StorageServer
-from allmydata.storage.crawler import BucketCountingCrawler
+from allmydata.storage.crawler import (
-from allmydata.storage.expirer import LeaseCheckingCrawler
+    BucketCountingCrawler,
+    _LeaseStateSerializer,
+)
+from allmydata.storage.expirer import (
+    LeaseCheckingCrawler,
+    _HistorySerializer,
+)
 from allmydata.web.storage import (
     StorageStatus,
     StorageStatusElement,
     remove_prefix
 )
+from allmydata.scripts.admin import (
+    migrate_crawler,
+)
+from allmydata.scripts.runner import (
+    Options,
+)
 from .common_util import FakeCanary
 
 from .common_web import (
@@ -376,7 +391,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             self.failUnlessEqual(type(lah), list)
             self.failUnlessEqual(len(lah), 1)
             self.failUnlessEqual(lah, [ (0.0, DAY, 1) ] )
-            self.failUnlessEqual(so_far["leases-per-share-histogram"], {1: 1})
+            self.failUnlessEqual(so_far["leases-per-share-histogram"], {"1": 1})
             self.failUnlessEqual(so_far["corrupt-shares"], [])
             sr1 = so_far["space-recovered"]
             self.failUnlessEqual(sr1["examined-buckets"], 1)
@@ -427,9 +442,9 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             self.failIf("cycle-to-date" in s)
             self.failIf("estimated-remaining-cycle" in s)
             self.failIf("estimated-current-cycle" in s)
-            last = s["history"][0]
+            last = s["history"]["0"]
             self.failUnlessIn("cycle-start-finish-times", last)
-            self.failUnlessEqual(type(last["cycle-start-finish-times"]), tuple)
+            self.failUnlessEqual(type(last["cycle-start-finish-times"]), list)
             self.failUnlessEqual(last["expiration-enabled"], False)
             self.failUnlessIn("configured-expiration-mode", last)
 
@@ -437,9 +452,9 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             lah = last["lease-age-histogram"]
             self.failUnlessEqual(type(lah), list)
             self.failUnlessEqual(len(lah), 1)
-            self.failUnlessEqual(lah, [ (0.0, DAY, 6) ] )
+            self.failUnlessEqual(lah, [ [0.0, DAY, 6] ] )
 
-            self.failUnlessEqual(last["leases-per-share-histogram"], {1: 2, 2: 2})
+            self.failUnlessEqual(last["leases-per-share-histogram"], {"1": 2, "2": 2})
             self.failUnlessEqual(last["corrupt-shares"], [])
 
             rec = last["space-recovered"]
@@ -485,17 +500,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
         return d
 
     def backdate_lease(self, sf, renew_secret, new_expire_time):
-        # ShareFile.renew_lease ignores attempts to back-date a lease (i.e.
+        sf.renew_lease(renew_secret, new_expire_time, allow_backdate=True)
-        # "renew" a lease with a new_expire_time that is older than what the
-        # current lease has), so we have to reach inside it.
-        for i,lease in enumerate(sf.get_leases()):
-            if lease.renew_secret == renew_secret:
-                lease.expiration_time = new_expire_time
-                f = open(sf.home, 'rb+')
-                sf._write_lease_record(f, i, lease)
-                f.close()
-                return
-        raise IndexError("unable to renew non-existent lease")
 
     def test_expire_age(self):
         basedir = "storage/LeaseCrawler/expire_age"
@@ -597,12 +602,12 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             self.failUnlessEqual(count_leases(mutable_si_3), 1)
 
             s = lc.get_state()
-            last = s["history"][0]
+            last = s["history"]["0"]
 
             self.failUnlessEqual(last["expiration-enabled"], True)
             self.failUnlessEqual(last["configured-expiration-mode"],
-                                 ("age", 2000, None, ("mutable", "immutable")))
+                                 ["age", 2000, None, ["mutable", "immutable"]])
-            self.failUnlessEqual(last["leases-per-share-histogram"], {1: 2, 2: 2})
+            self.failUnlessEqual(last["leases-per-share-histogram"], {"1": 2, "2": 2})
 
             rec = last["space-recovered"]
             self.failUnlessEqual(rec["examined-buckets"], 4)
@@ -741,14 +746,14 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             self.failUnlessEqual(count_leases(mutable_si_3), 1)
 
             s = lc.get_state()
-            last = s["history"][0]
+            last = s["history"]["0"]
 
             self.failUnlessEqual(last["expiration-enabled"], True)
             self.failUnlessEqual(last["configured-expiration-mode"],
-                                 ("cutoff-date", None, then,
+                                 ["cutoff-date", None, then,
-                                  ("mutable", "immutable")))
+                                  ["mutable", "immutable"]])
             self.failUnlessEqual(last["leases-per-share-histogram"],
-                                 {1: 2, 2: 2})
+                                 {"1": 2, "2": 2})
 
             rec = last["space-recovered"]
             self.failUnlessEqual(rec["examined-buckets"], 4)
@@ -934,8 +939,8 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
             s = lc.get_state()
             h = s["history"]
             self.failUnlessEqual(len(h), 10)
-            self.failUnlessEqual(max(h.keys()), 15)
+            self.failUnlessEqual(max(int(k) for k in h.keys()), 15)
-            self.failUnlessEqual(min(h.keys()), 6)
+            self.failUnlessEqual(min(int(k) for k in h.keys()), 6)
         d.addCallback(_check)
         return d
 
@@ -1024,7 +1029,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
 
         def _check(ignored):
             s = lc.get_state()
-            last = s["history"][0]
+            last = s["history"]["0"]
             rec = last["space-recovered"]
             self.failUnlessEqual(rec["configured-buckets"], 4)
             self.failUnlessEqual(rec["configured-shares"], 4)
@@ -1120,7 +1125,7 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
 
         def _after_first_cycle(ignored):
             s = lc.get_state()
-            last = s["history"][0]
+            last = s["history"]["0"]
             rec = last["space-recovered"]
             self.failUnlessEqual(rec["examined-buckets"], 5)
             self.failUnlessEqual(rec["examined-shares"], 3)
@@ -1149,6 +1154,390 @@ class LeaseCrawler(unittest.TestCase, pollmixin.PollMixin):
         d.addBoth(_cleanup)
         return d
 
+    @skipIf(platform.isWindows(), "pickle test-data can't be loaded on windows")
+    def test_deserialize_pickle(self):
+        """
+        The crawler can read existing state from the old pickle format
+        """
+        # this file came from an "in the wild" tahoe version 1.16.0
+        original_pickle = FilePath(__file__).parent().child("data").child("lease_checker.state.txt")
+        root = FilePath(self.mktemp())
+        storage = root.child("storage")
+        storage.makedirs()
+        test_pickle = storage.child("lease_checker.state")
+        with test_pickle.open("wb") as local, original_pickle.open("rb") as remote:
+            local.write(remote.read())
+
+        # convert from pickle format to JSON
+        top = Options()
+        top.parseOptions([
+            "admin", "migrate-crawler",
+            "--basedir", storage.parent().path,
+        ])
+        options = top.subOptions
+        while hasattr(options, "subOptions"):
+            options = options.subOptions
+        options.stdout = StringIO()
+        migrate_crawler(options)
+
+        # the (existing) state file should have been upgraded to JSON
+        self.assertFalse(test_pickle.exists())
+        self.assertTrue(test_pickle.siblingExtension(".json").exists())
+        serial = _LeaseStateSerializer(test_pickle.path)
+
+        self.assertEqual(
+            serial.load(),
+            {
+                u'last-complete-prefix': None,
+                u'version': 1,
+                u'current-cycle-start-time': 1635003106.611748,
+                u'last-cycle-finished': 312,
+                u'cycle-to-date': {
+                    u'leases-per-share-histogram': {
+                        u'1': 36793,
+                        u'2': 1,
+                    },
+                    u'space-recovered': {
+                        u'examined-buckets-immutable': 17183,
+                        u'configured-buckets-mutable': 0,
+                        u'examined-shares-mutable': 1796,
+                        u'original-shares-mutable': 1563,
+                        u'configured-buckets-immutable': 0,
+                        u'original-shares-immutable': 27926,
+                        u'original-diskbytes-immutable': 431149056,
+                        u'examined-shares-immutable': 34998,
+                        u'original-buckets': 14661,
+                        u'actual-shares-immutable': 0,
+                        u'configured-shares': 0,
+                        u'original-buckets-mutable': 899,
+                        u'actual-diskbytes': 4096,
+                        u'actual-shares-mutable': 0,
+                        u'configured-buckets': 1,
+                        u'examined-buckets-unknown': 14,
+                        u'actual-sharebytes': 0,
+                        u'original-shares': 29489,
+                        u'actual-buckets-immutable': 0,
+                        u'original-sharebytes': 312664812,
+                        u'examined-sharebytes-immutable': 383801602,
+                        u'actual-shares': 0,
+                        u'actual-sharebytes-immutable': 0,
+                        u'original-diskbytes': 441643008,
+                        u'configured-diskbytes-mutable': 0,
+                        u'configured-sharebytes-immutable': 0,
+                        u'configured-shares-mutable': 0,
+                        u'actual-diskbytes-immutable': 0,
+                        u'configured-diskbytes-immutable': 0,
+                        u'original-diskbytes-mutable': 10489856,
+                        u'actual-sharebytes-mutable': 0,
+                        u'configured-sharebytes': 0,
+                        u'examined-shares': 36794,
+                        u'actual-diskbytes-mutable': 0,
+                        u'actual-buckets': 1,
+                        u'original-buckets-immutable': 13761,
+                        u'configured-sharebytes-mutable': 0,
+                        u'examined-sharebytes': 390369660,
+                        u'original-sharebytes-immutable': 308125753,
+                        u'original-sharebytes-mutable': 4539059,
+                        u'actual-buckets-mutable': 0,
+                        u'examined-buckets-mutable': 1043,
+                        u'configured-shares-immutable': 0,
+                        u'examined-diskbytes': 476598272,
+                        u'examined-diskbytes-mutable': 9154560,
+                        u'examined-sharebytes-mutable': 6568058,
+                        u'examined-buckets': 18241,
+                        u'configured-diskbytes': 4096,
+                        u'examined-diskbytes-immutable': 467443712},
+                    u'corrupt-shares': [
+                        [u'2dn6xnlnsqwtnapwxfdivpm3s4', 4],
+                        [u'2dn6xnlnsqwtnapwxfdivpm3s4', 1],
+                        [u'2rrzthwsrrxolevmwdvbdy3rqi', 4],
+                        [u'2rrzthwsrrxolevmwdvbdy3rqi', 1],
+                        [u'2skfngcto6h7eqmn4uo7ntk3ne', 4],
+                        [u'2skfngcto6h7eqmn4uo7ntk3ne', 1],
+                        [u'32d5swqpqx2mwix7xmqzvhdwje', 4],
+                        [u'32d5swqpqx2mwix7xmqzvhdwje', 1],
+                        [u'5mmayp66yflmpon3o6unsnbaca', 4],
+                        [u'5mmayp66yflmpon3o6unsnbaca', 1],
+                        [u'6ixhpvbtre7fnrl6pehlrlflc4', 4],
+                        [u'6ixhpvbtre7fnrl6pehlrlflc4', 1],
+                        [u'ewzhvswjsz4vp2bqkb6mi3bz2u', 4],
+                        [u'ewzhvswjsz4vp2bqkb6mi3bz2u', 1],
+                        [u'fu7pazf6ogavkqj6z4q5qqex3u', 4],
+                        [u'fu7pazf6ogavkqj6z4q5qqex3u', 1],
+                        [u'hbyjtqvpcimwxiyqbcbbdn2i4a', 4],
+                        [u'hbyjtqvpcimwxiyqbcbbdn2i4a', 1],
+                        [u'pmcjbdkbjdl26k3e6yja77femq', 4],
+                        [u'pmcjbdkbjdl26k3e6yja77femq', 1],
+                        [u'r6swof4v2uttbiiqwj5pi32cm4', 4],
+                        [u'r6swof4v2uttbiiqwj5pi32cm4', 1],
+                        [u't45v5akoktf53evc2fi6gwnv6y', 4],
+                        [u't45v5akoktf53evc2fi6gwnv6y', 1],
+                        [u'y6zb4faar3rdvn3e6pfg4wlotm', 4],
+                        [u'y6zb4faar3rdvn3e6pfg4wlotm', 1],
+                        [u'z3yghutvqoqbchjao4lndnrh3a', 4],
+                        [u'z3yghutvqoqbchjao4lndnrh3a', 1],
+                    ],
+                    u'lease-age-histogram': {
+                        "1641600,1728000": 78,
+                        "12441600,12528000": 78,
+                        "8640000,8726400": 32,
+                        "1814400,1900800": 1860,
+                        "2764800,2851200": 76,
+                        "11491200,11577600": 20,
+                        "10713600,10800000": 183,
+                        "47865600,47952000": 7,
+                        "3110400,3196800": 328,
+                        "10627200,10713600": 43,
+                        "45619200,45705600": 4,
+                        "12873600,12960000": 5,
+                        "7430400,7516800": 7228,
+                        "1555200,1641600": 492,
+                        "38880000,38966400": 3,
+                        "12528000,12614400": 193,
+                        "7344000,7430400": 12689,
+                        "2678400,2764800": 278,
+                        "2332800,2419200": 12,
+                        "9244800,9331200": 73,
+                        "12787200,12873600": 218,
+                        "49075200,49161600": 19,
+                        "10368000,10454400": 117,
+                        "4665600,4752000": 256,
+                        "7516800,7603200": 993,
+                        "42336000,42422400": 33,
+                        "10972800,11059200": 122,
+                        "39052800,39139200": 51,
+                        "12614400,12700800": 210,
+                        "7603200,7689600": 2004,
+                        "10540800,10627200": 16,
+                        "950400,1036800": 4435,
+                        "42076800,42163200": 4,
+                        "8812800,8899200": 57,
+                        "5788800,5875200": 954,
+                        "36374400,36460800": 3,
+                        "9331200,9417600": 12,
+                        "30499200,30585600": 5,
+                        "12700800,12787200": 25,
+                        "2073600,2160000": 388,
+                        "12960000,13046400": 8,
+                        "11923200,12009600": 89,
+                        "3369600,3456000": 79,
+                        "3196800,3283200": 628,
+                        "37497600,37584000": 11,
+                        "33436800,33523200": 7,
+                        "44928000,45014400": 2,
+                        "37929600,38016000": 3,
+                        "38966400,39052800": 61,
+                        "3283200,3369600": 86,
+                        "11750400,11836800": 7,
+                        "3801600,3888000": 32,
+                        "46310400,46396800": 1,
+                        "4838400,4924800": 386,
+                        "8208000,8294400": 38,
+                        "37411200,37497600": 4,
+                        "12009600,12096000": 329,
+                        "10454400,10540800": 1239,
+                        "40176000,40262400": 1,
+                        "3715200,3801600": 104,
+                        "44409600,44496000": 13,
+                        "38361600,38448000": 5,
+                        "12268800,12355200": 2,
+                        "28771200,28857600": 6,
+                        "41990400,42076800": 10,
+                        "2592000,2678400": 40,
+                    },
+                },
+                'current-cycle': None,
+                'last-complete-bucket': None,
+            }
+        )
+        second_serial = _LeaseStateSerializer(serial._path.path)
+        self.assertEqual(
+            serial.load(),
+            second_serial.load(),
+        )
+
+    @skipIf(platform.isWindows(), "pickle test-data can't be loaded on windows")
+    def test_deserialize_history_pickle(self):
+        """
+        The crawler can read existing history state from the old pickle
+        format
+        """
+        # this file came from an "in the wild" tahoe version 1.16.0
+        original_pickle = FilePath(__file__).parent().child("data").child("lease_checker.history.txt")
+        root = FilePath(self.mktemp())
+        storage = root.child("storage")
+        storage.makedirs()
+        test_pickle = storage.child("lease_checker.history")
+        with test_pickle.open("wb") as local, original_pickle.open("rb") as remote:
+            local.write(remote.read())
+
+        # convert from pickle format to JSON
+        top = Options()
+        top.parseOptions([
+            "admin", "migrate-crawler",
+            "--basedir", storage.parent().path,
+        ])
+        options = top.subOptions
+        while hasattr(options, "subOptions"):
+            options = options.subOptions
+        options.stdout = StringIO()
+        migrate_crawler(options)
+
+        serial = _HistorySerializer(test_pickle.path)
+
+        self.maxDiff = None
+        self.assertEqual(
+            serial.load(),
+            {
+                "363": {
+                    'configured-expiration-mode': ['age', None, None, ['immutable', 'mutable']],
+                    'expiration-enabled': False,
+                    'leases-per-share-histogram': {
+                        '1': 39774,
+                    },
+                    'lease-age-histogram': [
+                        [0, 86400, 3125],
+                        [345600, 432000, 4175],
+                        [950400, 1036800, 141],
+                        [1036800, 1123200, 345],
+                        [1123200, 1209600, 81],
+                        [1296000, 1382400, 1832],
+                        [1555200, 1641600, 390],
+                        [1728000, 1814400, 12],
+                        [2073600, 2160000, 84],
+                        [2160000, 2246400, 228],
+                        [2246400, 2332800, 75],
+                        [2592000, 2678400, 644],
+                        [2678400, 2764800, 273],
+                        [2764800, 2851200, 94],
+                        [2851200, 2937600, 97],
+                        [3196800, 3283200, 143],
+                        [3283200, 3369600, 48],
+                        [4147200, 4233600, 374],
+                        [4320000, 4406400, 534],
+                        [5270400, 5356800, 1005],
+                        [6739200, 6825600, 8704],
+                        [6825600, 6912000, 3986],
+                        [6912000, 6998400, 7592],
+                        [6998400, 7084800, 2607],
+                        [7689600, 7776000, 35],
+                        [8035200, 8121600, 33],
+                        [8294400, 8380800, 54],
+                        [8640000, 8726400, 45],
+                        [8726400, 8812800, 27],
+                        [8812800, 8899200, 12],
+                        [9763200, 9849600, 77],
+                        [9849600, 9936000, 91],
+                        [9936000, 10022400, 1210],
+                        [10022400, 10108800, 45],
+                        [10108800, 10195200, 186],
+                        [10368000, 10454400, 113],
+                        [10972800, 11059200, 21],
+                        [11232000, 11318400, 5],
+                        [11318400, 11404800, 19],
+                        [11404800, 11491200, 238],
+                        [11491200, 11577600, 159],
+                        [11750400, 11836800, 1],
+                        [11836800, 11923200, 32],
+                        [11923200, 12009600, 192],
+                        [12009600, 12096000, 222],
+                        [12096000, 12182400, 18],
+                        [12182400, 12268800, 224],
+                        [12268800, 12355200, 9],
+                        [12355200, 12441600, 9],
+                        [12441600, 12528000, 10],
+                        [12528000, 12614400, 6],
+                        [12614400, 12700800, 6],
+                        [12700800, 12787200, 18],
+                        [12787200, 12873600, 6],
+                        [12873600, 12960000, 62],
+                    ],
+                    'cycle-start-finish-times': [1634446505.241972, 1634446666.055401],
+                    'space-recovered': {
+                        'examined-buckets-immutable': 17896,
+                        'configured-buckets-mutable': 0,
+                        'examined-shares-mutable': 2473,
+                        'original-shares-mutable': 1185,
+                        'configured-buckets-immutable': 0,
+                        'original-shares-immutable': 27457,
+                        'original-diskbytes-immutable': 2810982400,
+                        'examined-shares-immutable': 37301,
+                        'original-buckets': 14047,
+                        'actual-shares-immutable': 0,
+                        'configured-shares': 0,
+                        'original-buckets-mutable': 691,
+                        'actual-diskbytes': 4096,
+                        'actual-shares-mutable': 0,
+                        'configured-buckets': 1,
+                        'examined-buckets-unknown': 14,
+                        'actual-sharebytes': 0,
+                        'original-shares': 28642,
+                        'actual-buckets-immutable': 0,
+                        'original-sharebytes': 2695552941,
+                        'examined-sharebytes-immutable': 2754798505,
+                        'actual-shares': 0,
+                        'actual-sharebytes-immutable': 0,
+                        'original-diskbytes': 2818981888,
+                        'configured-diskbytes-mutable': 0,
+                        'configured-sharebytes-immutable': 0,
+                        'configured-shares-mutable': 0,
+                        'actual-diskbytes-immutable': 0,
+                        'configured-diskbytes-immutable': 0,
+                        'original-diskbytes-mutable': 7995392,
+                        'actual-sharebytes-mutable': 0,
+                        'configured-sharebytes': 0,
+                        'examined-shares': 39774,
+                        'actual-diskbytes-mutable': 0,
+                        'actual-buckets': 1,
+                        'original-buckets-immutable': 13355,
+                        'configured-sharebytes-mutable': 0,
+                        'examined-sharebytes': 2763646972,
+                        'original-sharebytes-immutable': 2692076909,
+                        'original-sharebytes-mutable': 3476032,
+                        'actual-buckets-mutable': 0,
+                        'examined-buckets-mutable': 1286,
+                        'configured-shares-immutable': 0,
+                        'examined-diskbytes': 2854801408,
+                        'examined-diskbytes-mutable': 12161024,
+                        'examined-sharebytes-mutable': 8848467,
+                        'examined-buckets': 19197,
+                        'configured-diskbytes': 4096,
+                        'examined-diskbytes-immutable': 2842640384
+                    },
+                    'corrupt-shares': [
+                        ['2dn6xnlnsqwtnapwxfdivpm3s4', 3],
+                        ['2dn6xnlnsqwtnapwxfdivpm3s4', 0],
+                        ['2rrzthwsrrxolevmwdvbdy3rqi', 3],
+                        ['2rrzthwsrrxolevmwdvbdy3rqi', 0],
+                        ['2skfngcto6h7eqmn4uo7ntk3ne', 3],
+                        ['2skfngcto6h7eqmn4uo7ntk3ne', 0],
+                        ['32d5swqpqx2mwix7xmqzvhdwje', 3],
+                        ['32d5swqpqx2mwix7xmqzvhdwje', 0],
+                        ['5mmayp66yflmpon3o6unsnbaca', 3],
+                        ['5mmayp66yflmpon3o6unsnbaca', 0],
+                        ['6ixhpvbtre7fnrl6pehlrlflc4', 3],
+                        ['6ixhpvbtre7fnrl6pehlrlflc4', 0],
+                        ['ewzhvswjsz4vp2bqkb6mi3bz2u', 3],
+                        ['ewzhvswjsz4vp2bqkb6mi3bz2u', 0],
+                        ['fu7pazf6ogavkqj6z4q5qqex3u', 3],
+                        ['fu7pazf6ogavkqj6z4q5qqex3u', 0],
+                        ['hbyjtqvpcimwxiyqbcbbdn2i4a', 3],
+                        ['hbyjtqvpcimwxiyqbcbbdn2i4a', 0],
+                        ['pmcjbdkbjdl26k3e6yja77femq', 3],
+                        ['pmcjbdkbjdl26k3e6yja77femq', 0],
+                        ['r6swof4v2uttbiiqwj5pi32cm4', 3],
+                        ['r6swof4v2uttbiiqwj5pi32cm4', 0],
+                        ['t45v5akoktf53evc2fi6gwnv6y', 3],
+                        ['t45v5akoktf53evc2fi6gwnv6y', 0],
+                        ['y6zb4faar3rdvn3e6pfg4wlotm', 3],
+                        ['y6zb4faar3rdvn3e6pfg4wlotm', 0],
+                        ['z3yghutvqoqbchjao4lndnrh3a', 3],
+                        ['z3yghutvqoqbchjao4lndnrh3a', 0],
+                    ]
+                }
+            }
+        )
+
 
 class WebStatus(unittest.TestCase, pollmixin.PollMixin):
 

src/allmydata/test/test_system.py

@@ -23,6 +23,7 @@ from twisted.internet import defer
 
 from allmydata import uri
 from allmydata.storage.mutable import MutableShareFile
+from allmydata.storage.immutable import ShareFile
 from allmydata.storage.server import si_a2b
 from allmydata.immutable import offloaded, upload
 from allmydata.immutable.literal import LiteralFileNode
@@ -1290,8 +1291,8 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
                 # are sharefiles here
                 filename = os.path.join(dirpath, filenames[0])
                 # peek at the magic to see if it is a chk share
-                magic = open(filename, "rb").read(4)
+                with open(filename, "rb") as f:
-                if magic == b'\x00\x00\x00\x01':
+                    if ShareFile.is_valid_header(f.read(32)):
                         break
         else:
             self.fail("unable to find any uri_extension files in %r"

src/allmydata/test/test_util.py

@@ -553,11 +553,6 @@ class JSONBytes(unittest.TestCase):
                 o, cls=jsonbytes.AnyBytesJSONEncoder)),
             expected,
         )
-        self.assertEqual(
-            json.loads(jsonbytes.dumps(o, any_bytes=True)),
-            expected
-        )
-
 
 
 class FakeGetVersion(object):

src/allmydata/test/web/test_grid.py

@@ -18,7 +18,6 @@ from six.moves import StringIO
 from bs4 import BeautifulSoup
 
 from twisted.web import resource
-from twisted.trial import unittest
 from allmydata import uri, dirnode
 from allmydata.util import base32
 from allmydata.util.encodingutil import to_bytes
@@ -43,6 +42,21 @@ from .common import (
     unknown_rwcap,
 )
 
+from ..common import (
+    AsyncTestCase,
+)
+
+from testtools.matchers import (
+    Equals,
+    Contains,
+    Not,
+    HasLength,
+    EndsWith,
+)
+
+from testtools.twistedsupport import flush_logged_errors
+
+
 DIR_HTML_TAG = '<html lang="en">'
 
 class CompletelyUnhandledError(Exception):
@@ -53,7 +67,7 @@ class ErrorBoom(resource.Resource, object):
     def render(self, req):
         raise CompletelyUnhandledError("whoops")
 
-class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMixin, unittest.TestCase):
+class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMixin, AsyncTestCase):
 
     def CHECK(self, ign, which, args, clientnum=0):
         fileurl = self.fileurls[which]
@@ -117,37 +131,37 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "good", "t=check")
         def _got_html_good(res):
-            self.failUnlessIn("Healthy", res)
+            self.assertThat(res, Contains("Healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy", )))
             soup = BeautifulSoup(res, 'html5lib')
             assert_soup_has_favicon(self, soup)
 
         d.addCallback(_got_html_good)
         d.addCallback(self.CHECK, "good", "t=check&return_to=somewhere")
         def _got_html_good_return_to(res):
-            self.failUnlessIn("Healthy", res)
+            self.assertThat(res, Contains("Healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
-            self.failUnlessIn('<a href="somewhere">Return to file', res)
+            self.assertThat(res, Contains('<a href="somewhere">Return to file'))
         d.addCallback(_got_html_good_return_to)
         d.addCallback(self.CHECK, "good", "t=check&output=json")
         def _got_json_good(res):
             r = json.loads(res)
             self.failUnlessEqual(r["summary"], "Healthy")
             self.failUnless(r["results"]["healthy"])
-            self.failIfIn("needs-rebalancing", r["results"])
+            self.assertThat(r["results"], Not(Contains("needs-rebalancing",)))
             self.failUnless(r["results"]["recoverable"])
         d.addCallback(_got_json_good)
 
         d.addCallback(self.CHECK, "small", "t=check")
         def _got_html_small(res):
-            self.failUnlessIn("Literal files are always healthy", res)
+            self.assertThat(res, Contains("Literal files are always healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
         d.addCallback(_got_html_small)
         d.addCallback(self.CHECK, "small", "t=check&return_to=somewhere")
         def _got_html_small_return_to(res):
-            self.failUnlessIn("Literal files are always healthy", res)
+            self.assertThat(res, Contains("Literal files are always healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
-            self.failUnlessIn('<a href="somewhere">Return to file', res)
+            self.assertThat(res, Contains('<a href="somewhere">Return to file'))
         d.addCallback(_got_html_small_return_to)
         d.addCallback(self.CHECK, "small", "t=check&output=json")
         def _got_json_small(res):
@@ -158,8 +172,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "smalldir", "t=check")
         def _got_html_smalldir(res):
-            self.failUnlessIn("Literal files are always healthy", res)
+            self.assertThat(res, Contains("Literal files are always healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
         d.addCallback(_got_html_smalldir)
         d.addCallback(self.CHECK, "smalldir", "t=check&output=json")
         def _got_json_smalldir(res):
@@ -170,43 +184,43 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "sick", "t=check")
         def _got_html_sick(res):
-            self.failUnlessIn("Not Healthy", res)
+            self.assertThat(res, Contains("Not Healthy"))
         d.addCallback(_got_html_sick)
         d.addCallback(self.CHECK, "sick", "t=check&output=json")
         def _got_json_sick(res):
             r = json.loads(res)
             self.failUnlessEqual(r["summary"],
                                  "Not Healthy: 9 shares (enc 3-of-10)")
-            self.failIf(r["results"]["healthy"])
+            self.assertThat(r["results"]["healthy"], Equals(False))
             self.failUnless(r["results"]["recoverable"])
-            self.failIfIn("needs-rebalancing", r["results"])
+            self.assertThat(r["results"], Not(Contains("needs-rebalancing")))
         d.addCallback(_got_json_sick)
 
         d.addCallback(self.CHECK, "dead", "t=check")
         def _got_html_dead(res):
-            self.failUnlessIn("Not Healthy", res)
+            self.assertThat(res, Contains("Not Healthy"))
         d.addCallback(_got_html_dead)
         d.addCallback(self.CHECK, "dead", "t=check&output=json")
         def _got_json_dead(res):
             r = json.loads(res)
             self.failUnlessEqual(r["summary"],
                                  "Not Healthy: 1 shares (enc 3-of-10)")
-            self.failIf(r["results"]["healthy"])
+            self.assertThat(r["results"]["healthy"], Equals(False))
-            self.failIf(r["results"]["recoverable"])
+            self.assertThat(r["results"]["recoverable"], Equals(False))
-            self.failIfIn("needs-rebalancing", r["results"])
+            self.assertThat(r["results"], Not(Contains("needs-rebalancing")))
         d.addCallback(_got_json_dead)
 
         d.addCallback(self.CHECK, "corrupt", "t=check&verify=true")
         def _got_html_corrupt(res):
-            self.failUnlessIn("Not Healthy! : Unhealthy", res)
+            self.assertThat(res, Contains("Not Healthy! : Unhealthy"))
         d.addCallback(_got_html_corrupt)
         d.addCallback(self.CHECK, "corrupt", "t=check&verify=true&output=json")
         def _got_json_corrupt(res):
             r = json.loads(res)
-            self.failUnlessIn("Unhealthy: 9 shares (enc 3-of-10)", r["summary"])
+            self.assertThat(r["summary"], Contains("Unhealthy: 9 shares (enc 3-of-10)"))
-            self.failIf(r["results"]["healthy"])
+            self.assertThat(r["results"]["healthy"], Equals(False))
             self.failUnless(r["results"]["recoverable"])
-            self.failIfIn("needs-rebalancing", r["results"])
+            self.assertThat(r["results"], Not(Contains("needs-rebalancing")))
             self.failUnlessReallyEqual(r["results"]["count-happiness"], 9)
             self.failUnlessReallyEqual(r["results"]["count-shares-good"], 9)
             self.failUnlessReallyEqual(r["results"]["count-corrupt-shares"], 1)
@@ -261,9 +275,9 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "good", "t=check&repair=true")
         def _got_html_good(res):
-            self.failUnlessIn("Healthy", res)
+            self.assertThat(res, Contains("Healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
-            self.failUnlessIn("No repair necessary", res)
+            self.assertThat(res, Contains("No repair necessary", ))
             soup = BeautifulSoup(res, 'html5lib')
             assert_soup_has_favicon(self, soup)
 
@@ -271,9 +285,9 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "sick", "t=check&repair=true")
         def _got_html_sick(res):
-            self.failUnlessIn("Healthy : healthy", res)
+            self.assertThat(res, Contains("Healthy : healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
-            self.failUnlessIn("Repair successful", res)
+            self.assertThat(res, Contains("Repair successful"))
         d.addCallback(_got_html_sick)
 
         # repair of a dead file will fail, of course, but it isn't yet
@@ -290,9 +304,9 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "corrupt", "t=check&verify=true&repair=true")
         def _got_html_corrupt(res):
-            self.failUnlessIn("Healthy : Healthy", res)
+            self.assertThat(res, Contains("Healthy : Healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
-            self.failUnlessIn("Repair successful", res)
+            self.assertThat(res, Contains("Repair successful"))
         d.addCallback(_got_html_corrupt)
 
         d.addErrback(self.explain_web_error)
@@ -392,31 +406,31 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             if expect_rw_uri:
                 self.failUnlessReallyEqual(to_bytes(f[1]["rw_uri"]), unknown_rwcap, data)
             else:
-                self.failIfIn("rw_uri", f[1])
+                self.assertThat(f[1], Not(Contains("rw_uri")))
             if immutable:
                 self.failUnlessReallyEqual(to_bytes(f[1]["ro_uri"]), unknown_immcap, data)
             else:
                 self.failUnlessReallyEqual(to_bytes(f[1]["ro_uri"]), unknown_rocap, data)
-            self.failUnlessIn("metadata", f[1])
+            self.assertThat(f[1], Contains("metadata"))
         d.addCallback(_check_directory_json, expect_rw_uri=not immutable)
 
         def _check_info(res, expect_rw_uri, expect_ro_uri):
             if expect_rw_uri:
-                self.failUnlessIn(unknown_rwcap, res)
+                self.assertThat(res, Contains(unknown_rwcap))
             if expect_ro_uri:
                 if immutable:
-                    self.failUnlessIn(unknown_immcap, res)
+                    self.assertThat(res, Contains(unknown_immcap))
                 else:
-                    self.failUnlessIn(unknown_rocap, res)
+                    self.assertThat(res, Contains(unknown_rocap))
             else:
-                self.failIfIn(unknown_rocap, res)
+                self.assertThat(res, Not(Contains(unknown_rocap)))
             res = str(res, "utf-8")
-            self.failUnlessIn("Object Type: <span>unknown</span>", res)
+            self.assertThat(res, Contains("Object Type: <span>unknown</span>"))
-            self.failIfIn("Raw data as", res)
+            self.assertThat(res, Not(Contains("Raw data as")))
-            self.failIfIn("Directory writecap", res)
+            self.assertThat(res, Not(Contains("Directory writecap")))
-            self.failIfIn("Checker Operations", res)
+            self.assertThat(res, Not(Contains("Checker Operations")))
-            self.failIfIn("Mutable File Operations", res)
+            self.assertThat(res, Not(Contains("Mutable File Operations")))
-            self.failIfIn("Directory Operations", res)
+            self.assertThat(res, Not(Contains("Directory Operations")))
 
         # FIXME: these should have expect_rw_uri=not immutable; I don't know
         # why they fail. Possibly related to ticket #922.
@@ -432,7 +446,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             if expect_rw_uri:
                 self.failUnlessReallyEqual(to_bytes(data[1]["rw_uri"]), unknown_rwcap, data)
             else:
-                self.failIfIn("rw_uri", data[1])
+                self.assertThat(data[1], Not(Contains("rw_uri")))
 
             if immutable:
                 self.failUnlessReallyEqual(to_bytes(data[1]["ro_uri"]), unknown_immcap, data)
@@ -442,10 +456,10 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                 self.failUnlessReallyEqual(data[1]["mutable"], True)
             else:
                 self.failUnlessReallyEqual(to_bytes(data[1]["ro_uri"]), unknown_rocap, data)
-                self.failIfIn("mutable", data[1])
+                self.assertThat(data[1], Not(Contains("mutable")))
 
             # TODO: check metadata contents
-            self.failUnlessIn("metadata", data[1])
+            self.assertThat(data[1], Contains("metadata"))
 
         d.addCallback(lambda ign: self.GET("%s/%s?t=json" % (self.rooturl, str(name))))
         d.addCallback(_check_json, expect_rw_uri=not immutable)
@@ -519,14 +533,14 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         def _created(dn):
             self.failUnless(isinstance(dn, dirnode.DirectoryNode))
-            self.failIf(dn.is_mutable())
+            self.assertThat(dn.is_mutable(), Equals(False))
             self.failUnless(dn.is_readonly())
             # This checks that if we somehow ended up calling dn._decrypt_rwcapdata, it would fail.
-            self.failIf(hasattr(dn._node, 'get_writekey'))
+            self.assertThat(hasattr(dn._node, 'get_writekey'), Equals(False))
             rep = str(dn)
-            self.failUnlessIn("RO-IMM", rep)
+            self.assertThat(rep, Contains("RO-IMM"))
             cap = dn.get_cap()
-            self.failUnlessIn(b"CHK", cap.to_string())
+            self.assertThat(cap.to_string(), Contains(b"CHK"))
             self.cap = cap
             self.rootnode = dn
             self.rooturl = "uri/" + url_quote(dn.get_uri())
@@ -546,7 +560,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                 (name_utf8, ro_uri, rwcapdata, metadata_s), subpos = split_netstring(entry, 4)
                 name = name_utf8.decode("utf-8")
                 self.failUnlessEqual(rwcapdata, b"")
-                self.failUnlessIn(name, kids)
+                self.assertThat(kids, Contains(name))
                 (expected_child, ign) = kids[name]
                 self.failUnlessReallyEqual(ro_uri, expected_child.get_readonly_uri())
                 numkids += 1
@@ -572,27 +586,27 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
         d.addCallback(lambda ign: self.GET(self.rooturl))
         def _check_html(res):
             soup = BeautifulSoup(res, 'html5lib')
-            self.failIfIn(b"URI:SSK", res)
+            self.assertThat(res, Not(Contains(b"URI:SSK")))
             found = False
             for td in soup.find_all(u"td"):
                 if td.text != u"FILE":
                     continue
                 a = td.findNextSibling()(u"a")[0]
-                self.assertIn(url_quote(lonely_uri), a[u"href"])
+                self.assertThat(a[u"href"], Contains(url_quote(lonely_uri)))
-                self.assertEqual(u"lonely", a.text)
+                self.assertThat(a.text, Equals(u"lonely"))
-                self.assertEqual(a[u"rel"], [u"noreferrer"])
+                self.assertThat(a[u"rel"], Equals([u"noreferrer"]))
-                self.assertEqual(u"{}".format(len("one")), td.findNextSibling().findNextSibling().text)
+                self.assertThat(td.findNextSibling().findNextSibling().text, Equals(u"{}".format(len("one"))))
                 found = True
                 break
-            self.assertTrue(found)
+            self.assertThat(found, Equals(True))
 
             infos = list(
                 a[u"href"]
                 for a in soup.find_all(u"a")
                 if a.text == u"More Info"
             )
-            self.assertEqual(1, len(infos))
+            self.assertThat(infos, HasLength(1))
-            self.assertTrue(infos[0].endswith(url_quote(lonely_uri) + "?t=info"))
+            self.assertThat(infos[0], EndsWith(url_quote(lonely_uri) + "?t=info"))
         d.addCallback(_check_html)
 
         # ... and in JSON.
@@ -604,7 +618,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             self.failUnlessReallyEqual(sorted(listed_children.keys()), [u"lonely"])
             ll_type, ll_data = listed_children[u"lonely"]
             self.failUnlessEqual(ll_type, "filenode")
-            self.failIfIn("rw_uri", ll_data)
+            self.assertThat(ll_data, Not(Contains("rw_uri")))
             self.failUnlessReallyEqual(to_bytes(ll_data["ro_uri"]), lonely_uri)
         d.addCallback(_check_json)
         return d
@@ -744,8 +758,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             error_line = lines[first_error]
             error_msg = lines[first_error+1:]
             error_msg_s = "\n".join(error_msg) + "\n"
-            self.failUnlessIn("ERROR: UnrecoverableFileError(no recoverable versions)",
+            self.assertThat(error_line, Contains("ERROR: UnrecoverableFileError(no recoverable versions)"))
-                              error_line)
             self.failUnless(len(error_msg) > 2, error_msg_s) # some traceback
             units = [json.loads(line) for line in lines[:first_error]]
             self.failUnlessReallyEqual(len(units), 6) # includes subdir
@@ -765,8 +778,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             error_line = lines[first_error]
             error_msg = lines[first_error+1:]
             error_msg_s = "\n".join(error_msg) + "\n"
-            self.failUnlessIn("ERROR: UnrecoverableFileError(no recoverable versions)",
+            self.assertThat(error_line, Contains("ERROR: UnrecoverableFileError(no recoverable versions)"))
-                              error_line)
             self.failUnless(len(error_msg) > 2, error_msg_s) # some traceback
             units = [json.loads(line) for line in lines[:first_error]]
             self.failUnlessReallyEqual(len(units), 6) # includes subdir
@@ -936,8 +948,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
 
         d.addCallback(self.CHECK, "one", "t=check") # no add-lease
         def _got_html_good(res):
-            self.failUnlessIn("Healthy", res)
+            self.assertThat(res, Contains("Healthy"))
-            self.failIfIn("Not Healthy", res)
+            self.assertThat(res, Not(Contains("Not Healthy")))
         d.addCallback(_got_html_good)
 
         d.addCallback(self._count_leases, "one")
@@ -1111,7 +1123,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            self.GET, self.fileurls["0shares"]))
         def _check_zero_shares(body):
             body = str(body, "utf-8")
-            self.failIfIn("<html>", body)
+            self.assertThat(body, Not(Contains("<html>")))
             body = " ".join(body.strip().split())
             exp = ("NoSharesError: no shares could be found. "
                    "Zero shares usually indicates a corrupt URI, or that "
@@ -1129,7 +1141,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            self.GET, self.fileurls["1share"]))
         def _check_one_share(body):
             body = str(body, "utf-8")
-            self.failIfIn("<html>", body)
+            self.assertThat(body, Not(Contains("<html>")))
             body = " ".join(body.strip().split())
             msgbase = ("NotEnoughSharesError: This indicates that some "
                        "servers were unavailable, or that shares have been "
@@ -1154,17 +1166,16 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            self.GET, self.fileurls["imaginary"]))
         def _missing_child(body):
             body = str(body, "utf-8")
-            self.failUnlessIn("No such child: imaginary", body)
+            self.assertThat(body, Contains("No such child: imaginary"))
         d.addCallback(_missing_child)
 
         d.addCallback(lambda ignored: self.GET_unicode(self.fileurls["dir-0share"]))
         def _check_0shares_dir_html(body):
-            self.failUnlessIn(DIR_HTML_TAG, body)
+            self.assertThat(body, Contains(DIR_HTML_TAG))
             # we should see the regular page, but without the child table or
             # the dirops forms
             body = " ".join(body.strip().split())
-            self.failUnlessIn('href="?t=info">More info on this directory',
+            self.assertThat(body, Contains('href="?t=info">More info on this directory'))
-                              body)
             exp = ("UnrecoverableFileError: the directory (or mutable file) "
                    "could not be retrieved, because there were insufficient "
                    "good shares. This might indicate that no servers were "
@@ -1172,8 +1183,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                    "was corrupt, or that shares have been lost due to server "
                    "departure, hard drive failure, or disk corruption. You "
                    "should perform a filecheck on this object to learn more.")
-            self.failUnlessIn(exp, body)
+            self.assertThat(body, Contains(exp))
-            self.failUnlessIn("No upload forms: directory is unreadable", body)
+            self.assertThat(body, Contains("No upload forms: directory is unreadable"))
         d.addCallback(_check_0shares_dir_html)
 
         d.addCallback(lambda ignored: self.GET_unicode(self.fileurls["dir-1share"]))
@@ -1182,10 +1193,9 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             # and some-shares like we did for immutable files (since there
             # are different sorts of advice to offer in each case). For now,
             # they present the same way.
-            self.failUnlessIn(DIR_HTML_TAG, body)
+            self.assertThat(body, Contains(DIR_HTML_TAG))
             body = " ".join(body.strip().split())
-            self.failUnlessIn('href="?t=info">More info on this directory',
+            self.assertThat(body, Contains('href="?t=info">More info on this directory'))
-                              body)
             exp = ("UnrecoverableFileError: the directory (or mutable file) "
                    "could not be retrieved, because there were insufficient "
                    "good shares. This might indicate that no servers were "
@@ -1193,8 +1203,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                    "was corrupt, or that shares have been lost due to server "
                    "departure, hard drive failure, or disk corruption. You "
                    "should perform a filecheck on this object to learn more.")
-            self.failUnlessIn(exp, body)
+            self.assertThat(body, Contains(exp))
-            self.failUnlessIn("No upload forms: directory is unreadable", body)
+            self.assertThat(body, Contains("No upload forms: directory is unreadable"))
         d.addCallback(_check_1shares_dir_html)
 
         d.addCallback(lambda ignored:
@@ -1204,7 +1214,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            self.fileurls["dir-0share-json"]))
         def _check_unrecoverable_file(body):
             body = str(body, "utf-8")
-            self.failIfIn("<html>", body)
+            self.assertThat(body, Not(Contains("<html>")))
             body = " ".join(body.strip().split())
             exp = ("UnrecoverableFileError: the directory (or mutable file) "
                    "could not be retrieved, because there were insufficient "
@@ -1213,7 +1223,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                    "was corrupt, or that shares have been lost due to server "
                    "departure, hard drive failure, or disk corruption. You "
                    "should perform a filecheck on this object to learn more.")
-            self.failUnlessIn(exp, body)
+            self.assertThat(body, Contains(exp))
         d.addCallback(_check_unrecoverable_file)
 
         d.addCallback(lambda ignored:
@@ -1245,7 +1255,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            headers={"accept": "*/*"}))
         def _internal_error_html1(body):
             body = str(body, "utf-8")
-            self.failUnlessIn("<html>", "expected HTML, not '%s'" % body)
+            self.assertThat("expected HTML, not '%s'" % body, Contains("<html>"))
         d.addCallback(_internal_error_html1)
 
         d.addCallback(lambda ignored:
@@ -1255,8 +1265,9 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            headers={"accept": "text/plain"}))
         def _internal_error_text2(body):
             body = str(body, "utf-8")
-            self.failIfIn("<html>", body)
+            self.assertThat(body, Not(Contains("<html>")))
             self.failUnless(body.startswith("Traceback "), body)
+
         d.addCallback(_internal_error_text2)
 
         CLI_accepts = "text/plain, application/octet-stream"
@@ -1267,7 +1278,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            headers={"accept": CLI_accepts}))
         def _internal_error_text3(body):
             body = str(body, "utf-8")
-            self.failIfIn("<html>", body)
+            self.assertThat(body, Not(Contains("<html>")))
             self.failUnless(body.startswith("Traceback "), body)
         d.addCallback(_internal_error_text3)
 
@@ -1276,12 +1287,12 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
                                            500, "Internal Server Error", None,
                                            self.GET, "ERRORBOOM"))
         def _internal_error_html4(body):
-            self.failUnlessIn(b"<html>", body)
+            self.assertThat(body, Contains(b"<html>"))
         d.addCallback(_internal_error_html4)
 
         def _flush_errors(res):
             # Trial: please ignore the CompletelyUnhandledError in the logs
-            self.flushLoggedErrors(CompletelyUnhandledError)
+            flush_logged_errors(CompletelyUnhandledError)
             return res
         d.addBoth(_flush_errors)
 
@@ -1312,8 +1323,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
         d.addCallback(_stash_dir)
         d.addCallback(lambda ign: self.GET_unicode(self.dir_url, followRedirect=True))
         def _check_dir_html(body):
-            self.failUnlessIn(DIR_HTML_TAG, body)
+            self.assertThat(body, Contains(DIR_HTML_TAG))
-            self.failUnlessIn("blacklisted.txt</a>", body)
+            self.assertThat(body, Contains("blacklisted.txt</a>"))
         d.addCallback(_check_dir_html)
         d.addCallback(lambda ign: self.GET(self.url))
         d.addCallback(lambda body: self.failUnlessEqual(DATA, body))
@@ -1336,8 +1347,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
         # We should still be able to list the parent directory, in HTML...
         d.addCallback(lambda ign: self.GET_unicode(self.dir_url, followRedirect=True))
         def _check_dir_html2(body):
-            self.failUnlessIn(DIR_HTML_TAG, body)
+            self.assertThat(body, Contains(DIR_HTML_TAG))
-            self.failUnlessIn("blacklisted.txt</strike>", body)
+            self.assertThat(body, Contains("blacklisted.txt</strike>"))
         d.addCallback(_check_dir_html2)
 
         # ... and in JSON (used by CLI).
@@ -1347,8 +1358,8 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             self.failUnless(isinstance(data, list), data)
             self.failUnlessEqual(data[0], "dirnode")
             self.failUnless(isinstance(data[1], dict), data)
-            self.failUnlessIn("children", data[1])
+            self.assertThat(data[1], Contains("children"))
-            self.failUnlessIn("blacklisted.txt", data[1]["children"])
+            self.assertThat(data[1]["children"], Contains("blacklisted.txt"))
             childdata = data[1]["children"]["blacklisted.txt"]
             self.failUnless(isinstance(childdata, list), data)
             self.failUnlessEqual(childdata[0], "filenode")
@@ -1387,7 +1398,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
             self.child_url = b"uri/"+dn.get_readonly_uri()+b"/child"
         d.addCallback(_get_dircap)
         d.addCallback(lambda ign: self.GET(self.dir_url_base, followRedirect=True))
-        d.addCallback(lambda body: self.failUnlessIn(DIR_HTML_TAG, str(body, "utf-8")))
+        d.addCallback(lambda body: self.assertThat(str(body, "utf-8"), Contains(DIR_HTML_TAG)))
         d.addCallback(lambda ign: self.GET(self.dir_url_json1))
         d.addCallback(lambda res: json.loads(res))  # just check it decodes
         d.addCallback(lambda ign: self.GET(self.dir_url_json2))

src/allmydata/test/web/test_introducer.py

@@ -83,12 +83,18 @@ def create_introducer_webish(reactor, port_assigner, basedir):
         with the node and its webish service.
     """
     node.create_node_dir(basedir, "testing")
-    _, port_endpoint = port_assigner.assign(reactor)
+    main_tub_location, main_tub_endpoint = port_assigner.assign(reactor)
+    _, web_port_endpoint = port_assigner.assign(reactor)
     with open(join(basedir, "tahoe.cfg"), "w") as f:
         f.write(
             "[node]\n"
-            "tub.location = 127.0.0.1:1\n" +
+            "tub.port = {main_tub_endpoint}\n"
-            "web.port = {}\n".format(port_endpoint)
+            "tub.location = {main_tub_location}\n"
+            "web.port = {web_port_endpoint}\n".format(
+                main_tub_endpoint=main_tub_endpoint,
+                main_tub_location=main_tub_location,
+                web_port_endpoint=web_port_endpoint,
+            )
         )
 
     intro_node = yield create_introducer(basedir)

src/allmydata/test/web/test_logs.py

@@ -17,10 +17,8 @@ if PY2:
 
 import json
 
-from twisted.trial import unittest
 from twisted.internet.defer import inlineCallbacks
 
-from eliot import log_call
 
 from autobahn.twisted.testing import create_memory_agent, MemoryReactorClockResolver, create_pumper
 
@@ -48,6 +46,7 @@ from .matchers import (
 
 from ..common import (
     SyncTestCase,
+    AsyncTestCase,
 )
 
 from ...web.logs import (
@@ -55,6 +54,8 @@ from ...web.logs import (
     TokenAuthenticatedWebSocketServerProtocol,
 )
 
+from eliot import log_call
+
 class StreamingEliotLogsTests(SyncTestCase):
     """
     Tests for the log streaming resources created by ``create_log_resources``.
@@ -75,18 +76,20 @@ class StreamingEliotLogsTests(SyncTestCase):
         )
 
 
-class TestStreamingLogs(unittest.TestCase):
+class TestStreamingLogs(AsyncTestCase):
     """
     Test websocket streaming of logs
     """
 
     def setUp(self):
+        super(TestStreamingLogs, self).setUp()
         self.reactor = MemoryReactorClockResolver()
         self.pumper = create_pumper()
         self.agent = create_memory_agent(self.reactor, self.pumper, TokenAuthenticatedWebSocketServerProtocol)
         return self.pumper.start()
 
     def tearDown(self):
+        super(TestStreamingLogs, self).tearDown()
         return self.pumper.stop()
 
     @inlineCallbacks
@@ -114,10 +117,10 @@ class TestStreamingLogs(unittest.TestCase):
         proto.transport.loseConnection()
         yield proto.is_closed
 
-        self.assertEqual(len(messages), 2)
+        self.assertThat(len(messages), Equals(3))
-        self.assertEqual(messages[0]["action_type"], "test:cli:some-exciting-action")
+        self.assertThat(messages[0]["action_type"], Equals("test:cli:some-exciting-action"))
-        self.assertEqual(messages[0]["arguments"],
+        self.assertThat(messages[0]["arguments"],
-                         ["hello", "good-\\xff-day", 123, {"a": 35}, [None]])
+                         Equals(["hello", "good-\\xff-day", 123, {"a": 35}, [None]]))
-        self.assertEqual(messages[1]["action_type"], "test:cli:some-exciting-action")
+        self.assertThat(messages[1]["action_type"], Equals("test:cli:some-exciting-action"))
-        self.assertEqual("started", messages[0]["action_status"])
+        self.assertThat("started", Equals(messages[0]["action_status"]))
-        self.assertEqual("succeeded", messages[1]["action_status"])
+        self.assertThat("succeeded", Equals(messages[1]["action_status"]))

src/allmydata/test/web/test_root.py

@@ -20,10 +20,11 @@ from bs4 import (
     BeautifulSoup,
 )
 
-from twisted.trial import unittest
 from twisted.web.template import Tag
 from twisted.web.test.requesthelper import DummyRequest
 from twisted.application import service
+from testtools.twistedsupport import succeeded
+from twisted.internet.defer import inlineCallbacks
 
 from ...storage_client import (
     NativeStorageServer,
@@ -44,7 +45,17 @@ from ..common import (
     EMPTY_CLIENT_CONFIG,
 )
 
-class RenderSlashUri(unittest.TestCase):
+from ..common import (
+    SyncTestCase,
+)
+
+from testtools.matchers import (
+    Equals,
+    Contains,
+    AfterPreprocessing,
+)
+
+class RenderSlashUri(SyncTestCase):
     """
     Ensure that URIs starting with /uri?uri= only accept valid
     capabilities
@@ -53,7 +64,9 @@ class RenderSlashUri(unittest.TestCase):
     def setUp(self):
         self.client = object()
         self.res = URIHandler(self.client)
+        super(RenderSlashUri, self).setUp()
 
+    @inlineCallbacks
     def test_valid_query_redirect(self):
         """
         A syntactically valid capability given in the ``uri`` query argument
@@ -64,9 +77,7 @@ class RenderSlashUri(unittest.TestCase):
             b"mukesarwdjxiyqsjinbfiiro6q7kgmmekocxfjcngh23oxwyxtzq:2:5:5874882"
         )
         query_args = {b"uri": [cap]}
-        response_body = self.successResultOf(
+        response_body = yield render(self.res, query_args)
-            render(self.res, query_args),
-        )
         soup = BeautifulSoup(response_body, 'html5lib')
         tag = assert_soup_has_tag_with_attributes(
             self,
@@ -74,9 +85,9 @@ class RenderSlashUri(unittest.TestCase):
             u"meta",
             {u"http-equiv": "refresh"},
         )
-        self.assertIn(
+        self.assertThat(
-            quote(cap, safe=""),
             tag.attrs.get(u"content"),
+            Contains(quote(cap, safe="")),
         )
 
     def test_invalid(self):
@@ -84,16 +95,14 @@ class RenderSlashUri(unittest.TestCase):
         A syntactically invalid capbility results in an error.
         """
         query_args = {b"uri": [b"not a capability"]}
-        response_body = self.successResultOf(
+        response_body = render(self.res, query_args)
-            render(self.res, query_args),
+        self.assertThat(
-        )
-        self.assertEqual(
             response_body,
-            b"Invalid capability",
+            succeeded(AfterPreprocessing(bytes, Equals(b"Invalid capability"))),
         )
 
 
-class RenderServiceRow(unittest.TestCase):
+class RenderServiceRow(SyncTestCase):
     def test_missing(self):
         """
         minimally-defined static servers just need anonymous-storage-FURL
@@ -127,5 +136,5 @@ class RenderServiceRow(unittest.TestCase):
         # Coerce `items` to list and pick the first item from it.
         item = list(items)[0]
 
-        self.assertEqual(item.slotData.get("version"), "")
+        self.assertThat(item.slotData.get("version"), Equals(""))
-        self.assertEqual(item.slotData.get("nickname"), "")
+        self.assertThat(item.slotData.get("nickname"), Equals(""))

src/allmydata/util/_eliot_updates.py

@@ -0,0 +1,195 @@
+"""
+Bring in some Eliot updates from newer versions of Eliot than we can
+depend on in Python 2.  The implementations are copied from Eliot 1.14 and
+only changed enough to add Python 2 compatibility.
+
+Every API in this module (except ``eliot_json_encoder``) should be obsolete as
+soon as we depend on Eliot 1.14 or newer.
+
+When that happens:
+
+* replace ``capture_logging``
+  with ``partial(eliot.testing.capture_logging, encoder_=eliot_json_encoder)``
+* replace ``validateLogging``
+  with ``partial(eliot.testing.validateLogging, encoder_=eliot_json_encoder)``
+* replace ``MemoryLogger``
+  with ``partial(eliot.MemoryLogger, encoder=eliot_json_encoder)``
+
+Ported to Python 3.
+"""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+
+from future.utils import PY2
+if PY2:
+    from 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
+
+import json as pyjson
+from functools import wraps, partial
+
+from eliot import (
+    MemoryLogger as _MemoryLogger,
+)
+
+from eliot.testing import (
+    check_for_errors,
+    swap_logger,
+)
+
+from .jsonbytes import AnyBytesJSONEncoder
+
+# There are currently a number of log messages that include non-UTF-8 bytes.
+# Allow these, at least for now.  Later when the whole test suite has been
+# converted to our SyncTestCase or AsyncTestCase it will be easier to turn
+# this off and then attribute log failures to specific codepaths so they can
+# be fixed (and then not regressed later) because those instances will result
+# in test failures instead of only garbage being written to the eliot log.
+eliot_json_encoder = AnyBytesJSONEncoder
+
+class _CustomEncoderMemoryLogger(_MemoryLogger):
+    """
+    Override message validation from the Eliot-supplied ``MemoryLogger`` to
+    use our chosen JSON encoder.
+
+    This is only necessary on Python 2 where we use an old version of Eliot
+    that does not parameterize the encoder.
+    """
+    def __init__(self, encoder=eliot_json_encoder):
+        """
+        @param encoder: A JSONEncoder subclass to use when encoding JSON.
+        """
+        self._encoder = encoder
+        super(_CustomEncoderMemoryLogger, self).__init__()
+
+    def _validate_message(self, dictionary, serializer):
+        """Validate an individual message.
+
+        As a side-effect, the message is replaced with its serialized contents.
+
+        @param dictionary: A message C{dict} to be validated.  Might be mutated
+            by the serializer!
+
+        @param serializer: C{None} or a serializer.
+
+        @raises TypeError: If a field name is not unicode, or the dictionary
+            fails to serialize to JSON.
+
+        @raises eliot.ValidationError: If serializer was given and validation
+            failed.
+        """
+        if serializer is not None:
+            serializer.validate(dictionary)
+        for key in dictionary:
+            if not isinstance(key, str):
+                if isinstance(key, bytes):
+                    key.decode("utf-8")
+                else:
+                    raise TypeError(dictionary, "%r is not unicode" % (key,))
+        if serializer is not None:
+            serializer.serialize(dictionary)
+
+        try:
+            pyjson.dumps(dictionary, cls=self._encoder)
+        except Exception as e:
+            raise TypeError("Message %s doesn't encode to JSON: %s" % (dictionary, e))
+
+if PY2:
+    MemoryLogger = partial(_CustomEncoderMemoryLogger, encoder=eliot_json_encoder)
+else:
+    MemoryLogger = partial(_MemoryLogger, encoder=eliot_json_encoder)
+
+def validateLogging(
+    assertion, *assertionArgs, **assertionKwargs
+):
+    """
+    Decorator factory for L{unittest.TestCase} methods to add logging
+    validation.
+
+    1. The decorated test method gets a C{logger} keyword argument, a
+       L{MemoryLogger}.
+    2. All messages logged to this logger will be validated at the end of
+       the test.
+    3. Any unflushed logged tracebacks will cause the test to fail.
+
+    For example:
+
+        from unittest import TestCase
+        from eliot.testing import assertContainsFields, validateLogging
+
+        class MyTests(TestCase):
+            def assertFooLogging(self, logger):
+                assertContainsFields(self, logger.messages[0], {"key": 123})
+
+
+    @param assertion: A callable that will be called with the
+       L{unittest.TestCase} instance, the logger and C{assertionArgs} and
+       C{assertionKwargs} once the actual test has run, allowing for extra
+       logging-related assertions on the effects of the test. Use L{None} if you
+       want the cleanup assertions registered but no custom assertions.
+
+    @param assertionArgs: Additional positional arguments to pass to
+        C{assertion}.
+
+    @param assertionKwargs: Additional keyword arguments to pass to
+        C{assertion}.
+
+    @param encoder_: C{json.JSONEncoder} subclass to use when validating JSON.
+    """
+    encoder_ = assertionKwargs.pop("encoder_", eliot_json_encoder)
+    def decorator(function):
+        @wraps(function)
+        def wrapper(self, *args, **kwargs):
+            skipped = False
+
+            kwargs["logger"] = logger = MemoryLogger(encoder=encoder_)
+            self.addCleanup(check_for_errors, logger)
+            # TestCase runs cleanups in reverse order, and we want this to
+            # run *before* tracebacks are checked:
+            if assertion is not None:
+                self.addCleanup(
+                    lambda: skipped
+                    or assertion(self, logger, *assertionArgs, **assertionKwargs)
+                )
+            try:
+                return function(self, *args, **kwargs)
+            except self.skipException:
+                skipped = True
+                raise
+
+        return wrapper
+
+    return decorator
+
+# PEP 8 variant:
+validate_logging = validateLogging
+
+def capture_logging(
+    assertion, *assertionArgs, **assertionKwargs
+):
+    """
+    Capture and validate all logging that doesn't specify a L{Logger}.
+
+    See L{validate_logging} for details on the rest of its behavior.
+    """
+    encoder_ = assertionKwargs.pop("encoder_", eliot_json_encoder)
+    def decorator(function):
+        @validate_logging(
+            assertion, *assertionArgs, encoder_=encoder_, **assertionKwargs
+        )
+        @wraps(function)
+        def wrapper(self, *args, **kwargs):
+            logger = kwargs["logger"]
+            previous_logger = swap_logger(logger)
+
+            def cleanup():
+                swap_logger(previous_logger)
+
+            self.addCleanup(cleanup)
+            return function(self, *args, **kwargs)
+
+        return wrapper
+
+    return decorator

src/allmydata/util/eliotutil.py

@@ -16,12 +16,14 @@ from __future__ import (
 )
 
 __all__ = [
+    "MemoryLogger",
     "inline_callbacks",
     "eliot_logging_service",
     "opt_eliot_destination",
     "opt_help_eliot_destinations",
     "validateInstanceOf",
     "validateSetMembership",
+    "capture_logging",
 ]
 
 from future.utils import PY2
@@ -32,7 +34,7 @@ from six import ensure_text
 from sys import (
     stdout,
 )
-from functools import wraps, partial
+from functools import wraps
 from logging import (
     INFO,
     Handler,
@@ -66,8 +68,6 @@ from eliot.twisted import (
     DeferredContext,
     inline_callbacks,
 )
-from eliot.testing import capture_logging as eliot_capture_logging
-
 from twisted.python.usage import (
     UsageError,
 )
@@ -87,8 +87,11 @@ from twisted.internet.defer import (
 )
 from twisted.application.service import Service
 
-from .jsonbytes import AnyBytesJSONEncoder
+from ._eliot_updates import (
-
+    MemoryLogger,
+    eliot_json_encoder,
+    capture_logging,
+)
 
 def validateInstanceOf(t):
     """
@@ -306,7 +309,7 @@ class _DestinationParser(object):
                     rotateLength=rotate_length,
                     maxRotatedFiles=max_rotated_files,
                 )
-        return lambda reactor: FileDestination(get_file(), AnyBytesJSONEncoder)
+        return lambda reactor: FileDestination(get_file(), eliot_json_encoder)
 
 
 _parse_destination_description = _DestinationParser().parse
@@ -327,10 +330,3 @@ def log_call_deferred(action_type):
                 return DeferredContext(d).addActionFinish()
         return logged_f
     return decorate_log_call_deferred
-
-# On Python 3, encoding bytes to JSON doesn't work, so we have a custom JSON
-# encoder we want to use when validating messages.
-if PY2:
-    capture_logging = eliot_capture_logging
-else:
-    capture_logging = partial(eliot_capture_logging, encoder_=AnyBytesJSONEncoder)

src/allmydata/web/storage.py

@@ -256,8 +256,8 @@ class StorageStatusElement(Element):
 
         if so_far["corrupt-shares"]:
             add("Corrupt shares:",
-                T.ul( (T.li( ["SI %s shnum %d" % corrupt_share
+                T.ul( (T.li( ["SI %s shnum %d" % (si, shnum)
-                              for corrupt_share in so_far["corrupt-shares"] ]
+                              for si, shnum in so_far["corrupt-shares"] ]
                              ))))
         return tag("Current cycle:", p)
 
@@ -267,7 +267,8 @@ class StorageStatusElement(Element):
         h = lc.get_state()["history"]
         if not h:
             return ""
-        last = h[max(h.keys())]
+        biggest = str(max(int(k) for k in h.keys()))
+        last = h[biggest]
 
         start, end = last["cycle-start-finish-times"]
         tag("Last complete cycle (which took %s and finished %s ago)"
@@ -290,8 +291,8 @@ class StorageStatusElement(Element):
 
         if last["corrupt-shares"]:
             add("Corrupt shares:",
-                T.ul( (T.li( ["SI %s shnum %d" % corrupt_share
+                T.ul( (T.li( ["SI %s shnum %d" % (si, shnum)
-                              for corrupt_share in last["corrupt-shares"] ]
+                              for si, shnum in last["corrupt-shares"] ]
                              ))))
 
         return tag(p)

tox.ini

@@ -217,13 +217,8 @@ commands =
 # your web browser.
 
 [testenv:docs]
-# we pin docutils because of https://sourceforge.net/p/docutils/bugs/301/
-# which asserts when it reads links to .svg files (e.g. about.rst)
 deps =
-     sphinx
+     -r docs/requirements.txt
-     docutils==0.12
-     recommonmark
-     sphinx_rtd_theme
 # normal install is not needed for docs, and slows things down
 skip_install = True
 commands =