Nixpkgs: pythonPackages.pycrypto: deprecate in favor of pycryptodome

[FRidh]

Issue description

pycrypto is a package that hasn't been maintained for a long time now. A maintained fork and drop-in replacement is pycryptodome. Therefore, we should get rid of pycrypto and use pycryptodome.

Packages typically have pycrypto as install_requires either in setup.py or requirements.txt. Packages that have this should be patched.

https://pycryptodome.readthedocs.io/en/latest/src/introduction.html

Technical details

This applies to both 16.09 and master.

[FRidh]

Can't we fake the package name or introduce a dummy package named pycrypto which pulls in pycryptodome and in turn the Crypto python module?

@fpletz Good idea. Working on that now

[FRidh]

The dummy is now in master and not in stable. I suggest we wait and see before we backport this dummy, since most dependents don't have any test suites.

[domenkozar]

This breaks https://github.com/NixOS/nixpkgs/issues/21730

[aszlig]

This also breaks Gajim with OTR and E2E.

[aszlig]

Also, after digging a bit on what exactly went wrong, it seems that pycrptodome is not suitable as a drop-in replacement and it has a lot of backwards-incompatible changes, see for example https://github.com/Legrandin/pycryptodome/blob/57548f05eec372932f54b552fecf0f0f1d54d941/Changelog.rst#breaks-in-compatibility.

For example this is the exception that's now thrown with the gotr plugin:

python traceback Traceback (most recent call last): File "/nix/store/i64sgga5635s5y9jmrwdbgybsvh182l0-gajim-0.16.6/share/gajim/src/common/ged.py", line 93, in raise_event if handler(*args, **kwargs): File "/home/aszlig/.local/share/gajim/plugins/gotr/otrmodule.py", line 562, in handle_incoming_msg appdata={'thread':event.session.thread_id if event.session else None}) File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 196, in receiveMessage self.handleQuery(message, appdata=appdata) File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 390, in handleQuery self.authStartV2(appdata=appdata) File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 398, in authStartV2 self.crypto.startAKE(appdata=appdata) File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/crypt.py", line 259, in startAKE outMsg = self.ake.startAKE() File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/crypt.py", line 352, in startAKE self.encgx = AESCTR(self.r).encrypt(gxmpi) File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/compatcrypto/pycrypto.py", line 52, in AESCTR return Cipher.AES.new(key, Cipher.AES.MODE_CTR, counter=counter) File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/AES.py", line 260, in new return _create_cipher(sys.modules[__name__], key, mode, *args, **kwargs) File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/__init__.py", line 130, in _create_cipher return modes[mode](factory, **kwargs) File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/_mode_ctr.py", line 319, in _create_ctr_cipher _counter = dict(counter) TypeError: 'Counter' object is not iterable

[FRidh]

@aszlig gajim master seems to have changed to pycryptodome and cryptography
https://dev.gajim.org/gajim/gajim/blob/master/requirements.txt

Quite frankly, I wouldn't trust any package that has been using an unsafe library for this long and still hasn't bothered to release a version that is using a safe solution.

[aszlig]

@FRidh: Mhm, I'm giving it a shot trying to backport the changes from master to use pycryptodome.

[Mic92]

@aszlig some test for python-axolotl also break with pycryptodome (which is used by the gajim omemo plugin).

[aszlig]

Yeah, which also is needed for Gajim and OMEMO. Also, backporting this is not so easy because first of all cherry-picking the right commits from master is quite difficult because both implementations use the same module names.

Aside from that, there are plugins that are relying on pycrypto. These however are downloaded from the gajim plugin repository so we don't have control over them. So in the long term we might want to package the plugins as well.

So, I'm not going to backport this because this is going to take a lot of time and I have more pressing issues to attend to at the moment.

But that aside: @FRidh: What exactly are the security issues of pycrypto? And what about patching those issues for pycrypto until every package that depends on it has switched over to pycryptodome?

[FRidh]

@aszlig pycrypto was on the list in https://github.com/NixOS/nixpkgs/issues/21642
The issue https://lwn.net/Vulnerabilities/710478/

As I wrote on the roundup issue:

python-crypto or pycrypto isn't being maintained anymore. According to this comment the issue has been resolved on master already 3 years ago, but it hasn't been included in any release.
A maintained fork of pycrypto is pycryptodome. We should see if we can replace pycrypto with pycryptodome.

We didn't backport the pycrypto dummy, so its just on unstable where some packages are broken. In my opinion packages that still depend on pycrypto by the time we release 17.03 should be marked broken.

[FRidh]

But that aside: @FRidh: What exactly are the security issues of pycrypto? And what about patching those issues for pycrypto until every package that depends on it has switched over to pycryptodome?

If someone feels like maintaining that package, feel free.

[fadenb]

Commit 3b7193604f8b05dcb08afab8801203825bd966c1 breaks otr in poezio as well.

[FRidh]

One solution is to add a pycrypto-original which packages can choose to use, This package would be marked as broken or insecure https://github.com/NixOS/nixpkgs/pull/22890. That means your users have to explicitly allow using pycrypto-original in order to use your package.

[grahamc]

pycrypto-original can be marked as insecure now.

[FRidh]

@grahamc we don't have a pycrypto-original yet. Right now pycrypto is a dummy that installs the mostly drop-in replacement pycryptodome. In certain cases this works, in others it doesn't. I proposed keeping the dummy, but adding a new pycrypto-original that would me marked insecure.

[grahamc]

Sorry, I should have said more. I meant to say now that the PR is merged, this is a workable idea.

[fadenb]

How should we proceed with this?

I was unable to find any non patched security issue with pycrypto (the last known security issue I found was patched by @fpletz in fe9373460c08c83449657b22c026c806bec92d21).

I created a pycrypto-original (in our nixpkgs fork) but I feel a bit uneasy marking it as insecure even there are no known security issues: https://github.com/mayflower/nixpkgs/blob/master/pkgs/development/python-modules/pycrypto-original/default.nix

[FRidh]

@grahamc what is the purpose of the new "insecure" feature. Is it to only mark packages as insecure that are known to be insecure, with the claim being backed somehow. Or could we also use it for packages that we do not trust, because its a package for security but isn't maintained?

[FRidh]

Task left to do:

• [ ] check for every package whether they expect pycypto or can use pycryptodome instead of our dummy pycrypto package.

[parthshah1509]

Also, after digging a bit on what exactly went wrong, it seems that pycrptodome is not suitable as a drop-in replacement and it has a lot of backwards-incompatible changes, see for example https://github.com/Legrandin/pycryptodome/blob/57548f05eec372932f54b552fecf0f0f1d54d941/Changelog.rst#breaks-in-compatibility.

For example this is the exception that's now thrown with the gotr plugin:

Traceback (most recent call last):
  File "/nix/store/i64sgga5635s5y9jmrwdbgybsvh182l0-gajim-0.16.6/share/gajim/src/common/ged.py", line 93, in raise_event
    if handler(*args, **kwargs):
  File "/home/aszlig/.local/share/gajim/plugins/gotr/otrmodule.py", line 562, in handle_incoming_msg
    appdata={'thread':event.session.thread_id if event.session else None})
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 196, in receiveMessage
    self.handleQuery(message, appdata=appdata)
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 390, in handleQuery
    self.authStartV2(appdata=appdata)
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/context.py", line 398, in authStartV2
    self.crypto.startAKE(appdata=appdata)
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/crypt.py", line 259, in startAKE
    outMsg = self.ake.startAKE()
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/crypt.py", line 352, in startAKE
    self.encgx = AESCTR(self.r).encrypt(gxmpi)
  File "/home/aszlig/.local/share/gajim/plugins/gotr/potr/compatcrypto/pycrypto.py", line 52, in AESCTR
    return Cipher.AES.new(key, Cipher.AES.MODE_CTR, counter=counter)
  File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/AES.py", line 260, in new
    return _create_cipher(sys.modules[__name__], key, mode, *args, **kwargs)
  File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/__init__.py", line 130, in _create_cipher
    return modes[mode](factory, **kwargs)
  File "/nix/store/8lcqn954v2vm86rv7d4wpn2yr95gwvfb-pycryptodome-3.4.3/lib/python2.7/site-packages/Crypto/Cipher/_mode_ctr.py", line 319, in _create_ctr_cipher
    _counter = dict(counter)
TypeError: 'Counter' object is not iterable

Hey,
How do I work around with this?
I am kinda new to this.

[Mic92]

Has somebody reported this upstream?

[fadenb]

@optimus-p1509

Hey,
How do I work around with this?
I am kinda new to this.

Take a look at https://nixos.wiki/wiki/Poezio_OTR for a workaround. It basically does what was suggested at https://github.com/NixOS/nixpkgs/issues/21671#issuecomment-281951215

[woffs]

FYI, this is possible with an override, too. For potr I use something like

nixpkgs.config.packageOverrides = pkgs: {
  python3Packages = pkgs.python3Packages.override {
    overrides = self: super: {
      pycrypto-original = self.callPackage ./pycrypto.nix { };
      potr = super.potr.overridePythonAttrs {
        pycrypto = self.pycrypto-original;
        propagatedBuildInputs = [ self.pycrypto-original ];
      };
    };
  };
};

with pycrypto.nix from https://raw.githubusercontent.com/mayflower/nixpkgs/master/pkgs/development/python-modules/pycrypto-original/default.nix

Although I think all this is temporary and should be solved in another, better way,.

[lheckemann]

IMHO this issue can be closed in favour of package-specific issues for packages that don't like our pycryptodome shim thing. Disagreement is welcome and will be taken into account :)