Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-31904: Add _crypt module support for VxWorks RTOS#12321
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
pxinwr commented Mar 14, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pxinwr commented Mar 26, 2019
@serhiy-storchaka I've fixed the problem you mentioned. Could you help to review again and do the merge if the code is OK? |
vstinner commented Mar 27, 2019
@tiran: Do you think that using OpenSSL DES_crypt() to implement crypt.crypt() is acceptable? |
vstinner commented Mar 27, 2019
Oh, the commit 32f5fdd already added OpenSSL dependency to _crypt Python module in setup.py. IMHO it would be better to not add it before this change, but well, since the commit is already pushed, it's too late and it's ok :-) |
tiran left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for implementing crypt as DES.
Do you really need the crypt module or are you simply implementing it for the sake of having a complete Python stdlib?
I'm planning to deprecate and drop the crypt module. It's not very useful these days and most hashing algorithms are weak. Especially DES is super weak.
bedevere-bot commented Mar 28, 2019
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
pxinwr commented Mar 29, 2019
@tiran Thanks for your comments. I am pinging my PM for the necessity of crypt module. Before I get the decision, could you tell what modules can be used to replace the crypt module? |
tiran commented Mar 29, 2019
It depends on your use case. For passwords the hashlib module provides PBKDF2-HMAC and scrypt. Both algorithms are much better than crypt. There are also external packages like bcrypt, cryptography, and passlib, that provide secure password derivation algorithms. The crypt module is really only useful for legacy systems or for use with the |
serhiy-storchaka commented Mar 29, 2019
Agree. If VxWorks RTOS for some (likely good) reasons does not provide the system |
pxinwr commented Apr 1, 2019
@tiran@serhiy-storchaka Thanks for your comments. My PM has confirmed that VxWorks can drop the crypt module support. So as you required, I have made the requested changes; please review again. |
bedevere-bot commented Apr 1, 2019
Thanks for making the requested changes! @serhiy-storchaka, @tiran: please review the changes made to this pull request. |
pxinwr commented Apr 1, 2019
Already checked the CI failures. That should be caused by other PRs other than this one. |
pxinwr commented Apr 3, 2019
Closing it and reopening it is to rerun the CI to update the regression result. |
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to get a new PR. It's surprising that a PR to add VxWorks support to crypt became "remove crypt". For example, you didn't update the PR title.
Please open a new PR with requested changes.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Apr 8, 2019
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
This is the successive PR after #11968. This PR adds _crypt module support for VxWorks RTOS.
VxWorks has no APIs crypt() and crypt_r() provided. Instead, DES_crypt() is defined in openssl/des.h to replace the crypt(). It has the same prototype as crypt(). This extension module _crypt will not be built if VxWorks users don't enable the OPENSSL features. See
cpython/setup.py
Lines 976 to 989 in f2f55e7
More and full support on modules for VxWorks will continuously be added by the coming PRs.
VxWorks is a product developed and owned by Wind River. For VxWorks introduction or more details, go to https://www.windriver.com/products/vxworks/
Wind River will have a dedicated engineering team to contribute to the support as maintainers.
We already have a working buildbot worker internally, but has not bound to master. We will check the process for the buildbot, then add it.
https://bugs.python.org/issue31904