New attack mode: Association attack#95
Conversation
|
Sorry for spam of pull requests.... |
|
Cool, thanks! I will do code review this weekend. |
There was a problem hiding this comment.
Initial comments.
"all_hashes" should be explained more.
Otherwise I will wait until your thesis is available, so I can better understand your implementation.
There are no tests, so please atleast create some and put them to @ihranicky's manual testing spreadsheet.
|
|
||
| # Entrypoint | ||
| RUN ["chmod", "+x", "/srv/fitcrack/entrypoint-fitcrack.sh"] | ||
| ENTRYPOINT ["/srv/fitcrack/entrypoint-fitcrack.sh"] |
There was a problem hiding this comment.
Why we need these changes?
| @@ -0,0 +1,3 @@ | |||
| (boinccmd --project 127.0.0.1/fitcrack/ detach || true) && | |||
There was a problem hiding this comment.
Not needed for our repo.
| args: | ||
| - COMPILER_THREADS=1 # Higher values may cause linker race conditions | ||
|
|
||
| command: ./entrypoint-fitcrack.sh |
There was a problem hiding this comment.
Possibly bad merge of dev branch?
| virtual bool requiresDicts() const override {return true;} | ||
|
|
||
| virtual bool hasStickyLeftDict() const override { | ||
| return m_job->getDistributionMode() == 1 || m_job->getDistributionMode() == 2; |
There was a problem hiding this comment.
Better to put these constants to enum
| /** | ||
| * @brief enum for distribution mode options readability | ||
| */ | ||
| enum DistributionMode { |
There was a problem hiding this comment.
You have them here, so move this declaration somewhere else, into some more general header, so we can use these named constants everywhere.
|
|
||
| case Config::AttackMode::AttackAssoc: | ||
| if (job->getDistributionMode() == 0){ // | ||
| if (job->getAttackSubmode() == 1) { |
There was a problem hiding this comment.
Use enum instead of numbers, as explained above.
| echo "Project already exists." | ||
|
|
||
| usermod -d /var/lib/mysql/ mysql | ||
| # Fix MySQL socket permissions |
| job['keyspace'] = job['hc_keyspace'] * ruleFileMultiplier | ||
|
|
||
| # in case of rule distribution hashcat keyspace is defined by rules | ||
| if job['attack_settings']['distribution_mode'] == 2: |
There was a problem hiding this comment.
Create same "enum" also in python, so you can use it here, instead of "2".
| if self.job.rulesFile: | ||
| rules = self.job.rulesFile.count | ||
| return self.hc_keyspace * rules if rules else self.hc_keyspace | ||
| elif self.job.attack_mode == 10 and self.job.distribution_mode == 2: |
There was a problem hiding this comment.
Same here and everywhere where you use magic constants.
| // All ok! | ||
| return true | ||
| case 'association': | ||
| return state.leftDicts.reduce((total, current)=>total+current.keyspace, 0) == state.validatedHashes.length |
There was a problem hiding this comment.
Please format properly.
davidbolvansky
force-pushed
the
dev
branch
2 times, most recently
from
35da608 to
3c6de50
Compare
alpatron
left a comment
alpatron
left a comment
There was a problem hiding this comment.
Okay. I will have to test the thing, then implement the changes requested, then re-test, and also investigate the remaining review questions.
Moreover, I need to check how this PR is impacted by some of the newer pulled PRs; most notably the PR to update the way cracking time is estimated and the performance updates.
Also, I think as of now this will not work because the code is made for FC before the hash list update, so I'll probably most definitely need to update it for that.
There was a problem hiding this comment.
Yeah. I also think the changes here are nonsense. Will test and remove.
There was a problem hiding this comment.
Yeah. I also think the changes here are nonsense. Will test and remove.
There was a problem hiding this comment.
I too have zero idea what the changes here are supposed to mean.
There was a problem hiding this comment.
Yeah. I also think the changes here are nonsense. Will test and remove.
There was a problem hiding this comment.
I suppose I agree with David's points and will implement them.
There was a problem hiding this comment.
This is a very long file. I haven't read through it yet, but I think I suppose that this file is largely based on the dictionary attack file? I think I'll want to do a diff with that to see the relevant parts.
There was a problem hiding this comment.
This is a very long file. I haven't read through it yet, but I think I suppose that this file is largely based on the dictionary attack file? I think I'll want to do a diff with that to see the relevant parts.
There was a problem hiding this comment.
I don't understand the need for the m_all_hashes. If it was mentioned in the thesis, I may have forgotten.
There was a problem hiding this comment.
Ahh, I see David's point. Yeah, we'll probably do it like that.
There was a problem hiding this comment.
Yeah. I also think the changes here are nonsense. Will test and remove.
3 participants
Changes in Webadmin, Generator, Runner.
No changes to Database models.