magic-wormhole-mailbox-server icon indicating copy to clipboard operation
magic-wormhole-mailbox-server copied to clipboard

send error:crowded to all sides when first discovered

Open warner opened this issue 3 years ago • 0 comments

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

warner avatar Mar 20 '21 22:03 warner