commit 8596e298f761c32cecff45424f5242cd14269292
Author: Zack Weinberg <zackw@panix.com>
Date: Tue Aug 7 21:35:12 2018 -0400
Add configure option --disable-failure-tokens.
When this option is given, crypt and crypt_r will return NULL on
failure, instead of a special "failure token" string that isn't the
hash of any passphrase. This was the historical behavior of glibc,
FreeBSD libc, and several other implementations.
diff --git a/configure.ac b/configure.ac
index a22a5926bd82f729..23651f9c5c886107 100644
--- a/configure.ac
+++ b/configure.ac
@@ -152,6 +152,25 @@ AC_CHECK_FUNCS_ONCE([
])
# Configure options.
+AC_ARG_ENABLE([failure-tokens],
+ AS_HELP_STRING(
+ [--disable-failure-tokens],
+ [Make crypt and crypt_r return NULL on failure, instead of a
+ special "failure token" string that isn't the hash of any
+ passphrase. This matches the behavior of several other
+ crypt implementations, but will break programs that assume these
+ functions never return NULL. crypt_rn and crypt_ra are not affected
+ by this option, and will always return NULL on failure.]
+ ),
+ [case "$enableval" in
+ yes) enable_failure_tokens=1;;
+ no) enable_failure_tokens=0;;
+ *) AC_MSG_ERROR([bad value ${enableval} for --enable-failure-tokens]);;
+ esac],
+ [enable_failure_tokens=1])
+AC_DEFINE_UNQUOTED([ENABLE_FAILURE_TOKENS], [$enable_failure_tokens],
+ [Define to 1 if crypt and crypt_r should return a "failure token" on
+ failure, or 0 if they should return NULL.])
AC_ARG_ENABLE([obsolete-api],
AS_HELP_STRING(
diff --git a/crypt.c b/crypt.c
index 9a3e19214e613097..839763afad14eaa9 100644
--- a/crypt.c
+++ b/crypt.c
@@ -235,7 +235,11 @@ crypt_r (const char *phrase, const char *setting, struct crypt_data *data)
{
make_failure_token (setting, data->output, sizeof data->output);
do_crypt (phrase, setting, data);
+#if ENABLE_FAILURE_TOKENS
return data->output;
+#else
+ return data->output[0] == '*' ? 0 : data->output;
+#endif
}
SYMVER_crypt_r;
#endif
diff --git a/crypt_rn.3 b/crypt_rn.3
index 24da44cfce19716b..d021c4ed4a046e04 100644
--- a/crypt_rn.3
+++ b/crypt_rn.3
@@ -204,17 +204,31 @@ multiple threads simultaneously, as long as a separate
object is used for each thread.
.PP
Upon error,
-.B crypt
-and
-.B crypt_r
-return a pointer to an
+.BR crypt_r ", " crypt_rn ", and " crypt_ra
+write an
.I invalid
-hashed passphrase.
+hashed passphrase to the
+.I output
+field of their
+.I crypt_data
+object, and
+.B crypt
+writes an invalid hash to its static storage area.
This string will be shorter than 13 characters,
will begin with a \(oq\fB*\fR\(cq,
and will not compare equal to
.IR setting .
-(This peculiar behavior is for compatibility
+.PP
+Upon error,
+.BR crypt_rn " and " crypt_ra
+return a null pointer.
+.BR crypt_r " and " crypt
+may also return a null pointer,
+or they may return a pointer to the invalid hash,
+depending on how
+.I libcrypt
+was configured.
+(The option to return the invalid hash is for compatibility
with old applications that assume that
.B crypt
cannot return a null pointer.
@@ -222,15 +236,6 @@ See
.B "PORTABILITY NOTES"
below.)
.PP
-.B crypt_rn
-and
-.B crypt_ra
-also write an invalid hashed passphrase to the
-.I output
-field of their
-.I crypt_data
-object when they fail, but they return a null pointer.
-.PP
All four functions set
.I errno
when they fail.
diff --git a/test-badsalt.c b/test-badsalt.c
index b2743373628b1f3f..3d2e47ac0e7647bd 100644
--- a/test-badsalt.c
+++ b/test-badsalt.c
@@ -222,12 +222,28 @@ check_crypt (const char *label, const char *fn,
const char *retval, const char *setting,
bool expected_to_succeed)
{
- /* crypt/crypt_r should never return null */
+#if ENABLE_FAILURE_TOKENS
+ /* crypt/crypt_r never return null when failure tokens are enabled */
if (!retval)
{
printf ("FAIL: %s/%s/%s: returned NULL\n", label, setting, fn);
return false;
}
+#else
+ if (expected_to_succeed && !retval)
+ {
+ printf ("FAIL: %s/%s/%s: returned NULL\n", label, setting, fn);
+ return false;
+ }
+ else if (!expected_to_succeed && retval)
+ {
+ printf ("FAIL: %s/%s/%s: returned %p, should be NULL\n",
+ label, setting, fn, (const void *)retval);
+ return false;
+ }
+ else if (!expected_to_succeed && !retval)
+ return true;
+#endif
if (!check_results (label, fn, retval, setting,
expected_to_succeed))
return false;
diff --git a/test-crypt-badargs.c b/test-crypt-badargs.c
index 0e6af1626a605086..6be24a99ca7f9015 100644
--- a/test-crypt-badargs.c
+++ b/test-crypt-badargs.c
@@ -169,6 +169,14 @@ test_crypt_ra (const char *tag,
check (tag, expect, got);
}
+#if ENABLE_FAILURE_TOKENS
+# define FT0 "*0"
+# define FT1 "*1"
+#else
+# define FT0 0
+# define FT1 0
+#endif
+
/* PAGE should point to PAGESIZE bytes of read-write memory followed
by another PAGESIZE bytes of inaccessible memory. */
@@ -187,55 +195,55 @@ do_tests(char *page, size_t pagesize)
size_t i;
/* When SETTING is null, it shouldn't matter what PHRASE is. */
- expect_no_fault ("0.0.crypt", 0, 0, "*0", test_crypt);
- expect_no_fault ("0.0.crypt_r", 0, 0, "*0", test_crypt_r);
+ expect_no_fault ("0.0.crypt", 0, 0, FT0, test_crypt);
+ expect_no_fault ("0.0.crypt_r", 0, 0, FT0, test_crypt_r);
expect_no_fault ("0.0.crypt_rn", 0, 0, 0, test_crypt_rn);
expect_no_fault ("0.0.crypt_ra", 0, 0, 0, test_crypt_ra);
- expect_no_fault ("''.0.crypt", "", 0, "*0", test_crypt);
- expect_no_fault ("''.0.crypt_r", "", 0, "*0", test_crypt_r);
+ expect_no_fault ("''.0.crypt", "", 0, FT0, test_crypt);
+ expect_no_fault ("''.0.crypt_r", "", 0, FT0, test_crypt_r);
expect_no_fault ("''.0.crypt_rn", "", 0, 0, test_crypt_rn);
expect_no_fault ("''.0.crypt_ra", "", 0, 0, test_crypt_ra);
- expect_no_fault ("ph.0.crypt", phrase, 0, "*0", test_crypt);
- expect_no_fault ("ph.0.crypt_r", phrase, 0, "*0", test_crypt_r);
+ expect_no_fault ("ph.0.crypt", phrase, 0, FT0, test_crypt);
+ expect_no_fault ("ph.0.crypt_r", phrase, 0, FT0, test_crypt_r);
expect_no_fault ("ph.0.crypt_rn", phrase, 0, 0, test_crypt_rn);
expect_no_fault ("ph.0.crypt_ra", phrase, 0, 0, test_crypt_ra);
- expect_no_fault ("p1.0.crypt", p1, 0, "*0", test_crypt);
- expect_no_fault ("p1.0.crypt_r", p1, 0, "*0", test_crypt_r);
+ expect_no_fault ("p1.0.crypt", p1, 0, FT0, test_crypt);
+ expect_no_fault ("p1.0.crypt_r", p1, 0, FT0, test_crypt_r);
expect_no_fault ("p1.0.crypt_rn", p1, 0, 0, test_crypt_rn);
expect_no_fault ("p1.0.crypt_ra", p1, 0, 0, test_crypt_ra);
- expect_no_fault ("p2.0.crypt", p2, 0, "*0", test_crypt);
- expect_no_fault ("p2.0.crypt_r", p2, 0, "*0", test_crypt_r);
+ expect_no_fault ("p2.0.crypt", p2, 0, FT0, test_crypt);
+ expect_no_fault ("p2.0.crypt_r", p2, 0, FT0, test_crypt_r);
expect_no_fault ("p2.0.crypt_rn", p2, 0, 0, test_crypt_rn);
expect_no_fault ("p2.0.crypt_ra", p2, 0, 0, test_crypt_ra);
/* Conversely, when PHRASE is null,
it shouldn't matter what SETTING is... */
- expect_no_fault ("0.''.crypt", 0, "", "*0", test_crypt);
- expect_no_fault ("0.''.crypt_r", 0, "", "*0", test_crypt_r);
+ expect_no_fault ("0.''.crypt", 0, "", FT0, test_crypt);
+ expect_no_fault ("0.''.crypt_r", 0, "", FT0, test_crypt_r);
expect_no_fault ("0.''.crypt_rn", 0, "", 0, test_crypt_rn);
expect_no_fault ("0.''.crypt_ra", 0, "", 0, test_crypt_ra);
- expect_no_fault ("0.'*'.crypt", 0, "*", "*0", test_crypt);
- expect_no_fault ("0.'*'.crypt_r", 0, "*", "*0", test_crypt_r);
+ expect_no_fault ("0.'*'.crypt", 0, "*", FT0, test_crypt);
+ expect_no_fault ("0.'*'.crypt_r", 0, "*", FT0, test_crypt_r);
expect_no_fault ("0.'*'.crypt_rn", 0, "*", 0, test_crypt_rn);
expect_no_fault ("0.'*'.crypt_ra", 0, "*", 0, test_crypt_ra);
- expect_no_fault ("0.'*0'.crypt", 0, "*0", "*1", test_crypt);
- expect_no_fault ("0.'*0'.crypt_r", 0, "*0", "*1", test_crypt_r);
+ expect_no_fault ("0.'*0'.crypt", 0, "*0", FT1, test_crypt);
+ expect_no_fault ("0.'*0'.crypt_r", 0, "*0", FT1, test_crypt_r);
expect_no_fault ("0.'*0'.crypt_rn", 0, "*0", 0, test_crypt_rn);
expect_no_fault ("0.'*0'.crypt_ra", 0, "*0", 0, test_crypt_ra);
- expect_no_fault ("0.'*1'.crypt", 0, "*1", "*0", test_crypt);
- expect_no_fault ("0.'*1'.crypt_r", 0, "*1", "*0", test_crypt_r);
+ expect_no_fault ("0.'*1'.crypt", 0, "*1", FT0, test_crypt);
+ expect_no_fault ("0.'*1'.crypt_r", 0, "*1", FT0, test_crypt_r);
expect_no_fault ("0.'*1'.crypt_rn", 0, "*1", 0, test_crypt_rn);
expect_no_fault ("0.'*1'.crypt_ra", 0, "*1", 0, test_crypt_ra);
- expect_no_fault ("0.p1.crypt", 0, p1, "*0", test_crypt);
- expect_no_fault ("0.p1.crypt_r", 0, p1, "*0", test_crypt_r);
+ expect_no_fault ("0.p1.crypt", 0, p1, FT0, test_crypt);
+ expect_no_fault ("0.p1.crypt_r", 0, p1, FT0, test_crypt_r);
expect_no_fault ("0.p1.crypt_rn", 0, p1, 0, test_crypt_rn);
expect_no_fault ("0.p1.crypt_ra", 0, p1, 0, test_crypt_ra);
@@ -245,8 +253,8 @@ do_tests(char *page, size_t pagesize)
bug, but it's impractical to fix without breaking the property
that 'crypt' _never_ creates a failure token that is equal to the
setting string, which is more important than this corner case. */
- expect_a_fault ("0.p2.crypt", 0, p2, "*0", test_crypt);
- expect_a_fault ("0.p2.crypt_r", 0, p2, "*0", test_crypt_r);
+ expect_a_fault ("0.p2.crypt", 0, p2, FT0, test_crypt);
+ expect_a_fault ("0.p2.crypt_r", 0, p2, FT0, test_crypt_r);
expect_a_fault ("0.p2.crypt_rn", 0, p2, 0, test_crypt_rn);
expect_a_fault ("0.p2.crypt_ra", 0, p2, 0, test_crypt_ra);
@@ -257,9 +265,9 @@ do_tests(char *page, size_t pagesize)
strcpy (page, "p1.'");
strcat (page, settings[i]);
strcat (page, "'.crypt");
- expect_a_fault (page, p1, settings[i], "*0", test_crypt);
+ expect_a_fault (page, p1, settings[i], FT0, test_crypt);
strcat (page, "_r");
- expect_a_fault (page, p1, settings[i], "*0", test_crypt_r);
+ expect_a_fault (page, p1, settings[i], FT0, test_crypt_r);
strcat (page, "n");
expect_a_fault (page, p1, settings[i], 0, test_crypt_rn);
page [strlen (page) - 1] = 'a';
@@ -268,9 +276,9 @@ do_tests(char *page, size_t pagesize)
strcpy (page, "p2.'");
strcat (page, settings[i]);
strcat (page, "'.crypt");
- expect_a_fault (page, p2, settings[i], "*0", test_crypt);
+ expect_a_fault (page, p2, settings[i], FT0, test_crypt);
strcat (page, "_r");
- expect_a_fault (page, p2, settings[i], "*0", test_crypt_r);
+ expect_a_fault (page, p2, settings[i], FT0, test_crypt_r);
strcat (page, "n");
expect_a_fault (page, p2, settings[i], 0, test_crypt_rn);
page [strlen (page) - 1] = 'a';
@@ -279,8 +287,8 @@ do_tests(char *page, size_t pagesize)
/* Conversely, when PHRASE is valid, passing an invalid string as SETTING
should crash reliably. */
- expect_a_fault ("ph.p2.crypt", phrase, p2, "*0", test_crypt);
- expect_a_fault ("ph.p2.crypt_r", phrase, p2, "*0", test_crypt_r);
+ expect_a_fault ("ph.p2.crypt", phrase, p2, FT0, test_crypt);
+ expect_a_fault ("ph.p2.crypt_r", phrase, p2, FT0, test_crypt_r);
expect_a_fault ("ph.p2.crypt_rn", phrase, p2, 0, test_crypt_rn);
expect_a_fault ("ph.p2.crypt_ra", phrase, p2, 0, test_crypt_ra);
@@ -292,9 +300,9 @@ do_tests(char *page, size_t pagesize)
strcpy (page, "ph.'");
strcat (page, settings[i]);
strcat (page, ".crypt");
- expect_a_fault (page, phrase, p1, "*0", test_crypt);
+ expect_a_fault (page, phrase, p1, FT0, test_crypt);
strcat (page, "_r");
- expect_a_fault (page, phrase, p1, "*0", test_crypt_r);
+ expect_a_fault (page, phrase, p1, FT0, test_crypt_r);
strcat (page, "n");
expect_a_fault (page, phrase, p1, 0, test_crypt_rn);
page [strlen (page) - 1] = 'a';
diff --git a/test-crypt-bcrypt.c b/test-crypt-bcrypt.c
index c984e4d47d8df2c6..bf149b405bd408c7 100644
--- a/test-crypt-bcrypt.c
+++ b/test-crypt-bcrypt.c
@@ -194,8 +194,12 @@ main (void)
errno = 0;
p = crypt (key, setting);
errnm = errno;
+#if ENABLE_FAILURE_TOKENS
match = strcmp (p, hash);
- if ((!ok && !errno) || strcmp (p, hash))
+#else
+ match = (ok ? strcmp (p, hash) : p != 0);
+#endif
+ if ((!ok && !errno) || match)
{
printf ("FAIL: %d/crypt.1: key=%s setting=%s: xhash=%s xerr=%d, "
"p=%s match=%d err=%s\n",