Merge pull request #1216 from tahoe-lafs/3904-http-storage-holistic

Address various http storage holistic review comments
This commit is contained in:
Itamar Turner-Trauring 2022-10-03 09:13:30 -04:00 committed by GitHub
commit bd74c3d971
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 77 additions and 69 deletions

View File

@ -395,8 +395,8 @@ Encoding
General
~~~~~~~
``GET /v1/version``
!!!!!!!!!!!!!!!!!!!
``GET /storage/v1/version``
!!!!!!!!!!!!!!!!!!!!!!!!!!!
Retrieve information about the version of the storage server.
Information is returned as an encoded mapping.
@ -409,14 +409,13 @@ For example::
"tolerates-immutable-read-overrun": true,
"delete-mutable-shares-with-zero-length-writev": true,
"fills-holes-with-zero-bytes": true,
"prevents-read-past-end-of-share-data": true,
"gbs-anonymous-storage-url": "pb://...#v=1"
"prevents-read-past-end-of-share-data": true
},
"application-version": "1.13.0"
}
``PUT /v1/lease/:storage_index``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``PUT /storage/v1/lease/:storage_index``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Either renew or create a new lease on the bucket addressed by ``storage_index``.
@ -468,8 +467,8 @@ Immutable
Writing
~~~~~~~
``POST /v1/immutable/:storage_index``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``POST /storage/v1/immutable/:storage_index``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Initialize an immutable storage index with some buckets.
The buckets may have share data written to them once.
@ -504,7 +503,7 @@ Handling repeat calls:
Discussion
``````````
We considered making this ``POST /v1/immutable`` instead.
We considered making this ``POST /storage/v1/immutable`` instead.
The motivation was to keep *storage index* out of the request URL.
Request URLs have an elevated chance of being logged by something.
We were concerned that having the *storage index* logged may increase some risks.
@ -539,8 +538,8 @@ Rejected designs for upload secrets:
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``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``PATCH /storage/v1/immutable/:storage_index/:share_number``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Write data for the indicated share.
The share number must belong to the storage index.
@ -580,8 +579,8 @@ Responses:
the response is ``CONFLICT``.
At this point the only thing to do is abort the upload and start from scratch (see below).
``PUT /v1/immutable/:storage_index/:share_number/abort``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``PUT /storage/v1/immutable/:storage_index/:share_number/abort``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
This cancels an *in-progress* upload.
@ -616,8 +615,8 @@ From RFC 7231::
PATCH method defined in [RFC5789]).
``POST /v1/immutable/:storage_index/:share_number/corrupt``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``POST /storage/v1/immutable/:storage_index/:share_number/corrupt``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Advise the server the data read from the indicated share was corrupt. The
request body includes an human-meaningful text string with details about the
@ -635,8 +634,8 @@ couldn't be found.
Reading
~~~~~~~
``GET /v1/immutable/:storage_index/shares``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``GET /storage/v1/immutable/:storage_index/shares``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Retrieve a list (semantically, a set) indicating all shares available for the
indicated storage index. For example::
@ -645,8 +644,8 @@ indicated storage index. For example::
An unknown storage index results in an empty list.
``GET /v1/immutable/:storage_index/:share_number``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``GET /storage/v1/immutable/:storage_index/:share_number``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Read a contiguous sequence of bytes from one share in one bucket.
The response body is the raw share data (i.e., ``application/octet-stream``).
@ -686,8 +685,8 @@ Mutable
Writing
~~~~~~~
``POST /v1/mutable/:storage_index/read-test-write``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``POST /storage/v1/mutable/:storage_index/read-test-write``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
General purpose read-test-and-write operation for mutable storage indexes.
A mutable storage index is also called a "slot"
@ -742,18 +741,18 @@ As a result, if there is no data at all, an empty bytestring is returned no matt
Reading
~~~~~~~
``GET /v1/mutable/:storage_index/shares``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``GET /storage/v1/mutable/:storage_index/shares``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Retrieve a set indicating all shares available for the indicated storage index.
For example (this is shown as list, since it will be list for JSON, but will be set for CBOR)::
[1, 5]
``GET /v1/mutable/:storage_index/:share_number``
``GET /storage/v1/mutable/:storage_index/:share_number``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Read data from the indicated mutable shares, just like ``GET /v1/immutable/:storage_index``
Read data from the indicated mutable shares, just like ``GET /storage/v1/immutable/:storage_index``
The ``Range`` header may be used to request exactly one ``bytes`` range, in which case the response code will be 206 (partial content).
Interpretation and response behavior is as specified in RFC 7233 § 4.1.
@ -765,8 +764,8 @@ The resulting ``Content-Range`` header will be consistent with the returned data
If the response to a query is an empty range, the ``NO CONTENT`` (204) response code will be used.
``POST /v1/mutable/:storage_index/:share_number/corrupt``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
``POST /storage/v1/mutable/:storage_index/:share_number/corrupt``
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Advise the server the data read from the indicated share was corrupt.
Just like the immutable version.
@ -779,7 +778,7 @@ 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
POST /storage/v1/immutable/AAAAAAAAAAAAAAAA
Authorization: Tahoe-LAFS nurl-swissnum
X-Tahoe-Authorization: lease-renew-secret efgh
X-Tahoe-Authorization: lease-cancel-secret jjkl
@ -792,7 +791,7 @@ Immutable Data
#. Upload the content for immutable share ``7``::
PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7
Authorization: Tahoe-LAFS nurl-swissnum
Content-Range: bytes 0-15/48
X-Tahoe-Authorization: upload-secret xyzf
@ -800,7 +799,7 @@ Immutable Data
200 OK
PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7
Authorization: Tahoe-LAFS nurl-swissnum
Content-Range: bytes 16-31/48
X-Tahoe-Authorization: upload-secret xyzf
@ -808,7 +807,7 @@ Immutable Data
200 OK
PATCH /v1/immutable/AAAAAAAAAAAAAAAA/7
PATCH /storage/v1/immutable/AAAAAAAAAAAAAAAA/7
Authorization: Tahoe-LAFS nurl-swissnum
Content-Range: bytes 32-47/48
X-Tahoe-Authorization: upload-secret xyzf
@ -818,7 +817,7 @@ Immutable Data
#. Download the content of the previously uploaded immutable share ``7``::
GET /v1/immutable/AAAAAAAAAAAAAAAA?share=7
GET /storage/v1/immutable/AAAAAAAAAAAAAAAA?share=7
Authorization: Tahoe-LAFS nurl-swissnum
Range: bytes=0-47
@ -827,7 +826,7 @@ Immutable Data
#. Renew the lease on all immutable shares in bucket ``AAAAAAAAAAAAAAAA``::
PUT /v1/lease/AAAAAAAAAAAAAAAA
PUT /storage/v1/lease/AAAAAAAAAAAAAAAA
Authorization: Tahoe-LAFS nurl-swissnum
X-Tahoe-Authorization: lease-cancel-secret jjkl
X-Tahoe-Authorization: lease-renew-secret efgh
@ -842,7 +841,7 @@ The special test vector of size 1 but empty bytes will only pass
if there is no existing share,
otherwise it will read a byte which won't match `b""`::
POST /v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
POST /storage/v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
Authorization: Tahoe-LAFS nurl-swissnum
X-Tahoe-Authorization: write-enabler abcd
X-Tahoe-Authorization: lease-cancel-secret efgh
@ -874,7 +873,7 @@ 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
POST /storage/v1/mutable/BBBBBBBBBBBBBBBB/read-test-write
Authorization: Tahoe-LAFS nurl-swissnum
X-Tahoe-Authorization: write-enabler abcd
X-Tahoe-Authorization: lease-cancel-secret efgh
@ -906,14 +905,14 @@ 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
GET /storage/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
PUT /storage/v1/lease/BBBBBBBBBBBBBBBB
Authorization: Tahoe-LAFS nurl-swissnum
X-Tahoe-Authorization: lease-cancel-secret efgh
X-Tahoe-Authorization: lease-renew-secret ijkl

View File

@ -103,11 +103,8 @@ Version 1
The hash component of a version 1 NURL differs in three ways from the prior version.
1. The hash function used is SHA3-224 instead of SHA1.
The security of SHA1 `continues to be eroded`_.
Contrariwise SHA3 is currently the most recent addition to the SHA family by NIST.
The 224 bit instance is chosen to keep the output short and because it offers greater collision resistance than SHA1 was thought to offer even at its inception
(prior to security research showing actual collision resistance is lower).
1. The hash function used is SHA-256, to match RFC 7469.
The security of SHA1 `continues to be eroded`_; Latacora `SHA-2`_.
2. The hash is computed over the certificate's SPKI instead of the whole certificate.
This allows certificate re-generation so long as the public key remains the same.
This is useful to allow contact information to be updated or extension of validity period.
@ -140,7 +137,8 @@ Examples
* ``pb://azEu8vlRpnEeYm0DySQDeNY3Z2iJXHC_bsbaAw@localhost:47877/64i4aokv4ej#v=1``
.. _`continues to be eroded`: https://en.wikipedia.org/wiki/SHA-1#Cryptanalysis_and_validation
.. _`explored by the web community`: https://www.imperialviolet.org/2011/05/04/pinning.html
.. _`SHA-2`: https://latacora.micro.blog/2018/04/03/cryptographic-right-answers.html
.. _`explored by the web community`: https://www.rfc-editor.org/rfc/rfc7469
.. _Foolscap: https://github.com/warner/foolscap
.. [1] ``foolscap.furl.decode_furl`` is taken as the canonical definition of the syntax of a fURL.

0
newsfragments/3904.minor Normal file
View File

View File

@ -392,7 +392,7 @@ class StorageClientGeneral(object):
"""
Return the version metadata for the server.
"""
url = self._client.relative_url("/v1/version")
url = self._client.relative_url("/storage/v1/version")
response = yield self._client.request("GET", url)
decoded_response = yield _decode_cbor(response, _SCHEMAS["get_version"])
returnValue(decoded_response)
@ -408,7 +408,7 @@ class StorageClientGeneral(object):
Otherwise a new lease is added.
"""
url = self._client.relative_url(
"/v1/lease/{}".format(_encode_si(storage_index))
"/storage/v1/lease/{}".format(_encode_si(storage_index))
)
response = yield self._client.request(
"PUT",
@ -457,7 +457,9 @@ def read_share_chunk(
always provided by the current callers.
"""
url = client.relative_url(
"/v1/{}/{}/{}".format(share_type, _encode_si(storage_index), share_number)
"/storage/v1/{}/{}/{}".format(
share_type, _encode_si(storage_index), share_number
)
)
response = yield client.request(
"GET",
@ -518,7 +520,7 @@ async def advise_corrupt_share(
):
assert isinstance(reason, str)
url = client.relative_url(
"/v1/{}/{}/{}/corrupt".format(
"/storage/v1/{}/{}/{}/corrupt".format(
share_type, _encode_si(storage_index), share_number
)
)
@ -563,7 +565,9 @@ class StorageClientImmutables(object):
Result fires when creating the storage index succeeded, if creating the
storage index failed the result will fire with an exception.
"""
url = self._client.relative_url("/v1/immutable/" + _encode_si(storage_index))
url = self._client.relative_url(
"/storage/v1/immutable/" + _encode_si(storage_index)
)
message = {"share-numbers": share_numbers, "allocated-size": allocated_size}
response = yield self._client.request(
@ -588,7 +592,9 @@ class StorageClientImmutables(object):
) -> Deferred[None]:
"""Abort the upload."""
url = self._client.relative_url(
"/v1/immutable/{}/{}/abort".format(_encode_si(storage_index), share_number)
"/storage/v1/immutable/{}/{}/abort".format(
_encode_si(storage_index), share_number
)
)
response = yield self._client.request(
"PUT",
@ -620,7 +626,9 @@ class StorageClientImmutables(object):
been uploaded.
"""
url = self._client.relative_url(
"/v1/immutable/{}/{}".format(_encode_si(storage_index), share_number)
"/storage/v1/immutable/{}/{}".format(
_encode_si(storage_index), share_number
)
)
response = yield self._client.request(
"PATCH",
@ -668,7 +676,7 @@ class StorageClientImmutables(object):
Return the set of shares for a given storage index.
"""
url = self._client.relative_url(
"/v1/immutable/{}/shares".format(_encode_si(storage_index))
"/storage/v1/immutable/{}/shares".format(_encode_si(storage_index))
)
response = yield self._client.request(
"GET",
@ -774,7 +782,7 @@ class StorageClientMutables:
are done and if they are valid the writes are done.
"""
url = self._client.relative_url(
"/v1/mutable/{}/read-test-write".format(_encode_si(storage_index))
"/storage/v1/mutable/{}/read-test-write".format(_encode_si(storage_index))
)
message = {
"test-write-vectors": {
@ -817,7 +825,7 @@ class StorageClientMutables:
List the share numbers for a given storage index.
"""
url = self._client.relative_url(
"/v1/mutable/{}/shares".format(_encode_si(storage_index))
"/storage/v1/mutable/{}/shares".format(_encode_si(storage_index))
)
response = await self._client.request("GET", url)
if response.code == http.OK:

View File

@ -545,7 +545,7 @@ class HTTPServer(object):
##### Generic APIs #####
@_authorized_route(_app, set(), "/v1/version", methods=["GET"])
@_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"])
def version(self, request, authorization):
"""Return version information."""
return self._send_encoded(request, self._storage_server.get_version())
@ -555,7 +555,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
{Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL, Secrets.UPLOAD},
"/v1/immutable/<storage_index:storage_index>",
"/storage/v1/immutable/<storage_index:storage_index>",
methods=["POST"],
)
def allocate_buckets(self, request, authorization, storage_index):
@ -591,7 +591,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
{Secrets.UPLOAD},
"/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>/abort",
"/storage/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>/abort",
methods=["PUT"],
)
def abort_share_upload(self, request, authorization, storage_index, share_number):
@ -622,7 +622,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
{Secrets.UPLOAD},
"/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>",
"/storage/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>",
methods=["PATCH"],
)
def write_share_data(self, request, authorization, storage_index, share_number):
@ -665,7 +665,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
set(),
"/v1/immutable/<storage_index:storage_index>/shares",
"/storage/v1/immutable/<storage_index:storage_index>/shares",
methods=["GET"],
)
def list_shares(self, request, authorization, storage_index):
@ -678,7 +678,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
set(),
"/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>",
"/storage/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>",
methods=["GET"],
)
def read_share_chunk(self, request, authorization, storage_index, share_number):
@ -694,7 +694,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
{Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL},
"/v1/lease/<storage_index:storage_index>",
"/storage/v1/lease/<storage_index:storage_index>",
methods=["PUT"],
)
def add_or_renew_lease(self, request, authorization, storage_index):
@ -715,7 +715,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
set(),
"/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>/corrupt",
"/storage/v1/immutable/<storage_index:storage_index>/<int(signed=False):share_number>/corrupt",
methods=["POST"],
)
def advise_corrupt_share_immutable(
@ -736,7 +736,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
{Secrets.LEASE_RENEW, Secrets.LEASE_CANCEL, Secrets.WRITE_ENABLER},
"/v1/mutable/<storage_index:storage_index>/read-test-write",
"/storage/v1/mutable/<storage_index:storage_index>/read-test-write",
methods=["POST"],
)
def mutable_read_test_write(self, request, authorization, storage_index):
@ -771,7 +771,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
set(),
"/v1/mutable/<storage_index:storage_index>/<int(signed=False):share_number>",
"/storage/v1/mutable/<storage_index:storage_index>/<int(signed=False):share_number>",
methods=["GET"],
)
def read_mutable_chunk(self, request, authorization, storage_index, share_number):
@ -795,7 +795,10 @@ class HTTPServer(object):
return read_range(request, read_data, share_length)
@_authorized_route(
_app, set(), "/v1/mutable/<storage_index:storage_index>/shares", methods=["GET"]
_app,
set(),
"/storage/v1/mutable/<storage_index:storage_index>/shares",
methods=["GET"],
)
def enumerate_mutable_shares(self, request, authorization, storage_index):
"""List mutable shares for a storage index."""
@ -805,7 +808,7 @@ class HTTPServer(object):
@_authorized_route(
_app,
set(),
"/v1/mutable/<storage_index:storage_index>/<int(signed=False):share_number>/corrupt",
"/storage/v1/mutable/<storage_index:storage_index>/<int(signed=False):share_number>/corrupt",
methods=["POST"],
)
def advise_corrupt_share_mutable(

View File

@ -255,7 +255,7 @@ class TestApp(object):
else:
return "BAD: {}".format(authorization)
@_authorized_route(_app, set(), "/v1/version", methods=["GET"])
@_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"])
def bad_version(self, request, authorization):
"""Return version result that violates the expected schema."""
request.setHeader("content-type", CBOR_MIME_TYPE)
@ -534,7 +534,7 @@ class GenericHTTPAPITests(SyncTestCase):
lease_secret = urandom(32)
storage_index = urandom(16)
url = self.http.client.relative_url(
"/v1/immutable/" + _encode_si(storage_index)
"/storage/v1/immutable/" + _encode_si(storage_index)
)
message = {"bad-message": "missing expected keys"}
@ -1418,7 +1418,7 @@ class SharedImmutableMutableTestsMixin:
self.http.client.request(
"GET",
self.http.client.relative_url(
"/v1/{}/{}/1".format(self.KIND, _encode_si(storage_index))
"/storage/v1/{}/{}/1".format(self.KIND, _encode_si(storage_index))
),
)
)
@ -1441,7 +1441,7 @@ class SharedImmutableMutableTestsMixin:
self.http.client.request(
"GET",
self.http.client.relative_url(
"/v1/{}/{}/1".format(self.KIND, _encode_si(storage_index))
"/storage/v1/{}/{}/1".format(self.KIND, _encode_si(storage_index))
),
headers=headers,
)