From 5553729a879136372f4c0f47c2e68753006f637e Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Wed, 25 May 2016 18:05:02 -0700 Subject: [PATCH] w.verify() now stalls until confirmation message is checked If it succeeds, you get back the verifier string, which can be compared against the other side. If it fails, the wormhole code didn't match. --- src/wormhole/test/test_wormhole.py | 99 +++++++++++++++++++++++++++--- src/wormhole/wormhole.py | 59 ++++++++++++------ 2 files changed, 129 insertions(+), 29 deletions(-) diff --git a/src/wormhole/test/test_wormhole.py b/src/wormhole/test/test_wormhole.py index 8948535..3570261 100644 --- a/src/wormhole/test/test_wormhole.py +++ b/src/wormhole/test/test_wormhole.py @@ -224,17 +224,16 @@ class Basic(unittest.TestCase): response(w, type=u"message", phase=u"pake", body=msg2_hex, side=side2) # hearing the peer's PAKE (msg2) makes us release the nameplate, send - # the confirmation message, delivered the verifier, and sends any - # queued phase messages + # the confirmation message, and sends any queued phase messages. It + # doesn't deliver the verifier because we're still waiting on the + # confirmation message. self.assertFalse(w._flag_need_to_see_mailbox_used) self.assertEqual(w._key, key) out = ws.outbound() self.assertEqual(len(out), 2, out) self.check_out(out[0], type=u"release") self.check_out(out[1], type=u"add", phase=u"confirm") - verifier = self.successResultOf(v) - self.assertEqual(verifier, - w.derive_key(u"wormhole:verifier", SecretBox.KEY_SIZE)) + self.assertNoResult(v) # hearing a valid confirmation message doesn't throw an error confkey = w.derive_key(u"wormhole:confirmation", SecretBox.KEY_SIZE) @@ -244,6 +243,11 @@ class Basic(unittest.TestCase): response(w, type=u"message", phase=u"confirm", body=confirm2_hex, side=side2) + # and it releases the verifier + verifier = self.successResultOf(v) + self.assertEqual(verifier, + w.derive_key(u"wormhole:verifier", SecretBox.KEY_SIZE)) + # an outbound message can now be sent immediately w.send(b"phase0-outbound") out = ws.outbound() @@ -506,10 +510,85 @@ class Basic(unittest.TestCase): self.assertEqual(len(pieces), 3) # nameplate plus two words self.assert_(re.search(r'^\d+-\w+-\w+$', code), code) + # make sure verify() can be called both before and after the verifier is + # computed + + def _test_verifier(self, when, order, success): + assert when in ("early", "middle", "late") + assert order in ("key-then-confirm", "confirm-then-key") + assert isinstance(success, bool) + #print(when, order, success) + + timing = DebugTiming() + w = wormhole._Wormhole(APPID, u"relay_url", reactor, None, timing) + w._drop_connection = mock.Mock() + w._ws_send_command = mock.Mock() + w._mailbox_state = wormhole.OPEN + d = None + + if success: + w._key = b"key" + else: + w._key = b"wrongkey" + confkey = w._derive_confirmation_key() + nonce = os.urandom(wormhole.CONFMSG_NONCE_LENGTH) + confmsg = wormhole.make_confmsg(confkey, nonce) + w._key = None + + if when == "early": + d = w.verify() + self.assertNoResult(d) + + if order == "key-then-confirm": + w._key = b"key" + w._event_established_key() + else: + w._event_received_confirm(confmsg) + + if when == "middle": + d = w.verify() + if d: + self.assertNoResult(d) # still waiting for other msg + + if order == "confirm-then-key": + w._key = b"key" + w._event_established_key() + else: + w._event_received_confirm(confmsg) + + if when == "late": + d = w.verify() + if success: + self.successResultOf(d) + else: + self.assertFailure(d, wormhole.WrongPasswordError) + self.flushLoggedErrors(WrongPasswordError) + def test_verifier(self): - # make sure verify() can be called both before and after the verifier - # is computed - pass + for when in ("early", "middle", "late"): + for order in ("key-then-confirm", "confirm-then-key"): + for success in (False, True): + self._test_verifier(when, order, success) + + def test_verifier_4(self): + # ask early, key-then-confirm, success + timing = DebugTiming() + w = wormhole._Wormhole(APPID, u"relay_url", reactor, None, timing) + w._drop_connection = mock.Mock() + w._ws_send_command = mock.Mock() + w._mailbox_state = wormhole.OPEN + d = w.verify() + self.assertNoResult(d) + + w._key = b"key" + w._event_established_key() + self.assertNoResult(d) # still waiting for confirmation message + confkey = w._derive_confirmation_key() + nonce = os.urandom(wormhole.CONFMSG_NONCE_LENGTH) + confmsg = wormhole.make_confmsg(confkey, nonce) + w._event_received_confirm(confmsg) + self.successResultOf(d) + def test_api_errors(self): # doing things you're not supposed to do @@ -578,8 +657,7 @@ class Basic(unittest.TestCase): msg2_hex = hexlify(msg2).decode("ascii") response(w, type=u"message", phase=u"pake", body=msg2_hex, side=u"s2") self.assertNoResult(d1) - self.successResultOf(d2) # early verify is unaffected - # TODO: change verify() to wait for "confirm" + self.assertNoResult(d2) # verify() waits for confirmation # sending a random confirm message will cause a confirmation error confkey = w.derive_key(u"WRONG", SecretBox.KEY_SIZE) @@ -590,6 +668,7 @@ class Basic(unittest.TestCase): side=u"s2") self.failureResultOf(d1, WrongPasswordError) + self.failureResultOf(d2, WrongPasswordError) # once the error is signalled, all API calls should fail self.assertRaises(WrongPasswordError, w.send, u"foo") diff --git a/src/wormhole/wormhole.py b/src/wormhole/wormhole.py index d239ed0..8513c3c 100644 --- a/src/wormhole/wormhole.py +++ b/src/wormhole/wormhole.py @@ -5,7 +5,7 @@ from binascii import hexlify, unhexlify from twisted.internet import defer, endpoints, error from twisted.internet.threads import deferToThread, blockingCallFromThread from twisted.internet.defer import inlineCallbacks, returnValue -from twisted.python import log +from twisted.python import log, failure from autobahn.twisted import websocket from nacl.secret import SecretBox from nacl.exceptions import CryptoError @@ -237,15 +237,19 @@ class _Wormhole: self._flag_need_to_build_msg1 = True self._flag_need_to_send_PAKE = True self._key = None + + self._confirmation_message = None + self._confirmation_checked = False + self._get_verifier_called = False + self._verifier = None # bytes + self._verify_result = None # bytes or a Failure + self._verifier_waiter = None + self._close_called = False # the close() API has been called self._closing = False # we've started shutdown self._disconnect_waiter = defer.Deferred() self._error = None - self._get_verifier_called = False - self._verifier = None - self._verifier_waiter = None - self._next_send_phase = 0 # send() queues plaintext here, waiting for a connection and the key self._plaintext_to_send = [] # (phase, plaintext) @@ -549,38 +553,55 @@ class _Wormhole: verifier = self._derive_key(b"wormhole:verifier") self._event_computed_verifier(verifier) + self._maybe_check_confirmation() self._maybe_send_phase_messages() def _API_verify(self): - # TODO: rename "verify()", make it stall until confirm received. If - # you want to discover WrongPasswordError before doing send(), call - # verify() first. If you also want to deny a successful MitM (and - # have some other way to check a long verifier), use the return value - # of verify(). if self._error: return defer.fail(self._error) if self._get_verifier_called: raise UsageError self._get_verifier_called = True - if self._verifier: - return defer.succeed(self._verifier) - # TODO: maybe have this wait on _event_received_confirm too + if self._verify_result: + return defer.succeed(self._verify_result) # bytes or Failure self._verifier_waiter = defer.Deferred() return self._verifier_waiter def _event_computed_verifier(self, verifier): self._verifier = verifier - if self._verifier_waiter: - self._verifier_waiter.callback(verifier) + self._maybe_notify_verify() + + def _maybe_notify_verify(self): + if not (self._verifier and self._confirmation_checked): + return + if self._error: + self._verify_result = failure.Failure(self._error) + else: + self._verify_result = self._verifier + if self._verifier_waiter and not self._verifier_waiter.called: + self._verifier_waiter.callback(self._verify_result) def _event_received_confirm(self, body): - # TODO: we might not have a master key yet, if the caller wasn't - # waiting in _get_master_key() when a back-to-back pake+_confirm - # message pair arrived. + # We ought to have the master key by now, because sensible peers + # should always send "pake" before sending "confirm". It might be + # nice to relax this requirement, which means storing the received + # confirmation message, and having _event_established_key call + # _check_confirmation() + self._confirmation_message = body + self._maybe_check_confirmation() + + def _maybe_check_confirmation(self): + if not (self._key and self._confirmation_message): + return + if self._confirmation_checked: + return confkey = self._derive_confirmation_key() + body = self._confirmation_message nonce = body[:CONFMSG_NONCE_LENGTH] if body != make_confmsg(confkey, nonce): # this makes all API calls fail if self.DEBUG: print("CONFIRM FAILED") - return self._signal_error(WrongPasswordError(), u"scary") + self._signal_error(WrongPasswordError(), u"scary") + self._confirmation_checked = True + self._maybe_notify_verify() def _API_send(self, outbound_data):