-
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
-C option inversion in wash #106
Comments
the -C tag in reaver is not "inverted" and it is there to execute a command after reaver got the PIN
you are speaking about wash i guess |
Dude, dont start again. I was talking about this project in general and wanted to bring a message to all of your developers to stay compatible and not to reverse meanings of any parameter randomly. wpsmon.c (or wash) is just an example |
i agree that in this specific case the inversion of the meaning is very confusing, to say the least. maybe @t6x recalls why this was done ? |
Sorry for the confusion everyone. I found this while trying to fix WPS support in wifite2 during Christmas break. I wrote a detailed explanation of the '-C inversion' problem here: My suspicion is that these lines should probably be reverted, because having different outcomes depending on which version of the wash, or the fork of it, are installed breaks runtime compatibility for scripts that try and communicate with arbitrary versions of wash. From what I found, it doesn't look like wifite-v1 necessarily needs versions of wash from the forked repositories, but then when you don't have them installed you lose some functionality from the forks (so, maintainers make this choice optional). That's why I think this was never an issue before for any distros shipping with the non-forked versions of reaver-wps-fork-t6x. Wifite-v2 looks like it has a hard dependency on leveraging the pixie attack from reaver-wps-fork-t6x, so, when they copied the logic over from v1, it doesn't look like the old passing '-C' works anymore. And that's precisely because it's meaning was inverted in this fork of wash. Might be easier for upstream just to revert the '-C' inversion, though I don't know enough about scripts that depend on this new version of wash to make that criticism. Btw @blshkv very thorough work on that patch, you fixed a lot of backwards-compatibility! 👍 |
My patch is very simple: the current -C flag is renamed as -F, while new -C does nothing since it is a new default behavior of this fork. It is a quick hack but it works. |
This reverts commit 63c5c02. Above commit made a lot of unrelated changes without proper explanations. One of the changes made for example causes inversal of the behaviour to wash's -C flag (t6x#106), and the other changes seem highly controversial as well. It is better to just revert them all and then with sufficient time trying to reproduce and analyze what each of the changes tried to achieve, and if indeed an improvement, reapply that specific change including proper documentation (commit message) why that change was made.
the flatr0ze "mod" just changed the behaviour of the existing -C flags, which broke 3rd party apps. It was reverted in the previous commit, now reintroduced with a new flag. see issue t6x#106 for explanation.
submitted a PR. ^ |
also note that the flatr0ze patch was so hasty that it did not even change the output of the help text:
the option was renamed to "announce-fcs", but the explanation text still talked about "Ignore". |
I would say exactly the same to you.
The focus here is to have a the most efficient minimal syntax with wash and that what this change is about. @ rofl0r thanks again for your effort, sox and i are on holliday right now and will come back full of energy soon and we will speak about this and a couple of things more if you agree; some details that can be improved. . . |
closed because we are dealing about it here: https://github.com/t6x/reaver-wps-fork-t6x/pull/107 |
We are facing an issue with wifite which is calling reaver with "-C" parameter:
pentoo/pentoo-overlay#139
However, the following commit has inverted its meaning:
63c5c02
This is just wrong. Please introduce a new flag if you do that and keep it compatible with original code
The text was updated successfully, but these errors were encountered: