[TECH DEBT] Migrating away from `M2Crypto`
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 15.7k
- Forks
- 5.6k
- Avg merge
- 2d 44m
- Merged PRs (30d)
- 80
Description
Description of the tech debt to be addressed, include links and screenshots
Following up on https://github.com/saltstack/salt/issues/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
PyCryptodomeor elsePyCryptoifM2Cryptois not found - Can be migrated to
cryptographyusingRSA.generate_private_key() - PR now open here: https://github.com/saltstack/salt/pull/66166
- Fallbacks to using
- x509 module: Already deprecated
- Its replacement module
x509v2was re-written to usepyca/cryptographyinstead
- Its replacement module
- 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
PyCryptodomeor elsePyCryptoifM2Cryptois 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.
- Fallbacks to using
- channel/server.py: Used to encrypt/decrypt with RSA with PKCS1 OAEP padding
- Fallbacks to using
PyCryptodomeor elsePyCryptoifM2Cryptois not found - Same as above, can be migrated to
pyca/cryptography
- Fallbacks to using
- auth/pki.py: Used to load and verify a X.509 certificate
- Fallbacks to using
pyOpenSSLandPyCryptodome/PyCryptoifM2Cryptois not available - Can be migrated to
pyca/cryptographyusing https://cryptography.io/en/latest/x509/verification/
- Fallbacks to using
- clouds/joyent.py: Used to calculate a SHA256 digest and sign using RSA
- Fallbacks to using
PyCryptodomeor elsePyCryptoifM2Cryptois not found - Same as above, can be migrated to
pyca/cryptography
- Fallbacks to using
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review the listed M2Crypto call sites in salt/crypt.py, salt/channel/server.py, salt/auth/pki.py, and salt/cloud/clouds/joyent.py, noting that runners/digicertapi.py already has a pull request and the x509 module is deprecated. Start by comparing each fallback path with the suggested pyca/cryptography APIs. Done means the remaining unchecked usages have a decided migration or deprecation path without relying on M2Crypto.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cryptography, security
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100