rewrite welcome handler

docs/api.md

@@ -288,6 +288,75 @@ sides are the same, call `send()` to continue the protocol. If you call
 `send()` before `verify()`, it will perform the complete protocol without
 pausing.
 
+## Welcome Messages
+
+The first message sent by the rendezvous server is a "welcome" message (a
+dictionary). Clients should not wait for this message, but when it arrives,
+they should process the keys it contains.
+
+The welcome message serves three main purposes:
+
+* notify users about important server changes, such as CAPTCHA requirements
+  driven by overload, or donation requests
+* enable future protocol negotiation between clients and the server
+* advise users of the CLI tools (`wormhole send`) to upgrade to a new version
+
+There are three keys currently defined for the welcome message, all of which
+are optional (the welcome message omits "error" and "motd" unless the server
+operator needs to signal a problem).
+
+* `motd`: if this key is present, it will be a string with embedded newlines.
+  The client should display this string to the user, including a note that it
+  comes from the magic-wormhole Rendezvous Server and that server's URL.
+* `error`: if present, the server has decided it cannot service this client.
+  The string will be wrapped in a `WelcomeError` (which is a subclass of
+  `WormholeError`), and all API calls will signal errors (pending Deferreds
+  will errback). The rendezvous connection will be closed.
+* `current_cli_version`: if present, the server is advising instances of the
+  CLI tools (the `wormhole` command included in the python distribution) that
+  there is a newer release available, thus users should upgrade if they can,
+  because more features will be available if both clients are running the
+  same version. The CLI tools compare this string against their `__version__`
+  and can print a short message to stderr if an upgrade is warranted.
+
+There is currently no facility in the server to actually send `motd`, but a
+static `error` string can be included by running the server with
+`--signal-error=MESSAGE`.
+
+The main idea of `error` is to allow the server to cleanly inform the client
+about some necessary action it didn't take. The server currently sends the
+welcome message as soon as the client connects (even before it receives the
+"claim" request), but a future server could wait for a required client
+message and signal an error (via the Welcome message) if it didn't see this
+extra message before the CLAIM arrived.
+
+This could enable changes to the protocol, e.g. requiring a CAPTCHA or
+proof-of-work token when the server is under DoS attack. The new server would
+send the current requirements in an initial message (which old clients would
+ignore). New clients would be required to send the token before their "claim"
+message. If the server sees "claim" before "token", it knows that the client
+is too old to know about this protocol, and it could send a "welcome" with an
+`error` field containing instructions (explaining to the user that the server
+is under attack, and they must either upgrade to a client that can speak the
+new protocol, or wait until the attack has passed). Either case is better
+than an opaque exception later when the required message fails to arrive.
+
+(Note that the server can also send an explicit ERROR message at any time,
+and the client should react with a ServerError. Versions 0.9.2 and earlier of
+the library did not pay attention to the ERROR message, hence the server
+should deliver errors in a WELCOME message if at all possible)
+
+The `error` field is handled internally by the Wormhole object. The other
+fields are processed by an application-supplied "welcome handler" function,
+supplied as an argument to the `wormhole()` constructor. This function will
+be called with the full welcome dictionary, so any other keys that a future
+server might send will be available to it. If the welcome handler raises
+`WelcomeError`, the connection will be closed just as if an `error` key had
+been received.
+
+The default welcome handler will print `motd` to stderr, and will ignore
+`current_cli_version`.
+
 ## Events
 
 As the wormhole connection is established, several events may be dispatched

docs/state-machines/boss.dot

@@ -22,6 +22,10 @@ digraph {
         P_close_error -> S_closing
         S0 -> P_close_lonely [label="close"]
 
+        S0 -> P_close_unwelcome [label="rx_unwelcome"]
+        P_close_unwelcome [shape="box" label="T.close(unwelcome)"]
+        P_close_unwelcome -> S_closing
+
         P0_build [shape="box" label="W.got_code"]
         P0_build -> S1
         S1 [label="S1: lonely" color="orange"]
@@ -30,6 +34,7 @@ digraph {
 
         S1 -> P_close_error [label="rx_error"]
         S1 -> P_close_scary [label="scared" color="red"]
+        S1 -> P_close_unwelcome [label="rx_unwelcome"]
         S1 -> P_close_lonely [label="close"]
         P_close_lonely [shape="box" label="T.close(lonely)"]
         P_close_lonely -> S_closing
@@ -52,6 +57,7 @@ digraph {
 
         S2 -> P_close_error [label="rx_error"]
         S2 -> P_close_scary [label="scared" color="red"]
+        S2 -> P_close_unwelcome [label="rx_unwelcome"]
 
         S_closing [label="closing"]
         S_closing -> P_closed [label="closed\nerror"]
@@ -67,7 +73,7 @@ digraph {
 
         {rank=same; Other S_closed}
         Other [shape="box" style="dashed"
-               label="rx_welcome -> process\nsend -> S.send\ngot_message -> got_version or got_phase\ngot_key -> W.got_key\ngot_verifier -> W.got_verifier\nallocate_Code -> C.allocate_code\ninput_code -> C.input_code\nset_code -> C.set_code"
+               label="rx_welcome -> process (maybe rx_unwelcome)\nsend -> S.send\ngot_message -> got_version or got_phase\ngot_key -> W.got_key\ngot_verifier -> W.got_verifier\nallocate_Code -> C.allocate_code\ninput_code -> C.input_code\nset_code -> C.set_code"
                ]
 
 

src/wormhole/_boss.py

@@ -21,7 +21,8 @@ from ._code import Code
 from ._terminator import Terminator
 from ._wordlist import PGPWordList
 from .errors import (ServerError, LonelyError, WrongPasswordError,
-                     KeyFormatError, OnlyOneCodeError, _UnknownPhaseError)
+                     KeyFormatError, OnlyOneCodeError, _UnknownPhaseError,
+                     WelcomeError)
 from .util import bytes_to_dict
 
 @attrs
@@ -162,11 +163,28 @@ class Boss(object):
     @m.input()
     def close(self): pass
 
-    # from RendezvousConnector. rx_error an error message from the server
+    # from RendezvousConnector:
-    # (probably because of something we did, or due to CrowdedError). error
+    # * "rx_welcome" is the Welcome message, which might signal an error, or
-    # is when an exception happened while it tried to deliver something else
+    #   our welcome_handler might signal one
+    # * "rx_error" is error message from the server (probably because of
+    #   something we said badly, or due to CrowdedError)
+    # * "error" is when an exception happened while it tried to deliver
+    #   something else
+    def rx_welcome(self, welcome):
+        try:
+            if "error" in welcome:
+                raise WelcomeError(welcome["error"])
+            # TODO: it'd be nice to not call the handler when we're in
+            # S3_closing or S4_closed states. I tried to implement this with
+            # rx_Welcome as an @input, but in the error case I'd be
+            # delivering a new input (rx_error or something) while in the
+            # middle of processing the rx_welcome input, and I wasn't sure
+            # Automat would handle that correctly.
+            self._welcome_handler(welcome) # can raise WelcomeError too
+        except WelcomeError as welcome_error:
+            self.rx_unwelcome(welcome_error)
     @m.input()
-    def rx_welcome(self, welcome): pass
+    def rx_unwelcome(self, welcome_error): pass
     @m.input()
     def rx_error(self, errmsg, orig): pass
     @m.input()
@@ -207,11 +225,6 @@ class Boss(object):
     @m.input()
     def closed(self): pass
 
-
-    @m.output()
-    def process_welcome(self, welcome):
-        self._welcome_handler(welcome)
-
     @m.output()
     def do_got_code(self, code):
         self._W.got_code(code)
@@ -232,6 +245,11 @@ class Boss(object):
         self._S.send("%d" % phase, plaintext)
 
     @m.output()
+    def close_unwelcome(self, welcome_error):
+        #assert isinstance(err, WelcomeError)
+        self._result = welcome_error
+        self._T.close("unwelcome")
+    @m.output()
     def close_error(self, errmsg, orig):
         self._result = ServerError(errmsg)
         self._T.close("errory")
@@ -275,12 +293,12 @@ class Boss(object):
 
     S0_empty.upon(close, enter=S3_closing, outputs=[close_lonely])
     S0_empty.upon(send, enter=S0_empty, outputs=[S_send])
-    S0_empty.upon(rx_welcome, enter=S0_empty, outputs=[process_welcome])
+    S0_empty.upon(rx_unwelcome, enter=S3_closing, outputs=[close_unwelcome])
     S0_empty.upon(got_code, enter=S1_lonely, outputs=[do_got_code])
     S0_empty.upon(rx_error, enter=S3_closing, outputs=[close_error])
     S0_empty.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
-    S1_lonely.upon(rx_welcome, enter=S1_lonely, outputs=[process_welcome])
+    S1_lonely.upon(rx_unwelcome, enter=S3_closing, outputs=[close_unwelcome])
     S1_lonely.upon(happy, enter=S2_happy, outputs=[])
     S1_lonely.upon(scared, enter=S3_closing, outputs=[close_scared])
     S1_lonely.upon(close, enter=S3_closing, outputs=[close_lonely])
@@ -290,7 +308,7 @@ class Boss(object):
     S1_lonely.upon(rx_error, enter=S3_closing, outputs=[close_error])
     S1_lonely.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
-    S2_happy.upon(rx_welcome, enter=S2_happy, outputs=[process_welcome])
+    S2_happy.upon(rx_unwelcome, enter=S3_closing, outputs=[close_unwelcome])
     S2_happy.upon(_got_phase, enter=S2_happy, outputs=[W_received])
     S2_happy.upon(_got_version, enter=S2_happy, outputs=[process_version])
     S2_happy.upon(scared, enter=S3_closing, outputs=[close_scared])
@@ -299,7 +317,7 @@ class Boss(object):
     S2_happy.upon(rx_error, enter=S3_closing, outputs=[close_error])
     S2_happy.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
-    S3_closing.upon(rx_welcome, enter=S3_closing, outputs=[])
+    S3_closing.upon(rx_unwelcome, enter=S3_closing, outputs=[])
     S3_closing.upon(rx_error, enter=S3_closing, outputs=[])
     S3_closing.upon(_got_phase, enter=S3_closing, outputs=[])
     S3_closing.upon(_got_version, enter=S3_closing, outputs=[])
@@ -310,7 +328,7 @@ class Boss(object):
     S3_closing.upon(closed, enter=S4_closed, outputs=[W_closed])
     S3_closing.upon(error, enter=S4_closed, outputs=[W_close_with_error])
 
-    S4_closed.upon(rx_welcome, enter=S4_closed, outputs=[])
+    S4_closed.upon(rx_unwelcome, enter=S4_closed, outputs=[])
     S4_closed.upon(_got_phase, enter=S4_closed, outputs=[])
     S4_closed.upon(_got_version, enter=S4_closed, outputs=[])
     S4_closed.upon(happy, enter=S4_closed, outputs=[])

src/wormhole/cli/cmd_receive.py

@@ -5,11 +5,12 @@ from humanize import naturalsize
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.python import log
-from .. import wormhole
+from .. import wormhole, __version__
 from ..transit import TransitReceiver
 from ..errors import TransferError, WormholeClosedError, NoTorError
 from ..util import (dict_to_bytes, bytes_to_dict, bytes_to_hexstr,
                     estimate_free_space)
+from .welcome import CLIWelcomeHandler
 
 APPID = u"lothar.com/wormhole/text-or-file-xfer"
 VERIFY_TIMER = 1
@@ -61,10 +62,13 @@ class TwistedReceiver:
             # with the user handing off the wormhole code
             yield self._tor_manager.start()
 
+        wh = CLIWelcomeHandler(self.args.relay_url, __version__,
+                               self.args.stderr)
         w = wormhole.create(self.args.appid or APPID, self.args.relay_url,
                             self._reactor,
                             tor_manager=self._tor_manager,
-                            timing=self.args.timing)
+                            timing=self.args.timing,
+                            welcome_handler=wh.handle_welcome)
         # I wanted to do this instead:
         #
         #    try:

src/wormhole/cli/cmd_send.py

@@ -7,9 +7,10 @@ from twisted.protocols import basic
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks, returnValue
 from ..errors import TransferError, WormholeClosedError, NoTorError
-from .. import wormhole
+from .. import wormhole, __version__
 from ..transit import TransitSender
 from ..util import dict_to_bytes, bytes_to_dict, bytes_to_hexstr
+from .welcome import CLIWelcomeHandler
 
 APPID = u"lothar.com/wormhole/text-or-file-xfer"
 VERIFY_TIMER = 1
@@ -52,10 +53,13 @@ class Sender:
             # with the user handing off the wormhole code
             yield self._tor_manager.start()
 
+        wh = CLIWelcomeHandler(self._args.relay_url, __version__,
+                               self._args.stderr)
         w = wormhole.create(self._args.appid or APPID, self._args.relay_url,
                             self._reactor,
                             tor_manager=self._tor_manager,
-                            timing=self._timing)
+                            timing=self._timing,
+                            welcome_handler=wh.handle_welcome)
         d = self._go(w)
 
         # if we succeed, we should close and return the w.close results

src/wormhole/cli/welcome.py

@@ -0,0 +1,24 @@
+from __future__ import print_function, absolute_import, unicode_literals
+import sys
+from ..wormhole import _WelcomeHandler
+
+class CLIWelcomeHandler(_WelcomeHandler):
+    def __init__(self, url, cli_version, stderr=sys.stderr):
+        _WelcomeHandler.__init__(self, url, stderr)
+        self._current_version = cli_version
+        self._version_warning_displayed = False
+
+    def handle_welcome(self, welcome):
+        # Only warn if we're running a release version (e.g. 0.0.6, not
+        # 0.0.6-DISTANCE-gHASH). Only warn once.
+        if ("current_cli_version" in welcome
+            and "-" not in self._current_version
+            and not self._version_warning_displayed
+            and welcome["current_cli_version"] != self._current_version):
+            print("Warning: errors may occur unless both sides are running the same version", file=self.stderr)
+            print("Server claims %s is current, but ours is %s"
+                  % (welcome["current_cli_version"], self._current_version),
+                  file=self.stderr)
+            self._version_warning_displayed = True
+        _WelcomeHandler.handle_welcome(self, welcome)
+

src/wormhole/test/test_cli.py

@@ -0,0 +1,45 @@
+from __future__ import print_function, absolute_import, unicode_literals
+import io
+from twisted.trial import unittest
+from ..cli import welcome
+
+class Welcome(unittest.TestCase):
+    def do(self, welcome_message, my_version="2.0", twice=False):
+        stderr = io.StringIO()
+        h = welcome.CLIWelcomeHandler("url", my_version, stderr)
+        h.handle_welcome(welcome_message)
+        if twice:
+            h.handle_welcome(welcome_message)
+        return stderr.getvalue()
+
+    def test_empty(self):
+        stderr = self.do({})
+        self.assertEqual(stderr, "")
+
+    def test_version_current(self):
+        stderr = self.do({"current_cli_version": "2.0"})
+        self.assertEqual(stderr, "")
+
+    def test_version_old(self):
+        stderr = self.do({"current_cli_version": "3.0"})
+        expected = ("Warning: errors may occur unless both sides are running the same version\n" +
+                    "Server claims 3.0 is current, but ours is 2.0\n")
+        self.assertEqual(stderr, expected)
+
+    def test_version_old_twice(self):
+        stderr = self.do({"current_cli_version": "3.0"}, twice=True)
+        # the handler should only emit the version warning once, even if we
+        # get multiple Welcome messages (which could happen if we lose the
+        # connection and then reconnect)
+        expected = ("Warning: errors may occur unless both sides are running the same version\n" +
+                    "Server claims 3.0 is current, but ours is 2.0\n")
+        self.assertEqual(stderr, expected)
+
+    def test_version_unreleased(self):
+        stderr = self.do({"current_cli_version": "3.0"},
+                         my_version="2.5-middle-something")
+        self.assertEqual(stderr, "")
+
+    def test_motd(self):
+        stderr = self.do({"motd": "hello"})
+        self.assertEqual(stderr, "Server (at url) says:\n hello\n")

src/wormhole/test/test_scripts.py

@@ -713,6 +713,9 @@ class NotWelcome(ServerBase, unittest.TestCase):
         send_d = cmd_send.send(self.cfg)
         f = yield self.assertFailure(send_d, WelcomeError)
         self.assertEqual(str(f), "please upgrade XYZ")
+        # TODO: this comes from log.err() in cmd_send.Sender.go._bad, and I'm
+        # undecided about whether that ought to be doing log.err or not
+        self.flushLoggedErrors(WelcomeError)
 
     @inlineCallbacks
     def test_receiver(self):
@@ -721,7 +724,7 @@ class NotWelcome(ServerBase, unittest.TestCase):
         receive_d = cmd_receive.receive(self.cfg)
         f = yield self.assertFailure(receive_d, WelcomeError)
         self.assertEqual(str(f), "please upgrade XYZ")
-NotWelcome.skip = "not yet"
+        self.flushLoggedErrors(WelcomeError)
 
 class Cleanup(ServerBase, unittest.TestCase):
 

src/wormhole/test/test_wormhole.py

@@ -1,5 +1,5 @@
 from __future__ import print_function, unicode_literals
-import os, json, re, gc
+import os, json, re, gc, io
 from binascii import hexlify, unhexlify
 import mock
 from twisted.trial import unittest
@@ -38,70 +38,20 @@ def response(w, **kwargs):
 
 class Welcome(unittest.TestCase):
     def test_tolerate_no_current_version(self):
-        w = wormhole._WelcomeHandler("relay_url", "current_cli_version", None)
+        w = wormhole._WelcomeHandler("relay_url")
         w.handle_welcome({})
 
     def test_print_motd(self):
-        w = wormhole._WelcomeHandler("relay_url", "current_cli_version", None)
+        stderr = io.StringIO()
-        with mock.patch("sys.stderr") as stderr:
+        w = wormhole._WelcomeHandler("relay_url", stderr=stderr)
-            w.handle_welcome({"motd": "message of\nthe day"})
+        w.handle_welcome({"motd": "message of\nthe day"})
-        self.assertEqual(stderr.method_calls,
+        self.assertEqual(stderr.getvalue(),
-                         [mock.call.write("Server (at relay_url) says:\n"
+                         "Server (at relay_url) says:\n message of\n the day\n")
-                                          " message of\n the day"),
-                          mock.call.write("\n")])
         # motd can be displayed multiple times
-        with mock.patch("sys.stderr") as stderr2:
+        w.handle_welcome({"motd": "second message"})
-            w.handle_welcome({"motd": "second message"})
+        self.assertEqual(stderr.getvalue(),
-        self.assertEqual(stderr2.method_calls,
+                         ("Server (at relay_url) says:\n message of\n the day\n"
-                         [mock.call.write("Server (at relay_url) says:\n"
+                          "Server (at relay_url) says:\n second message\n"))
-                                          " second message"),
-                          mock.call.write("\n")])
-
-    def test_current_version(self):
-        w = wormhole._WelcomeHandler("relay_url", "2.0", None)
-        with mock.patch("sys.stderr") as stderr:
-            w.handle_welcome({"current_cli_version": "2.0"})
-        self.assertEqual(stderr.method_calls, [])
-
-        with mock.patch("sys.stderr") as stderr:
-            w.handle_welcome({"current_cli_version": "3.0"})
-        exp1 = ("Warning: errors may occur unless both sides are"
-                " running the same version")
-        exp2 = ("Server claims 3.0 is current, but ours is 2.0")
-        self.assertEqual(stderr.method_calls,
-                         [mock.call.write(exp1),
-                          mock.call.write("\n"),
-                          mock.call.write(exp2),
-                          mock.call.write("\n"),
-                          ])
-
-        # warning is only displayed once
-        with mock.patch("sys.stderr") as stderr:
-            w.handle_welcome({"current_cli_version": "3.0"})
-        self.assertEqual(stderr.method_calls, [])
-
-    def test_non_release_version(self):
-        w = wormhole._WelcomeHandler("relay_url", "2.0-dirty", None)
-        with mock.patch("sys.stderr") as stderr:
-            w.handle_welcome({"current_cli_version": "3.0"})
-        self.assertEqual(stderr.method_calls, [])
-
-    def test_signal_error(self):
-        se = mock.Mock()
-        w = wormhole._WelcomeHandler("relay_url", "2.0", se)
-        w.handle_welcome({})
-        self.assertEqual(se.mock_calls, [])
-
-        w.handle_welcome({"error": "oops"})
-        self.assertEqual(len(se.mock_calls), 1)
-        self.assertEqual(len(se.mock_calls[0][1]), 2) # posargs
-        we = se.mock_calls[0][1][0]
-        self.assertIsInstance(we, WelcomeError)
-        self.assertEqual(we.args, ("oops",))
-        mood = se.mock_calls[0][1][1]
-        self.assertEqual(mood, "unwelcome")
-        # alas WelcomeError instances don't compare against each other
-        #self.assertEqual(se.mock_calls, [mock.call(WelcomeError("oops"))])
 
 class Basic(unittest.TestCase):
     def tearDown(self):

src/wormhole/wormhole.py

@@ -10,7 +10,7 @@ from .timing import DebugTiming
 from .journal import ImmediateJournal
 from ._boss import Boss
 from ._key import derive_key
-from .errors import WelcomeError, NoKeyError, WormholeClosed
+from .errors import NoKeyError, WormholeClosed
 from .util import to_bytes
 
 # We can provide different APIs to different apps:
@@ -39,34 +39,16 @@ def _log(client_name, machine_name, old_state, input, new_state):
                                     old_state, input, new_state))
 
 class _WelcomeHandler:
-    def __init__(self, url, current_version, signal_error):
+    def __init__(self, url, stderr=sys.stderr):
-        self._ws_url = url
+        self.relay_url = url
-        self._version_warning_displayed = False
+        self.stderr = stderr
-        self._current_version = current_version
-        self._signal_error = signal_error
 
     def handle_welcome(self, welcome):
         if "motd" in welcome:
             motd_lines = welcome["motd"].splitlines()
             motd_formatted = "\n ".join(motd_lines)
             print("Server (at %s) says:\n %s" %
-                  (self._ws_url, motd_formatted), file=sys.stderr)
+                  (self.relay_url, motd_formatted), file=self.stderr)
-
-        # Only warn if we're running a release version (e.g. 0.0.6, not
-        # 0.0.6-DISTANCE-gHASH). Only warn once.
-        if ("current_cli_version" in welcome
-            and "-" not in self._current_version
-            and not self._version_warning_displayed
-            and welcome["current_cli_version"] != self._current_version):
-            print("Warning: errors may occur unless both sides are running the same version", file=sys.stderr)
-            print("Server claims %s is current, but ours is %s"
-                  % (welcome["current_cli_version"], self._current_version),
-                  file=sys.stderr)
-            self._version_warning_displayed = True
-
-        if "error" in welcome:
-            return self._signal_error(WelcomeError(welcome["error"]),
-                                      "unwelcome")
 
 @attrs
 @implementer(IWormhole)
@@ -118,8 +100,6 @@ class _DelegatedWormhole(object):
     # from below
     def got_code(self, code):
         self._delegate.wormhole_code(code)
-    def got_welcome(self, welcome):
-        pass # TODO
     def got_key(self, key):
         self._key = key # for derive_key()
     def got_verifier(self, verifier):
@@ -232,8 +212,6 @@ class _DeferredWormhole(object):
         for d in self._code_observers:
             d.callback(code)
         self._code_observers[:] = []
-    def got_welcome(self, welcome):
-        pass # TODO
     def got_key(self, key):
         self._key = key # for derive_key()
     def got_verifier(self, verifier):
@@ -280,10 +258,7 @@ def create(appid, relay_url, reactor, versions={},
     side = bytes_to_hexstr(os.urandom(5))
     journal = journal or ImmediateJournal()
     if not welcome_handler:
-        from . import __version__
+        welcome_handler = _WelcomeHandler(relay_url).handle_welcome
-        signal_error = NotImplemented # TODO
-        wh = _WelcomeHandler(relay_url, __version__, signal_error)
-        welcome_handler = wh.handle_welcome
     if delegate:
         w = _DelegatedWormhole(delegate)
     else: