Make crypt and crypt_gensalt use thread-local output buffers.

.gitignore

@@ -70,6 +70,7 @@ test/crypt-des
 test/crypt-gost-yescrypt
 test/crypt-kat
 test/crypt-md5
+test/crypt-multithread
 test/crypt-nthash
 test/crypt-pbkdf1-sha1
 test/crypt-scrypt

LICENSING

@@ -74,9 +74,10 @@ source tree.  For specific licensing terms consult the files themselves.
  * Public domain, written by Zack Weinberg et al.:
    byteorder.h, randombytes.c, test-byteorder.c
    test-alg-pbkdf-hmac-sha256.c
-   test-badsetting.c, test-crypt-badargs.c, test-getrandom-fallbacks.c,
-   test-getrandom-interface.c, test-symbols-compat.sh,
-   test-symbols-renames.sh, test-symbols-static.sh,
+   test-badsetting.c, test-crypt-badargs.c, test-crypt-multithread.c,
+   test-getrandom-fallbacks.c, test-getrandom-interface.c,
+   test-symbols-compat.sh, test-symbols-renames.sh,
+   test-symbols-static.sh,
    build-aux/scripts/gen-crypt-h,
    build-aux/scripts/gen-crypt-symbol-vers-h,
    build-aux/scripts/gen-libcrypt-map,
@@ -88,6 +89,14 @@ source tree.  For specific licensing terms consult the files themselves.
    GPL (v3 or later), with Autoconf exception:
    build-aux/m4/zw_automodern.m4, build-aux/m4/zw_simple_warnings.m4
 
+*  Copyright Steven G. Johnson, Daniel Richard G., Marc Stevens;
+   GPL (v3 or later), with Autoconf exception:
+   build-aux/m4/ax_pthread.m4
+
+*  Copyright Alan Woodland, Diego Elio Petteno;
+   GPL (v3 or later), with Autoconf exception:
+   build-aux/m4/ax_tls.m4
+
  * Copyright <vt at altlinux.org>; 0-clause BSD:
    crypt-yescrypt.c, test-crypt-yescrypt.c
 

Makefile.am

@@ -162,6 +162,7 @@ APPLY_SYMVERS = no
 endif
 
 libcrypt_la_LDFLAGS += $(UNDEF_FLAG) $(TEXT_RELOC_FLAG) $(AM_LDFLAGS)
+libcrypt_la_LDFLAGS += $(NO_TLS_GET_ADDR_OPT_FLAG)
 
 libcrypt_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_LIBCRYPT
 
@@ -375,6 +376,7 @@ check_PROGRAMS = \
 	test/compile-strong-alias \
 	test/crypt-badargs \
 	test/crypt-gost-yescrypt \
+	test/crypt-multithread \
 	test/explicit-bzero \
 	test/gensalt \
 	test/gensalt-extradata \
@@ -478,6 +480,10 @@ test/symbols-compat.log test/symbols-compat.trs: test/TestCommon.pm
 test/symbols-renames.log test/symbols-renames.trs: test/TestCommon.pm
 test/symbols-static.log test/symbols-static.trs: test/TestCommon.pm
 
+# test/crypt-multithread.c uses pthreads.
+test_crypt_multithread_CFLAGS = $(PTHREAD_CFLAGS)
+test_crypt_multithread_LIBS = $(PTHREAD_LIBS)
+
 COMMON_TEST_OBJECTS = libcrypt.la
 
 test_badsalt_LDADD = $(COMMON_TEST_OBJECTS)
@@ -488,6 +494,7 @@ test_checksalt_LDADD = $(COMMON_TEST_OBJECTS)
 test_des_obsolete_LDADD = $(COMMON_TEST_OBJECTS)
 test_des_obsolete_r_LDADD = $(COMMON_TEST_OBJECTS)
 test_crypt_badargs_LDADD = $(COMMON_TEST_OBJECTS)
+test_crypt_multithread_LDADD = $(COMMON_TEST_OBJECTS)
 test_short_outbuf_LDADD = $(COMMON_TEST_OBJECTS)
 test_preferred_method_LDADD = $(COMMON_TEST_OBJECTS)
 test_special_char_salt_LDADD = $(COMMON_TEST_OBJECTS)

NEWS

@@ -4,6 +4,15 @@ Please send bug reports, questions and suggestions to
 <https://github.com/besser82/libxcrypt/issues>.
 
 Version 4.4.39
+* crypt and crypt_gensalt now use per-thread storage areas for their
+  output, allocated upon the first call in each thread that uses them.
+  This makes it safe to call these functions from multiple threads
+  simultaneously (but consecutive calls will still clobber the
+  previous output).
+  This feature is a safety net against sloppy coding.  Programs are
+  still strongly encouraged to use the reentrant functions instead,
+  because this safety net is not guaranteed by any standard
+  (although we are informed that Solaris also does this).
 
 Version 4.4.38
 * Fix several "-Wunterminated-string-initialization", which are seen by

TODO.md

@@ -67,15 +67,6 @@ It was last updated 20 October 2018.
     * If we do, should it know how to trigger the trusted-path
       password prompt in modern GUI environments? (probably)
 
-  * Make the crypt and crypt_gensalt static state thread-specific?
-    * Solaris 11 may have done this (its `crypt(3)` manpage describes
-      it as MT-Safe and I don’t see any other way they could have
-      accomplished that).
-    * if allocated on first use, this would also shave 32kB of
-      data segment off the shared library
-    * alternatively, add a global lock and *crash the program* if we
-      detect concurrent calls
-
   * Allow access to more of yescrypt’s tunable parameters and ROM
     feature, in a way that’s generic enough that we could also use it
     for e.g. Argon2’s tunable parameters

build-aux/m4/ax_compare_version.m4

@@ -0,0 +1,177 @@
+# ===========================================================================
+#    https://www.gnu.org/software/autoconf-archive/ax_compare_version.html
+# ===========================================================================
+#
+# SYNOPSIS
+#
+#   AX_COMPARE_VERSION(VERSION_A, OP, VERSION_B, [ACTION-IF-TRUE], [ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   This macro compares two version strings. Due to the various number of
+#   minor-version numbers that can exist, and the fact that string
+#   comparisons are not compatible with numeric comparisons, this is not
+#   necessarily trivial to do in a autoconf script. This macro makes doing
+#   these comparisons easy.
+#
+#   The six basic comparisons are available, as well as checking equality
+#   limited to a certain number of minor-version levels.
+#
+#   The operator OP determines what type of comparison to do, and can be one
+#   of:
+#
+#    eq  - equal (test A == B)
+#    ne  - not equal (test A != B)
+#    le  - less than or equal (test A <= B)
+#    ge  - greater than or equal (test A >= B)
+#    lt  - less than (test A < B)
+#    gt  - greater than (test A > B)
+#
+#   Additionally, the eq and ne operator can have a number after it to limit
+#   the test to that number of minor versions.
+#
+#    eq0 - equal up to the length of the shorter version
+#    ne0 - not equal up to the length of the shorter version
+#    eqN - equal up to N sub-version levels
+#    neN - not equal up to N sub-version levels
+#
+#   When the condition is true, shell commands ACTION-IF-TRUE are run,
+#   otherwise shell commands ACTION-IF-FALSE are run. The environment
+#   variable 'ax_compare_version' is always set to either 'true' or 'false'
+#   as well.
+#
+#   Examples:
+#
+#     AX_COMPARE_VERSION([3.15.7],[lt],[3.15.8])
+#     AX_COMPARE_VERSION([3.15],[lt],[3.15.8])
+#
+#   would both be true.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq],[3.15.8])
+#     AX_COMPARE_VERSION([3.15],[gt],[3.15.8])
+#
+#   would both be false.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq2],[3.15.8])
+#
+#   would be true because it is only comparing two minor versions.
+#
+#     AX_COMPARE_VERSION([3.15.7],[eq0],[3.15])
+#
+#   would be true because it is only comparing the lesser number of minor
+#   versions of the two values.
+#
+#   Note: The characters that separate the version numbers do not matter. An
+#   empty string is the same as version 0. OP is evaluated by autoconf, not
+#   configure, so must be a string, not a variable.
+#
+#   The author would like to acknowledge Guido Draheim whose advice about
+#   the m4_case and m4_ifvaln functions make this macro only include the
+#   portions necessary to perform the specific comparison specified by the
+#   OP argument in the final configure script.
+#
+# LICENSE
+#
+#   Copyright (c) 2008 Tim Toolan <[email protected]>
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 13
+
+dnl #########################################################################
+AC_DEFUN([AX_COMPARE_VERSION], [
+  AC_REQUIRE([AC_PROG_AWK])
+
+  # Used to indicate true or false condition
+  ax_compare_version=false
+
+  # Convert the two version strings to be compared into a format that
+  # allows a simple string comparison.  The end result is that a version
+  # string of the form 1.12.5-r617 will be converted to the form
+  # 0001001200050617.  In other words, each number is zero padded to four
+  # digits, and non digits are removed.
+  AS_VAR_PUSHDEF([A],[ax_compare_version_A])
+  A=`echo "$1" | sed -e 's/\([[0-9]]*\)/Z\1Z/g' \
+                     -e 's/Z\([[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/[[^0-9]]//g'`
+
+  AS_VAR_PUSHDEF([B],[ax_compare_version_B])
+  B=`echo "$3" | sed -e 's/\([[0-9]]*\)/Z\1Z/g' \
+                     -e 's/Z\([[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/Z\([[0-9]][[0-9]][[0-9]]\)Z/Z0\1Z/g' \
+                     -e 's/[[^0-9]]//g'`
+
+  dnl # In the case of le, ge, lt, and gt, the strings are sorted as necessary
+  dnl # then the first line is used to determine if the condition is true.
+  dnl # The sed right after the echo is to remove any indented white space.
+  m4_case(m4_tolower($2),
+  [lt],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort -r | sed "s/x${A}/false/;s/x${B}/true/;1q"`
+  ],
+  [gt],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort | sed "s/x${A}/false/;s/x${B}/true/;1q"`
+  ],
+  [le],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort | sed "s/x${A}/true/;s/x${B}/false/;1q"`
+  ],
+  [ge],[
+    ax_compare_version=`echo "x$A
+x$B" | sed 's/^ *//' | sort -r | sed "s/x${A}/true/;s/x${B}/false/;1q"`
+  ],[
+    dnl Split the operator from the subversion count if present.
+    m4_bmatch(m4_substr($2,2),
+    [0],[
+      # A count of zero means use the length of the shorter version.
+      # Determine the number of characters in A and B.
+      ax_compare_version_len_A=`echo "$A" | $AWK '{print(length)}'`
+      ax_compare_version_len_B=`echo "$B" | $AWK '{print(length)}'`
+
+      # Set A to no more than B's length and B to no more than A's length.
+      A=`echo "$A" | sed "s/\(.\{$ax_compare_version_len_B\}\).*/\1/"`
+      B=`echo "$B" | sed "s/\(.\{$ax_compare_version_len_A\}\).*/\1/"`
+    ],
+    [[0-9]+],[
+      # A count greater than zero means use only that many subversions
+      A=`echo "$A" | sed "s/\(\([[0-9]]\{4\}\)\{m4_substr($2,2)\}\).*/\1/"`
+      B=`echo "$B" | sed "s/\(\([[0-9]]\{4\}\)\{m4_substr($2,2)\}\).*/\1/"`
+    ],
+    [.+],[
+      AC_WARNING(
+        [invalid OP numeric parameter: $2])
+    ],[])
+
+    # Pad zeros at end of numbers to make same length.
+    ax_compare_version_tmp_A="$A`echo $B | sed 's/./0/g'`"
+    B="$B`echo $A | sed 's/./0/g'`"
+    A="$ax_compare_version_tmp_A"
+
+    # Check for equality or inequality as necessary.
+    m4_case(m4_tolower(m4_substr($2,0,2)),
+    [eq],[
+      test "x$A" = "x$B" && ax_compare_version=true
+    ],
+    [ne],[
+      test "x$A" != "x$B" && ax_compare_version=true
+    ],[
+      AC_WARNING([invalid OP parameter: $2])
+    ])
+  ])
+
+  AS_VAR_POPDEF([A])dnl
+  AS_VAR_POPDEF([B])dnl
+
+  dnl # Execute ACTION-IF-TRUE / ACTION-IF-FALSE.
+  if test "$ax_compare_version" = "true" ; then
+    m4_ifvaln([$4],[$4],[:])dnl
+    m4_ifvaln([$5],[else $5])dnl
+  fi
+]) dnl AX_COMPARE_VERSION