openwall / openwall/john

Hybrid external mode never calls the original mode's fix_state(), breaking --restore

Open
#5,814 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
C
Stars
13.6k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

I feel I may be missing something here, because @jfoug's documentation for hybrid external added to doc/EXTERNAL talks about defining restore() or not and related efficiency aspects, and the code certainly tries to support restore, but I just don't see how it can possibly work when the original mode's fix_state() is never called?

In particular, I observe this problem with -w --external=Shuffle, where actual progress through the wordlist is never saved (all writes to the .rec files write the line number as 0). Desired behavior is that without a restore() the shuffling for current word should start from scratch, but some progress through the wordlist (as limited by keys-per-crypt granularity) should be saved/restored.

The below patch appears to achieve the desired behavior in the above case, but I worry it may break something else?

+++ b/src/external.c
@@ -454,6 +454,8 @@ static void fix_state(void)
        rec_seq = seq;
 }
 
+static void (*saved_crk_fix_state)(void);
+
 void ext_hybrid_fix_state(void)
 {
        strcpy(hybrid_rec_word, int_word);
@@ -462,6 +464,9 @@ void ext_hybrid_fix_state(void)
        hybrid_actual_completed_resume = ext_hybrid_resume;
        hybrid_actual_completed_total  = ext_hybrid_total;
        strcpy(hybrid_actual_completed_base_word, int_hybrid_base_word);
+
+       if (saved_crk_fix_state)
+               saved_crk_fix_state();
 }
 
 void do_external_crack(struct db_main *db)
@@ -594,14 +599,13 @@ void do_external_crack(struct db_main *db)
  */
 
 
-extern void(*crk_fix_state)(void);
-void(*saved_crk_fix_state)(void);
-void save_fix_state(void(*new_crk_fix_state)(void))
+extern void (*crk_fix_state)(void);
+static void save_fix_state(void(*new_crk_fix_state)(void))
 {
        saved_crk_fix_state = crk_fix_state;
        crk_fix_state = new_crk_fix_state;
 }
-void restore_fix_state(void)
+static void restore_fix_state(void)
 {
        crk_fix_state = saved_crk_fix_state;
 }

(additions of static are not directly related, but helped me make sure those symbols were local to this file)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/external.c, focusing on ext_hybrid_fix_state(), fix_state(), save_fix_state(), and restore_fix_state(); read doc/EXTERNAL for the hybrid external restore behavior. Reproduce with -w --external=Shuffle and inspect the .rec files. Done means progress through the wordlist is saved and restored while shuffling for the current word restarts when restore() is absent.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
cli, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.