KWallet format: Support new partially-endianness-unbroken wallets, proper valid(), tunable costs, fix old KDF, support/produce truncated "hashes"

[solardiz]

Fixes #5866 (edit: and #5874)

In my testing, this cracks all samples from our john-samples repo as well as those attached to #5866.

[exploide]

Describe in one term how bizarre a file format can be. "partially-endianness-unbroken" 😅

Awesome, the patch seems to work and I'm able to crack the wallets. 👍

[solardiz]

Thank you @exploide! I'd merge this now, but I'm also thinking of possibly not decrypting beyond the first 64 bytes (8+4+52) when we're not checking SHA-1 anyway, for some slight speedup. Maybe I should implement that.

[solardiz]

I'm also thinking of possibly not decrypting beyond the first 64 bytes (8+4+52) when we're not checking SHA-1 anyway

This is now implemented.

[solardiz]

This format's valid was really poor, would miss a possible buffer overflow.

I guess our fuzzing just didn't test 64K+, maybe it should, @AlekseyCherepanov.

[solardiz]

This format's valid was really poor, would miss a possible buffer overflow.

Fixed with a commit in this PR now.

I guess our fuzzing just didn't test 64K+, maybe it should, @AlekseyCherepanov.

Or rather, maybe it's hard for a fuzzer to simultaneously update a length field and the following data to be of the exact updated length. So maybe we should implement this as a special case in the fuzzer.

[solardiz]

We just got the familiar ASan false positive for Argon2 here, in the asan CI job, despite of us keeping it at checkout@v4. I wonder what else has changed recently. Could be some Ubuntu 24.04 update.

https://github.com/openwall/john/actions/runs/18964974991/job/54159710415?pr=5869

AddressSanitizer:DEADLYSIGNAL
=================================================================
==9360==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x564f9c28ae6f bp 0x7ffeef9ce6c0 sp 0x7ffeef9ce400 T0)
==9360==The signal is caused by a READ memory access.
==9360==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 0x564f9c28ae6f in memcpy /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29
    #1 0x564f9c28ae6f in blake2b_long /home/runner/work/john/john/src/blake2b_plug.c:420
    #2 0x564f9c25dd37 in argon2_fill_first_blocks /home/runner/work/john/john/src/argon2_core_plug.c:533
    #3 0x564f9c25ef00 in argon2_initialize /home/runner/work/john/john/src/argon2_core_plug.c:652
    #4 0x564f9c26c85d in argon2_ctx /home/runner/work/john/john/src/argon2_plug.c:82
    #5 0x564f9c260f37 in crypt_all._omp_fn.0 /home/runner/work/john/john/src/argon2_fmt_plug.c:343
    #6 0x7fa9cb89f976 in GOMP_parallel (/lib/x86_64-linux-gnu/libgomp.so.1+0x18976) (BuildId: be297f11b9b4baa3674f3d9e232b2a87b3abf6b2)
    #7 0x564f9c2613fe in crypt_all /home/runner/work/john/john/src/argon2_fmt_plug.c:323
    #8 0x564f9c4c9e1d in is_key_right /home/runner/work/john/john/src/formats.c:594
    #9 0x564f9c4d1f1e in fmt_self_test_body /home/runner/work/john/john/src/formats.c:1329
    #10 0x564f9c4d3036 in fmt_self_test /home/runner/work/john/john/src/formats.c:2079
    #11 0x564f9c4acd8e in benchmark_all /home/runner/work/john/john/src/bench.c:883
    #12 0x564f9c4e14c7 in john_run /home/runner/work/john/john/src/john.c:1695
    #13 0x564f9c4e14c7 in main /home/runner/work/john/john/src/john.c:2117
    #14 0x7fa9cb62a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #15 0x7fa9cb62a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #16 0x564f9bffc1c4 in _start (/home/runner/work/john/john/run/john+0x20c1c4) (BuildId: d6c880ba2b3d7ebd5a6e968289b013459f6f2bcc)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29 in memcpy
==9360==ABORTING

[solardiz]

@exploide This passes all of my tests with the sample wallets you provided, but you could want to retest before I merge. The new code in John is supposed to work with "hashes" produced by both old and new kwallet2john, and even recognize that they're effectively the same (for the purposes of duplicate detection and --show).

[exploide]

Great, these are a lot of improvements and fixes to the format 👍
I tested again and everything seems to work as I would expect it.

Just added two minor notes concerning code comments. But is also fine as is.

[exploide]

Don't know if using an older version of JtR would be an option given that there were a lot of cases which did not work at all.

[solardiz]

Good point. Maybe I should add a warning to this comment, or drop the suggestion to comment-out for the old version.

[solardiz]

I've just revised the comment and force-pushed.

[exploide]

Isn't this the optimization you implemented? The comment sounds a bit hypothetical. But maybe I'm mistaken.

[solardiz]

I implemented the most important part of this optimization. The comment remains valid as-is, but the meaning of "the whole thing" changes from "full original encrypted file" to "full data still included in our truncated non-hashes". It is still possible to optimize further, by decrypting just 8 rather than 56 bytes most of the time.

[solardiz]

I tested again and everything seems to work as I would expect it.

Thank you very much @exploide for supporting me in this effort. I'm going to merge this now and move on to other tasks.