[TECH DEBT] Migrating away from M2Crypto

[facutuesca]

Description of the tech debt to be addressed, include links and screenshots

Following up on #63066: Salt currently uses M2Crypto for some of its cryptographic operations. M2Crypto is a wrapper around OpenSSL's APIs that has some drawbacks:

• It does not provide memory safety
• It's currently in maintenance mode (the recommended alternative is pyca/cryptography)
• It does not provide wheels on PyPI, which means users installing it also have to build it from source
• The build/installation process requires installing swig, something that adds complexity and results in user installation issues.

I'm opening this issue to list the places where M2Crypto is still being used inside Salt, and to assess the interest in migrating away from it towards more well-maintained alternatives, such as pyca/cryptography. To note: Salt already depends on pyca/cryptography, so this migration would not add new dependencies.

Going through the codebase, these are the usages that I could find, along with the feasibility of migrating to alternatives:

• runners/digicertapi.py: Used to generate a RSA private key
  • Fallbacks to using PyCryptodome or else PyCrypto if M2Crypto is not found
  • Can be migrated to cryptography using RSA.generate_private_key()
  • PR now open here: [master] Migrate the digicert runner from M2Crypto to PyCA/cryptography #66166
• x509 module: Already deprecated
  • Its replacement module x509v2 was re-written to use pyca/cryptography instead
• salt/crypt.py: Used to generate an RSA key pair, load/encrypt/sign using a RSA private key, load/encrypt/decrypt/verify using a RSA public key, load an RSA private key, load an RSA public key, signing and verifying using RSA, encrypts and decrypts using RSA with X931 padding, decrypts using RSA with PKCS1.5 padding, and others.
  • Fallbacks to using PyCryptodome or else PyCrypto if M2Crypto is not found
  • Most uses can be migrated to pyca/cryptography. An exception is encryption/decryption with X931 padding, but Salt already has its own fallback implementation for it.
• channel/server.py: Used to encrypt/decrypt with RSA with PKCS1 OAEP padding
  • Fallbacks to using PyCryptodome or else PyCrypto if M2Crypto is not found
  • Same as above, can be migrated to pyca/cryptography
• auth/pki.py: Used to load and verify a X.509 certificate
  • Fallbacks to using pyOpenSSL and PyCryptodome/PyCrypto if M2Crypto is not available
  • Can be migrated to pyca/cryptography using https://cryptography.io/en/latest/x509/verification/
• clouds/joyent.py: Used to calculate a SHA256 digest and sign using RSA
  • Fallbacks to using PyCryptodome or else PyCrypto if M2Crypto is not found
  • Same as above, can be migrated to pyca/cryptography

I'm opening the discussion to see if there is interest in migrating away from M2Crypto, and to discuss related issues such as blockers, deprecation paths, fallback alternatives, etc.

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Community Wiki
• Salt’s Contributor Guide
• Join our Community Slack
• IRC on LiberaChat
• Salt Project YouTube channel
• Salt Project Twitch channel

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

[dwoz]

@facutuesca The intent is to migrate everything to cryptography. We just have not had a chance to prioritize it. Any contributions moving things in that direction are welcome.

[mcepl]

A couple more comments from tired maintainer of M2Crypto, who doesn’t wish anything more than to let M2Crypto leave its duties for peace of retirement.

Following up on #63066: Salt currently uses M2Crypto for some of its cryptographic operations. M2Crypto is a wrapper around OpenSSL's APIs that has some drawbacks:

Besides, M2Crypto follows OpenSSL API very closely, so it is very easy to create insecure application with it, no, sometimes it is actually quite difficult to call OpenSSL APIs in secure manner. If cryptography is doing more in preventing its users to shoot into their feet, it is great advantage of it.

* The build/installation process requires [installing `swig`](https://gitlab.com/m2crypto/m2crypto/-/blob/0.41.0/INSTALL.rst?ref_type=tags&plain=1#L21-22), something that adds complexity and results in [user installation issues](https://github.com/saltstack/salt/issues?q=is%3Aissue+swig+is%3Aclosed).

And the header files for OpenSSL (which might be cryptography problem as well).

  * Fallbacks to using `PyCryptodome` or else `PyCrypto` if `M2Crypto` is not found

Whatever I said about M2Crypto however pale in comparison with PyCrypto* with its home-brew cryptographic algorithms and much lesser level of audit than what OpenSSL gets. I would strongly encourage getting rid of any traces of relation to it.

[facutuesca]

@facutuesca The intent is to migrate everything to cryptography. We just have not had a chance to prioritize it. Any contributions moving things in that direction are welcome.

First PR for the migration is up here: #66166

[dwoz]

@facutuesca I will take care of salt/crypt.py and salt/channel/*

[dwoz]

Most uses can be migrated to pyca/cryptography. An exception is encryption/decryption with X931 padding, but Salt already has its own fallback

This module should also be migrated to use cryptography's openssl backend instead of ctypes.

[dwoz]

Most uses can be migrated to pyca/cryptography. An exception is encryption/decryption with X931 padding, but Salt already has its own fallback

This module should also be migrated to use cryptography's openssl backend instead of ctypes.

On second thought, let's get off x9.31 all together as it's likely considered to be insecure: openssl/openssl#10919 (comment)