Commit bbde1c0d authored by Joanne Hugé's avatar Joanne Hugé

Fix two issues related to handleHello

In some circumstances, the hello_protocol attribute could get modified
on the wrong peer, which would raise an AttributeError.
On reception of seqno 1 from a peer with protocol < 7, protocol could be
equal to zero which would cause handleHello to not return True, causing
the handshake to fail.
parent 85d77bd8
...@@ -384,14 +384,14 @@ class BaseTunnelManager(object): ...@@ -384,14 +384,14 @@ class BaseTunnelManager(object):
peer = self._getPeer(prefix) peer = self._getPeer(prefix)
msg = peer.decode(msg) msg = peer.decode(msg)
if type(msg) is tuple: if type(msg) is tuple:
real_seqno, msg = msg seqno, msg, protocol = msg
def handleHello(peer, seqno, msg): def handleHello(peer, seqno, msg, retry):
if seqno == 2: if seqno == 2:
i = len(msg) // 2 i = len(msg) // 2
h = msg[:i] h = msg[:i]
try: try:
peer.verify(msg[i:], h) peer.verify(msg[i:], h)
peer.newSession(self.cert.decrypt(h)) peer.newSession(self.cert.decrypt(h), protocol)
except (AttributeError, crypto.Error, x509.NewSessionError, except (AttributeError, crypto.Error, x509.NewSessionError,
subprocess.CalledProcessError): subprocess.CalledProcessError):
logging.debug('ignored new session key from %r', logging.debug('ignored new session key from %r',
...@@ -412,7 +412,7 @@ class BaseTunnelManager(object): ...@@ -412,7 +412,7 @@ class BaseTunnelManager(object):
if serial in self.cache.crl: if serial in self.cache.crl:
raise ValueError("revoked") raise ValueError("revoked")
except (x509.VerifyError, ValueError), e: except (x509.VerifyError, ValueError), e:
if real_seqno and peer.hello_protocol: if retry:
return True return True
logging.debug('ignored invalid certificate from %r (%s)', logging.debug('ignored invalid certificate from %r (%s)',
address, e.args[-1]) address, e.args[-1])
...@@ -430,17 +430,17 @@ class BaseTunnelManager(object): ...@@ -430,17 +430,17 @@ class BaseTunnelManager(object):
peer.stop_date = stop_date peer.stop_date = stop_date
self.selectTimeout(stop_date, self.invalidatePeers, False) self.selectTimeout(stop_date, self.invalidatePeers, False)
if seqno: if seqno:
self._sendto(to, peer.hello(self.cert)) self._sendto(to, peer.hello(self.cert, protocol))
else: else:
msg = peer.hello0(self.cert.cert) msg = peer.hello0(self.cert.cert)
if msg and self._sendto(to, msg): if msg and self._sendto(to, msg):
peer.hello0Sent() peer.hello0Sent()
if handleHello(peer, real_seqno, msg): if handleHello(peer, seqno, msg, seqno):
# It is possible to reconstruct the original message because # It is possible to reconstruct the original message because
# the serialization of the protocol version is always unique. # the serialization of the protocol version is always unique.
msg = utils.packInteger(peer.hello_protocol) + msg msg = utils.packInteger(protocol) + msg
peer.hello_protocol = 0 protocol = 0
handleHello(peer, real_seqno, msg) handleHello(peer, seqno, msg, False)
elif msg: elif msg:
# We got a valid and non-empty message. Always reply # We got a valid and non-empty message. Always reply
# something so that the sender knows we're still connected. # something so that the sender knows we're still connected.
......
...@@ -241,26 +241,26 @@ class Peer(object): ...@@ -241,26 +241,26 @@ class Peer(object):
def hello0Sent(self): def hello0Sent(self):
self._hello = time.time() + 60 self._hello = time.time() + 60
def hello(self, cert): def hello(self, cert, protocol):
key = self._key = newHmacSecret() key = self._key = newHmacSecret()
h = encrypt(crypto.dump_certificate(crypto.FILETYPE_PEM, self.cert), h = encrypt(crypto.dump_certificate(crypto.FILETYPE_PEM, self.cert),
key) key)
self._i = self._j = 2 self._i = self._j = 2
self._last = 0 self._last = 0
self.protocol = self.hello_protocol self.protocol = protocol
return ''.join(('\0\0\0\2', PACKED_PROTOCOL if self.protocol else '', return ''.join(('\0\0\0\2', PACKED_PROTOCOL if protocol else '',
h, cert.sign(h))) h, cert.sign(h)))
def _hmac(self, msg): def _hmac(self, msg):
return hmac.HMAC(self._key, msg, hashlib.sha1).digest() return hmac.HMAC(self._key, msg, hashlib.sha1).digest()
def newSession(self, key): def newSession(self, key, protocol):
if key <= self._key: if key <= self._key:
raise NewSessionError(self._key, key) raise NewSessionError(self._key, key)
self._key = key self._key = key
self._i = self._j = 2 self._i = self._j = 2
self._last = None self._last = None
self.protocol = self.hello_protocol self.protocol = protocol
def verify(self, sign, data): def verify(self, sign, data):
crypto.verify(self.cert, sign, data, 'sha512') crypto.verify(self.cert, sign, data, 'sha512')
...@@ -272,9 +272,11 @@ class Peer(object): ...@@ -272,9 +272,11 @@ class Peer(object):
if seqno <= 2: if seqno <= 2:
msg = msg[4:] msg = msg[4:]
if seqno: if seqno:
self.hello_protocol, n = utils.unpackInteger(msg) or (0, 0) protocol, n = utils.unpackInteger(msg) or (0, 0)
msg = msg[n:] msg = msg[n:]
return seqno, msg else:
protocol = None
return seqno, msg, protocol
i = -utils.HMAC_LEN i = -utils.HMAC_LEN
if self._hmac(msg[:i]) == msg[i:] and self._i < seqno: if self._hmac(msg[:i]) == msg[i:] and self._i < seqno:
self._last = None self._last = None
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment