[Twisted-Python] conch problem with ecdsa-sha2-nistp256 host key?

[Glyph Lefkowitz]

> On Dec 2, 2016, at 12:37 AM, Glyph Lefkowitz <glyph at twistedmatrix.com> wrote:
> 
> 
>> On Dec 2, 2016, at 12:27 AM, Glyph Lefkowitz <glyph at twistedmatrix.com <mailto:glyph at twistedmatrix.com>> wrote:
>> 
>> 
>>> On Dec 2, 2016, at 12:19 AM, Glyph Lefkowitz <glyph at twistedmatrix.com <mailto:glyph at twistedmatrix.com>> wrote:
>>> 
>>> 
>>>> On Dec 1, 2016, at 7:01 PM, Mark Williams <markrwilliams at gmail.com <mailto:markrwilliams at gmail.com>> wrote:
>>>> 
>>>> On Thu, Dec 01, 2016 at 05:11:37PM -0800, Craig Rodrigues wrote:
>>>>> Hi,
>>>>> 
>>>>> I filed this bug:
>>>>> https://twistedmatrix.com/trac/ticket/8931 <https://twistedmatrix.com/trac/ticket/8931>
>>>>> 
>>>>> At least for me, conch fails to parse a host key created by OpenSSH
>>>>> in ~/.ssh/known_hosts
>>>>> which is of type ecdsa-sha2-nistp256.
>>>>> 
>>>>> Anyone have an idea as to how to fix this?
>>>>> 
>>>> 
>>>> As usual you've found a fantastically interesting issue.
>>>> 
>>>> This is conch, the client, right?  I'm guessing so because
>>>> ~/.ssh/known_hosts contains the servers the ssh client trusts.
>>>> (Specifically, among other things it contains a hostname and that
>>>> host's sshd server's public key fingerprint).
>>>> 
>>>> If it is conch, the-client, then deleting the offending entry from
>>>> ~/.ssh/known_hosts and getting a new one makes sense.  That's because
>>>> sshd usually generates a couple of different keys in case clients
>>>> don't support the latest and greatest technology.
>>>> 
>>>> I think deleting the entry in ~/.ssh/known_hosts allows conch to ask
>>>> the server for a different key that it *can* understand.  You should
>>>> be able to find out which server key conch negotiated by doing thing
>>>> following after deleting the offending ~/.ssh/known_hosts entry:
>>>> 
>>>> (<hostname is the problematic OS X server>)
>>>> $ ssh-keygen -H -F <hostname> | awk '{ print $NF }'
>>>> dGhpcyBpcyB2ZXJ5IHZlcnkgdmVyeSB2ZXJ5IHZlcnkgbG9uZyBob3N0IGtleQ==
>>>> 
>>>> Then on the OS X server, grep for that in /etc/ssh/*.pub
>>>> 
>>>> I bet the key negotiated by conch is not an ECDSA key but rather an
>>>> RSA key.  If this is all the case, then I think you've found a key
>>>> that LibreSSL supports but your client's libssl (which conch calls
>>>> into via cryptography) does not.  What version of libssl do you have?
>>>> 
>>>> If any of this is helpful or relevant I'll ask more questions in the
>>>> ticket.
>>> 
>>> I think there might be a regression in 16.6.0.
>>> 
>>> For every version up to 16.6.0, I can do 'conch twistedmatrix.com <http://twistedmatrix.com/>' in a shell and it works fine.
>>> 
>>> On 16.6.0, I get:
>>> 
>>> Connection to twistedmatrix.com <http://twistedmatrix.com/> closed.
>>> conch: exiting with error [Failure instance: Traceback (failure with no frames): <class 'twisted.conch.error.ConchError'>: ('bad host key', 9)
>>> ]
>>> 
>>> instead.
>>> 
>>> Worth noting: the keys I have for twistedmatrix.com <http://twistedmatrix.com/> are RSA keys.
>>> 
>>> What did we add recently that changes key parsing?
>> 
>> The offending commit is 8164d89104a453947215b9296e8b406f15e63252.  Clearly something went wrong when introducing ECDSA parsing.
> 
> The problem is not quite as bad as breaking RSA parsing, at least; the issue is that my known_hosts file includes an ecdsa-sha2-nistp256 entry that _precedes_ my ssh-rsa entry.  So the problem is that parsing one of those entries raises an exception which propagates.  From a superficial investigation, it would appear that _all_ ecdsa keys cause this failure, though.

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.

-glyph

-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20161202/740bdb5e/attachment-0002.html>