rpms/libxcrypt
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Remove unused patches

Browse Source
This commit is contained in:
Björn Esser 2018-11-11 16:33:29 +01:00
parent b11249655f
commit 31d7e3a133
No known key found for this signature in database
GPG Key ID: F52E98007594C21D
2 changed files with 0 additions and 418 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

378
libxcrypt-4.3.1-add_test_gensalt_extradata.patch
View File

@ -1,378 +0,0 @@
From ea4d762cab0649dcbdac2de19dcc49dcc89c009d Mon Sep 17 00:00:00 2001
From: Zack Weinberg <zackw@panix.com>
Date: Sat, 10 Nov 2018 20:28:38 -0500
Subject: [PATCH] New test: crypt_gensalt should look only at the prefix.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
For quite some time there’s been a chunk of code at the end of
test-gensalt.c labeled
/* FIXME: This test is a little too specific. It used to be in
test-bcrypt.c and I'm not sure what it's meant to be testing. */
I finally figured out what the point of this code is, which means I
can generalize it from $2a$-variant bcrypt to all hashing methods.
It’s testing that, when you supply a full setting string (or even a
full hashed passphrase) as the “prefix” argument to crypt_gensalt,
crypt_gensalt does not use any information from that string other than
the method-selection prefix. (For instance, if you give it "$2a$05"
but a cost of 12, you should get back a string beginning "$2a$12".)
I wasn’t expecting the generalized test to detect any bugs, because in
libxcrypt, individual hashing methods’ gensalt procedures don’t get to
see the “prefix” string at all. But it did! Turns out the salt
generation for NTHASH fails to NUL-terminate its output properly and
we never noticed till now (mostly because nobody uses that thing, I
hope). So that’s fixed and there’s extra code in the new test to make
sure we notice if the same mistake ever pops up again in the future.
---
Makefile.am | 2 +
crypt-nthash.c | 2 +-
test-badsetting.c | 3 +-
test-gensalt-extradata.c | 242 +++++++++++++++++++++++++++++++++++++++
test-gensalt.c | 32 ------
5 files changed, 246 insertions(+), 35 deletions(-)
create mode 100644 test-gensalt-extradata.c
diff --git a/Makefile.am b/Makefile.am
index fa41938..0eecf0f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -180,6 +180,7 @@ check_PROGRAMS = \
test-crypt-pbkdf1-sha1 test-crypt-scrypt test-crypt-sha256 \
test-crypt-sha512 test-crypt-sunmd5 test-crypt-yescrypt \
test-byteorder test-badsalt test-badsetting test-gensalt \
+ test-gensalt-extradata \
test-crypt-badargs test-short-outbuf test-compile-strong-alias \
test-getrandom-interface test-getrandom-fallbacks
@@ -225,6 +226,7 @@ test_crypt_yescrypt_LDADD = crypt-common.lo libcrypt.la
test_badsalt_LDADD = crypt-common.lo libcrypt.la
test_badsetting_LDADD = crypt-common.lo libcrypt.la
test_gensalt_LDADD = crypt-common.lo libcrypt.la
+test_gensalt_extradata_LDADD = crypt-common.lo libcrypt.la
test_checksalt_LDADD = crypt-common.lo libcrypt.la
test_des_obsolete_LDADD = crypt-common.lo libcrypt.la
test_des_obsolete_r_LDADD = crypt-common.lo libcrypt.la
diff --git a/crypt-nthash.c b/crypt-nthash.c
index 78fdd63..5ae7842 100644
--- a/crypt-nthash.c
+++ b/crypt-nthash.c
@@ -144,7 +144,7 @@ gensalt_nt_rn (unsigned long count,
sprintf (&(hashstr[i * 2]), "%02x", hashbuf[i]);
memcpy (output, salt, 15);
- memcpy (output + 15, hashstr, 14);
+ memcpy (output + 15, hashstr, 14+1);
}
#endif
diff --git a/test-badsetting.c b/test-badsetting.c
index 8a96cd9..20b3d3e 100644
--- a/test-badsetting.c
+++ b/test-badsetting.c
@@ -8,11 +8,10 @@
details. */
#include "crypt-port.h"
-#include <crypt.h>
+
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
-#include <string.h>
/* Supply 64 bytes of "random" data to each gensalt call, for
determinism. */
diff --git a/test-gensalt-extradata.c b/test-gensalt-extradata.c
new file mode 100644
index 0000000..27b87eb
--- /dev/null
+++ b/test-gensalt-extradata.c
@@ -0,0 +1,242 @@
+/* Test that the prefix argument to crypt_gensalt affects only the
+ choice of hashing method, not any of the parameters or the salt.
+
+ Written by Zack Weinberg <zackw at panix.com> in 2018.
+ To the extent possible under law, Zack Weinberg has waived all
+ copyright and related or neighboring rights to this work.
+
+ See https://creativecommons.org/publicdomain/zero/1.0/ for further
+ details. */
+
+#include "crypt-port.h"
+
+#include <errno.h>
+#include <stdio.h>
+
+/* Random bytes used when calling crypt_gensalt; for determinism, these
+ are fixed from run to run. */
+#define N_RBYTES 64ul
+
+static const char rbytes1[] =
+ "90idUkI2+mu2E/tMTViD418j2sPdEYq9LYq0yRW7RYhr4RqQ+oVzIIEcfJBqpf/D";
+
+static const char rbytes2[] =
+ "sEwXQxrjBTEADauxCpvOQqq7iU9oq6uJ+Iux/fbhtLRj1MWgBFyo/t+nh/nzm0Kn";
+
+static_assert(sizeof rbytes1 == N_RBYTES + 1, "rbytes1 is wrong length");
+static_assert(sizeof rbytes2 == N_RBYTES + 1, "rbytes2 is wrong length");
+
+struct testcase
+{
+ const char prefix[8];
+ unsigned long count1;
+ unsigned long count2;
+};
+
+/* This list should include one entry for each potentially-supported
+ hash prefix. If the hash method has tunable cost, set count1 and
+ count2 to two different nonzero values, within the supported cost
+ range. Neither value should equal the default cost. If the hash
+ method does not have tunable cost, set count1 and count2 to zero. */
+static const struct testcase testcases[] =
+{
+#if INCLUDE_descrypt || INCLUDE_bigcrypt
+ { "", 0, 0 },
+#endif
+#if INCLUDE_bsdicrypt
+ { "_", 7019, 1120211 },
+#endif
+#if INCLUDE_nt
+ { "$3$", 0, 0 },
+#endif
+#if INCLUDE_md5crypt
+ { "$1$", 0, 0 },
+#endif
+#if INCLUDE_sunmd5
+ { "$md5", 7019, 1120211 },
+#endif
+#if INCLUDE_sha1crypt
+ { "$sha1", 7019, 1120211 },
+#endif
+#if INCLUDE_sha256crypt
+ { "$5$", 7019, 1120211 },
+#endif
+#if INCLUDE_sha512crypt
+ { "$6$", 7019, 1120211 },
+#endif
+#if INCLUDE_bcrypt
+ { "$2b$", 7, 11 },
+#endif
+#if INCLUDE_bcrypt_y
+ { "$2y$", 7, 11 },
+#endif
+#if INCLUDE_bcrypt_a
+ { "$2a$", 7, 11 },
+#endif
+#if INCLUDE_bcrypt_x
+ { "$2x$", 7, 11 },
+#endif
+#if INCLUDE_scrypt
+ { "$7$", 7, 11, },
+#endif
+#if INCLUDE_yescrypt
+ { "$y$", 7, 11, },
+#endif
+#if INCLUDE_gost_yescrypt
+ { "$gy$", 7, 11, },
+#endif
+};
+
+static int
+do_crypt_gensalt(const char *prefix,
+ const char rbytes[MIN_SIZE(N_RBYTES)],
+ unsigned long count,
+ char outbuf[MIN_SIZE(CRYPT_GENSALT_OUTPUT_SIZE)])
+{
+ /* Detect failure to NUL-terminate the output properly. */
+ static int ncalls = 0;
+ memset(outbuf, '!' + (ncalls % ('~' - '!' + 1)),
+ CRYPT_GENSALT_OUTPUT_SIZE - 1);
+ outbuf[CRYPT_GENSALT_OUTPUT_SIZE - 1] = 0;
+ ncalls++;
+
+ char *rv = crypt_gensalt_rn(prefix, count, rbytes, N_RBYTES,
+ outbuf, CRYPT_GENSALT_OUTPUT_SIZE);
+ if (rv == 0)
+ {
+ printf("ERROR: gensalt(%s, %lu, %c%c..., %lu, outbuf, %lu) = 0/%s\n",
+ prefix, count, rbytes[0], rbytes[1],
+ N_RBYTES, (unsigned long)CRYPT_GENSALT_OUTPUT_SIZE,
+ strerror(errno));
+ outbuf[0] = '*';
+ memset (outbuf+1, 0, CRYPT_GENSALT_OUTPUT_SIZE-1);
+ return 1;
+ }
+ else if (rv[0] == '*')
+ {
+ printf("ERROR: gensalt(%s, %lu, %c%c..., %lu, outbuf, %lu) = %s/%s\n",
+ prefix, count, rbytes[0], rbytes[1],
+ N_RBYTES, (unsigned long)CRYPT_GENSALT_OUTPUT_SIZE,
+ outbuf, strerror(errno));
+ outbuf[0] = '*';
+ memset (outbuf+1, 0, CRYPT_GENSALT_OUTPUT_SIZE-1);
+ return 1;
+ }
+ else
+ return 0;
+}
+
+static int
+do_check_equal(const char *stst, const char *sref,
+ const char *prefix, const char rbytes[N_RBYTES],
+ unsigned long count, const char *setting)
+{
+ if (!strcmp(stst, sref))
+ return 0;
+
+ printf("FAIL: expected %s\n"
+ " got %s\n"
+ " from %s, %lu, %c%c...\n"
+ " and %s\n",
+ sref, stst, prefix, count, rbytes[0], rbytes[1], setting);
+ return 1;
+}
+
+int
+main(void)
+{
+ int status = 0;
+ char sref[6][CRYPT_GENSALT_OUTPUT_SIZE];
+ char stst[CRYPT_GENSALT_OUTPUT_SIZE];
+
+ for (size_t i = 0; i < ARRAY_SIZE(testcases); i++)
+ {
+ const char *prefix = testcases[i].prefix;
+ unsigned long count1 = testcases[i].count1;
+ unsigned long count2 = testcases[i].count2;
+ int ncases;
+
+ memset(sref, 0, sizeof sref);
+
+ /* If count1 and count2 are both nonzero, then they should also
+ be unequal, and we have six reference cases:
+ (0, count1, count2) x (rbytes1, rbytes2).
+ If count1 and count2 are both zero, then we only have two
+ reference cases: 0 x (rbytes1, rbytes2) (this happens when the
+ hash method doesn't have tunable cost).
+ It is incorrect for only one of count1 and count2 to be zero,
+ or for them to be equal but nonzero. */
+ if (count1 == 0 && count2 == 0)
+ {
+ ncases = 2;
+ status |= do_crypt_gensalt(prefix, rbytes1, 0, sref[0]);
+ status |= do_crypt_gensalt(prefix, rbytes2, 0, sref[1]);
+ }
+ else if (count1 != 0 && count2 != 0 && count1 != count2)
+ {
+ ncases = 6;
+ status |= do_crypt_gensalt(prefix, rbytes1, 0, sref[0]);
+ status |= do_crypt_gensalt(prefix, rbytes2, 0, sref[1]);
+ status |= do_crypt_gensalt(prefix, rbytes1, count1, sref[2]);
+ status |= do_crypt_gensalt(prefix, rbytes2, count1, sref[3]);
+ status |= do_crypt_gensalt(prefix, rbytes1, count2, sref[4]);
+ status |= do_crypt_gensalt(prefix, rbytes2, count2, sref[5]);
+ }
+ else
+ {
+ printf ("ERROR: %zu/%s: inappropriate count1=%lu count2=%lu\n",
+ i, prefix, count1, count2);
+ status = 1;
+ continue;
+ }
+
+ /* At this point, sref[0..ncases] are filled with setting
+ strings corresponding to different combinations of salt and
+ cost. If we reuse those strings as prefixes for crypt_gensalt,
+ none of the additional information should affect the output. */
+ for (int j = 0; j < ncases; j++)
+ {
+ if (sref[j][0] == '*')
+ continue; /* initial crypt_gensalt call failed */
+ if (count1 == 0 && count2 == 0)
+ {
+ status |= do_crypt_gensalt(sref[j], rbytes1, 0, stst);
+ status |= do_check_equal(stst, sref[0],
+ prefix, rbytes1, 0, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes2, 0, stst);
+ status |= do_check_equal(stst, sref[1],
+ prefix, rbytes2, 0, sref[j]);
+ }
+ else
+ {
+ status |= do_crypt_gensalt(sref[j], rbytes1, 0, stst);
+ status |= do_check_equal(stst, sref[0],
+ prefix, rbytes1, 0, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes2, 0, stst);
+ status |= do_check_equal(stst, sref[1],
+ prefix, rbytes2, 0, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes1, count1, stst);
+ status |= do_check_equal(stst, sref[2],
+ prefix, rbytes1, count1, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes2, count1, stst);
+ status |= do_check_equal(stst, sref[3],
+ prefix, rbytes2, count1, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes1, count2, stst);
+ status |= do_check_equal(stst, sref[4],
+ prefix, rbytes1, count2, sref[j]);
+
+ status |= do_crypt_gensalt(sref[j], rbytes2, count2, stst);
+ status |= do_check_equal(stst, sref[5],
+ prefix, rbytes2, count2, sref[j]);
+ }
+ }
+
+ }
+
+ return status;
+}
diff --git a/test-gensalt.c b/test-gensalt.c
index 2099cd2..0ef3809 100644
--- a/test-gensalt.c
+++ b/test-gensalt.c
@@ -556,37 +556,5 @@ main (void)
free (setting);
}
#endif
-
-#if INCLUDE_bcrypt_a
- /* FIXME: This test is a little too specific. It used to be in
- test-bcrypt.c and I'm not sure what it's meant to be testing. */
- {
- char *setting1, *setting2;
- const char *which = "$2a$05$CCCCCCCCCCCCCCCCCCCCC.E5YPO9kmyuRGyh0XouQYb4YMJKvyOeW";
- setting1 = crypt_gensalt (which, 12, "CCCCCCCCCCCCCCCCCCCCC", 21);
- if (!setting1 || strncmp (setting1, "$2a$12$", 7))
- {
- printf ("FAILED (crypt_gensalt: wrong prefix) s1=%s\n", setting1);
- status = 1;
- }
-
- setting2 = crypt_gensalt_ra (setting1, 12, "CCCCCCCCCCCCCCCCCCCCC", 21);
- if (strcmp (setting1, setting2))
- {
- printf ("FAILED (crypt_gensalt_ra/1: s1=%s s2=%s)\n", setting1, setting2);
- status = 1;
- }
-
- setting1 = crypt_gensalt_ra (setting2, 12, "DCCCCCCCCCCCCCCCCCCCC", 21);
- if (!strcmp (setting1, setting2))
- {
- printf ("FAILED (crypt_gensalt_ra/2: s1=%s s2=%s)\n", setting1, setting2);
- status = 1;
- }
-
- free (setting1);
- free (setting2);
- }
-#endif
return status;
}

40
libxcrypt-4.3.1-nthash_output_buffer_gensalt.patch
View File

@ -1,40 +0,0 @@
From 0a58300cdea4733afad31df8a4ff283f3b42caca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= <besser82@fedoraproject.org>
Date: Sun, 11 Nov 2018 12:25:50 +0100
Subject: [PATCH] nthash: The output buffer for gensalt must be at least 30
bytes long.
The size of the buffer provided by 'o_size' must be at least 30 bytes
long to fit the terminating null byte.
Also use 'XCRYPT_STRCPY_OR_ABORT' over plain 'memcpy', since it is
the preferred method to copy strings.
---
crypt-nthash.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/crypt-nthash.c b/crypt-nthash.c
index 5ae7842..bb7c1ff 100644
--- a/crypt-nthash.c
+++ b/crypt-nthash.c
@@ -119,7 +119,7 @@ gensalt_nt_rn (unsigned long count,
At least 1 byte of RBYTES is needed
to calculate the MD4 hash used in the
fake salt. */
- if ((o_size < 29) || (nrbytes < 1))
+ if ((o_size < 30) || (nrbytes < 1))
{
errno = ERANGE;
return;
@@ -142,9 +142,10 @@ gensalt_nt_rn (unsigned long count,
for (i = 0; i < 7; i++)
sprintf (&(hashstr[i * 2]), "%02x", hashbuf[i]);
+ hashstr[14] = '\0';
- memcpy (output, salt, 15);
- memcpy (output + 15, hashstr, 14+1);
+ XCRYPT_STRCPY_OR_ABORT (output, o_size, salt);
+ XCRYPT_STRCPY_OR_ABORT (output + 15, o_size - 15, hashstr);
}
#endif