[Twisted-Python] INCOMPATIBLE CHANGE: removing dependency on PyCrypto

[Glyph Lefkowitz]

There are a few places within Conch which currently export PyCrypto objects as part of a public interface in Twisted.

These include:
twisted.conch.ssh.keys.Key.keyObject
twisted.conch.ssh.keys.objectType
I'm working on a ticket - https://twistedmatrix.com/trac/ticket/7413 - to eliminate the dependency on PyCrypto. Right now, in that branch, those objects are Cryptography key objects instead of PyCrypto key objects.

It is possible to preserve compatibility with keyObject, and we could deprecate and then remove objectType, with conditional dependencies on PyCrypto. But before I go through the effort there, I'm wondering if any users of conch actually care.

-glyph

[Adi Roiban]

Post by Glyph Lefkowitz
There are a few places within Conch which currently export PyCrypto objects
as part of a public interface in Twisted.
twisted.conch.ssh.keys.Key.keyObject
twisted.conch.ssh.keys.objectType
I'm working on a ticket - https://twistedmatrix.com/trac/ticket/7413 - to
eliminate the dependency on PyCrypto. Right now, in that branch, those
objects are Cryptography key objects instead of PyCrypto key objects.
It is possible to preserve compatibility with keyObject, and we could
deprecate and then remove objectType, with conditional dependencies on
PyCrypto. But before I go through the effort there, I'm wondering if any
users of conch actually care.

I care about conch and PyCrypto, but I prefer to update my own code now.
... rather than work on Twisted to support the compatibility and later
still work on my own code to make it work once compatibility is
removed.

I care about PyCrypto in the sense that I would like to see it gone.

In the same time, I am commited to Twisted deprecation policy and I am
happy to work on having a lean PyCrypto deprecation path.

------------

I see that objectType document that it accepts an
Crypto.PublicKey.pubkey.pubkey.... the latest patch ask for
cryptography.hazmat.primitives.interfaces

I would say that we can deprecate it now without changing the current
implementation and without replacing it with something.

Disclaimer: I have no idea why someone would need that function.

-----------

I think that twisted.conch.ssh.keys.Key.keyObject should also be
deprecated without being replace with something else... as such things
should stay private.


I can work at implementing a compatibility getter / setter for
keyObject while deprecating it in the same time.

-------------

I am happy to work or review any code which tries to remove PyCrypto
dependency and work on deprecating current PyCrytpo dependency.

--------

Many thanks for working on this!

[Brian Warner]

Post by Glyph Lefkowitz
I'm working on a ticket - https://twistedmatrix.com/trac/ticket/7413 -
to eliminate the dependency on PyCrypto. Right now, in that branch,
those objects are Cryptography key objects instead of PyCrypto key
objects.
It is /possible/ to preserve compatibility with keyObject, and we
could deprecate and then remove objectType, with conditional
dependencies on PyCrypto. But before I go through the effort there,
I'm wondering if any users of conch actually care.

Tahoe-LAFS uses Conch for two features: an SFTP frontend, and a
"manhole" repl-inside-the-app debugging interface. Neither uses the keys
objects.

Tahoe *does* indicate a dependency on PyCrypto because conch uses it,
and at the time we found it was more reliable to depend upon the
transitive closure of our subdependencies. We'll need to fix that (as we
bring our packaging up to modern standards).

Our SFTP frontend currently uses "from Crypto import Util" as a test to
see whether twisted.conch.ssh.filetransfer is going to work, so the
failure happens early and we can provide a better "your configuration
isn't going to work" error message. I'm guessing we imported pycrypto
rather than t.c.ssh directly to work around some old importer bug (I
vaguely remember something about half-complete imports causing confusion
in some old version of python). The code in question is ancient and
we'll need to update that when Twisted stops using PyCrypto.

But in general, yeah, we'd love to see PyCrypto go away. We currently
depend on pycryptopp (for AES/RSA/Ed25519), PyCrypto (via conch for SFTP
and manhole), and 'cryptography' (via pyOpenSSL via Foolscap for server
connections). Reducing the dependency graph by one node would be great.

cheers,
-Brian

[Zooko Wilcox-OHearn]

Yay for removing the dependency on PyCrypto! This would allow these
Twisted tickets to be closed:

* https://twistedmatrix.com/trac/ticket/4633# allow applications to
"bring their own crypto" to avoid the dependency of conch on PyCrypto

* https://twistedmatrix.com/trac/ticket/5577# Using manhole_tap ends
up requiring pycryto, even though only using telnet manhole

* https://twistedmatrix.com/trac/ticket/5805#
twisted.test.test_strcred fails on Python without pycrypto

And this Tahoe-LAFS tickets:

* https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2094# rebuild (if
necessary) PyCrypto eggs to use libgmp >= 5, to mitigate RSA timing
attack

Possibly also this Tahoe-LAFS ticket:

* https://tahoe-lafs.org/trac/tahoe-lafs/ticket/774# pycrypto package
is required for manhole

And it allows us to remove this warning label about potential timing
attacks against the SFTP connection:

https://tahoe-lafs.org/trac/tahoe-lafs/browser/trunk/docs/frontends/FTP-and-SFTP.rst#configuring-sftp-access

Regards,

Zooko

[Glyph Lefkowitz]

Post by Brian Warner

Post by Glyph Lefkowitz
I'm working on a ticket - https://twistedmatrix.com/trac/ticket/7413 -
to eliminate the dependency on PyCrypto. Right now, in that branch,
those objects are Cryptography key objects instead of PyCrypto key
objects.
It is /possible/ to preserve compatibility with keyObject, and we
could deprecate and then remove objectType, with conditional
dependencies on PyCrypto. But before I go through the effort there,
I'm wondering if any users of conch actually care.

Tahoe-LAFS uses Conch for two features: an SFTP frontend, and a
"manhole" repl-inside-the-app debugging interface. Neither uses the keys
objects.

Thanks for checking on that.

Post by Brian Warner
Tahoe *does* indicate a dependency on PyCrypto because conch uses it,
and at the time we found it was more reliable to depend upon the
transitive closure of our subdependencies. We'll need to fix that (as we
bring our packaging up to modern standards).

I am definitely feeling the pain with you on that one :).

Post by Brian Warner
Our SFTP frontend currently uses "from Crypto import Util" as a test to
see whether twisted.conch.ssh.filetransfer is going to work, so the
failure happens early and we can provide a better "your configuration
isn't going to work" error message. I'm guessing we imported pycrypto
rather than t.c.ssh directly to work around some old importer bug (I
vaguely remember something about half-complete imports causing confusion
in some old version of python). The code in question is ancient and
we'll need to update that when Twisted stops using PyCrypto.

That interpreter bug was something Twisted had workarounds for too, and I am fairly certain it was fixed in python *2.5*, so it's been QUITE a while since it was a real concern :). If you can, I'd recommend fixing that conditional import now rather than waiting for a Twisted release that has switched backends.

Post by Brian Warner
But in general, yeah, we'd love to see PyCrypto go away. We currently
depend on pycryptopp (for AES/RSA/Ed25519), PyCrypto (via conch for SFTP
and manhole), and 'cryptography' (via pyOpenSSL via Foolscap for server
connections). Reducing the dependency graph by one node would be great.

Excellent.

It's worth noting that Cryptography now provides good support for AES and RSA (pretty sure this was not true when I was last discussing Cryptography in the context of Tahoe), and has plans for Ed25519 - https://github.com/pyca/cryptography/issues/856 - so you may be able to reduce this even further in the future :).

-glyph

[Zooko Wilcox-O'Hearn]

Post by Glyph Lefkowitz
It's worth noting that Cryptography now provides good support for AES and RSA (pretty sure this was not true when I was last discussing Cryptography in the context of Tahoe), and has plans for Ed25519 - https://github.com/pyca/cryptography/issues/856 - so you may be able to reduce this even further in the future :).

FWIW I would be uncomfortable relying on https://cryptography.io for
Tahoe-LAFS's purposes. I expect to continue to rely on
https://pypi.python.org/pypi/pycryptopp/ for that for the forseeable
future.

Regards,

Zooko

[Ray Cote]

Post by Glyph Lefkowitz
here are a few places within Conch which currently export PyCrypto objects
as part of a public interface in Twisted.
- twisted.conch.ssh.keys.Key.keyObject
- twisted.conch.ssh.keys.objectType
I'm working on a ticket - https://twistedmatrix.com/trac/ticket/7413 - to
eliminate the dependency on PyCrypto. Right now, in that branch, those
objects are Cryptography key objects instead of PyCrypto key objects.
It is *possible* to preserve compatibility with keyObject, and we could
deprecate and then remove objectType, with conditional dependencies on
PyCrypto. But before I go through the effort there, I'm wondering if any
users of conch actually care.

We have a custom SFTP server based on conch.
In reviewing the code, it looks like I don’t care about the change (aside
from appreciating one less dependency).
In reviewing our code, the only key references are as follows:

from twisted.conch.ssh.factory import SSHFactory
public_key = getRSAKey('id_rsa.pub')
private_key = getRSAKey(‘id_rsa')factory = SSHFactory()
factory.publicKeys = {'ssh-rsa': public_key}
factory.privateKeys = {'ssh-rsa': private_key}
—Ray