#3904 closed task (fixed)

Perform a holistic review of the GBS HTTP server implementation

Reported by: exarkun Owned by: exarkun
Priority: normal Milestone: HTTP Storage Protocol
Component: code Version: n/a
Keywords: Cc:
Launchpad Bug:

Description

The GBS HTTP server implementation is now largely complete. We have reviewed each piece as it was developed. We should now review the whole thing to make sure it is consistent and in particular to build confidence that it is safe to ship a version of Tahoe-LAFS in which it is _turned on_.

Change History (7)

comment:1 Changed at 2022-06-30T18:00:27Z by itamarst

  • Milestone changed from undecided to HTTP Storage Protocol

comment:2 Changed at 2022-06-30T18:32:52Z by itamarst

Things that should be considered:

  • Correctness.
  • Security concerns.
  • Whether or not the API endpoints should be in current structure. I would like to add a /storage/ prefix personally.

comment:3 Changed at 2022-08-08T14:24:19Z by itamarst

  • Summary changed from Perform a wholistic review of the GBS HTTP server implementation to Perform a holistic review of the GBS HTTP server implementation

comment:4 Changed at 2022-09-06T12:48:02Z by exarkun

I started by re-reading the specification documents and just lightly comparing them to implementation where I didn't remember how things work. The outcome of that is the following list:

  • We should review the hash function chosen for the SPKI hash in v1 NURLs. At minimum, the inconsistency between http-storage-node-protocol.rst and url.rst needs to be addressed (the former says SHA256, the latter says SHA3-224.
  • http-storage-node-protocol.rst says the servers' version response will include gbs-anonymous-storage-url. This is intended to make automatic upgrade to GBS simple for clients. I'm not sure the version response is the right place to put this information though.
  • It seems reasonable to elaborate on the /v1/ prefix. The main motivation for doing so, to me, seems to make it more clear what a URL relates to in the face of multiple services living side-by-side. Right now you could have the client-facing API (mostly at "/uri") next to the storage API (beneath "/v1") and maybe there will be an introducer API (at ... ???) someday. Using "v1" was an attempt to provide some coherent versioning for the endpoints. If we put the first version of GBS at "/v1/..." then maybe we make a new, incompatible version available at "/v2/..." later. Clearly this segment doesn't have anything to do with versioning _other_ APIs a node exposes. Should we make the stem "/storage/v1" instead? This fits somewhat with "/uri" - ie, the top is not a version, it's some general hint about the kind of stuff you might expect to find below it.

While reading the text I also noticed some simpler/smaller issues so I created https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3922 / https://github.com/tahoe-lafs/tahoe-lafs/pull/1213

comment:5 Changed at 2022-09-15T14:40:17Z by itamarst

Do you believe any additional review is needed before HTTPS becomes public (i.e. #3902)? It wouldn't actually be in use at this stage, but it would be exposed to potentially malicious attackers.

comment:6 Changed at 2022-10-03T14:54:53Z by itamarst

Sounds like no additional review is needed (at least for now), and the bullet points above were addressed, so going to close this.

comment:7 Changed at 2022-10-03T14:54:58Z by itamarst

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.