From 6b57d7d05dc99488b7405cf6121352d03a3d3ff9 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Mon, 16 Nov 2015 18:24:39 -0800 Subject: [PATCH 1/4] check key-confirmation messages, if present --- src/wormhole/blocking/transcribe.py | 17 ++++++++++++--- src/wormhole/test/test_blocking.py | 32 ++++++++++++++++++++++++++++- src/wormhole/test/test_twisted.py | 29 +++++++++++++++++++++++++- src/wormhole/twisted/transcribe.py | 27 ++++++++++++++++++------ 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/src/wormhole/blocking/transcribe.py b/src/wormhole/blocking/transcribe.py index 63ef206..01fed57 100644 --- a/src/wormhole/blocking/transcribe.py +++ b/src/wormhole/blocking/transcribe.py @@ -214,6 +214,7 @@ class Wormhole: self.verifier = None self._sent_data = set() # phases self._got_data = set() + self._got_confirmation = False self._closed = False def __enter__(self): @@ -355,10 +356,20 @@ class Wormhole: if self._channel is None: raise UsageError self._got_data.add(phase) self._get_key() - data_key = self.derive_key(u"wormhole:phase:%s" % phase) - inbound_encrypted = self._channel.get(phase) + phases = [] + if not self._got_confirmation: + phases.append(u"_confirm") + phases.append(phase) + (got_phase, body) = self._channel.get_first_of(phases) + if got_phase == u"_confirm": + if body != self.derive_key(u"wormhole:confirmation"): + raise WrongPasswordError + self._got_confirmation = True + (got_phase, body) = self._channel.get_first_of([phase]) + assert got_phase == phase try: - inbound_data = self._decrypt_data(data_key, inbound_encrypted) + data_key = self.derive_key(u"wormhole:phase:%s" % phase) + inbound_data = self._decrypt_data(data_key, body) return inbound_data except CryptoError: raise WrongPasswordError diff --git a/src/wormhole/test/test_blocking.py b/src/wormhole/test/test_blocking.py index 5801e99..8556df5 100644 --- a/src/wormhole/test/test_blocking.py +++ b/src/wormhole/test/test_blocking.py @@ -3,7 +3,8 @@ import json from twisted.trial import unittest from twisted.internet.defer import gatherResults, succeed from twisted.internet.threads import deferToThread -from ..blocking.transcribe import Wormhole, UsageError, ChannelManager +from ..blocking.transcribe import (Wormhole, UsageError, ChannelManager, + WrongPasswordError) from ..blocking.eventsource import EventSourceFollower from .common import ServerBase @@ -246,6 +247,35 @@ class Blocking(ServerBase, unittest.TestCase): d.addCallback(_got_2) return d + def test_wrong_password(self): + w1 = Wormhole(APPID, self.relayurl) + w2 = Wormhole(APPID, self.relayurl) + + # make sure we can detect WrongPasswordError even if one side only + # does get_data() and not send_data(), like "wormhole receive" does + d = deferToThread(w1.get_code) + d.addCallback(lambda code: w2.set_code(code+"not")) + + # w2 can't throw WrongPasswordError until it sees a CONFIRM message, + # and w1 won't send CONFIRM until it sees a PAKE message, which w2 + # won't send until we call get_data. So we need both sides to be + # running at the same time for this test. + def _w1_sends(): + w1.send_data(b"data1") + def _w2_gets(): + self.assertRaises(WrongPasswordError, w2.get_data) + d.addCallback(lambda _: self.doBoth([_w1_sends], [_w2_gets])) + + # and now w1 should have enough information to throw too + d.addCallback(lambda _: deferToThread(self.assertRaises, + WrongPasswordError, w1.get_data)) + def _done(_): + # both sides are closed automatically upon error, but it's still + # legal to call .close(), and should be idempotent + return self.doBoth([w1.close], [w2.close]) + d.addCallback(_done) + return d + def test_verifier(self): w1 = Wormhole(APPID, self.relayurl) w2 = Wormhole(APPID, self.relayurl) diff --git a/src/wormhole/test/test_twisted.py b/src/wormhole/test/test_twisted.py index fd20498..f522b06 100644 --- a/src/wormhole/test/test_twisted.py +++ b/src/wormhole/test/test_twisted.py @@ -2,7 +2,8 @@ from __future__ import print_function import sys, json from twisted.trial import unittest from twisted.internet.defer import gatherResults, succeed -from ..twisted.transcribe import Wormhole, UsageError, ChannelManager +from ..twisted.transcribe import (Wormhole, UsageError, ChannelManager, + WrongPasswordError) from ..twisted.eventsource_twisted import EventSourceParser from .common import ServerBase @@ -229,6 +230,32 @@ class Basic(ServerBase, unittest.TestCase): d.addCallback(_got_2) return d + def test_wrong_password(self): + w1 = Wormhole(APPID, self.relayurl) + w2 = Wormhole(APPID, self.relayurl) + d = w1.get_code() + d.addCallback(lambda code: w2.set_code(code+"not")) + + # w2 can't throw WrongPasswordError until it sees a CONFIRM message, + # and w1 won't send CONFIRM until it sees a PAKE message, which w2 + # won't send until we call get_data. So we need both sides to be + # running at the same time for this test. + def _w1_sends(): + return w1.send_data(b"data1") + def _w2_gets(): + return self.assertFailure(w2.get_data(), WrongPasswordError) + d.addCallback(lambda _: self.doBoth(_w1_sends(), _w2_gets())) + + # and now w1 should have enough information to throw too + d.addCallback(lambda _: self.assertFailure(w1.get_data(), + WrongPasswordError)) + def _done(_): + # both sides are closed automatically upon error, but it's still + # legal to call .close(), and should be idempotent + return self.doBoth(w1.close(), w2.close()) + d.addCallback(_done) + return d + def test_verifier(self): w1 = Wormhole(APPID, self.relayurl) w2 = Wormhole(APPID, self.relayurl) diff --git a/src/wormhole/twisted/transcribe.py b/src/wormhole/twisted/transcribe.py index 5a36d90..d92123b 100644 --- a/src/wormhole/twisted/transcribe.py +++ b/src/wormhole/twisted/transcribe.py @@ -206,6 +206,7 @@ class Wormhole: self._started_get_code = False self._sent_data = set() # phases self._got_data = set() + self._got_confirmation = False def _set_side(self, side): self._side = side @@ -375,16 +376,30 @@ class Wormhole: self._got_data.add(phase) d = self._get_key() def _get(key): - data_key = self.derive_key(u"wormhole:phase:%s" % phase) - d1 = self._channel.get(phase) - def _decrypt(inbound_encrypted): + phases = [] + if not self._got_confirmation: + phases.append(u"_confirm") + phases.append(phase) + d1 = self._channel.get_first_of(phases) + def _maybe_got_confirm(phase_and_body): + (got_phase, body) = phase_and_body + if got_phase == u"_confirm": + if body != self.derive_key(u"wormhole:confirmation"): + raise WrongPasswordError + self._got_confirmation = True + return self._channel.get_first_of([phase]) + return phase_and_body + d1.addCallback(_maybe_got_confirm) + def _got(phase_and_body): + (got_phase, body) = phase_and_body + assert got_phase == phase try: - inbound_data = self._decrypt_data(data_key, - inbound_encrypted) + data_key = self.derive_key(u"wormhole:phase:%s" % phase) + inbound_data = self._decrypt_data(data_key, body) return inbound_data except CryptoError: raise WrongPasswordError - d1.addCallback(_decrypt) + d1.addCallback(_got) return d1 d.addCallback(_get) return d From 1ad001bbc3693d202cc966215c913a9cc4e0d9ba Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Mon, 16 Nov 2015 18:25:28 -0800 Subject: [PATCH 2/4] WIP: test that we tolerate missing key-confirmation messages --- src/wormhole/blocking/transcribe.py | 3 +++ src/wormhole/test/test_blocking.py | 18 ++++++++++++++++++ src/wormhole/test/test_twisted.py | 18 ++++++++++++++++++ src/wormhole/twisted/transcribe.py | 3 +++ 4 files changed, 42 insertions(+) diff --git a/src/wormhole/blocking/transcribe.py b/src/wormhole/blocking/transcribe.py index 01fed57..b51d898 100644 --- a/src/wormhole/blocking/transcribe.py +++ b/src/wormhole/blocking/transcribe.py @@ -194,6 +194,7 @@ def close_on_error(f): # method decorator class Wormhole: motd_displayed = False version_warning_displayed = False + _send_confirm = True def __init__(self, appid, relay_url, wait=0.5*SECOND, timeout=3*MINUTE): if not isinstance(appid, type(u"")): raise TypeError(type(appid)) @@ -315,6 +316,8 @@ class Wormhole: pake_msg = self._channel.get(u"pake") self.key = self.sp.finish(pake_msg) self.verifier = self.derive_key(u"wormhole:verifier") + if not self._send_confirm: + return conf = self.derive_key(u"wormhole:confirmation") self._channel.send(u"_confirm", conf) diff --git a/src/wormhole/test/test_blocking.py b/src/wormhole/test/test_blocking.py index 8556df5..44c5a0e 100644 --- a/src/wormhole/test/test_blocking.py +++ b/src/wormhole/test/test_blocking.py @@ -276,6 +276,24 @@ class Blocking(ServerBase, unittest.TestCase): d.addCallback(_done) return d + def test_no_confirm(self): + # newer versions (which check confirmations) should will work with + # older versions (that don't send confirmations) + w1 = Wormhole(APPID, self.relayurl) + w1._send_confirm = False + w2 = Wormhole(APPID, self.relayurl) + + d = deferToThread(w1.get_code) + d.addCallback(lambda code: w2.set_code(code)) + d.addCallback(lambda _: self.doBoth([w1.send_data, b"data1"], + [w2.get_data])) + d.addCallback(lambda dl: self.assertEqual(dl[1], b"data1")) + d.addCallback(lambda _: self.doBoth([w1.get_data], + [w2.send_data, b"data2"])) + d.addCallback(lambda dl: self.assertEqual(dl[0], b"data2")) + d.addCallback(lambda _: self.doBoth([w1.close], [w2.close])) + return d + def test_verifier(self): w1 = Wormhole(APPID, self.relayurl) w2 = Wormhole(APPID, self.relayurl) diff --git a/src/wormhole/test/test_twisted.py b/src/wormhole/test/test_twisted.py index f522b06..341a44a 100644 --- a/src/wormhole/test/test_twisted.py +++ b/src/wormhole/test/test_twisted.py @@ -256,6 +256,24 @@ class Basic(ServerBase, unittest.TestCase): d.addCallback(_done) return d + def test_no_confirm(self): + # newer versions (which check confirmations) should will work with + # older versions (that don't send confirmations) + w1 = Wormhole(APPID, self.relayurl) + w1._send_confirm = False + w2 = Wormhole(APPID, self.relayurl) + + d = w1.get_code() + d.addCallback(lambda code: w2.set_code(code)) + d.addCallback(lambda _: self.doBoth(w1.send_data(b"data1"), + w2.get_data())) + d.addCallback(lambda dl: self.assertEqual(dl[1], b"data1")) + d.addCallback(lambda _: self.doBoth(w1.get_data(), + w2.send_data(b"data2"))) + d.addCallback(lambda dl: self.assertEqual(dl[0], b"data2")) + d.addCallback(lambda _: self.doBoth(w1.close(), w2.close())) + return d + def test_verifier(self): w1 = Wormhole(APPID, self.relayurl) w2 = Wormhole(APPID, self.relayurl) diff --git a/src/wormhole/twisted/transcribe.py b/src/wormhole/twisted/transcribe.py index d92123b..9081ba1 100644 --- a/src/wormhole/twisted/transcribe.py +++ b/src/wormhole/twisted/transcribe.py @@ -192,6 +192,7 @@ class ChannelManager: class Wormhole: motd_displayed = False version_warning_displayed = False + _send_confirm = True def __init__(self, appid, relay_url): if not isinstance(appid, type(u"")): raise TypeError(type(appid)) @@ -333,6 +334,8 @@ class Wormhole: key = self.sp.finish(pake_msg) self.key = key self.verifier = self.derive_key(u"wormhole:verifier") + if not self._send_confirm: + return key conf = self.derive_key(u"wormhole:confirmation") d1 = self._channel.send(u"_confirm", conf) d1.addCallback(lambda _: key) From fd9a62e8ff01020e8b08235f4494039fc802460b Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 19 Nov 2015 16:06:30 -0800 Subject: [PATCH 3/4] change confirmation message: must be different on each side The previous same-message-for-both-sides approach failed, because the Channel filters out duplicates. --- src/wormhole/blocking/transcribe.py | 15 ++++++++++++--- src/wormhole/twisted/transcribe.py | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/wormhole/blocking/transcribe.py b/src/wormhole/blocking/transcribe.py index b51d898..167fd9f 100644 --- a/src/wormhole/blocking/transcribe.py +++ b/src/wormhole/blocking/transcribe.py @@ -16,6 +16,11 @@ from ..channel_monitor import monitor SECOND = 1 MINUTE = 60*SECOND +CONFMSG_NONCE_LENGTH = 128//8 +CONFMSG_MAC_LENGTH = 256//8 +def make_confmsg(confkey, nonce): + return nonce+HKDF(confkey, CONFMSG_MAC_LENGTH, nonce) + def to_bytes(u): return unicodedata.normalize("NFC", u).encode("utf-8") @@ -318,8 +323,10 @@ class Wormhole: self.verifier = self.derive_key(u"wormhole:verifier") if not self._send_confirm: return - conf = self.derive_key(u"wormhole:confirmation") - self._channel.send(u"_confirm", conf) + confkey = self.derive_key(u"wormhole:confirmation") + nonce = os.urandom(CONFMSG_NONCE_LENGTH) + confmsg = make_confmsg(confkey, nonce) + self._channel.send(u"_confirm", confmsg) @close_on_error def get_verifier(self): @@ -365,7 +372,9 @@ class Wormhole: phases.append(phase) (got_phase, body) = self._channel.get_first_of(phases) if got_phase == u"_confirm": - if body != self.derive_key(u"wormhole:confirmation"): + confkey = self.derive_key(u"wormhole:confirmation") + nonce = body[:CONFMSG_NONCE_LENGTH] + if body != make_confmsg(confkey, nonce): raise WrongPasswordError self._got_confirmation = True (got_phase, body) = self._channel.get_first_of([phase]) diff --git a/src/wormhole/twisted/transcribe.py b/src/wormhole/twisted/transcribe.py index 9081ba1..c38f702 100644 --- a/src/wormhole/twisted/transcribe.py +++ b/src/wormhole/twisted/transcribe.py @@ -18,6 +18,11 @@ from ..errors import ServerError, WrongPasswordError, UsageError from ..util.hkdf import HKDF from ..channel_monitor import monitor +CONFMSG_NONCE_LENGTH = 128//8 +CONFMSG_MAC_LENGTH = 256//8 +def make_confmsg(confkey, nonce): + return nonce+HKDF(confkey, CONFMSG_MAC_LENGTH, nonce) + def to_bytes(u): return unicodedata.normalize("NFC", u).encode("utf-8") @@ -336,8 +341,10 @@ class Wormhole: self.verifier = self.derive_key(u"wormhole:verifier") if not self._send_confirm: return key - conf = self.derive_key(u"wormhole:confirmation") - d1 = self._channel.send(u"_confirm", conf) + confkey = self.derive_key(u"wormhole:confirmation") + nonce = os.urandom(CONFMSG_NONCE_LENGTH) + confmsg = make_confmsg(confkey, nonce) + d1 = self._channel.send(u"_confirm", confmsg) d1.addCallback(lambda _: key) return d1 d.addCallback(_got_pake) @@ -387,7 +394,9 @@ class Wormhole: def _maybe_got_confirm(phase_and_body): (got_phase, body) = phase_and_body if got_phase == u"_confirm": - if body != self.derive_key(u"wormhole:confirmation"): + confkey = self.derive_key(u"wormhole:confirmation") + nonce = body[:CONFMSG_NONCE_LENGTH] + if body != make_confmsg(confkey, nonce): raise WrongPasswordError self._got_confirmation = True return self._channel.get_first_of([phase]) From 9827a2e50c8a3aa8ae317c284e52d2b713514ea0 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Thu, 19 Nov 2015 16:21:10 -0800 Subject: [PATCH 4/4] add twisted/blocking interop test --- src/wormhole/test/test_interop.py | 64 +++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 src/wormhole/test/test_interop.py diff --git a/src/wormhole/test/test_interop.py b/src/wormhole/test/test_interop.py new file mode 100644 index 0000000..7a8bab6 --- /dev/null +++ b/src/wormhole/test/test_interop.py @@ -0,0 +1,64 @@ +from __future__ import print_function +import sys +from twisted.trial import unittest +from twisted.internet.defer import gatherResults +from twisted.internet.threads import deferToThread +from ..twisted.transcribe import Wormhole as twisted_Wormhole +from ..blocking.transcribe import Wormhole as blocking_Wormhole +from .common import ServerBase + +# make sure the two implementations (Twisted-style and blocking-style) can +# interoperate + +APPID = u"appid" + +class Basic(ServerBase, unittest.TestCase): + + def doBoth(self, call1, d2): + f1 = call1[0] + f1args = call1[1:] + return gatherResults([deferToThread(f1, *f1args), d2], True) + + def test_twisted_to_blocking(self): + tw = twisted_Wormhole(APPID, self.relayurl) + bw = blocking_Wormhole(APPID, self.relayurl) + d = tw.get_code() + def _got_code(code): + bw.set_code(code) + return self.doBoth([bw.send_data, b"data2"], tw.send_data(b"data1")) + d.addCallback(_got_code) + def _sent(res): + return self.doBoth([bw.get_data], tw.get_data()) + d.addCallback(_sent) + def _done(dl): + (dataX, dataY) = dl + self.assertEqual(dataX, b"data1") + self.assertEqual(dataY, b"data2") + return self.doBoth([bw.close], tw.close()) + d.addCallback(_done) + return d + + def test_blocking_to_twisted(self): + bw = blocking_Wormhole(APPID, self.relayurl) + tw = twisted_Wormhole(APPID, self.relayurl) + d = deferToThread(bw.get_code) + def _got_code(code): + tw.set_code(code) + return self.doBoth([bw.send_data, b"data1"], tw.send_data(b"data2")) + d.addCallback(_got_code) + def _sent(res): + return self.doBoth([bw.get_data], tw.get_data()) + d.addCallback(_sent) + def _done(dl): + (dataX, dataY) = dl + self.assertEqual(dataX, b"data2") + self.assertEqual(dataY, b"data1") + return self.doBoth([bw.close], tw.close()) + d.addCallback(_done) + return d + +if sys.version_info[0] >= 3: + Basic.skip = "twisted is not yet sufficiently ported to py3" + # as of 15.4.0, Twisted is still missing: + # * web.client.Agent (for all non-EventSource POSTs in transcribe.py) + # * python.logfile (to allow daemonization of 'wormhole server')