Opened 7 years ago

Closed 12 months ago

#4633 enhancement closed duplicate (duplicate)

allow applications to "bring their own crypto" to avoid the dependency of conch on PyCrypto

Reported by: davidsarah Owned by:
Priority: normal Milestone:
Component: conch Keywords: pycrypto
Cc: david-sarah@…, zooko@…, ali, Adi Roiban, davidsarah Branch: branches/pycryptopp-4633
branch-diff, diff-cov, branch-cov, buildbot
Author: z3p

Description

The dependency of Twisted conch on PyCrypto is problematic for some applications that want to use conch: it introduces more C code to compile, which is more code that may have bugs and cause packaging problems (especially for Windows).

Since conch only depends (since the fix to #3849) on 3DES, RSA and DSA from PyCrypto, an application might be able to provide those itself, or from another library. Tahoe-LAFS is in this position: it depends in any case on pycryptopp (a Python binding of Crypto++), which has well-debugged and tested implementations of 3DES, RSA and DSA.

The simplest way to support this would be to allow the app to provide classes that are API-compatible with the PyCrypto classes, so that the only code that needs to change in twisted is a few import statements.

If the general idea meets with approval, I'll prepare a patch.

Change History (37)

comment:1 in reply to:  description Changed 7 years ago by davidsarah

Replying to davidsarah:

Tahoe-LAFS is in this position: it depends in any case on pycryptopp (a Python binding of Crypto++), which has well-debugged and tested implementations of 3DES, RSA and DSA.

Clarification: Crypto++ has has well-debugged and tested implementations of 3DES, RSA and DSA. pycryptopp doesn't currently have bindings for 3DES and DSA, but we would fix that.

comment:2 Changed 7 years ago by Jean-Paul Calderone

Something to consider here is that Conch may wish to widen its crypto requirements at some point. At the moment, this is fairly straightforward, particularly if the new requirements are already satisfied by PyCrypto. If the crypto module's interface becomes part of Conch's public interface, this gets more complicated.

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

Of course, also worth consideration is the number of times PyCrypto has changed in a way which has broken Conch.

comment:4 Changed 7 years ago by davidsarah

It seems that PyCrypto 2.2 has again changed in a way that breaks conch: https://bugs.launchpad.net/pycrypto/+bug/620253/ (Looking at the diff there, it is a trivial import error that could have been caught by simply running pyflakes or some other linter.)

I'm busy with work for the next couple of weeks, but after that I'll start work on a patch. (Please don't let that stop anyone else working on this, though.) I'll try to make it general enough that it won't fall apart if conch needs to widen its use of crypto. Presumably, if it did that then the new algorithms would be optional?

comment:5 Changed 7 years ago by zooko

Cc: zooko@… added

comment:6 Changed 7 years ago by ali

Cc: ali added

comment:7 Changed 7 years ago by zooko

related ticket: http://tahoe-lafs.org/trac/tahoe-lafs/ticket/1192 (warn users not to rely on PyCrypto)

comment:8 Changed 7 years ago by Adi Roiban

Cc: Adi Roiban added

Hi,

I like the idea.

Beside the fact that PyCrypto add extra dependencies that requires to be compiled on each system, Twisted already depends on PyOpenSSL which is a wrapper for OpenSSL.

OpenSSL already implements the required ciphers and DSA/RSA http://www.openssl.org/docs/crypto/crypto.html OpenSSL is well tested and was audited many times and got the FIPS-140 validation.

Right now PyOpenSSL does not provide access to OpenSSL ciphers. It would make sense to me to extend PyOpenSSL so that it will provide access to OpenSSL ciphers.

Maybe we should add Jean-Paul Calderone to the CC list of this bug as he is the current PyOpenSSL maintainer.

PS: I am new to twisted and maybe I am talking non-senses...

comment:9 Changed 7 years ago by Jean-Paul Calderone

I wonder if davidsarah would be as satisfied if Conch switched to (a hypothetical future version of) pyOpenSSL and stopped depending on PyCrypto. This seems like a simpler change to me, since it doesn't involve arbitrary combinations of Twisted and crypto, and it doesn't raise the same concerns about future expansion, as Twisted would still just be using a particular crypto library.

As far as how Conch actually depends on PyCrypto right now, don't overlook the Crypto.number module which Conch uses for some fast number/string conversions, and the XOR "cipher" which is needed to implement some SSH MAC stuff.

comment:10 Changed 7 years ago by <automation>

Owner: z3p deleted

comment:11 in reply to:  9 Changed 6 years ago by davidsarah

Cc: davidsarah added

Replying to exarkun:

I wonder if davidsarah would be as satisfied if Conch switched to (a hypothetical future version of) pyOpenSSL and stopped depending on PyCrypto.

For Tahoe-LAFS, that would be an improvement since we already depend on pyOpenSSL, so it would reduce the number of crypto libraries we depend on from three to two. Some of the other Tahoe-LAFS developers have expressed reservations about OpenSSL, and I'm not a big fan of it myself (it doesn't have an especially good security bug record), but it would still be better than PyCrypto.

comment:12 Changed 6 years ago by zooko

https://tahoe-lafs.org/pipermail/tahoe-dev/2011-October/006772.html

I'm extremely annoyed at the fact that we depend on PyCrypto, which I regard as too sloppily-written to be secure, and also as too sloppily-written to build and install on all the platforms we support. For a while there the pain had lessened because there were no new releases of PyCrypto, and people had worked-around all of the known problems in the most recent release. But unfortunately they are apparently maintaining PyCrypto again, so now it is going to resume interfering with our work.

comment:13 Changed 6 years ago by zooko

Here is the bug in the new release of PyCrypto which apparently prevents it from building with setuptools, zc.buildout, or virtualenv:

https://bugs.launchpad.net/pycrypto/+bug/881130

comment:15 Changed 5 years ago by z3p

While I think "pluggable crypto" is too complicated to be worth the effort, I would not be opposed to switching away from PyCrypto. However, that means we need a new library to use.

Here's a list of the algorithms we're using from PyCrypto:

  • DES3
  • AES
  • RSA
  • DSA
  • XOR (probably implementable on our own?)
  • _fastmath stuff, but some of that is inside Twisted

and we're adding ECDSA support as part of #5350. Libraries mentioned in this ticket:

  • PyOpenSSL: OpenSSL supports the protocols(?), but the bindings don't expose them.
  • pycryptopp: only supports AES and a public-key protocol we don't use. Currently supports RSA/ECDSA, but they're deprecated.
  • KeyCzar: only supports AES, and doesn't appear to expose anything low-level enough for us to use.

Did I miss any libraries?

comment:16 Changed 5 years ago by Glyph

One point which hasn't been mentioned yet but may be implicit in some discussants' understanding here is that PyCrypto won't accept our patches because Twisted is - largely, if not entirely - a US-based project, and the author of PyCrypto understood the nature of the export regulations on crypto as restricting his ability to accept code from residents of the US.

The wayback machine confirms that I'm not hallucinating about this, as its most recent snapshot of the PyCrypto page (from november of 2010) includes this declaration. However, the PyCrypto site today no longer includes that text, and neither do the explicit submission requirements now on the site. So perhaps it's a bit easier to just get PyCrypto fixed than we've been thinking.

At the very least, whenever PyCrypto breaks compatibility in the future, there should be an accompanying launchpad bug filed against PyCrypto so the maintainer knows there's a problem. A brief glimpse at the previous compatibility-breaking tickets indicates to me that we haven't been doing that.

comment:17 Changed 5 years ago by Glyph

Also, I should note that there are some license considerations for alternate libraries:

  • PyCrypto: public domain/Python license. Awesome! (Like, for real public domain, with affidavits and everything.)
  • PyOpenSSL: I'd like for Twisted to depend less on OpenSSL in general, for a couple of reasons. OpenSSL's slightly weird license is one of them, but there's also the issue of monoculture. Despite PyCrypto's potential issues, it at least has different issues than the wildly popular PyOpenSSL, which also underlies OpenSSH. It's nice to know that if there's a problem found in OpenSSL it won't necessarily affect Conch's crypto.
  • PyCryptoPP: crypto++ itself has a funky license which is fairly permissive but wholly original and does weird stuff where the individual components of the library are all public domain individually but copyrighted with restrictions as a whole work, which I am pretty sure would give some lawyers heartburn. The PyCryptoPP wrapper layer is optionally under two licenses: GPL2 (NOT LGPL2, so unacceptable for any required dependency of Twisted), and TGGPL (not written by a lawyer as far as I know, not verified as open source by the OSI, kind of an interesting idea hypothetically, but as with any new license, a nightmare of legal ambiguities and differing interpretations)
  • KeyCzar: Apache2, which is a few extra restrictions on top of MIT, but then, so's PyOpenSSL and we already depend on that, so meh. According to the readme though, this actually depends on PyCrypto anyway so what's the point?

A pluggable crypto API would allow us to sidestep these issues by allowing the crypto providers to (potentially) become derivative works of Twisted rather than vice-versa, by defining an interface that they can each implement. Or, at least (c.f. qt4reactor) allowing the plug-in that lets Twisted work with a particular crypto API be distributed separately.

I certainly wouldn't say that it's worth it for that alone, but having spent some time recently weeding through testing dependencies in conch looking at the way that PyCrypto gets imported all over the place, it would be nice to have a more sensibly controlled choke-point through which crypto API accesses were performed. This could probably be accomplished with substantially less effort than full aribtrary pluggability though.

comment:18 Changed 5 years ago by Glyph

Another crypto library to possibly consider, which I believe is open source: Common Crypto, which I believe has a fairly permissive license?

comment:19 Changed 5 years ago by Adi Roiban

Maybe this table is a bit old, but here is a comparison for Python cryptography modules (date 2010): http://www.scribd.com/doc/48627853/Python-Crypto

m2crypto looks like a decent wrapper for OpenSSL.


If Jean-Paul is interested in having the ciphers exposed in pyOpenSSL, I can try to see if I am able to produce the required patches for pyOpenSSL.

Cheers, Adi

comment:20 Changed 5 years ago by zooko

Botan is a C++ library that comes with Python wrappers maintained by the author.

comment:21 Changed 5 years ago by zooko

I refuse to believe that anyone actually requires triple-DES.

comment:23 Changed 5 years ago by davidsarah

PyCryptoPP: crypto++ itself has a funky license which is fairly permissive but wholly original and does weird stuff where the individual components of the library are all public domain individually but copyrighted with restrictions as a whole work, which I am pretty sure would give some lawyers heartburn. The PyCryptoPP wrapper layer is optionally under two licenses: GPL2 (NOT LGPL2, so unacceptable for any required dependency of Twisted), and TGGPL (not written by a lawyer as far as I know, not verified as open source by the OSI, kind of an interesting idea hypothetically, but as with any new license, a nightmare of legal ambiguities and differing interpretations)

The pycryptopp wrappers could probably be relicensed as MIT or BSD. (I asked zooko and he said, "Please do suggest that such a thing is possible. Any 'permissive' licence is probably just as good as any other for this.")

The Crypto++ license looks like BSD with advertising clause, plus two clauses that state people's responsibility to obtain export and patent licenses. Is that a problem?

comment:24 Changed 5 years ago by davidsarah

Oh, I didn't read clause 2 of the Crypto++ license carefully enough when comparing it to BSD; it is a little funky, but harmless I think (I interpret it as only affecting direct modifications to Crypto++ code):

  1. Users of this software agree that any modification or extension they provide to Wei Dai will be considered public domain and not copyrighted unless it includes an explicit copyright notice.

comment:25 in reply to:  23 Changed 5 years ago by Glyph

Replying to davidsarah:

PyCryptoPP: crypto++ itself has a funky license which is fairly permissive but wholly original and does weird stuff where the individual components of the library are all public domain individually but copyrighted with restrictions as a whole work, which I am pretty sure would give some lawyers heartburn. The PyCryptoPP wrapper layer is optionally under two licenses: GPL2 (NOT LGPL2, so unacceptable for any required dependency of Twisted), and TGGPL (not written by a lawyer as far as I know, not verified as open source by the OSI, kind of an interesting idea hypothetically, but as with any new license, a nightmare of legal ambiguities and differing interpretations)

The pycryptopp wrappers could probably be relicensed as MIT or BSD. (I asked zooko and he said, "Please do suggest that such a thing is possible. Any 'permissive' licence is probably just as good as any other for this.")

That's very gracious of Zooko, thanks for asking.

The Crypto++ license looks like BSD with advertising clause, plus two clauses that state people's responsibility to obtain export and patent licenses. Is that a problem? ... Oh, I didn't read clause 2 of the Crypto++ license carefully enough when comparing it to BSD; it is a little funky ...

Yes, it is a little funky. The fact that it's not exactly BSD or MIT or Apache is the problem; probably the interpretation is equivalent, but I'm not a lawyer and anywhere that Twisted might be used it will have to be re-evaluated by their lawyers. Every savvy large company has at least one lawyer dedicated to this process; many less-savvy large companies (including some where Twisted is used) just have a blanket policy like "we accept only code under the X, Y, and Z license, no others will ever be considered".

And hey, maybe there are some legitimate problems too. What does "modification or extension" mean? Those terms aren't defined by copyright law as far as I know. Could he mean every derivative work? What does "provide to Wei Dai" mean? Does that mean making it available in any place where it would be retrievable by that individual? I can see why he would want to do this, but the license is the wrong place to put it.

comment:26 Changed 5 years ago by zooko

I asked Wei Dai and he says he'll release Crypto++ under a more common, OSI-recognized, permissive licence:

http://www.mail-archive.com/cryptopp-users@googlegroups.com/msg06563.html

comment:27 in reply to:  26 Changed 5 years ago by Glyph

Replying to zooko:

I asked Wei Dai and he says he'll release Crypto++ under a more common, OSI-recognized, permissive licence:

http://www.mail-archive.com/cryptopp-users@googlegroups.com/msg06563.html

This is excellent news. Thanks for asking.

Just one note (since I'm not on that mailing list): there's really no need to put the "only copyrighted as a compilation" thing into the license itself. It can be an informative note somewhere else in the code or on the website, since the license by definition only applies to the copyrighted material. (Putting anything into the license itself can have weird knock-on effects. For example, let's say someone wants to release a crypto++ enhancement that is open source, but cannot be not public domain for some reason. Now, their enhancement is copyrighted as an individual file and as a compilation. But one of the requirements of the license is to distribute the license verbatim. Can they do that? One thing is for sure: they'll have to ask a lawyer.)

comment:28 Changed 5 years ago by zooko

http://www.mail-archive.com/cryptopp-users@googlegroups.com/msg06589.html

Wei Dai writes:

""" Thanks for everyone's comments. Since there weren't any objections, please consider Crypto++ Library version 5.6.1 to be dual licensed under the current Crypto++ license and the Boost Software License. The next version will probably be licensed under BSL only, unless someone has an argument against that. """

Is that licensing sufficient for Twisted's purposes?

comment:29 Changed 5 years ago by z3p

Author: z3p
Branch: branches/pycryptopp-4633

(In [35139]) Branching to 'pycryptopp-4633'

comment:30 Changed 5 years ago by Jean-Paul Calderone

Does pycryptopp work on PyPy?

comment:31 in reply to:  28 ; Changed 5 years ago by Itamar Turner-Trauring

Replying to zooko:

Is that licensing sufficient for Twisted's purposes?

It seems like a fine license, yeah. How about pycrytopp's license, will that be changed?

comment:32 in reply to:  30 Changed 5 years ago by zooko

Replying to exarkun:

Does pycryptopp work on PyPy?

I haven't tried. Pycryptopp is written in the CPython API, so if it works on PyPy it is by relying on PyPy's "cpyext" feature. I would be keen to see that work! Try it and report to tahoe-dev@… mailing list or open a ticket on https://tahoe-lafs.org/trac/pycryptopp . I would be interested in contributing patches to pycryptopp and/or the PyPy to make it work.

Also, someone who wants this to happen could help by volunteering to administer a buildslave to run the pycryptopp unit tests on PyPy and join this buildslave herd: https://tahoe-lafs.org/buildbot-pycryptopp/waterfall

comment:33 in reply to:  31 ; Changed 5 years ago by zooko

I've updated pycryptopp to be available under your choice of GPLv2+, TGPPLv1+, MIT ¹, or Simple Permissive Licence ².

¹ https://github.com/tahoe-lafs/pycryptopp/blob/82eca33b7234483836f8c268b1cc1c75a825d999/COPYING.MIT.txt

² https://github.com/tahoe-lafs/pycryptopp/blob/82eca33b7234483836f8c268b1cc1c75a825d999/COPYING.SPL.txt

Since Twisted and Pycryptopp now have the MIT licence in common, hopefully there will be no further licence-based reason to hesitate to combine then.

Note that pycryptopp does not currently implement RSA encryption -- only RSA digital signatures, and using a padding scheme that probably isn't the one that ssh requires. Also it doesn't currently implement 3DES.

comment:34 in reply to:  33 Changed 5 years ago by Glyph

Replying to zooko:

I've updated pycryptopp to be available under your choice of GPLv2+, TGPPLv1+, MIT ¹, or Simple Permissive Licence ².

¹ https://github.com/tahoe-lafs/pycryptopp/blob/82eca33b7234483836f8c268b1cc1c75a825d999/COPYING.MIT.txt

² https://github.com/tahoe-lafs/pycryptopp/blob/82eca33b7234483836f8c268b1cc1c75a825d999/COPYING.SPL.txt

Since Twisted and Pycryptopp now have the MIT licence in common, hopefully there will be no further licence-based reason to hesitate to combine then.

This seems to me to be sufficient, but... wouldn't just switching to straight MIT for everybody be functionally the same thing expressed in a lot fewer lines of potentially problematic licensing text?

Note that pycryptopp does not currently implement RSA encryption -- only RSA digital signatures, and using a padding scheme that probably isn't the one that ssh requires. Also it doesn't currently implement 3DES.

So now that the license problem is resolved, the "only" issue is that it doesn't cover 2/3 of our requirements, as stated by the ticket description ;-).

comment:35 Changed 3 years ago by zooko

related: #7413

comment:36 Changed 3 years ago by davidsarah

It seems like #7413 completely supercedes this bug. (Having already bitten the bullet by accepting the cryptography as a dependency, there's no point in using something different for conch.)

comment:37 Changed 12 months ago by Craig Rodrigues

Resolution: duplicate
Status: newclosed

Duplicate of ticket:7413

Note: See TracTickets for help on using tickets.