Skip to content

Commit

Permalink
pkey: __eq__() should not use hash()
Browse files Browse the repository at this point in the history
... because `hash()` is too weak and not intended for this.
Also simplify `__cmp__()` (which did work fine).

This fixes a security flaw. If you are using Paramiko with Python 2,
or a Python 3 which is running with PYTHONHASHSEED=0, it is possible
for an attacker to craft a new keypair from an exfiltrated public key,
which Paramiko would consider equal to the original key.

This could enable attacks such as:
 * Paramiko server processes would incorrectly authenticate the attacker
   (using their generated private key) as if they were the victim.
 * Paramiko client processes would incorrectly validate a connected
   server (when host key verification is enabled) while subjected to a
   man-in-the-middle attack. This impacts more users than the server-side
   version, but also carries higher requirements for the attacker, namely
   successful DNS poisoning or other MITM techniques.

Reported by @jun66j5 in paramiko#908
Vulnerability description by Jeff Forcier <jeff@bitprophet.org>
  • Loading branch information
ploxiln committed Nov 30, 2021
1 parent 09289a5 commit 11025ca
Showing 1 changed file with 7 additions and 13 deletions.
20 changes: 7 additions & 13 deletions paramiko/pkey.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,26 +112,20 @@ def asbytes(self):
def __str__(self):
return self.asbytes()

# noinspection PyUnresolvedReferences
# TODO: The comparison functions should be removed as per:
# https://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons
def __cmp__(self, other):
# python-2 only, same purpose as __eq__()
return cmp(self.asbytes(), other.asbytes()) # noqa

def __eq__(self, other):
"""
Compare this key to another. Returns 0 if this key is equivalent to
the given key, or non-0 if they are different. Only the public parts
Compare this key to another. Returns True if this key is equivalent to
the given key, or False if they are different. Only the public parts
of the key are compared, so a public key will compare equal to its
corresponding private key.
:param .PKey other: key to compare to.
"""
hs = hash(self)
ho = hash(other)
if hs != ho:
return cmp(hs, ho) # noqa
return cmp(self.asbytes(), other.asbytes()) # noqa

def __eq__(self, other):
return hash(self) == hash(other)
return self.asbytes() == other.asbytes()

def get_name(self):
"""
Expand Down

0 comments on commit 11025ca

Please sign in to comment.