Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSH module is not iterating on the credential list properly #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

SSH module is not iterating on the credential list properly #99

wants to merge 1 commit into from

Conversation

schischi
Copy link
Contributor

The logic to get the credentials for the SSH module is wrong:

  • Pairwise: the module is using the wrong pairs of credentials: user[idx] with pass[idx+6];
  • Default: the first 6 passwords are ignored for all the usernames except the first one;

With pairwise:

  $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) ssh://127.0.0.1 | grep Login
  (EID 1) Login failed: '1' '1'
  (EID 1) Login failed: '1' '2'          << should be '2' '2'
  (EID 1) Login failed: '1' '3'          << should be '3' '3'
  (EID 1) Login failed: '1' '4'          << ...
  (EID 1) Login failed: '1' '5'          << ...
  (EID 1) Login failed: '1' '6'          << ...
  (EID 1) Login failed: '2' '7'          << should be '7' '7'
  ...
  (EID 24) Login failed: '32' '38'
  ...
  (EID 83) Login failed: '81' '87'
  ...
  (EID 105) Login failed: '99' '5'
  (EID 106) Login failed: '100' '6'

As we can see, ncrack is using the wrong pairs of credentials: user[idx] with pass[idx+6].

With default option:

  $ ./ncrack -vvv -d10 -U <(seq 3) -P <(seq 10) ssh://127.0.0.1 | grep Login
  (EID 1) Connection closed by peer
  (EID 1) Login failed: '1' '1'
  (EID 1) Login failed: '1' '2'
  (EID 1) Login failed: '1' '3'
  (EID 1) Login failed: '1' '4'
  (EID 1) Login failed: '1' '5'
  (EID 1) Login failed: '1' '6'
  (EID 2) Login failed: '1' '7'
  (EID 4) Login failed: '1' '8'
  (EID 6) Login failed: '1' '9'
  (EID 8) Login failed: '1' '10'
  (EID 3) Login failed: '2' '7'   << missing data here (6 creds)
  (EID 5) Login failed: '2' '8'
  (EID 7) Login failed: '2' '9'
  (EID 9) Login failed: '2' '10'
  (EID 3) Login failed: '3' '7'   << missing data here (6 creds)
  (EID 5) Login failed: '3' '8'
  (EID 7) Login failed: '3' '9'
  (EID 9) Login failed: '3' '10'

Ncrack is skipping the first 6 passwords for all the users except the first one.

Note that if we run the same command but with the mysql module, everything looks good:
Example:

  $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) mysql://127.0.0.1 | grep Login
  mysql://127.0.0.1:3306 (EID 1) Login failed: '1' '1'
  mysql://127.0.0.1:3306 (EID 2) Login failed: '2' '2'
  ...
  mysql://127.0.0.1:3306 (EID 99) Login failed: '99' '99'
  mysql://127.0.0.1:3306 (EID 100) Login failed: '100' '100'

The commit f2270de introduced this regression and reverting it fix the issue.
The description of the commit lacks a bit of context, but I guess the intent was that if we are using the same usernames for different attempts, then we could keep the same connection open and speed up things during the first timing probe.
Even if reverting this commits might cause a bit of performance drop, I think it's more important to have code that behave as it should.

With this commit applied:

  $ ./ncrack -vvv -d10 --pairwise -U <(seq 20) -P <(seq 20) ssh://127.0.0.1 | grep Login
  (EID 1) Login failed: '1' '1'
  ...
  (EID 21) Login failed: '20' '20'

The logic to get the credentials for the SSH module is wrong:
 - Pairwise: the module is using the wrong pairs of credentials: user[idx] with pass[idx+6];
 - Default: the first 6 passwords are ignored for all the usernames except the first one;

With pairwise:
  $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) ssh://127.0.0.1 | grep Login
  (EID 1) Login failed: '1' '1'
  (EID 1) Login failed: '1' '2'          << should be '2' '2'
  (EID 1) Login failed: '1' '3'          << should be '3' '3'
  (EID 1) Login failed: '1' '4'          << ...
  (EID 1) Login failed: '1' '5'          << ...
  (EID 1) Login failed: '1' '6'          << ...
  (EID 1) Login failed: '2' '7'          << should be '7' '7'
  ...
  (EID 24) Login failed: '32' '38'
  ...
  (EID 83) Login failed: '81' '87'
  ...
  (EID 105) Login failed: '99' '5'
  (EID 106) Login failed: '100' '6'

As we can see, ncrack is using the wrong pairs of credentials: user[idx] with pass[idx+6].

With default option:
  $ ./ncrack -vvv -d10 -U <(seq 3) -P <(seq 10) ssh://127.0.0.1 | grep Login
  (EID 1) Connection closed by peer
  (EID 1) Login failed: '1' '1'
  (EID 1) Login failed: '1' '2'
  (EID 1) Login failed: '1' '3'
  (EID 1) Login failed: '1' '4'
  (EID 1) Login failed: '1' '5'
  (EID 1) Login failed: '1' '6'
  (EID 2) Login failed: '1' '7'
  (EID 4) Login failed: '1' '8'
  (EID 6) Login failed: '1' '9'
  (EID 8) Login failed: '1' '10'
  (EID 3) Login failed: '2' '7'   << missing data here (6 creds)
  (EID 5) Login failed: '2' '8'
  (EID 7) Login failed: '2' '9'
  (EID 9) Login failed: '2' '10'
  (EID 3) Login failed: '3' '7'   << missing data here (6 creds)
  (EID 5) Login failed: '3' '8'
  (EID 7) Login failed: '3' '9'
  (EID 9) Login failed: '3' '10'

Ncrack is skipping the first 6 passwords for all the users except the first one.

Note that if we run the same command but with the mysql module, everything looks good:
Example:
  $ ./ncrack -vvv -d10 --pairwise -U <(seq 100) -P <(seq 100) mysql://127.0.0.1 | grep Login
  mysql://127.0.0.1:3306 (EID 1) Login failed: '1' '1'
  mysql://127.0.0.1:3306 (EID 2) Login failed: '2' '2'
  ...
  mysql://127.0.0.1:3306 (EID 99) Login failed: '99' '99'
  mysql://127.0.0.1:3306 (EID 100) Login failed: '100' '100'

The commit f2270de introduced this regression and reverting it fix the issue.
The description of the commit lacks a bit of context, but I guess the intent was that if we are using the same usernames for different attempts, then we could keep the same connection open and speed up things during the first timing probe.
Even if reverting this commits might cause a bit of performance drop, I think it's more important to have code that behave as it should.

With this commit applied:
  $ ./ncrack -vvv -d10 --pairwise -U <(seq 20) -P <(seq 20) ssh://127.0.0.1 | grep Login
  (EID 1) Login failed: '1' '1'
  ...
  (EID 21) Login failed: '20' '20'
@dguerri
Copy link

dguerri commented Jul 19, 2021

@ithilgore could this be merged in your opinion? It looks sensible as some credentials are not being used while bruteforcing!

Copy link
Collaborator

@ithilgore ithilgore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Place inside #if 0 and #endif instead of deleting. Might have to fix this later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants