Opened 10 years ago

Closed 10 years ago

#4639 defect closed worksforme (worksforme)

PyCrypto compiled without libgmp causes test failures in twisted.conch

Reported by: Glyph Owned by:
Priority: high Milestone:
Component: conch Keywords:
Cc: Branch:
Author:

Description

For example, here are some test failures from an Ubuntu Lucid machine:

glyph@kurma:~/Scratch/Test$ python -c 'import Crypto; print Crypto.__version__'; trial twisted.conch.test.test_keys.KeyTestCase.test_toLSH
2.0.1
twisted.conch.test.test_keys
  KeyTestCase
    test_toLSH ...                                                         [OK]

-------------------------------------------------------------------------------
Ran 1 tests in 0.004s

PASSED (successes=1)
glyph@kurma:~/Scratch/Test$ pip install -I pycrypto > /dev/null
glyph@kurma:~/Scratch/Test$ python -c 'import Crypto; print Crypto.__version__'; trial twisted.conch.test.test_keys.KeyTestCase.test_toLSH
2.2
twisted.conch.test.test_keys
  KeyTestCase
    test_toLSH ...                                                      [ERROR]

===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/glyph/Projects/Twisted/trunk/twisted/conch/test/test_keys.py", line 424, in test_toLSH
    self.assertEquals(key.toString('lsh'), keydata.privateRSA_lsh)
  File "/home/glyph/Projects/Twisted/trunk/twisted/conch/ssh/keys.py", line 581, in toString
    return method()
  File "/home/glyph/Projects/Twisted/trunk/twisted/conch/ssh/keys.py", line 661, in _toString_LSH
    ['c', common.MP(data['u'])[4:]]]]])
exceptions.KeyError: 'u'

twisted.conch.test.test_keys.KeyTestCase.test_toLSH
-------------------------------------------------------------------------------
Ran 1 tests in 0.013s

FAILED (errors=1)

Change History (11)

comment:1 Changed 10 years ago by Glyph

If someone more SSH-knowledgeable would attach an explanation of the nature of these failures, I'd appreciate it, as I'd like to fix the problem but "KeyError: 'u'" doesn't do much for me (and neither does data['u']).

comment:2 Changed 10 years ago by Glyph

Priority: normalhigh
Type: enhancementdefect

comment:3 Changed 10 years ago by Jean-Paul Calderone

On Karmic with PyCrypto 2.2 I wasn't able to reproduce this:

exarkun@boson:/tmp$ python -c 'import Crypto; print Crypto.__version__'
2.2
exarkun@boson:/tmp$ trial twisted.conch.test.test_keys.KeyTestCase.test_toLSH
twisted.conch.test.test_keys
  KeyTestCase
    test_toLSH ...                                                         [OK]

-------------------------------------------------------------------------------
Ran 1 tests in 0.004s

PASSED (successes=1)
exarkun@boson:/tmp$ 

comment:4 Changed 10 years ago by Glyph

I don't have a Karmic machine handy, but I've reproduced this on multiple MacOS X Snow Leopard and Ubuntu Lucid machines. It happens with PyCrypto 2.1 as well as 2.2.

comment:5 Changed 10 years ago by Glyph

Summary: Versions of PyCrypto greater than 2.0.1 cause test failures in twisted.conchPyCrypto compiled without libgmp causes test failures in twisted.conch

OK, we tracked down the problems. It's the non-GMP implementation of some things in pycrypto.

comment:7 Changed 10 years ago by Glyph

Keywords: review added
Owner: z3p deleted

The following patch fixes the tests:

Index: twisted/conch/ssh/keys.py
===================================================================
--- twisted/conch/ssh/keys.py	(revision 29940)
+++ twisted/conch/ssh/keys.py	(working copy)
@@ -489,6 +489,9 @@
             value = getattr(self.keyObject, name, None)
             if value is not None:
                 keyData[name] = value
+        if self.type() == 'RSA':
+            if 'p' in keyData and 'q' in keyData and 'u' not in keyData:
+                keyData['u'] = 1 / keyData['p'] % keyData['q']
         return keyData

what say we apply it?

NEWS message: "Work around a bug in PyCrypto https://bugs.launchpad.net/pycrypto/+bug/622879"

comment:8 Changed 10 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Glyph

This fix doesn't look right. Apparently Conch's key tests don't exercise the crypto parts of the keys. Here's an example of how the above implementation differs from the PyCrypto _fastmath implementation.

With the patch applied:

$ python -c '
import sys
sys.modules["Crypto.PublicKey._fastmath"] = None
from twisted.conch.ssh.keys import Key
from Crypto.PublicKey.RSA import construct
k = Key(construct((115275351804388053458541050948848139783336596202263902055454559652088006187483627974753643424894109351045969114938053282529987873848584092202027179058888569842367308089572272454873302454640046926374964395939018122476452688883797099321821682378558931185190240603923891176582715884269499585794141055507266920263, 65537L, 47230934382889482452183412424713829584237213806295532264111354437613365612498441771489156251358112582118289861823345693920918479463237255959363928957524380119173566772973907720301655113709193586681728451676899903113308656511657244296929841536377222637924553573548632626453525316690554607781623572983412710241, 10141845831285486840400809110041804080636258493969213587784113974481213645045593886840566270090163179452047396758708036149076219889356051597721495097125539, 11366308828003236668823563314594157304734539215937765613976440203910716313203009631139117161698164061232934984724586828829258520752888383081688599764059917))).data()
print k["u"]
'
0

This shows a computed value for u of 0. The sys.modules line makes sure _fastmath won't be available so the new code above will have to compute u. The parameters passed to generate are valid for an actual key (unlike the bogus values used in the Key unit tests).

Compare this to the correct (presumably) results when _fastmath is used instead:

exarkun@boson:~$ python -c '
import sys
#sys.modules["Crypto.PublicKey._fastmath"] = None
from twisted.conch.ssh.keys import Key
from Crypto.PublicKey.RSA import construct
k = Key(construct((115275351804388053458541050948848139783336596202263902055454559652088006187483627974753643424894109351045969114938053282529987873848584092202027179058888569842367308089572272454873302454640046926374964395939018122476452688883797099321821682378558931185190240603923891176582715884269499585794141055507266920263, 65537L, 47230934382889482452183412424713829584237213806295532264111354437613365612498441771489156251358112582118289861823345693920918479463237255959363928957524380119173566772973907720301655113709193586681728451676899903113308656511657244296929841536377222637924553573548632626453525316690554607781623572983412710241, 10141845831285486840400809110041804080636258493969213587784113974481213645045593886840566270090163179452047396758708036149076219889356051597721495097125539, 11366308828003236668823563314594157304734539215937765613976440203910716313203009631139117161698164061232934984724586828829258520752888383081688599764059917))).data()
print k["u"]
'
10904781574998398681800563342679010232755436849694622639495214563295467839908656129167913974623837350114189816556719537879858334105207452532599541254241636

If someone knows how u is actually supposed to be computed, maybe we can do something similar. I can't figure it out though.

comment:9 Changed 10 years ago by therve

The fix has been released in PyCrypto 2.3.

comment:10 Changed 10 years ago by Glyph

Resolution: worksforme
Status: newclosed

PyCrypto 2.3 fixes this for me. Hooray for responsive maintainers!

comment:11 Changed 9 years ago by <automation>

Owner: Glyph deleted
Note: See TracTickets for help on using tickets.