-
Notifications
You must be signed in to change notification settings - Fork 397
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
Add arbitrary string PIN feature #133
Conversation
bb09f99
to
5b06019
Compare
since a pin string cannot advance (i.e. it's "one-shot") i'd rather not introduce a new option and just re-use the existing -p flag. edit: note that such a one-shot flag exists already, so it's even easier. |
Alright, I'll modify existing option. Thanks for the advice! |
btw, i just noticed who you are. i recently stumbled upon your database, which rocks. |
Thanks :)
Did you mean Router Scan? |
Commited new changes, not tested yet. |
Yes indeed. 😺
Yes: -g 1,
A fake NACK is systematically detected when it shouldn't be the case. That happens when we use -p option and -g option become useless. Reaver will never stop as the failed attempt for a wrong PIN is never correctly acknowledged. |
True, I also noticed that even with correct known PIN codes it fails with "Fake NACK" warning. |
I tried the commit and it worked fine... as far as I can say.
I don't have it with the master one from here. |
Hm... I didn't change
By the way, on the antichat forum we have some success using this new feature :) But the password was not received for some reason. |
Indeed. btw i've got a new cleaned up reaver in the work; it's based on latest official reaver with all known good changes from this repo backported, the only thing it's missing i'm aware of right now is the pixiedust attack feature. once i've got that in too, we'll probably continue development from there. so it may well be that when you apply your patch there it'll work. |
Oh, many thanks!
There is no source code publicly available, since it's closed source project (for security reasons). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work. i hope you understand that we don't want mixing up unrelated changes (for example whitespace) as this makes it much easier to see what a commit does, and use it as a standalone patch to apply to other branches.
src/cracker.c
Outdated
@@ -162,7 +162,14 @@ void crack() | |||
} | |||
else | |||
{ | |||
cprintf(WARNING, "[+] Trying pin %s\n", pin); | |||
if (get_pin_string_mode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this hunk needed at all ? afaics it just prints additional quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we may instead just put the string always in quotes
README.md
Outdated
@@ -77,12 +77,12 @@ Optional Arguments: | |||
-5, --5ghz Use 5GHz 802.11 channels | |||
-v, --verbose Display non-critical warnings (-vv for more) | |||
-q, --quiet Only display critical messages | |||
-K --pixie-dust=<number> [1] Run pixiewps with PKE, PKR, E-Hash1, E-Hash2 and E-Nonce (Ralink, Broadcom & Realtek) | |||
-K, --pixie-dust=<number> [1] Run pixiewps with PKE, PKR, E-Hash1, E-Hash2 and E-Nonce (Ralink, Broadcom & Realtek) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please do not add unrelated whitespace changes, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok!
src/argsparser.c
Outdated
@@ -46,8 +46,8 @@ int process_arguments(int argc, char **argv) | |||
char mac[MAC_ADDR_LEN] = { 0 }; | |||
char *short_options = "W:K:b:e:m:i:t:d:c:T:x:r:g:l:o:p:s:C:1:2:F:R:ZA5ELfnqvDShwXNPH0I"; | |||
struct option long_options[] = { | |||
{ "generate-pin", required_argument, NULL, 'W' }, | |||
{ "stop-in-m1", no_argument, NULL, '0' }, | |||
{ "generate-pin", required_argument, NULL, 'W' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace
src/argsparser.c
Outdated
@@ -85,8 +85,8 @@ int process_arguments(int argc, char **argv) | |||
{ "win7", no_argument, NULL, 'w' }, | |||
{ "exhaustive", no_argument, NULL, 'X' }, | |||
{ "help", no_argument, NULL, 'h' }, | |||
{ "pixiedust-loop", no_argument, NULL, 'P' }, | |||
{ "pixiedust-log", no_argument, NULL, 'H' }, | |||
{ "pixiedust-loop", no_argument, NULL, 'P' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace
src/argsparser.c
Outdated
@@ -301,6 +302,20 @@ void parse_recurring_delay(char *arg) | |||
free(x); | |||
} | |||
|
|||
int isnumber(char *pin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this shouldn't be "isnumber" but rather "is_valid_pin". there's a function wps_pin_checksum()
which you could use to check if the checksum is valid (after checking that the pin is numeric and of the right length). if the pin has an invalid checksum, one-shot mode needs to be activated as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code section at line 332 can be executed when the specified argument pin contains 8, 7 or 4 digits. If there is only 4 digits, it will set it as first part of pin, and continue bruteforcing second part. If there's 7 digits, it will calculate checksum digit and append it to the pin, entering one-shot mode. It also accepts 8 digit pin in two possible scenarios: with correct checksum and without it (-X "exhaustive" option is required for that).
So, any other pin values would set it into arbitrary string mode, that's why I added this function.
If we use wps_pin_checksum()
here, then we can't set 4 or 7 pin anymore (they will be interpreted as arbitrary strings).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's also the reason why I decided to make a new option at first - to be able specifying just any string without handling possible different situations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your research.
so in that case would it make sense to change the semantics of the function such that it checks:
- whether string is not numeric. if not, one-shot mode.
- if string is numeric and of size 8, check checksum. if checksum mismatch, one-shot mode, else normal operation.
- if string is numeric and of size 4,7, assume normal operation.
- if string is numeric and of any other size, one-shot mode.
?
as for your comment
without handling possible different situations
:-) well, this should probably have been done from the start, so you can see it as a general improvement. it doesn't make sense to try to advance the counter when the pin at hand doesn't fit the scheme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if string is numeric and of size 8, check checksum. if checksum mismatch, one-shot mode, else normal operation.
If checksum mismatch & "exhaustive" mode enabled, normal operation (it will bruteforce all 8 digits). Yep, this would require adding some more logics to "is_valid_pin" function.
as for your comment
This is exactly what it will do + the comment above.
Maybe revert to the initial solution with new option? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not my intention to annoy you, i just thought "if you do it, do it right". anyway, if i'm asking for more than you're willing to do, let's leave it at "isnumber" and i will fix it myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok!
src/wpscrack.c
Outdated
@@ -170,12 +170,12 @@ int usage(char *prog_name) | |||
fprintf(stderr, "\t-v, --verbose Display non-critical warnings (-vv for more)\n"); | |||
fprintf(stderr, "\t-q, --quiet Only display critical messages\n"); | |||
//fprintf(stderr, "\t-K, --pixie-dust Test Pixie Dust [1] Basic(-S) [2] With E-Once(-S) [3] With PKR \n"); | |||
fprintf(stderr, "\t-K --pixie-dust=<number> [1] Run pixiewps with PKE, PKR, E-Hash1, E-Hash2 and E-Nonce (Ralink, Broadcom & Realtek)\n"); | |||
fprintf(stderr, "\t-K, --pixie-dust=<number> [1] Run pixiewps with PKE, PKR, E-Hash1, E-Hash2 and E-Nonce (Ralink, Broadcom & Realtek)\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whitespace
Commited new changes, should be good to go. |
@rofl0r it seems like I did not got the idea about |
ok looks much better now, thanks! i wonder if we could get away without calling ..exhaustive, since iirc we don't have that in my cleaned up branch. btw @kcdtv i pushed initial support for pixiewps, could you please test my branch |
Hello!
ZTE ZXHN H118N (CPU - RTL8196C, WLAN - RTL8188C) It does not have WPS configuration in web interface at all, but exposes enabled WPS protocol. |
Thanks! |
sorry for the inconvenience, but we just made the switch of the codebase which we prepared since ~ 6 months. on the wiki in the FAQ is a how-to get current master into your fork. |
I updated the patch to new codebase. Did not tested yet. |
Im testing it right now.. here is error with output from make: argsparser.c: In function ‘is_valid_pin’: argsparser.o: In function |
So... Ok, fixed. |
Jepp, fixed. Now goes to further testing (^^,) got a tip for debugging the this (feature) only? |
Hi there!
Reaver shouldn't create a *wpc file when such feature is used and shouldn't modify an existing *wpc file. |
It's very simple, just enter
Ok, I'll fix that :) |
|
kcdtv said:
wpc should not be used at all when specifying any pin manually ? or only string-like pins ? |
Done! Please retest :) |
Brilliant!!!! 😺 |
Perfect!!!!
I used the -p "" argument between two PINS fully checked. |
@binarymaster & @kcdtv are you fighting those ciphers all day long you two?? :) is the arb. pin patching done? |
thanks, rebased and merged as 40862b8...493f72c. |
This check will be skipped, and function would return |
oh ok. that means that "666666" would be recognized as valid pin too. i.e. any numeric pins that are not 8 bytes long will be detected as valid. |
That's right. Any other length checks are performed outside this function. |
oh right! that's a bit confusing though since conceptually it belongs into the function. maybe i'll just merge the size checks into isvalidpin(). |
Hi there , |
please don't hijack existing issues for unrelated questions. open a new issue instead. |
It's not even an issue so don't open a new one, it's a question that can be easily answered with Google or the readme. |
well, the first part of his question was not answered: yes, if it shows |
This pull request resolves #132.
UPD: tested in real environment, fixed the bugs.