From d62f55fdbccc7911ef17f93cc56189ac7b360859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Esser?= Date: Mon, 4 Feb 2019 22:46:40 +0100 Subject: [PATCH 1/3] test-crypt-kat: Add cosistency test for fcrypt. --- crypt-port.h | 5 ++--- test-crypt-kat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/crypt-port.h b/crypt-port.h index 063d12d0..44d278f1 100644 --- a/crypt-port.h +++ b/crypt-port.h @@ -381,9 +381,8 @@ extern void crypt_yescrypt_rn (const char *, size_t, const char *, size_t, uint8_t *, size_t, void *, size_t); #endif -/* We need a prototype for fcrypt, when building with stubs for the - deprecated and unsafe functions. */ -#if ENABLE_OBSOLETE_API_ENOSYS +/* We need a prototype for fcrypt for some tests. */ +#if ENABLE_OBSOLETE_API char *fcrypt (const char *key, const char *setting); #endif diff --git a/test-crypt-kat.c b/test-crypt-kat.c index 206c5181..52bd2d90 100644 --- a/test-crypt-kat.c +++ b/test-crypt-kat.c @@ -18,6 +18,10 @@ #include #endif +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS +symver_ref("fcrypt", fcrypt, SYMVER_FLOOR); +#endif + /* The precalculated hashes in test-crypt-kat.inc, and some of the relationships among groups of test cases (see test-crypt-kat-gen.py) are invalidated if the execution character set is not ASCII. */ @@ -63,6 +67,9 @@ struct testresult char *h_crypt_rn; char *h_crypt_ra; char *h_recrypt; +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + char *h_fcrypt; +#endif }; /* Summarize the result of a single hashing operation in a format that @@ -252,6 +259,26 @@ calc_hashes_crypt_ra (void *results_) return 0; } +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS +static void * +calc_hashes_fcrypt (void *results_) +{ + struct testresult *results = results_; + char *hash; + size_t i; + + for (i = 0; i < ntests; i++) + { + errno = 0; + hash = fcrypt (tests[i].input, tests[i].salt); + record_result (&results[i].h_fcrypt, hash, errno, &tests[i], + ENABLE_FAILURE_TOKENS); + } + + return 0; +} +#endif + static void print_escaped (const char *s) { @@ -299,6 +326,9 @@ do_ka_tests (struct testresult *results) } calc_hashes_crypt (results); +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + calc_hashes_fcrypt (results); +#endif err = pthread_join (t_r, 0); if (err) { @@ -321,6 +351,9 @@ do_ka_tests (struct testresult *results) calc_hashes_crypt_r (results); calc_hashes_crypt_rn (results); calc_hashes_crypt_ra (results); +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + calc_hashes_fcrypt (results); +#endif #endif int rv = 0; @@ -352,6 +385,13 @@ do_ka_tests (struct testresult *results) report_ka_error (results[i].h_recrypt, &tests[i], 0, "_rn (recrypt)"); failed = 1; } +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + if (strchr (results[i].h_fcrypt, '!')) + { + report_ka_error (results[i].h_fcrypt, &tests[i], 0, " (fcrypt)"); + failed = 1; + } +#endif if (!failed) { @@ -379,6 +419,14 @@ do_ka_tests (struct testresult *results) results[i].h_crypt, "_rn (recrypt)"); failed = 1; } +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + if (strcmp (results[i].h_recrypt, results[i].h_crypt)) + { + report_ka_error (results[i].h_fcrypt, &tests[i], + results[i].h_crypt, " (fcrypt)"); + failed = 1; + } +#endif /* Tell the collision tests to skip this one if it's inconsistent. */ if (failed) @@ -687,6 +735,9 @@ main (void) free (results[i].h_crypt_rn); free (results[i].h_crypt_ra); free (results[i].h_recrypt); +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + free (results[i].h_fcrypt); +#endif } free (results); From 4fe4be1f2a14ead9f5666f5d748036a6ff64bf2f Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Tue, 5 Feb 2019 18:18:32 -0500 Subject: [PATCH 2/3] Restructure parallelism in test-crypt-kat so fcrypt can be tested. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fcrypt, like crypt, is not thread-safe, so have the main thread call crypt() and then fcrypt() for each test case; meanwhile, another thread calls crypt_r() and then crypt_rn(), and a third thread calls crypt_ra() and then repeats the call with the output hash instead of the original setting string. All are checked solely against the expected hash from test-crypt-kat.inc; transitivity means there’s no point in testing them against each other once they passed that check. Also, errors are reported immediately instead of being buffered up, which means we can scrap a bunch of memory management code. The collisions test, meanwhile, is moved to test-crypt-kat-gen.py. If we know that the only collisions among the *expected* hashes are the ones we anticipate, then checking the freshly generated hashes against the expected hashes is sufficient to detect new collisions. So we don’t need to do any of that work at build time. --- configure.ac | 1 - test-crypt-kat-gen.py | 306 ++++++++++++++---- test-crypt-kat.c | 733 +++++++++--------------------------------- 3 files changed, 381 insertions(+), 659 deletions(-) diff --git a/configure.ac b/configure.ac index a8ddb607..9ff3825f 100644 --- a/configure.ac +++ b/configure.ac @@ -225,7 +225,6 @@ AC_CHECK_FUNCS_ONCE([ memset_s open64 syscall - vasprintf ]) # Disable valgrind tools for checking multithreaded diff --git a/test-crypt-kat-gen.py b/test-crypt-kat-gen.py index c4c15fc6..b6ac3ced 100755 --- a/test-crypt-kat-gen.py +++ b/test-crypt-kat-gen.py @@ -24,6 +24,7 @@ # libxcrypt uses itself, so that we really are testing libxcrypt # against known answers generated with a different implementation.) +import array import ctypes import multiprocessing import os @@ -190,11 +191,10 @@ # # Each shim function takes at least the arguments phrase, rounds, and # salt, in that order. Additional optional arguments are allowed. It -# should do 'yield format_case(phrase, setting, expected)' at least -# once, where phrase is the phrase argument, setting is a setting -# string generated from rounds and salt, and expected is the hashed -# passphrase expected to be generated from that combination of phrase -# and setting. format_case is defined far below. +# should do 'yield (phrase, setting, expected)' at least once, where +# phrase is the phrase argument, setting is a setting string generated +# from rounds and salt, and expected is the hashed passphrase expected +# to be generated from that combination of phrase and setting. # # When implementing new shims, use of passlib's pure-Python "backends" # is strongly preferred where possible, because the speed of this @@ -210,7 +210,7 @@ def h_descrypt(phrase, rounds, salt): salt=salt, truncate_error=False ).hash(phrase) setting = expected[:2] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) BIGCRYPT = passlib.hash.bigcrypt # BIGCRYPT.set_backend("builtin") # currently p.h.bigcrypt always uses builtin @@ -227,7 +227,7 @@ def h_bigcrypt(phrase, rounds, salt): # descrypt. For bigcrypt to be used, the setting must be too long # to be a traditional DES hashed password. setting = expected[:2] + ".............." - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) BSDI_CRYPT = passlib.hash.bsdi_crypt BSDI_CRYPT.set_backend("builtin") @@ -236,7 +236,7 @@ def h_bsdicrypt(phrase, rounds, salt): salt=salt, rounds=rounds ).hash(phrase) setting = expected[:9] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) MD5_CRYPT = passlib.hash.md5_crypt MD5_CRYPT.set_backend("builtin") @@ -245,7 +245,7 @@ def h_md5crypt(phrase, rounds, salt): salt=salt ).hash(phrase) setting = expected[:expected.rfind('$')] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) BSD_NTHASH = passlib.hash.bsd_nthash #BSD_NTHASH.set_backend("builtin") # has only the built-in backend @@ -257,8 +257,8 @@ def h_nt(phrase, rounds, salt): # NTHash doesn't have a salt. # Older versions of libxcrypt generated a fake salt which # we should ensure is ignored. - yield format_case(phrase, "$3$", expected) - yield format_case(phrase, "$3$__not_used__0123456789abcdef", expected) + yield (phrase, "$3$", expected) + yield (phrase, "$3$__not_used__0123456789abcdef", expected) SHA1_CRYPT = passlib.hash.sha1_crypt SHA1_CRYPT.set_backend("builtin") @@ -267,7 +267,7 @@ def h_sha1crypt(phrase, rounds, salt): salt=salt, rounds=rounds ).hash(phrase) setting = expected[:expected.rfind('$')] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) SHA256_CRYPT = passlib.hash.sha256_crypt SHA256_CRYPT.set_backend("builtin") @@ -276,7 +276,7 @@ def h_sha256crypt(phrase, rounds, salt): salt=salt, rounds=rounds ).hash(phrase) setting = expected[:expected.rfind('$')] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) SHA512_CRYPT = passlib.hash.sha512_crypt SHA512_CRYPT.set_backend("builtin") @@ -285,7 +285,7 @@ def h_sha512crypt(phrase, rounds, salt): salt=salt, rounds=rounds ).hash(phrase) setting = expected[:expected.rfind('$')] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) # these need to do more work by hand @@ -308,12 +308,10 @@ def h_sunmd5(phrase, rounds, salt): bare_cksum = bare_cksum.decode("ascii") suff_cksum = suff_cksum.decode("ascii") - yield format_case(phrase, bare_setting, bare_setting + "$" + bare_cksum) - yield format_case(phrase, bare_setting + "$x", - bare_setting + "$" + bare_cksum) - yield format_case(phrase, suff_setting, suff_setting + "$" + suff_cksum) - yield format_case(phrase, suff_setting + "$", - suff_setting + "$" + suff_cksum) + yield (phrase, bare_setting, bare_setting + "$" + bare_cksum) + yield (phrase, bare_setting + "$x", bare_setting + "$" + bare_cksum) + yield (phrase, suff_setting, suff_setting + "$" + suff_cksum) + yield (phrase, suff_setting + "$", suff_setting + "$" + suff_cksum) # testing bcrypt $2b$ and $2y$ is easy, but ... BCRYPT = passlib.hash.bcrypt @@ -323,14 +321,14 @@ def h_bcrypt(phrase, rounds, salt): salt=salt, rounds=rounds, ident="2b" ).hash(phrase) setting = expected[:-31] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) def h_bcrypt_y(phrase, rounds, salt): expected = BCRYPT.using( salt=salt, rounds=rounds, ident="2y" ).hash(phrase) setting = expected[:-31] - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) # ...passlib doesn't implement the quirks of crypt_blowfish's $2a$ or # $2x$, but we really must test them. It is theoretically possible to @@ -363,7 +361,7 @@ def h_bcrypt_a(phrase, rounds, salt): setting = "$2a$" + base_setting expected = setting + output - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) bcrypt_x_substitutions = { 'xGPMyJSPyyeICKolPQ2gecm8rOgHwz.': '.sDifhVkUxvjPx6U4yeM2tC411Wuc.W', @@ -441,7 +439,7 @@ def h_bcrypt_x(phrase, rounds, salt): setting = "$2x$" + base_setting expected = setting + output - yield format_case(phrase, setting, expected) + yield (phrase, setting, expected) # passlib includes an scrypt implementation, but its encoded password # format is not the $7$ format we implement, so instead we use the @@ -468,8 +466,7 @@ def h_scrypt(phrase, rounds, salt): binhash = raw_scrypt(phrase, salt=bytesalt, p=p, r=r, n=N, dklen=32) - yield format_case(phrase, setting, - setting + b'$' + hash64.encode_bytes(binhash)) + yield (phrase, setting, setting + b'$' + hash64.encode_bytes(binhash)) # # passlib does not support either yescrypt or gost-yescrypt. In fact, @@ -523,11 +520,11 @@ def yescrypt_gensalt(ident, rounds, salt): def h_yescrypt(phrase, rounds, salt): setting = yescrypt_gensalt("y", rounds, salt) - yield format_case(phrase, setting, xcrypt_crypt(phrase, setting)) + yield (phrase, setting, xcrypt_crypt(phrase, setting)) def h_gost_yescrypt(phrase, rounds, salt): setting = yescrypt_gensalt("gy", rounds, salt) - yield format_case(phrase, setting, xcrypt_crypt(phrase, setting)) + yield (phrase, setting, xcrypt_crypt(phrase, setting)) # Each method should contribute a group of parameters to the array # below. Each block has the form @@ -660,6 +657,177 @@ def h_gost_yescrypt(phrase, rounds, salt): ]), ] +# Normally, we expect that (1) for fixed salt, no two phrases hash to +# the same string; (2) for fixed phrase, no two settings produce the +# same string. The known exceptions are all due to limitations and/or +# bugs in the hashing method. Check the table produced by this +# program to ensure that all of the collisions in the ->expected +# strings are due to one of the known exceptions. test-crypt-kat.c +# itself doesn't need to do this test; as long as all of the hashes +# produced by the just-built crypt() match the appropriate ->expected +# string, no new collisions can have been introduced. + +def strneq_7bit (p1, p2, limit): + n1 = len(p1) + n2 = len(p2) + for i in range(limit): + if i == n1 and i == n2: + # strings are the same length, within the limit, and no + # mismatched characters were found + return True + if i == n1 or i == n2: + # one string is longer than the other, within the limit + return False + if (p1[i] & 0x7F) != (p2[i] & 0x7F): + # characters not equal, after masking the 8th bit + return False + # reached the limit, no mismatches found + return True + +# The bug in bcrypt mode "x" (preserved from the original +# implementation of bcrypt) is, at its root, that the code below +# sign- rather than zero-extends *p before or-ing it into 'tmp'. +# When *p has its 8th bit set, it is therefore or-ed in as +# 0xFF_FF_FF_xx rather than 0x00_00_00_xx, and clobbers the other +# three bytes in 'tmp'. Depending on its position within the input, +# this can erase up to three other characters of the passphrase. +# The exact set of strings involved in any one group of collisions is +# difficult to describe in words and may depend on the endianness of +# the CPU. The test cases in this file have only been verified on +# a little-endian CPU. +BF_KEY_LEN = 18 + +def buggy_expand_BF_key(phrase): + p = 0 + lp = len(phrase) + expanded = [0]*BF_KEY_LEN + if lp > 0: + for i in range(BF_KEY_LEN): + tmp = 0 + for j in range(4): + if p == lp: + c = 0 + else: + c = phrase[p] + stmp = ((c & 0x7F) - (c & 0x80)) & 0xFFFFFFFF + tmp = ((tmp << 8) | stmp) & 0xFFFFFFFF + p += 1 + if p == lp + 1: + p = 0 + expanded[i] = tmp + return expanded + +def sign_extension_collision_p(p1, p2): + return buggy_expand_BF_key(p1) == buggy_expand_BF_key(p2) + +def equivalent_sunmd5_settings_p(s1, s2): + if s1[:4] != "$md5": return False + if s2[:4] != "$md5": return False + + l1 = len(s1) + l2 = len(s2) + if l1 < l2: + ll = l1 + lh = l2 + sl = s1 + sh = s2 + else: + ll = l2 + lh = l1 + sl = s2 + sh = s1 + if sl[:ll] != sh[:ll]: + return False + + # The two cases where sunmd5 settings are equivalent: + # $md5...$ and $md5...$$ + # $md5... and $md5...$x + if sl[ll-1] == '$': + if ll+1 != lh or sh[ll] != '$': + return False + else: + if ll+2 != lh or sh[ll] != '$' or sh[ll+1] != 'x': + return False + return True + +def collision_expected(p1, p2, s1, s2): + if not isinstance(p1, bytes): p1 = p1.encode("iso_8859_1") + if not isinstance(p2, bytes): p2 = p2.encode("iso_8859_1") + if isinstance(s1, bytes): s1 = s1.decode("ascii") + if isinstance(s2, bytes): s2 = s2.decode("ascii") + # Under no circumstances should two hashes with different settings + # collide, except... + if s1 != s2: + # a descrypt hash can collide with a bigcrypt hash when the phrase + # input to bigcrypt was fewer than 8 characters long + if ( s1[0] != '$' and s1[0] != '_' + and s2[0] != '$' and s2[0] != '_' + and ( (len(s1) == 2 and len(s2) > 2 and len(p2) <= 8) + or (len(s2) == 2 and len(s1) > 2 and len(p1) <= 8))): + return strneq_7bit(p1, p2, 8) + + # all settings for NTHASH are equivalent + if s1[:3] == '$3$' and s2[:3] == '$3$': + return p1 == p2 + + # sunmd5 has pairs of equivalent settings + if equivalent_sunmd5_settings_p (s1, s2): + return p1 == p2 + + return False + + if s1[:2] == '$2': + # bcrypt truncates passphrases to 72 characters + if p1[:72] == p2[:72]: + return True + # preserved bcrypt $2x bug? + if s1[:3] == '$2x' and sign_extension_collision_p(p1, p2): + return True + return False + + if s1[0] != '$' and s1[0] != '_': + if len(s1) == 2: + # descrypt truncates passphrases to 8 characters and strips the + # 8th bit + return strneq_7bit(p1, p2, 8) + else: + # bigcrypt truncates passphrases to 128 characters and strips the + # 8th bit + return strneq_7bit(p1, p2, 128) + + if s1[0] == '_': + # bsdicrypt does not truncate but does still strip the 8th bit + return strneq_7bit(p1, p2, max(len(p1), len(p2))) + + return False + +def report_unexpected_collision(p1, p2, s1, s2, expected): + sys.stderr.write("UNEXPECTED HASH COLLISION:\n" + " hash = {}\n" + " p1 = {!r}\n" + " p2 = {!r}\n" + " s1 = {!r}\n" + " s2 = {!r}\n" + "\n".format(expected, p1, p2, s1, s2)) + +# Master control. +# +# To reduce the painful slowness of this program _somewhat_, +# we use a multiprocessing pool to compute all of the hashes. + +def generate_phrase_setting_combs(): + for macro_name, settings in SETTINGS: + for phrase in PHRASES: + for setting in settings: + yield (macro_name, phrase, setting) + +def worker_compute_one(args): + method, phrase, setting = args + + import __main__ + sfunc = getattr(__main__, 'h_' + method) + return [(method, case) for case in sfunc(phrase, *setting)] + # Python specifies that an \x escape in a string literal consumes # exactly two subsequent hexadecimal digits. C, on the other hand, # specifies that \x in a string literal consumes *any number of* @@ -680,53 +848,53 @@ def c_hex_escape(s): return c_hex_escape_fixup_re_.sub(r'\1""\2', s) def format_case(phrase, setting, expected): - if expected is None: - return (' {{ "{}", 0, "{}" }},\n' - .format(c_hex_escape(setting), - c_hex_escape(phrase))) - else: - return (' {{ "{}", "{}", "{}" }},\n' - .format(c_hex_escape(setting), - c_hex_escape(expected), - c_hex_escape(phrase))) - -# To reduce the painful slowness of this program _somewhat_, -# we use a multiprocessing pool to compute all of the hashes. - -def generate_phrase_setting_combs(): - for macro_name, settings in SETTINGS: - for phrase in PHRASES: - for setting in settings: - yield (macro_name, phrase, setting) - -def worker_compute_one(args): - method, phrase, setting = args - - import __main__ - sfunc = getattr(__main__, 'h_' + method) - return [(method, case) for case in sfunc(phrase, *setting)] + return (' {{ "{}", "{}", "{}" }},\n' + .format(c_hex_escape(setting), + c_hex_escape(expected), + c_hex_escape(phrase))) def main(): + # FIXME: This only detects collisions that actually happen, not + # collisions that ought to have happened but didn't. (Detecting + # collisions that ought to have happened, but didn't, would be + # unavoidably quadratic in the total number of test cases, so I'm + # not sure it's worth it.) + items = [] + collisions = {} + collision_error = False + with multiprocessing.Pool() as pool: + for group in pool.imap(worker_compute_one, + generate_phrase_setting_combs(), + chunksize=100): + for method, (phrase, setting, expected) in group: + if expected in collisions: + p1, s1 = collisions[expected] + if not collision_expected(p1, phrase, s1, setting): + report_unexpected_collision(p1, phrase, s1, setting, + expected) + collision_error = True + else: + collisions[expected] = (phrase, setting) + items.append((method, format_case(phrase, setting, expected))) + + if collision_error: + sys.exit(1) + sys.stdout.write( "/* Automatically generated by test-crypt-kat-gen.py.\n" " Do not edit this file by hand. */\n\n") - with multiprocessing.Pool() as pool: - prev_method = None - for items in pool.imap(worker_compute_one, - generate_phrase_setting_combs(), - chunksize=100): - for method, case in items: - if method != prev_method: - if prev_method is not None: - sys.stdout.write("#endif // {}\n\n" - .format(prev_method)) - sys.stdout.write("#if INCLUDE_{}\n".format(method)) - prev_method = method - sys.stdout.write(case) - - if prev_method is not None: - sys.stdout.write("#endif // {}\n".format(prev_method)) + prev_method = None + for method, case in items: + if method != prev_method: + if prev_method is not None: + sys.stdout.write("#endif // {}\n\n".format(prev_method)) + sys.stdout.write("#if INCLUDE_{}\n".format(method)) + prev_method = method + sys.stdout.write(case) + + if prev_method is not None: + sys.stdout.write("#endif // {}\n".format(prev_method)) if __name__ == '__main__': main() diff --git a/test-crypt-kat.c b/test-crypt-kat.c index 52bd2d90..00971db3 100644 --- a/test-crypt-kat.c +++ b/test-crypt-kat.c @@ -9,13 +9,15 @@ #include "crypt-port.h" -#include #include #include #include #ifdef HAVE_PTHREAD #include +#else +#define flockfile(fp) do { } while (0) +#define funlockfile(fp) do { } while (0) #endif #if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS @@ -28,20 +30,39 @@ symver_ref("fcrypt", fcrypt, SYMVER_FLOOR); static_assert(' ' == 0x20 && 'C' == 0x43 && '~' == 0x7E, "Execution character set does not appear to be ASCII"); -/* This test verifies four things at once: - - crypt, crypt_r, crypt_rn, and crypt_ra all produce the - same outputs for the same inputs. +/* This test verifies three things at once: + - crypt, crypt_r, crypt_rn, crypt_ra, and fcrypt (if enabled) + all produce the same outputs for the same inputs. - given hash <- crypt(phrase, setting), then hash == crypt(phrase, hash) also. - crypt(phrase, setting) == crypt'(phrase, setting) where crypt' is an independent implementation of the same hashing method. (This is the "known answer" part of the test.) - - Except for certain known cases, whenever p1 != p2, - crypt(p1, s) != crypt(p2, s). The independent implementations come from the Python 'passlib' library: . - See test-crypt-kat-gen.py for more detail. */ + See test-crypt-kat-gen.py for more detail. + + The test program has been structured to make the most expensive + part (computing a whole bunch of hashes) somewhat parallelizable. + crypt and fcrypt have to be called serially for all inputs; we do + this on the main thread. When pthreads are available, a second + thread calls crypt_r and crypt_rn for all inputs, and a third + thread calls crypt_ra for each input and then repeats that call + with the hash output by the first call as the setting string. Each + thread compares its own two results to the expected hash. If there + are any errors, it reports them to stdout. Each thread returns a + boolean failure flag (cast to void*, because pthreads) and main + will exit unsuccessfully if any flag is set. + + More threads would not reduce the overall time required for the + test, because of crypt and fcrypt having to be called serially + for each hash. We can't reduce the runtime of the parallel + section below the time that takes; the above division of labor + gives the second and third threads the same amount of work to + do as the main thread. In principle we could split things up + more finely when fcrypt is configured out, but it isn't worth + the additional ifdeffage. */ struct testcase { @@ -56,690 +77,224 @@ static const struct testcase tests[] = }; #define ntests ARRAY_SIZE (tests) -/* The test logic is structured the way it is in order to make the most - expensive part (computing a whole bunch of hashes) parallelizable. */ - -struct testresult -{ - const struct testcase *tc; - char *h_crypt; - char *h_crypt_r; - char *h_crypt_rn; - char *h_crypt_ra; - char *h_recrypt; -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - char *h_fcrypt; -#endif -}; - -/* Summarize the result of a single hashing operation in a format that - will be easy for main to process. Specifically: if the output is - as expected, the string written to 'dest' will be the hash string. - If the output is _not_ as expected, the string written to 'dest' - will contain at least one '!', and will record enough information - to diagnose the failure. main will report a test failure for any - string containing an '!', and will also report a failure if any of - the fields of a 'struct testresult' is not the same as the others. */ - -#ifndef HAVE_VASPRINTF -#define INITIAL_LEN 128 -static int -vasprintf (char **strp, const char *fmt, va_list ap) -{ - va_list aq; - va_copy (aq, ap); - - char *buf = malloc (INITIAL_LEN); - if (!buf) return -1; - int len = snprintf (buf, INITIAL_LEN, fmt, aq); - va_end (aq); - - buf = realloc (buf, len + 1); - if (!buf) return -1; - if (len >= INITIAL_LEN) - /* There wasn't enough space initially; now there is. */ - if (len != snprintf (buf, len + 1, fmt, ap)) - abort (); - - *strp = buf; - return len; -} -#endif - -static char * -xasprintf (const char *fmt, ...) +/* Print out a string, using \xXX escapes for any characters that are + not printable ASCII. Backslash, single quote, and double quote are + also escaped, by preceding them with another backslash. If machine- + parsing the output, note that we use the Python semantics of \x, not + the C semantics: each \x consumes _exactly two_ subsequent hex digits. + (For instance, \x123 means 0x12 0x33.) */ +static void +print_escaped (const char *s) { - char *rv; - va_list ap; - va_start (ap, fmt); - if (vasprintf (&rv, fmt, ap) < 0) + const unsigned char *p = (const unsigned char *)s; + for (; *p; p++) { - perror ("asprintf"); - exit (1); + unsigned char c = *p; + if (c == '\\' || c == '\"' || c == '\'') + { + putchar ('\\'); + putchar (c); + } + else if (0x20 <= c && c <= 0x7E) + putchar (c); + else + printf ("\\x%02x", (unsigned int)c); } - va_end (ap); - return rv; } +/* Subroutine of report_result. */ static void -record_result (char **dest, const char *hash, int errnm, - const struct testcase *tcase, - bool expect_failure_tokens) +begin_error_report (const struct testcase *tc, const char *tag) +{ + printf ("FAIL: %s/", tc->salt); + print_escaped (tc->input); + printf (": %s ", tag); +} + +/* Summarize the result of a single hashing operation. + If everything is as expected, prints nothing and returns 0. + Otherwise, prints a diagnostic message to stdout (not stderr!) + and returns 1. */ +static int +report_result (const char *tag, const char *hash, int errnm, + const struct testcase *tc, bool expect_failure_tokens) { if (hash && hash[0] != '*') { - if (!strcmp (hash, tcase->expected)) - *dest = xasprintf ("%s", hash); - else - *dest = xasprintf ("!not as expected: %s !=\t %s", - hash, tcase->expected); + /* We don't look at errno in this branch, because errno is + allowed to be set by successful operations. */ + if (!strcmp (hash, tc->expected)) + return 0; + + flockfile (stdout); + begin_error_report (tc, tag); + printf ("mismatch: expected %s got %s\n", tc->expected, hash); + funlockfile (stdout); + return 1; } else { /* Ill-formed setting string arguments to 'crypt' are tested in a different program, so we never _expect_ a failure. However, if we do get a failure, we want to log it in detail. */ + flockfile (stdout); + begin_error_report (tc, tag); + + if (hash == 0) + printf ("failure: got (null)"); + else + printf ("failure: got %s", hash); /* errno should have been set. */ - const char *errmsg; if (errnm) - errmsg = strerror (errnm); + printf (", errno = %s", strerror (errnm)); else - errmsg = "errno not set"; + printf (", errno not set"); /* Should the API used have generated a NULL or a failure token? */ - const char *ftstatus = ""; - if (hash == 0 && expect_failure_tokens) - ftstatus = ", failure token not generated"; + if (hash == 0 && expect_failure_tokens) + printf (", failure token not generated"); if (hash != 0 && !expect_failure_tokens) - ftstatus = ", failure token wrongly generated"; + printf (", failure token wrongly generated"); /* A failure token must never compare equal to the setting string that was used in the computation. N.B. recrypt uses crypt_rn, which never produces failure tokens, so in this branch we can - safely assume that the setting string used was tcase->salt + safely assume that the setting string used was tc->salt (if it generates one anyway that's an automatic failure). */ - const char *ftmatch = ""; - if (hash != 0 && !strcmp (tcase->salt, hash)) - ftmatch = ", failure token == salt"; - - if (hash == 0) - hash = "(null)"; + if (hash != 0 && !strcmp (tc->salt, hash)) + printf (", failure token == salt"); - *dest = xasprintf ("!got %s: %s%s%s", - hash, errmsg, ftstatus, ftmatch); + putchar ('\n'); + funlockfile (stdout); + return 1; } } static void * -calc_hashes_crypt (void *results_) +calc_hashes_crypt_fcrypt (ARG_UNUSED (void *unused)) { - struct testresult *results = results_; char *hash; size_t i; + int status = 0; for (i = 0; i < ntests; i++) { - results[i].tc = &tests[i]; errno = 0; hash = crypt (tests[i].input, tests[i].salt); - record_result (&results[i].h_crypt, hash, errno, &tests[i], - ENABLE_FAILURE_TOKENS); + status |= report_result ("crypt", hash, errno, &tests[i], + ENABLE_FAILURE_TOKENS); + +#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS + errno = 0; + hash = fcrypt (tests[i].input, tests[i].salt); + status |= report_result ("fcrypt", hash, errno, &tests[i], + ENABLE_FAILURE_TOKENS); +#endif } - return 0; + return (void *)(uintptr_t)status; } static void * -calc_hashes_crypt_r (void *results_) +calc_hashes_crypt_r_rn (ARG_UNUSED (void *unused)) { - struct testresult *results = results_; char *hash; size_t i; struct crypt_data data; + int status = 0; memset (&data, 0, sizeof data); for (i = 0; i < ntests; i++) { errno = 0; hash = crypt_r (tests[i].input, tests[i].salt, &data); - record_result (&results[i].h_crypt_r, hash, errno, &tests[i], - ENABLE_FAILURE_TOKENS); - } - - return 0; -} + status |= report_result ("crypt_r", hash, errno, &tests[i], + ENABLE_FAILURE_TOKENS); -static void * -calc_hashes_crypt_rn (void *results_) -{ - struct testresult *results = results_; - char *hash; - size_t i; - struct crypt_data data; - - memset (&data, 0, sizeof data); - for (i = 0; i < ntests; i++) - { errno = 0; hash = crypt_rn (tests[i].input, tests[i].salt, &data, (int)sizeof data); - record_result (&results[i].h_crypt_rn, hash, errno, &tests[i], false); - - if (results[i].h_crypt_rn[0] != '!') - { - errno = 0; - hash = crypt_rn (tests[i].input, results[i].h_crypt_rn, &data, - (int)sizeof data); - record_result (&results[i].h_recrypt, hash, errno, &tests[i], false); - } - else - results[i].h_recrypt = "!skipped"; + status |= report_result ("crypt_rn", hash, errno, &tests[i], false); } - return 0; + return (void *)(uintptr_t)status; } static void * -calc_hashes_crypt_ra (void *results_) +calc_hashes_crypt_ra_recrypt (ARG_UNUSED (void *unused)) { - struct testresult *results = results_; char *hash; size_t i; void *datap = 0; int datasz = 0; + int status = 0; for (i = 0; i < ntests; i++) { errno = 0; hash = crypt_ra (tests[i].input, tests[i].salt, &datap, &datasz); - record_result (&results[i].h_crypt_ra, hash, errno, &tests[i], false); + if (report_result ("crypt_ra", hash, errno, &tests[i], false)) + status = 1; + else + { + /* if we get here, we know hash == tests[i].expected */ + errno = 0; + hash = crypt_ra (tests[i].input, tests[i].expected, + &datap, &datasz); + status |= report_result ("recrypt", hash, errno, &tests[i], false); + } } free (datap); - return 0; + return (void *)(uintptr_t)status; } -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS -static void * -calc_hashes_fcrypt (void *results_) -{ - struct testresult *results = results_; - char *hash; - size_t i; - - for (i = 0; i < ntests; i++) - { - errno = 0; - hash = fcrypt (tests[i].input, tests[i].salt); - record_result (&results[i].h_fcrypt, hash, errno, &tests[i], - ENABLE_FAILURE_TOKENS); - } - - return 0; -} -#endif - -static void -print_escaped (const char *s) +int +main (void) { - const unsigned char *p = (const unsigned char *)s; - for (; *p; p++) - if (0x20 <= *p && *p <= 0x7E && *p != '\\' && *p != '\"') - putchar (*p); - else - printf ("\\x%02x", (unsigned int)*p); -} + int status = 0; -static void -report_ka_error (const char *badhash, const struct testcase *tc, - const char *mismatched, const char *tag) -{ - printf ("FAIL: %s/", tc->salt); - print_escaped (tc->input); - printf (": crypt%s: got %s", tag, badhash); - if (mismatched) - printf (" (mismatch: %s)", mismatched); - putchar ('\n'); -} + if (ntests == 0) + return 77; /* UNSUPPORTED if there are no tests to run */ -static int -do_ka_tests (struct testresult *results) -{ #ifdef HAVE_PTHREAD { - pthread_t t_r, t_rn, t_ra; + pthread_t t1, t2; int err; - err = pthread_create (&t_r, 0, calc_hashes_crypt_r, results); + void *xstatus; + err = pthread_create (&t1, 0, calc_hashes_crypt_r_rn, 0); if (err) { fprintf (stderr, "pthread_create (crypt_r): %s\n", strerror (err)); return 1; } - err = pthread_create (&t_rn, 0, calc_hashes_crypt_rn, results); - if (err) { - fprintf (stderr, "pthread_create (crypt_rn): %s\n", strerror (err)); - return 1; - } - err = pthread_create (&t_ra, 0, calc_hashes_crypt_ra, results); + err = pthread_create (&t2, 0, calc_hashes_crypt_ra_recrypt, 0); if (err) { fprintf (stderr, "pthread_create (crypt_ra): %s\n", strerror (err)); return 1; } - calc_hashes_crypt (results); -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - calc_hashes_fcrypt (results); -#endif + status |= !!calc_hashes_crypt_fcrypt (0); - err = pthread_join (t_r, 0); + err = pthread_join (t1, &xstatus); if (err) { fprintf (stderr, "pthread_join (crypt_r): %s\n", strerror (err)); - return 1; + status = 1; + } else { + status |= !!xstatus; } - err = pthread_join (t_rn, 0); + err = pthread_join (t2, &xstatus); if (err) { fprintf (stderr, "pthread_join (crypt_rn): %s\n", strerror (err)); - return 1; - } - err = pthread_join (t_ra, 0); - if (err) { - fprintf (stderr, "pthread_join (crypt_ra): %s\n", strerror (err)); - return 1; + status = 1; + } else { + status |= !!xstatus; } } #else - calc_hashes_crypt (results); - calc_hashes_crypt_r (results); - calc_hashes_crypt_rn (results); - calc_hashes_crypt_ra (results); -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - calc_hashes_fcrypt (results); + status |= !!calc_hashes_crypt_fcrypt (results); + status |= !!calc_hashes_crypt_r_rn (results); + status |= !!calc_hashes_crypt_ra_recrypt (results); #endif -#endif - - int rv = 0; - for (size_t i = 0; i < ntests; i++) - { - int failed = 0; - if (strchr (results[i].h_crypt, '!')) - { - report_ka_error (results[i].h_crypt, &tests[i], 0, ""); - failed = 1; - } - if (strchr (results[i].h_crypt_r, '!')) - { - report_ka_error (results[i].h_crypt_r, &tests[i], 0, "_r"); - failed = 1; - } - if (strchr (results[i].h_crypt_rn, '!')) - { - report_ka_error (results[i].h_crypt_rn, &tests[i], 0, "_rn"); - failed = 1; - } - if (strchr (results[i].h_crypt_ra, '!')) - { - report_ka_error (results[i].h_crypt_ra, &tests[i], 0, "_ra"); - failed = 1; - } - if (strchr (results[i].h_recrypt, '!')) - { - report_ka_error (results[i].h_recrypt, &tests[i], 0, "_rn (recrypt)"); - failed = 1; - } -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - if (strchr (results[i].h_fcrypt, '!')) - { - report_ka_error (results[i].h_fcrypt, &tests[i], 0, " (fcrypt)"); - failed = 1; - } -#endif - - if (!failed) - { - if (strcmp (results[i].h_crypt_r, results[i].h_crypt)) - { - report_ka_error (results[i].h_crypt_r, &tests[i], - results[i].h_crypt, "_r"); - failed = 1; - } - if (strcmp (results[i].h_crypt_rn, results[i].h_crypt)) - { - report_ka_error (results[i].h_crypt_rn, &tests[i], - results[i].h_crypt, "_rn"); - failed = 1; - } - if (strcmp (results[i].h_crypt_ra, results[i].h_crypt)) - { - report_ka_error (results[i].h_crypt_ra, &tests[i], - results[i].h_crypt, "_ra"); - failed = 1; - } - if (strcmp (results[i].h_recrypt, results[i].h_crypt)) - { - report_ka_error (results[i].h_recrypt, &tests[i], - results[i].h_crypt, "_rn (recrypt)"); - failed = 1; - } -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - if (strcmp (results[i].h_recrypt, results[i].h_crypt)) - { - report_ka_error (results[i].h_fcrypt, &tests[i], - results[i].h_crypt, " (fcrypt)"); - failed = 1; - } -#endif - - /* Tell the collision tests to skip this one if it's inconsistent. */ - if (failed) - results[i].h_crypt[0] = '!'; - } - - rv |= failed; - } - - return rv; -} - -/* Collision test */ - -#if INCLUDE_descrypt || INCLUDE_bsdicrypt || INCLUDE_bigcrypt -static bool -strneq_7bit (const char *p1, const char *p2, size_t limit) -{ - for (size_t i = 0; i < limit; i++) - { - if ((p1[i] & 0x7F) != (p2[i] & 0x7F)) - return true; - if (p1[i] == '\0') - break; - } - return false; -} -#endif - -#if INCLUDE_bcrypt_x -/* Must match the definition of BF_key in crypt-bcrypt.c. */ -typedef uint32_t BF_key[18]; - - -/* The bug in bcrypt mode "x" (preserved from the original - implementation of bcrypt) is, at its root, that the code below - sign- rather than zero-extends *p before or-ing it into 'tmp'. - When *p has its 8th bit set, it is therefore or-ed in as - 0xFF_FF_FF_xx rather than 0x00_00_00_xx, and clobbers the other - three bytes in 'tmp'. Depending on its position within the input, - this can erase up to three other characters of the passphrase. - The exact set of strings involved in any one group of collisions is - difficult to describe in words and may depend on the endianness of - the CPU. The test cases in this file have only been verified on - a little-endian CPU. */ -static void -buggy_expand_BF_key (BF_key *expanded, const char *phrase) -{ - const char *p = phrase; - for (int i = 0; i < (int)ARRAY_SIZE (*expanded); i++) - { - uint32_t tmp = 0; - int32_t stmp; - for (int j = 0; j < 4; j++) - { - tmp <<= 8; - stmp = (int32_t) (signed char)*p; - tmp |= (uint32_t) stmp; - if (!*p) - p = phrase; - else - p++; - } - (*expanded)[i] = tmp; - } -} - -static bool -sign_extension_collision_p (const char *p1, const char *p2) -{ - BF_key expanded_1, expanded_2; - buggy_expand_BF_key (&expanded_1, p1); - buggy_expand_BF_key (&expanded_2, p2); - return !memcmp (expanded_1, expanded_2, sizeof (BF_key)); -} -#endif - -#if INCLUDE_sunmd5 -static bool -equivalent_sunmd5_settings_p (const char *s1, const char *s2) -{ - if (strncmp (s1, "$md5", 4)) - return false; - if (strncmp (s2, "$md5", 4)) - return false; - - size_t l1 = strlen (s1); - size_t l2 = strlen (s2); - size_t ll; - const char *sl, *sh; - if (l1 < l2) - { - ll = l1; - sl = s1; - sh = s2; - } - else - { - ll = l2; - sl = s2; - sh = s1; - } - if (strncmp (sl, sh, ll)) - return false; - /* The two cases where sunmd5 settings are equivalent: - $md5...$ and $md5...$$ - $md5... and $md5...$x - */ - if (sl[ll-1] == '$') - { - if (sh[ll] != '$' || sh[ll+1] != '\0') - return false; - } - else - { - if (sh[ll] != '$' || sh[ll+1] != 'x' || sh[ll+2] != '\0') - return false; - } - - return true; -} -#endif - -static bool -collision_expected (const struct testresult *a, const struct testresult *b) -{ - const char *p1 = a->tc->input; - const char *p2 = b->tc->input; - const char *s1 = a->tc->salt; - const char *s2 = b->tc->salt; - - /* Under no circumstances should two hashes with different settings - collide, except... */ - if (strcmp (s1, s2)) - { -#if INCLUDE_bigcrypt && INCLUDE_descrypt - /* a DES hash can collide with a bigcrypt hash when the phrase - input to bigcrypt was fewer than 8 characters long; */ - if (s1[0] != '$' && s1[0] != '_' && - s2[0] != '$' && s2[0] != '_' && - s1[0] == s2[0] && s1[1] == s2[1] && - ((s1[2] != '\0' && s2[2] == '\0' && strlen (p1) <= 8) || - (s1[2] == '\0' && s2[2] != '\0' && strlen (p2) <= 8))) - return !strneq_7bit (p1, p2, 8); -#endif - -#if INCLUDE_nt - /* all settings for NTHASH are equivalent; */ - if (!strncmp (s1, "$3$", 3) && !strncmp(s2, "$3$", 3)) - return !strncmp (p1, p2, 128); -#endif - -#if INCLUDE_sunmd5 - /* $md5... and $md5...$x are equivalent. */ - if (equivalent_sunmd5_settings_p (s1, s2)) - return !strcmp (p1, p2); -#endif - - return false; - } - -#if INCLUDE_bcrypt || INCLUDE_bcrypt_a || INCLUDE_bcrypt_x || INCLUDE_bcrypt_y - if (!strncmp (s1, "$2", 2)) /* bcrypt */ - { - /* bcrypt truncates passphrases to 72 characters. */ - if (!strncmp (p1, p2, 72)) - return true; - -#if INCLUDE_bcrypt_x - if (!strncmp (s1, "$2x", 3) /* bcrypt with preserved bug */ - && sign_extension_collision_p (p1, p2)) - return true; -#endif - - return false; - } -#endif - -#if INCLUDE_descrypt - if (s1[0] != '$' && s1[0] != '_' && s1[2] == '\0') - /* descrypt truncates passphrases to 8 characters and strips the - 8th bit. */ - return !strneq_7bit (p1, p2, 8); -#endif - -#if INCLUDE_bigcrypt - if (s1[0] != '$' && s1[0] != '_' && s1[2] != '\0') - /* bigcrypt truncates passphrases to 128 characters and strips the - 8th bit. */ - return !strneq_7bit (p1, p2, 128); -#endif - -#if INCLUDE_bsdicrypt - if (s1[0] == '_') - /* bsdicrypt does not truncate the passphrase, but it does still - strip the 8th bit. */ - return !strneq_7bit (p1, p2, (size_t)-1); -#endif - -#if INCLUDE_nt - if (!strcmp (s1, "$3$")) - /* nthash truncates the passphrase to 128 characters */ - return !strncmp (p1, p2, 128); -#endif - - return false; -} - -static void -report_collision (const struct testresult *a, const struct testresult *b) -{ - fputs ("FAIL: collision:\n A: '", stdout); - print_escaped (a->tc->input); - fputs ("' with '", stdout); - print_escaped (a->tc->salt); - fputs ("'\n B: '", stdout); - print_escaped (b->tc->input); - fputs ("' with '", stdout); - print_escaped (b->tc->salt); - fputs ("'\n H: '", stdout); - print_escaped (a->h_crypt); - fputs ("'\n", stdout); -} - -static void -report_no_collision (const struct testresult *a, const struct testresult *b) -{ - fputs ("FAIL: no collision:\n A: '", stdout); - print_escaped (a->tc->input); - fputs ("' with '", stdout); - print_escaped (a->tc->salt); - fputs ("'\n B: '", stdout); - print_escaped (b->tc->input); - fputs ("' with '", stdout); - print_escaped (b->tc->salt); - fputs ("'\n AH: '", stdout); - print_escaped (a->h_crypt); - fputs ("'\n BH: '", stdout); - print_escaped (b->h_crypt); - fputs ("'\n", stdout); -} - -static int -cmp_testresult_by_hash (const void *ax, const void *bx) -{ - const struct testresult *a = ax; - const struct testresult *b = bx; - return strcmp (a->h_crypt, b->h_crypt); -} - -static int -do_collision_tests (struct testresult *results) -{ - qsort (results, ntests, sizeof (struct testresult), cmp_testresult_by_hash); - - /* Sorting the test result records by hash means that if there are - any collisions, the records involved will be adjacent. */ - int rv = 0; - for (size_t i = 1; i < ntests; i++) - { - const struct testresult *a = &results[i-1]; - const struct testresult *b = &results[i]; - if (a->h_crypt[0] == '!' || b->h_crypt[0] == '!') - continue; - - bool collision = !strcmp (a->h_crypt, b->h_crypt); - bool x_collision = collision_expected (a, b); - - if (collision && !x_collision) - { - rv = 1; - report_collision (a, b); - } - else if (!collision && x_collision) - { - rv = 1; - report_no_collision (a, b); - } - } - return rv; -} - -int -main (void) -{ - if (ntests == 0) - return 77; /* UNSUPPORTED if there are no tests to run */ - - struct testresult *results = calloc (ntests, sizeof (struct testresult)); - if (!results) - { - fprintf (stderr, "failed to allocate %zu bytes: %s\n", - ntests * sizeof (struct testresult), strerror (errno)); - return 1; - } - - int rv = 0; - rv |= do_ka_tests (results); - rv |= do_collision_tests (results); - - /* Scrupulously free all allocations so valgrind is happy. */ - for (size_t i = 0; i < ntests; i++) - { - free (results[i].h_crypt); - free (results[i].h_crypt_r); - free (results[i].h_crypt_rn); - free (results[i].h_crypt_ra); - free (results[i].h_recrypt); -#if ENABLE_OBSOLETE_API && !ENABLE_OBSOLETE_API_ENOSYS - free (results[i].h_fcrypt); -#endif - } - free (results); - return rv; + return status; } From 0a7ada6dbf00a025e0f25768a3493811dd4763b5 Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Wed, 6 Feb 2019 14:45:08 -0500 Subject: [PATCH 3/3] Use ASan+UBSan for the Clang checked build on Travis, not Valgrind. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For some reason the code generated by Clang is significantly slower than the code generated by GCC, when run under Valgrind. It’s only about ten additional minutes of wall-clock time, but it makes the test suite trip not only Travis’s documented ten-minute no-output timeout (which can be worked around), but their undocumented 45?-minute no-matter-what timeout. Switch the Clang checked build on Travis to use ASan+UBSan instead of Valgrind. This is much faster and may also detect a different subset of potential bugs. --- .travis.yml | 6 +++--- .travis_script.sh | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 55b1f9cb..2125e2f3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -74,12 +74,12 @@ matrix: env: - CONF="--enable-obsolete-api --enable-hashes=all --enable-valgrind-memcheck" - VALGRIND=1 - - name: "Clang, all hashes, obsolete API, Valgrind" + - name: "Clang, all hashes, obsolete API, ASan+UBSan" compiler: clang os: linux env: - - CONF="--enable-obsolete-api --enable-hashes=all --enable-valgrind-memcheck" - - VALGRIND=1 + - CONF="--enable-obsolete-api --enable-hashes=all" + - SANITIZER=1 - name: "GCC, all hashes, static lib" compiler: gcc os: linux diff --git a/.travis_script.sh b/.travis_script.sh index 814327f5..1f79d735 100755 --- a/.travis_script.sh +++ b/.travis_script.sh @@ -108,6 +108,14 @@ else export LDFLAGS="$(dpkg-buildflags --get LDFLAGS)" fi +MAKE_ARGS= +if [[ "$SANITIZER" == "1" ]]; then + # ASan is incompatible with -z defs. + MAKE_ARGS="UNDEF_FLAG=" + export CFLAGS="$CFLAGS -fsanitize=undefined,address" + export CXXFLAGS="$CXXFLAGS -fsanitize=undefined,address" +fi + rm -fr build mkdir -p build pushd build @@ -118,12 +126,13 @@ log_time preparation log_time configure if [[ "$DISTCHECK" == "1" ]]; then - make -j$NPROCS distcheck + make -j$NPROCS $MAKE_ARGS distcheck log_time distcheck else - make -j$NPROCS + make -j$NPROCS $MAKE_ARGS all log_time build - make check -j$NPROCS || (cat test-suite.log && exit 1) + travis_wait 60 \ + make -j$NPROCS $MAKE_ARGS check || (cat test-suite.log && exit 1) log_time test fi @@ -132,7 +141,7 @@ if [[ "$VALGRIND" == "1" ]]; then # Travis no-output timeout on individual tests, just because # that's how slow memcheck is. travis_wait 60 \ - make -j$NPROCS check-valgrind-memcheck || \ + make -j$NPROCS $MAKE_ARGS check-valgrind-memcheck || \ (cat test-suite-memcheck.log && exit 1) log_time test-memcheck fi