321 lines
12 KiB
Diff
321 lines
12 KiB
Diff
|
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",
|