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

send error:crowded to all sides when first discovered #19

Open
warner opened this issue Mar 20, 2021 · 0 comments
Open

send error:crowded to all sides when first discovered #19

warner opened this issue Mar 20, 2021 · 0 comments

Comments

@warner
Copy link
Collaborator

warner commented Mar 20, 2021

I observed a transter fail in the following way:

  • Alice started a transfer (intended for Carol), claimed nameplate N
  • it took a while for Carol to react
  • in the meantime, Bob ran wormhole rx, claimed nameplate N (by typing N <TAB> <TAB>), then realized his mistake, and hit control-c to quit
  • Bob's client disconnected
  • later, Carol runs wormhole rx and N-<TAB> as usual
  • Carol's client exits with a "crowded" error
  • Alice's client keeps waiting forever

The server keeps track of how many sides are participating in a given mailbox. Alice's initial connection adds sideA. Bob's interruption adds sideB. Carol's connection adds sideC, at which point there are more than 2 sides, and the server throws a CrowdedError, which is returned to Carol (because it happened in response to her claim call, as she attempted to claim nameplate N).

The problem is that Bob chose to disconnect before he sent any messages, and Carol was disconnected (with an error) before she had a chance to send any messages, so Alice is still waiting. Alice holds on to the nameplate until she sees evidence that her one expected counterparty has claimed the mailbox, and that evidence must be in the form of a message received from a side other than her own. Since neither Bob nor Carol sent any messges, Alice hangs on to the nameplate (and her client sits waiting) forever.

Note that it's legitmate for Alice to wait until Carol shows up. When Bob connected and then immediately disconnected, that doesn't rule out Bob as the real counterparty. His server-visible behavior is identical to what would happen if Bob's client had connected, started the protocol, and then the network dropped (maybe he's on a train, and his laptop was connected to the station WiFi, and the train pulled out of the station and he lost network access. Remember when we got to ride trains?). If he reconnects again later, still with the same wormhole rx client running (and thus using the same sideB), then he could proceed with the protocol as planned. There would be no error in that case.

It's not until Carol shows up that we know we're in trouble. Once we see three sides in the conversation, we cannot proceed correctly: PAKE is a two-party protocol.

The primary bug here is that Alice waits forever. I think the fix would be: if the server ever sends an error:crowded to one side, it should send it to all sides. That would cause Alice (whose Boss state machine ought to be in the Lonely state) to exit with the error.

In addition, technically we might be able to tolerate this case. Bob claimed the nameplate and opened the mailbox, but didn't actually send any messages. We might defer adding sideB to the mailbox list until Bob actually did an add to add a message. A wormhole rx client who <TAB>s their way into a nameplate but doesn't hit <RETURN> to finish entering the PAKE code will not send a phase:pake message, so they haven't really interfered with the two-party conversation yet.

So the tasks are:

  • add a broadcast_error method to Mailbox
  • change Mailbox.open to perform the three-sides check internally, and call self.broadcast_error(CrowdedError) in that case
    • provide a return value so the caller knows whether the mailbox was opened or not
  • change AppNamespace.open_mailbox to remove the three-sides error check, in favor of mailbox.open doing it
  • remove the check from AppNamespace.claim_mailbox too
  • pass the Mailbox.open success flag far enough back out to let server_websocket.py 's handle_claim to not return claimed message if in fact the attempt caused an error
  • same for handle_open

(looking more closely at the code, I see we track both nameplate_sides and mailbox_sides, so maybe that plan isn't complete)

And the bonus task is:

  • rather than checking the three-sides error by looking at the nameplate_sides or mailbox_sides tables, look at the messages in the mailbox, and only signal an error if there are actual messages from three or more sides
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

No branches or pull requests

1 participant