On Fri, Dec 2, 2016 at 1:23 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:


Investigating further, I think I've figured it out.  Here's a patch that fixes the problem:

diff --git a/src/twisted/conch/ssh/keys.py b/src/twisted/conch/ssh/keys.py
index d47db7f..570f524 100644
--- a/src/twisted/conch/ssh/keys.py
+++ b/src/twisted/conch/ssh/keys.py
@@ -247,8 +247,10 @@ class Key(object):
                 ).public_key(default_backend())
             )
         elif keyType in [b'ecdsa-sha2-' + curve for curve in list(_curveTable.keys())]:
-            x, y, rest = common.getMP(rest, 2)
-            return cls._fromECComponents(x=x, y=y, curve=keyType)
+            return cls(load_ssh_public_key(
+                keyType + b' ' + base64.encodestring(blob),
+                default_backend())
+            )
         else:
             raise BadKeyError('unknown blob type: %s' % (keyType,))
 
I suspect, but do not fully understand, that the problem here is point compression.  Key._fromString_BLOB naively just does getMP(blob, 2) and expects that the x,y will be the EC point.  However, to quote https://tools.ietf.org/html/rfc5656#section-3.1, "point compression MAY be used".  I don't know exactly how point compression works, but I do understand that it means you do funky stuff with the Y value.  I do not believe that EllipticCurvePrivateNumbers understands said funky stuff.

A specific ECC integration test with ssh-keygen from OpenSSH and twisted.conch.client.knownhosts.KnownHostsFile would have spotted this specific manifestation of the issue.  However, another underlying bug is that KnownHostsFile _should_ ignore lines that it can't parse.  And, there's another potential manifestation of the issue where loading from an actual key blob might not work; arguably the problem here is that KnownHostsFile made the questionable decision to do its own base64-decoding and pass the blob straight to the key rather than just pass the portion of the line after the hostname and load it as an openssh-format key directly.


Yes, you are on the right track.  As the known_hosts file is parsed line by line,
if an exception is thrown during parsing, then any valid keys further in the file are ignored,
and you get the "bad host key" error.

I tried your patch, and while I don't get the same error, the patch doesn't solve the problem for me.

I did some more debugging, and when I tried to put some print statements in the code to
figure out what is going on, I found these errors:

Traceback (most recent call last):                                                                      

  File "<pudb command line>", line 1, in <module>
  File "/Users/crodrigues/twisted8/src/twisted/conch/ssh/keys.py", line 787, in __repr__
    self._keyObject.key_size)]                                                                          
AttributeError: '_EllipticCurvePublicKey' object has no attribute 'key_size'


and also this:

  File "/Users/crodrigues/twisted8/src/twisted/conch/client/knownhosts.py", line 107, in matchesKey
    print("SELF.PUBLICKEY ", self.publickey)
AttributeError: 'PlainEntry' object has no attribute 'publickey'

--
Craig