Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from PyCrypto to pyca/cryptography #1492

Draft
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@plettich
Copy link
Member

plettich commented Mar 7, 2019

Part 1: AES

  • remove all AES encryption/decryption based on PyCrypto

  • add only one AES (CBC mode) encryption/decryption function which uses
    pyca/cryptography without padding

  • renamed yubikey AES (ECB mode) decryption function.

  • currently the (un)padding must be done in the calling functions

  • removed the security/pkcs11.py module since it is not used.

  • refactored some parameter names

Working on #337

Migrate from PyCrypto to pyca/cryptography
Part 1: AES

- remove all AES encryption/decryption based on `PyCrypto`
- add only one AES (CBC mode) encryption/decryption function which uses
  `pyca/cryptography` without padding
- renamed yubikey AES (ECB mode) decryption function.
- currently the (un)padding must be done in the calling functions

- removed the `security/pkcs11.py` module since it is not used.
- refactored some parameter names

@plettich plettich requested a review from privacyidea/core Mar 7, 2019

Show resolved Hide resolved privacyidea/lib/crypto.py
Show resolved Hide resolved privacyidea/lib/crypto.py Outdated
Show resolved Hide resolved privacyidea/lib/crypto.py Outdated
Show resolved Hide resolved tests/test_lib_crypto.py
Fix remarks from review
- Switch to PKCS7 padding/unpadding in aes_(encrypt|decrypt)_b64
- Add tests for decrypting values generated with previous versions
@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 8, 2019

Codecov Report

Merging #1492 into master will increase coverage by 0.09%.
The diff coverage is 89.92%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1492      +/-   ##
=========================================
+ Coverage    96.8%   96.9%   +0.09%     
=========================================
  Files         144     143       -1     
  Lines       17250   17261      +11     
=========================================
+ Hits        16699   16726      +27     
+ Misses        551     535      -16
Impacted Files Coverage Δ
privacyidea/lib/security/aeshsm.py 83.33% <ø> (ø) ⬆️
privacyidea/lib/subscriptions.py 90.98% <100%> (+2.88%) ⬆️
privacyidea/lib/security/default.py 97.61% <100%> (-0.16%) ⬇️
privacyidea/lib/tokens/yubikeytoken.py 97.61% <100%> (ø) ⬆️
privacyidea/lib/importotp.py 91.39% <100%> (+0.05%) ⬆️
privacyidea/api/lib/postpolicy.py 95.88% <63.63%> (-1.13%) ⬇️
privacyidea/lib/auditmodules/sqlaudit.py 95.36% <71.42%> (-1.93%) ⬇️
privacyidea/lib/crypto.py 97.77% <93.18%> (+1.85%) ⬆️
privacyidea/lib/error.py 95% <0%> (+3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c89c545...803fa69. Read the comment docs.

Migrate from PyCrypto to pyca/cryptography
Part 2: RSA

This commit removes all RSA related functionality based on PyCrypto and
replaces them with `pyca/cryptography`
The RSA functionality is changed to mitigate cryptographic weaknesses:
- All Signatures are calculated using the PSS padding scheme [1] and are
  prefixed with a version string to identify the type.
- Old style textbook-RSA signatures in the audit log can still be validated
  by setting `PI_CHECK_OLD_SIGNATURES` to `True` in the configuration
- The `Sign` Object now accepts only the key data (not the file names) and
  is possible to use it either with only a private key or a public key
- Some tests now check for old style (textbook RSA) signatures.

[1] https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#signing
@fredreichbier
Copy link
Member

fredreichbier left a comment

looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.