> On Dec 2, 2016, at 12:37 AM, Glyph Lefkowitz <gl...@twistedmatrix.com> wrote:
> 
> 
>> On Dec 2, 2016, at 12:27 AM, Glyph Lefkowitz <gl...@twistedmatrix.com 
>> <mailto:gl...@twistedmatrix.com>> wrote:
>> 
>> 
>>> On Dec 2, 2016, at 12:19 AM, Glyph Lefkowitz <gl...@twistedmatrix.com 
>>> <mailto:gl...@twistedmatrix.com>> wrote:
>>> 
>>> 
>>>> On Dec 1, 2016, at 7:01 PM, Mark Williams <markrwilli...@gmail.com 
>>>> <mailto:markrwilli...@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

_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

Reply via email to