Skip to content

bpo-28503: _crypt: fix implicit declaration of crypt(), use crypt_r() if available. #4691

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.

Sign up for GitHub

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

Closed
EdSchouten wants to merge 1 commit into from
Closed

bpo-28503: _crypt: fix implicit declaration of crypt(), use crypt_r() if available. #4691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
_crypt: fix implicit declaration of crypt(), use crypt_r() if available. 
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.
  • Loading branch information
@EdSchouten
EdSchouten committed Dec 3, 2017
commit f8e4237bc2a37ebd6b7adc1da02bb5386f91d19f
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2017-12-03-21-25-10.bpo-28503.D93bf3.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Let the `_crypt` module use `crypt_r()` when available for thread safety.
11 changes: 11 additions & 0 deletions Modules/_cryptmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@

#include <sys/types.h>

#ifdef HAVE_CRYPT_H
#include <crypt.h>
#endif
#include <unistd.h>

/* Module crypt */

/*[clinic input]
Expand Down Expand Up @@ -34,7 +39,13 @@ static PyObject *
crypt_crypt_impl(PyObject *module, const char *word, const char *salt)
/*[clinic end generated code: output=0512284a03d2803c input=0e8edec9c364352b]*/
{
#ifdef HAVE_CRYPT_R
struct crypt_data data;
data.initialized = 0;
return Py_BuildValue("s", crypt_r(word, salt, &data));
#else
return Py_BuildValue("s", crypt(word, salt));
#endif
}


Expand Down
51 changes: 48 additions & 3 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7681,7 +7681,7 @@ $as_echo "#define STDC_HEADERS 1" >>confdefs.h

fi

for ac_header in asm/types.h conio.h direct.h dlfcn.h errno.h \
for ac_header in asm/types.h conio.h crypt.h direct.h dlfcn.h errno.h \
fcntl.h grp.h \
ieeefp.h io.h langinfo.h libintl.h process.h pthread.h \
sched.h shadow.h signal.h stropts.h termios.h \
Expand Down Expand Up @@ -9502,6 +9502,51 @@ _ACEOF

fi
# Dynamic linking for HP-UX
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for crypt in -lcrypt" >&5
$as_echo_n "checking for crypt in -lcrypt... " >&6; }
if ${ac_cv_lib_crypt_crypt+:} false; then :
$as_echo_n "(cached) " >&6
else
ac_check_lib_save_LIBS=$LIBS
LIBS="-lcrypt $LIBS"
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */

/* Override any GCC internal prototype to avoid an error.
Use char because int might match the return type of a GCC
builtin and then its argument prototype would still apply. */
#ifdef __cplusplus
extern "C"
#endif
char crypt ();
int
main ()
{
return crypt ();
;
return 0;
}
_ACEOF
if ac_fn_c_try_link "$LINENO"; then :
ac_cv_lib_crypt_crypt=yes
else
ac_cv_lib_crypt_crypt=no
fi
rm -f core conftest.err conftest.$ac_objext \
conftest$ac_exeext conftest.$ac_ext
LIBS=$ac_check_lib_save_LIBS
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_crypt_crypt" >&5
$as_echo "$ac_cv_lib_crypt_crypt" >&6; }
if test "x$ac_cv_lib_crypt_crypt" = xyes; then :
cat >>confdefs.h <<_ACEOF
#define HAVE_LIBCRYPT 1
_ACEOF

LIBS="-lcrypt $LIBS"

fi
# crypt() on Linux

{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for uuid_generate_time_safe" >&5
$as_echo_n "checking for uuid_generate_time_safe... " >&6; }
Expand Down Expand Up @@ -11141,8 +11186,8 @@ fi

# checks for library functions
for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
clock confstr ctermid dup3 execv faccessat fchmod fchmodat fchown fchownat \
fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \
clock confstr crypt_r ctermid dup3 execv faccessat fchmod fchmodat fchown \
fchownat fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \
futimens futimes gai_strerror getentropy \
getgrouplist getgroups getlogin getloadavg getpeername getpgid getpid \
getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \
Expand Down
7 changes: 4 additions & 3 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2043,7 +2043,7 @@ dnl AC_MSG_RESULT($cpp_type)

# checks for header files
AC_HEADER_STDC
AC_CHECK_HEADERS(asm/types.h conio.h direct.h dlfcn.h errno.h \
AC_CHECK_HEADERS(asm/types.h conio.h crypt.h direct.h dlfcn.h errno.h \
fcntl.h grp.h \
ieeefp.h io.h langinfo.h libintl.h process.h pthread.h \
sched.h shadow.h signal.h stropts.h termios.h \
Expand Down Expand Up @@ -2679,6 +2679,7 @@ AC_MSG_RESULT($SHLIBS)
AC_CHECK_LIB(sendfile, sendfile)
AC_CHECK_LIB(dl, dlopen) # Dynamic linking for SunOS/Solaris and SYSV
AC_CHECK_LIB(dld, shl_load) # Dynamic linking for HP-UX
AC_CHECK_LIB(crypt, crypt) # crypt() on Linux

@berkerpeksag berkerpeksag Dec 5, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAVE_LIBCRYPT created by this doesn't seem to be used in Modules/_cryptmodule.c. I recently tried to use AC_CHECK_LIB for the same use case, but there were some comments concerning the test program created by autotools. I end up using the following approach:

AC_MSG_CHECKING(for uuid_generate_time_safe)
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <uuid/uuid.h>]], [[
#ifndef uuid_generate_time_safe
void *x = uuid_generate_time_safe
#endif
]])],
  [AC_DEFINE(HAVE_UUID_GENERATE_TIME_SAFE, 1, Define if uuid_generate_time_safe() exists.)
   AC_MSG_RESULT(yes)],
  [AC_MSG_RESULT(no)]
)


AC_MSG_CHECKING(for uuid_generate_time_safe)
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <uuid/uuid.h>]], [[
Expand Down Expand Up @@ -3409,8 +3410,8 @@ fi

# checks for library functions
AC_CHECK_FUNCS(alarm accept4 setitimer getitimer bind_textdomain_codeset chown \
clock confstr ctermid dup3 execv faccessat fchmod fchmodat fchown fchownat \
fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \
clock confstr crypt_r ctermid dup3 execv faccessat fchmod fchmodat fchown \
fchownat fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \
futimens futimes gai_strerror getentropy \
getgrouplist getgroups getlogin getloadavg getpeername getpgid getpid \
getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \
Expand Down
9 changes: 9 additions & 0 deletions pyconfig.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@
/* Define to 1 if you have the `copysign' function. */
#undef HAVE_COPYSIGN

/* Define to 1 if you have the <crypt.h> header file. */
#undef HAVE_CRYPT_H

/* Define to 1 if you have the `crypt_r' function. */
#undef HAVE_CRYPT_R

/* Define to 1 if you have the `ctermid' function. */
#undef HAVE_CTERMID

Expand Down Expand Up @@ -547,6 +553,9 @@
/* Define to 1 if you have the `lgamma' function. */
#undef HAVE_LGAMMA

/* Define to 1 if you have the `crypt' library (-lcrypt). */
#undef HAVE_LIBCRYPT

/* Define to 1 if you have the `dl' library (-ldl). */
#undef HAVE_LIBDL

Expand Down