classification
Title: [Patch] '_crypt' module: fix implicit declaration of crypt(), use crypt_r() where available
Type: Stage: resolved
Components: Extension Modules Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: EdSchouten, benjamin.peterson, gregory.p.smith, pitrou, serhiy.storchaka, xdegaye
Priority: normal Keywords: patch

Created on 2016-10-22 07:52 by EdSchouten, last changed 2018-12-31 02:01 by gregory.p.smith. This issue is now closed.

Files
File name Uploaded Description Edit
crypt.diff EdSchouten, 2016-10-22 07:52 Fix implicit declaration of crypt(), use crypt_r() where available review
Pull Requests
URL Status Linked Edit
PR 4691 closed python-dev, 2017-12-03 21:17
PR 11373 merged gregory.p.smith, 2018-12-30 23:03
PR 11373 merged gregory.p.smith, 2018-12-30 23:03
PR 11373 merged gregory.p.smith, 2018-12-30 23:03
PR 11376 merged miss-islington, 2018-12-30 23:42
PR 11376 merged miss-islington, 2018-12-30 23:42
PR 11376 merged miss-islington, 2018-12-30 23:42
Messages (10)
msg279186 - (view) Author: Ed Schouten (EdSchouten) * Date: 2016-10-22 07:52
The '_crypt' module provides a binding to the C crypt(3) function. It is used by the crypt.crypt() function.

Looking at the C code, there are a couple of things we can improve:

- Because crypt() only depends on primitive C types, we currently get away with calling it without it being declared. Ensure that we include <unistd.h>, which is the POSIX header file declaring this.

- The disadvantage of crypt() is that it's thread-unsafe. Systems like Linux and recent versions of FreeBSD nowadays provide crypt_r() as a replacement. This function allows you to pass in a 'crypt_data' object that will hold the resulting string for you. Extend the code to use this function when available.

This patch is actually needed to make this module build on CloudABI (https://mail.python.org/pipermail/python-dev/2016-July/145708.html). CloudABI's C library doesn't provide any thread-unsafe functions, meaning that crypt_r() is the only way you can crypt passwords.
msg304943 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-24 20:53
What is the performance of crypt_r() in comparison with crypt()?
msg304965 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-10-25 07:23
I'm not sure performance matters.  Modern crypt() algorithms should actually be slow enough (using a large number of rounds) to make brute-force attacks impractical...
msg304966 - (view) Author: Ed Schouten (EdSchouten) * Date: 2017-10-25 08:07
Having looked at various implementations of crypt() and crypt_r(), I can't think of a reason why there would be any significant difference in performance. On systems like FreeBSD, crypt() is just a simple wrapper around crypt_r():

https://svnweb.freebsd.org/base/head/lib/libcrypt/crypt.c?view=markup#l134

If there would be any difference in performance, it would also be astronomically small compared to the computation time spent by the cryptographic hash functions themselves.
msg305026 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2017-10-26 06:20
This sgtm. Can you send a PR?
msg307525 - (view) Author: Ed Schouten (EdSchouten) * Date: 2017-12-03 21:16
Ah, you folks switched to Git + Github in the mean time. Nice!

I've just sent this pull request: https://github.com/python/cpython/pull/4691
msg332774 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-12-30 23:42
New changeset 387512c7ecde6446f2e29408af2e16b9fc043807 by Gregory P. Smith in branch 'master':
bpo-28503: Use crypt_r() when available instead of crypt() (GH-11373)
https://github.com/python/cpython/commit/387512c7ecde6446f2e29408af2e16b9fc043807
msg332775 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-12-30 23:56
As this is internal only rather than a feature, i'll bring this into 3.7 as well.
msg332785 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-12-31 01:59
New changeset a144feeb7ec501aaf30072d50e70d54b200e5ef0 by Gregory P. Smith (Miss Islington (bot)) in branch '3.7':
bpo-28503: Use crypt_r() when available instead of crypt() (GH-11373) (GH-11376)
https://github.com/python/cpython/commit/a144feeb7ec501aaf30072d50e70d54b200e5ef0
msg332786 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2018-12-31 02:01
I'm going to close this as 3.7 and 3.8 are good now, but if someone wants to see the same thing done in 2.7 it should be possible for them make a PR.  This is primarily just a configure.ac change.
History
Date User Action Args
2018-12-31 02:01:37gregory.p.smithsetstatus: open -> closed
resolution: fixed
messages: + msg332786

stage: commit review -> resolved
2018-12-31 01:59:54gregory.p.smithsetmessages: + msg332785
2018-12-30 23:57:00gregory.p.smithsetstage: patch review -> commit review
2018-12-30 23:56:24gregory.p.smithsetmessages: + msg332775
versions: + Python 3.7
2018-12-30 23:43:01miss-islingtonsetpull_requests: + pull_request10720
2018-12-30 23:42:52miss-islingtonsetpull_requests: + pull_request10719
2018-12-30 23:42:44miss-islingtonsetpull_requests: + pull_request10718
2018-12-30 23:42:34gregory.p.smithsetmessages: + msg332774
2018-12-30 23:03:24gregory.p.smithsetpull_requests: + pull_request10714
2018-12-30 23:03:15gregory.p.smithsetpull_requests: + pull_request10713
2018-12-30 23:03:07gregory.p.smithsetpull_requests: + pull_request10712
2018-12-30 22:54:26gregory.p.smithsetversions: + Python 3.8, - Python 3.7
2018-12-30 22:54:02gregory.p.smithsetassignee: gregory.p.smith

nosy: + gregory.p.smith
2017-12-03 21:17:32python-devsetstage: patch review
pull_requests: + pull_request4604
2017-12-03 21:16:33EdSchoutensetmessages: + msg307525
2017-10-26 06:20:11benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg305026
2017-10-25 08:07:23EdSchoutensetmessages: + msg304966
2017-10-25 07:23:29pitrousetnosy: + pitrou

messages: + msg304965
versions: + Python 3.7, - Python 3.6
2017-10-24 20:53:39serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg304943
2016-10-28 18:01:06xdegayesetnosy: + xdegaye
2016-10-22 07:52:55EdSchoutencreate