[#8325] Adding in ECDH for SSH. #432
Conversation
|
Well there's no conflicts. That's progress. :) Not too surprised it failed, as mentioned before it's known that the mismatchedHostKey test fails. Looking at the doc of the test it seems that it's expecting a certain host key in known_hosts but is getting a different one. Why this is I don't know, in my testing the clients make use of known_hosts fine, but as the test was running I got an idea of what could be happening, so I'll take a look at it. The other failure was because I didn't have a top file in my manifest. I don't know what this means. Is there another file I need to commit or add in addition to my changes? |
|
I guess this is what happens when you forget to verify the fingerprint of the host key. Working on a patch. |
…twisted into 8370-the0id-sshECH-redux
…his will be needed when I add in ECDSA.
Current coverage is 90.65% (diff: 100%)@@ trunk #432 diff @@
==========================================
Files 837 814 -23
Lines 146069 145323 -746
Methods 0 0
Messages 0 0
Branches 12912 12827 -85
==========================================
- Hits 133104 131746 -1358
- Misses 10725 11306 +581
- Partials 2240 2271 +31
|
|
It appears I have too many diffs? Am I reading the condecov report correctly? What needs to be done about that? |
…twisted into 8370-the0id-sshECH-redux
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/env python3.3 | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
This change is unrelated to this branch; would you mind making it in a separate PR?
There was a problem hiding this comment.
Yeah that accidentally got pushed, I wasn't sure if it's something you'd want or not. I'll take it out. :)
…twisted into 8370-the0id-sshECH-redux
…, because it does now. Updating the key string in generateECDSAkey because this version stores the full keytype in the _curvetable. Changing the expected hash value of the ECDSA test key because this verstion stores the key in a slightly different format.
…anged because I think strictly adhering to the rules makes some code less readable.
…he hope that it would provide better detail when things go wrong, but when writing the tests I didn't see a way to reach those exceptions because of other checks that would catch the problem earlier.
…n ECDSA key. This should hopefully complete coverage for knownhosts.py.
…is in bytes because data() will never return a key as bytes.
…peased, and I believe this is the best sacrifice.
I think I'm learning how to Git.
It seems that some of the upstream commits created a few more errors in my code that I needed to address. Most of them involved putting b in front of strings, but I did need to make a possibly significant change on line 1282 and 1625 of transport.py.
I changed
sharedSecret = MP(int(self.ecPriv.exchange(ec.ECDH(), self.theirECPub).encode('hex')), 16))
to
sharedSecret = MP(int(binascii.hexlify(self.ecPriv.exchange(ec.ECDH(), self.theirECPub)), 16))
to work with python 3.
I still feel like this transformation could be done better, but it's the best I could come up with.
Also as I mentioned before, but will reiterate this code does not pass all trials. It still fails on the twisted.conch.test.test_endpoints.NewConnectionTests.test_mismatchedHostKey check.
While making changes to the code I found that it appears to have to do with the addition of or _kex.isEllipticCurve(kexAlgorithm)] to factory.py. Why I'm not exactly sure, and I'm not sure how to fix it, but hasn't caused any errors when I use Twisted as a client or a server.
I can add a ticket for this to be fixed.