make debug causes selftest to fail

[thatux]

i'm on the latest commit 933bbdb95, using wsl

using one of the fmt_tests hashes and building with debug causes a selftest fail

echo '$gpg$*1*667*2048*387de4c9e2c1018aed84af75922ecaa92d1bc68d48042144c77dfe168de1fd654e4db77bfbc60ec68f283483382413cbfddddcfad714922b2d558f8729f705fbf973ab1839e756c26207a4bc8796eeb567bf9817f73a2a81728d3e4bc0894f62ad96e04e60752d84ebc01316703b0fd0f618f6120289373347027924606712610c583b25be57c8a130bc4dd796964f3f03188baa057d6b8b1fd36675af94d45847eeefe7fff63b755a32e8abe26b7f3f58bb091e5c7b9250afe2180b3d0abdd2c1db3d4fffe25e17d5b7d5b79367d98c523a6c280aafef5c1975a42fd97242ba86ced73c5e1a9bcab82adadd11ef2b64c3aad23bc930e62fc8def6b1d362e954795d87fa789e5bc2807bfdc69bba7e66065e3e3c2df0c25eab0fde39fbe54f32b26f07d88f8b05202e55874a1fa37d540a5af541e28370f27fe094ca8758cd7ff7b28df1cbc475713d7604b1af22fd758ebb3a83876ed83f003285bc8fdc7a5470f7c5a9e8a93929941692a9ff9f1bc146dcc02aab47e2679297d894f28b62da16c8baa95cd393d838fa63efc9d3f88de93dc970c67022d5dc88dce25decec8848f8e6f263d7c2c0238d36aa0013d7edefd43dac1299a54eb460d9b82cb53cf86fcb7c8d5dba95795a1adeb729a705b47b8317594ac3906424b2c0e425343eca019e53d927e6bc32688bd9e87ee808fb1d8eeee8ab938855131b839776c7da79a33a6d66e57eadb430ef04809009794e32a03a7e030b8792be5d53ceaf480ffd98633d1993c43f536a90bdbec8b9a827d0e0a49155450389beb53af5c214c4ec09712d83b175671358d8e9d54da7a8187f72aaaca5203372841af9b89a07b8aadecafc0f2901b8aec13a5382c6f94712d629333b301afdf52bdfa62534de2b10078cd4d0e781c88efdfe4e5252e39a236af449d4d62081cee630ab*3*254*2*3*8*b1fdf3772bb57e1f*65536*2127ccd55e721ba0' > /mnt/e/GoogleDrive/Tools/Hashcat/JohnTheRipper/src/test.hash

 make clean; make debug -sj20
rm ../run/john.pot; ../run/john test.hash --mask='polished'
[...]
Self test failed (Could not find salt in db - salt() return inconsistent?)

building without the debug flag works without a problem and cracks the hash

make clean; make -sj20
rm ../run/john.pot; ../run/john test.hash --mask='polished'
polished         (?)

Attach details about your OS and about john, including:

• The output of ./john --list=build-info. build-info.log
• The command line you are using. see above

[thatux]

My goal original goal was to be able to debug openssl, but that also works without the debug This works for me to do that:

cd /tmp
git clone https://github.com/openssl/openssl
git clone https://github.com/openwall/john/
cd openssl
./Configure
make -sj20
cd ../john/src
./configure OPENSSL_LIBS="-L/tmp/openssl -lssl -lcrypto"
make -sj20

The original bug still exists

[claudioandre-br]

Thanks for your bug report.

This looks like an obscure bug in the gpg format as stated in cracker.c:

 * We should have already called fmt_self_test() from john.c.  This redundant
 * self-test is only to catch some more obscure bugs in debugging builds (it
 * is a no-op in normal builds).

We need the magnum's help to understand what's going on.

[magnumripper]

I can reproduce. Perhaps that extra self-test indeed found an obscure bug, but I have yet to understand what is happening. Using --skip-self-tests works around it and the hash is cracked. I can't find any other #if DEBUG that could interfere.

[claudioandre-br]

I was also able to reproduce. And failed to understand how could it be db != db.

[magnumripper]

OK, the problem is (I'm pretty sure) that at this point, db is the real database, not the test db made from test vectors. So that's just a minor bug that existed ever since the test db thing was introduced, and no-one stumbled upon it until the OP did now.

Except...

When I do a similar test with for example the NT format, there's no problem. How can that NOT be a problem?

[magnumripper]

Here's a fix:

diff --git a/src/cracker.c b/src/cracker.c
index 681be8a98..779763c51 100644
--- a/src/cracker.c
+++ b/src/cracker.c
@@ -176,11 +176,15 @@ void crk_init(struct db_main *db, void (*fix_state)(void),
  * or if the format has a custom reset() method (we've already called reset(db)
  * from john.c, and we don't want to mess with the format's state).
  */
-   if (db->loaded && db->format->methods.reset == fmt_default_reset)
-   if ((where = fmt_self_test(db->format, db))) {
-       log_event("! Self test failed (%s)", where);
-       fprintf(stderr, "Self test failed (%s)\n", where);
-       error();
+   if (db->loaded && db->format->methods.reset == fmt_default_reset && !(options.flags & FLG_NOTESTS)) {
+       struct db_main *test_db = ldr_init_test_db(db->format, db);
+
+       if ((where = fmt_self_test(db->format, test_db))) {
+           log_event("! Self test failed (%s)", where);
+           fprintf(stderr, "Self test failed (%s)\n", where);
+           error();
+       }
+       ldr_free_db(test_db, 1);
    }

 #if HAVE_OPENCL

...but I don't want to commit it until I understand why NT doesn't have any problems with the current code.

[claudioandre-br]

If NT loads the actual database and the test doesn't fail, then ssh is changing something in a way that might actually be a bug.

On the other hand, who knows, something needs to be changed during load.

The real issue is: what does this test do and how does it catch bugs.

[claudioandre-br]

I have 14 CPU formats turned off before the test.

• gpg is one of them.

[magnumripper]

If NT loads the actual database and the test doesn't fail, then ssh is changing something in a way that might actually be a bug.

(You meant gpg, but) they should fail, I can't see why NT (and probably some others, haven't had time to test yet) does not.

[solardiz]

Here's a fix: [...] ...but I don't want to commit it until I understand why NT doesn't have any problems with the current code.

Do we really want to keep this issue completely unfixed for 1+ year just because we didn't fully understand why it didn't show up in some cases? That could make sense if @magnumripper or someone else were available to investigate it fully, but that doesn't appear to be the case now. So maybe just try applying the patch we have here?

[claudioandre-br]

Before merging, we should check again (if and how it fails now). gpg is a format that was recently fixed.

[magnumripper]

I'd revisit it, but my backlog is kinda huge. I guess we should implement that fix.

[solardiz]

It's really great to see you back, @magnumripper!

I guess we should implement that fix.

Since the fix is yours, would you be the one to commit it, please?

I think the recent changes in our gpg* formats are unrelated to this issue; I really don't expect them to have made any difference here.