-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
replace Pycrypto - which is unsupported and buggy #2365
Comments
Just wanted to point out in this bug too that PyCrypto is abandoned and it may actually be less effort to move to pycryptodome then to try to get PyCrypto working on Python 3.6. |
Unfortunately using pycryptodome looks like a non-trivial problem. Aside from some easy-to-fix errors on Python 3, I received this daunting error:
I personally wouldn't benefit from this feature and I estimate that this could take up to a week to debug. If I may be so bold, I predict that this could turn into another fiasco like MERGE (which has been unfixed for well over a year now). |
Agreed it's non-trivial. pycryptodome is far from a drop-in replacement in this particular case. It may end up being a matter of re-writing the feature from the ground up. However, the feature is pretty limited in scope, so maybe that's not an absolute dealbreaker. Still easier than getting PyCrypto working in 3.6 ;) |
For what it's worth, there are non-commercial uses for byte-encryption, if it's not the code you're trying to obfuscate per se, but sensitive information within that code (generally things you wish you didn't have to do, like hard-coded passwords). While the perfect solution would be to not do this at all, byte-encryption is a desirable band-aid until the perfect solution can be implemented. We use this for in-house scripts, and we're not commercial. I'm afraid I've seen this sort of sensitive data in infrastructure/maintenance scripts in lots of places, so I don't think we're unique. |
FWIW, I've been using https://github.com/pyca/cryptography in conjunction with netmiko for controlling SSH sessions under Python 3.6 and this works very well. |
Copying this over from #3316 Note: If |
what would be a sufficient bounty for this? |
I placed a modest bounty on this bug. It may not be sufficient, but if everyone chips in what they can, it may become adequate to get it fixed. |
FWIW, a developer has placed a $600 bounty goal on this bug. I've already committed $300 and am tapped out. Assistance would be appreciated. I don't know how these bounty goals work exactly, but presumably there's some interest at least right now, and that's a good thing. |
I'll work on this issue tomorrow. |
It seems someone has already made a pull request regarding this issue. If that PR is insufficient, then I will work on the issue. |
I'll chip in $100 to the bounty pool to get this issue fixed. It worked for me with Python 3.4, but now I've had to move up to 3.7. |
Depends on if PR #4241 is insufficient to fix this issue. We may just be waiting for a merge. |
That pull request may be the solution we're looking for... but it hasn't been touched in a month, and it fails some automated checks, so it's not getting merged without some love first. Right now, the bug bounty is probably the place where we can do the most to make this happen: |
@ProjectThreepio @SlightlyCyborg Sorry, app for checking PR notifications was being buggy. I'm not sure what check it's failing, but I'm pretty sure it's codestyle |
Yep, it was line length :P, not used to it being 79, mine is usually 120 |
veeshi commented earlier about another effort:
While I'm not sure that means you must use a big established, long-supported module like PyCryptodome, cryptography is hard, so we should make an effort to ensure cryptographers are actively maintaining the modules we're using. That said, this is ultimately about code obfuscation and can be circumvented by a motivated attacker even under the best of circumstances, so it's not about the quality of the encryption per se, but about the long-term quality of the module's code. No matter what solution ends up working, consider contributing to the bug bounty: |
Maybe I will continue working on it in the near future. |
The "problem" with bigger libraries is the the attack surface is bigger, and, considering AES CTR mode was introduced in 1979 (wikipedia), I expect most implementations to be quite stable. The https://github.com/kokke/tiny-AES-c seems to be maintained (the author is responsive on issues and PRs), tested with official codes, and used / starred enough to have a lot of eyes pointed to highlight any security problem may arise in the future. The wrapper itself is so minimal that could be printed on a t-shirt, and exposes the plain API, please audit it for any problem I may have overlooked. In the case of PyCrypto, beside the unmaintained state, all the security advisories where not against the AES module, that is the only one included for encryption, but other modules. So, IMHO, we should not search for a library with a commit each week, if not adding new unrelated features, that is not a good sign for a base crypto library, each commit may introduce bugs, and if there are no known security problems, the library (using 1979 knowledge) is fair to be stale. That said, we already know that the encryption mode in PyInstaller is an easy obstacle for a motivated cracker, even without involving any crypto attack, so I'd better favor an easy implementation that has minimal impact on the overall code, easier to maintain and evolve, just to represent the security trade-off the feature is offering, avoid tampering and easy reverse engineering and not much more. Here is a WIP branch: #4652 (opened a PR to trigger the CI) PS. just in case, I'm not interested in reclaiming the bounty |
tinyaes https://github.com/naufraghi/tinyaes-py is a minimal AES-only library that wraps the C library https://github.com/kokke/tiny-AES-c See issue pyinstaller#2365
tinyaes https://github.com/naufraghi/tinyaes-py is a minimal AES-only library that wraps the C library https://github.com/kokke/tiny-AES-c See issue pyinstaller#2365
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. See issue pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy
The bounty for this bug has been boosted. BountySource's site appears to be having troubles right now, so it's hard to see this change. |
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. See issue pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy
@ProjectThreepio and anyone else who had committed to the bounty - is there a way for @htgoebel to claim it due to the fact @naufraghi isn't interested? To fund PyInstaller maintenance and development |
Oh and if @naufraghi's solution doesn't work/satisfy, I'll be happy to implement this with |
The bounty is for whoever can write code that makes whatever cut is required to get accepted into the production version of PyInstaller. So in a sense @htgoebel decides the matter, not me. If the person who writes that code doesn't want to claim it, I believe I can (and would) revert it to general PyInstaller project maintenance, or another PyInstaller bug. |
@ProjectThreepio I agree. It should be repurposed at the least. As my second comment says, I could do this, but don't see the point of doing it if naufraghi's solution works - and if I do end up doing it, I'd donate half of the bounty to the team. |
I'm OK to give the bounty to the dev team, I very thank them for their work with PyInstaller! I added a test suite to the wrapper, using hypothesis and found a bug, the bug is fixed, so I think I'll release |
I'll review the wrapper at some point soon as well. It's always good to have a second pair of eyes look over it! @naufraghi |
I had no idea PyInstaller was in such financial straits until now! Yes, absolutely redirect my bounty to general PyInstaller operations, ASAP! |
@ProjectThreepio thanks! I've only been on the team a week, and I offered htgoebel help with the GitHub end to start with. If he doesn't have to sort through the many support issues and other ones that don't need core developer attention, he'll be able to put more if his funded time towards PyInstaller core... |
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. See issue pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy - remove some unused imports - better explain test for issue pyinstaller#1663
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. See issue pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy - remove some unused imports - better explain test for issue pyinstaller#1663
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. Closes pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy - remove some unused imports - better explain test for issue pyinstaller#1663
[tinyaes](https://github.com/naufraghi/tinyaes-py) is a minimal AES-only library that wraps the C library [tiny-AES-c](https://github.com/kokke/tiny-AES-c). Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is [somewhat similar to CBC](https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB))) and CTR is suggested as a go-to stream cipher (https://crypto.stackexchange.com/questions/6029/aes-cbc-mode-or-aes-ctr-mode-recommended and links) that can be used instead of CBC. Closes pyinstaller#2365 PR iterations: - use tinyaes from pypi (installing from git triggers a key confirmation) - make lint job happy - remove some unused imports - better explain test for issue pyinstaller#1663
This re-enables encrypting the Python bytecode (option ``--key``). `tinyaes <https://github.com/naufraghi/tinyaes-py>`_ is a minimal AES-only library that wraps the C library `tiny-AES-c <https://github.com/kokke/tiny-AES-c>`_. Currently the library wraps only the CTR mode that can be used in PyInstaller instead of the CFB mode used before. Reading various sources, CFB is `somewhat similar to CBC <https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Feedback_(CFB)>`_ and CTR is suggested as a go-to stream cipher (see <https://crypto.stackexchange.com/questions/6029> and links) that can be used instead of CBC. Closes pyinstaller#2365.
#4652) This re-enables encrypting the Python bytecode (`--key`) by replacing PyCrypto with tinyaes. tinyaes is a minimal AES-only library that wraps the C library tiny-AES-c. See https://github.com/naufraghi/tinyaes-py for source code. Closes #2365.
pyinstaller#4652) This re-enables encrypting the Python bytecode (`--key`) by replacing PyCrypto with tinyaes. tinyaes is a minimal AES-only library that wraps the C library tiny-AES-c. See https://github.com/naufraghi/tinyaes-py for source code. Closes pyinstaller#2365.
[htgoebel is taking over this formerly empt comment]
pycrypto, which PyInstaller uses for byte-code encryption, needs to be replaced. Its officially "dead" and seriously buggy. Further pycrypto doe not work for Python 3.6 (see below). Replacement options seem to be:
pycryptdome
- which calls it self a drop in replacement but does not work as expected (see below)cryptography
.Maintainer note: If you want this to be fixed, please place an adequate(!) bounty or submit a pull-request. The requirement for byte-encryption is a commercial one (free software does not require byte-encryption since the cod is available anyway). So if you are earning money with using PyInstaller you should take parts of the development.
The text was updated successfully, but these errors were encountered: