Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 17 Jan 2012 15:32:33 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: Recent CVS patches

On 01/17/2012 01:00 PM, magnum wrote:
> They were: BFEgg and MYSQL (fmt_default_binary), DOMINOSEC
> (using a struct that can't ever have both binary and salt aligned), EPI,
> hmailserver and oracle (static char salt[]), PO (static char binary[])
> and oracle11 (other bugs, not alignment).
> 
> Of these, I believe BFEgg, MYSQL and DOMINOSEC does not really need the
> fixes while the others do. It now occurs to me you could test if a
> format is using fmt_default_binary or fmt_default_binary_hash, and/or
> fmt_default_salt or fmt_default_salt_hash and disable the tests for
> these. I believe that would have solved it for these three.

I gave this a try. First I reverted my format fixes so I'm back at
yesterday:

Benchmarking: Eggdrop [blowfish]... FAILED (binary)
Benchmarking: More Secure Internet Password [RSA MD defined by BSAFE 1.x
- Lotus v6]... FAILED (binary)
Benchmarking: MYSQL [mysql]... FAILED (binary)
Benchmarking: Post.Office MD5 [STD]... FAILED (binary)
Benchmarking: Oracle 11g [SSE2i 8x]... FAILED (salt)
Benchmarking: hmailserver [32/64]... FAILED (salt)
Benchmarking: SHA-crypt-512 [OpenSSL 64/64]... Segmentation fault

Ignoring the segfault for now, I hacked formats.c so it skips the
alignment tests for formats using fmt_default_[binary|salt]*

Benchmarking: Post.Office MD5 [STD]... FAILED (binary (alignment))
Benchmarking: Oracle 11g [SSE2i 8x]... FAILED (salt (alignment))
Benchmarking: SAP CODVN F/G (PASSCODE) [SSE2i 8x]... Segmentation fault

Note that the segfault moved... I'm not sure what this is about. But
still ignoring this, we got rid of some formats that does not need to be
fixed. Now I also hacked formats.c so it always serves misaligned
ciphertext:

Benchmarking: Post.Office MD5 [STD]... FAILED (binary (alignment))
Benchmarking: Oracle 11g [SSE2i 8x]... FAILED (salt (alignment))
Benchmarking: hmailserver [32/64]... FAILED (salt (alignment))
3 out of 115 tests have FAILED

The segfault disappeared (it still worries me a little) and now
hmailserver show up too.

Now, I applied the format fixes for the formats who really needed it. A
new problem surfaced:

Benchmarking: Post.Office MD5 [STD]... FAILED (salt (alignment))
1 out of 115 tests have FAILED

Aha, So I missed one fix yesterday because the self-test was aligned by
coincidence? Fixing that... ok, actually this fix is not strictly needed
but this is now the only such fix I had to do!

I enclose the experimental fixes I did to formats.c.

Now, back to that segfault. It does not happen now but there is a
problem somewhere. I get this from Valgrind, introduced by the late CVS
code:

==1926== Invalid read of size 8
==1926==    at 0x41D017: salt_hash (dynamic_fmt.c:2030)
==1926==    by 0x48A12B: fmt_self_test (formats.c:128)
==1926==    by 0x482BA4: benchmark_format (bench.c:138)
==1926==    by 0x483364: benchmark_all (bench.c:447)
==1926==    by 0x48E2DC: john_run (john.c:483)
==1926==    by 0x48E7D4: main (john.c:675)
==1926==  Address 0x614f4e1 is 1 bytes inside a block of size 4 alloc'd
==1926==    at 0x4C28F9F: malloc (vg_replace_malloc.c:236)
==1926==    by 0x4918DC: mem_alloc (memory.c:54)
==1926==    by 0x489EEB: fmt_self_test (formats.c:200)
==1926==    by 0x482BA4: benchmark_format (bench.c:138)
==1926==    by 0x483364: benchmark_all (bench.c:447)
==1926==    by 0x48E2DC: john_run (john.c:483)
==1926==    by 0x48E7D4: main (john.c:675)

This is dynamic_0 because it's the first that appears in -test=0 but a
LOT of formats show similar problems. Some formats also have "Invalid
write" too, which is probably why there is a segfault in some
situations. Maybe this is actually previously hidden by alignment, and a
lot of formats should bump their reported binary or salt size? I'm not
sure. If that's it, it's a good thing we catch it now, but it should be
detected so we don't need Valgrind.

Maybe you should alloc more memory than needed to the binary/salt
copies, but mark the excess with a canary and check afterwards? I'll try
that and see what happens.

magnum

View attachment "0002-Test-Deliberately-use-misaligned-ciphertext-in-self-.patch" of type "text/x-patch" (1207 bytes)

View attachment "0001-Test-Only-check-binary-salt-alignment-if-not-using-f.patch" of type "text/x-patch" (1435 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.