Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc for a new HTTP-based introducer #893

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Conversation

exarkun
Copy link
Member

@exarkun exarkun commented Nov 11, 2020

Does not really fix anything yet. Relates to https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3457 in some ways but even when implemented probably does not completely resolve that ticket.

This PR is mainly an artifact to facilitate collaboration at this point.

@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #893 (3c54538) into master (eab228f) will increase coverage by 0%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #893    +/-   ##
=======================================
  Coverage      92%     92%            
=======================================
  Files         158     158            
  Lines       27545   27996   +451     
  Branches     4124    4254   +130     
=======================================
+ Hits        25385   25821   +436     
- Misses       1487    1496     +9     
- Partials      673     679     +6     
Impacted Files Coverage Δ
src/allmydata/immutable/layout.py 91% <0%> (-1%) ⬇️
src/allmydata/mutable/retrieve.py 97% <0%> (+1%) ⬆️
src/allmydata/mutable/servermap.py 95% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eab228f...153ceb9. Read the comment docs.

Copy link
Contributor

@meejah meejah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!
This looks really good and I like the overall idea. I think with a couple updates to details (e.g WebSocket subprotocol stuff etc) this would be good to implement.

I also can't claim to have given this a full comparison to Foolscap's properties, so maybe that's a caveat :)

docs/proposed/http-introducer-internals.rst Show resolved Hide resolved

The *version* property must be **1**.
The values for the *listen-endpoints*, *certicate*, and *private-key* properties are those given to the ``create`` command.
The *swissnum* property is string that is randomly generated at ``create``-time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this HAS to be a string and not a number "because javascript"? (Does JSON itself import the 2^53 or whatever limit of javascript)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, JSON is fuzzy about numbers. There is no integer type, only a union integer/floating point type. Different implementations can't be relied on to differentiate between the two very well.

However, even in Foolscap the "swissnum" is actually just a mostly arbitrary string, so I don't think there's much of an issue here.

Several locations may be present and separated by ``,``.
``<swissnum>`` is the *swissnum* property value.

The result is an unguessable self-authenticating URL which can be used to establish a confidential channel to the HTTP Introducer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To compare directly to Foolscap, it would be good to note the relevant properties of that here (or maybe elsewhere, but reference it here?). That is, what parts of the fURL does Foolscap authenticate etc?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this is addressed by #896

On top of TLS,
it runs a WebSocket server with two endpoints.

The server does not require the client to present a certificate.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to explicitly compare this to the Foolscap version (e.g. does Foolscap authenticate any aspect of the client). That said, my recollection is that the behavior described here is the same as Foolscap (that is, anyone who knows the fURL can access the resource, the server-side doesn't limit anything) so this is good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also covered by #896

The server does not require the client to present a certificate.
The client requires the server to present a certificate with an SPKI hash matching that in the *introducer fURL*.

.. TODO: Add docs about the WebSocket protocol negotiation that happens for the pub/sub protocol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we'd be using Autobahn for the WebSocket library, it supports subprotocol negotiation. This is a comma-separated list of subprotocols so we could just list "introducer-v1" or similar for this. See e.g. https://github.com/crossbario/autobahn-python/blob/master/autobahn/websocket/types.py#L128 (<-- is returned from onConnecting() and you'd specify e.g. protocols=["introducer-v1"] for our case).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that leaves "what to do with the swissnum" since that's in the URIs below .. but so I guess that just means we just put "the one Introducer WebSocket endpoint at /<swissnum>" and do the duplix thing in your TODO below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that leaves "what to do with the swissnum" since that's in the URIs below .. but so I guess that just means we just put "the one Introducer WebSocket endpoint at /" and do the duplix thing in your TODO below?

Yea I reckon so. Adjusted the doc to reflect this.

docs/proposed/http-introducer.rst Show resolved Hide resolved
a new private key and self-signed certificate is generated and used.

This is the HTTP Introducer's state and is required by future commands.
It is the operator's responsibility to persist this state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth repeating here that this state is Seekrit Information and to treat accordingly (even if that's already strongly implied with --private-key etc etc) and the consequences (i.e. that you can forever impersonate this introducer and direct clients to malicious servers).

docs/proposed/http-introducer.rst Show resolved Hide resolved

[bob@bbb:~]$ cat >> storage-node/tahoe.cfg
[client]
http-introducer-path = storage-node/private/alicegrid.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier the docs said this would be private/alicegrid.json (relative to node path).

@@ -0,0 +1 @@
Client nodes can now be introduced to storage nodes using an HTTP-based introducer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly an "aside" here, but I'd like our "proposal process" to be able to merge a proposal in docs/proposed like this when we decide the proposal is good enough -- and then move it to docs/ when actually implemented. I still have "write a proposal proposal down" in my TODO but if you agree, the above could say that we introduced a "proposal for an http-based introducer" or so, and then we could actually merge these proposal-docs while the implementation happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants