-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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
Better CorpWatch modules (with more loot!) #80
Conversation
use print_line() instead of puts(), please :-) |
You might also wanna try out Rex::Ui::Text::Table instead of multiple print_status(), just a thought |
print_lines commited and pushed I might could get that done tonight as well |
tbh, I like the way the print_status looks compared to the rex ui table in this instance. However, if the general consensus is to put it into a table, I won't mind doing that. |
This pull request supercedes this redmine ticket: http://dev.metasploit.com/redmine/issues/5966 |
I don't think there's actually a rule (or guideline) on how modules should display data. The thing with Rex table is that you don't have to deal with formatting, and in case people want to convert to csv, it can do that easily with just a single line. And some people do ask us to convert data in csv... sometimes. There is a rule (that we've begun to enforce it more strictly) on storing data you gather, though. Typically we prefer modules to store data to database (report_note, report_auth_info, etc, etc), but for "looted" information you might also want to consider using store_loot(). By the way, I notice that you're using 'res.body == nil'. It's possible res might return nil, however, iirc the default value of 'body' should always be set to "". If you check out the request_raw() or request_cgi() function, you'll see. They do the actual work for send_request_cgi() and send_request_raw(). |
Sure, so I am saving the corpwatch_info data being retruned as loot. Do you think storing the search results from corpwatch_search should be saved as well? /me is getting another push ready |
The reason I am weary of using a rex table (unless you want it in key/value form rather than a horizontal row of values) is that the data being returned is widely variable, and would require a lot of horizontal screen space. Currently, the print_status allows the data to be displayed in a nicer way on both my netbook and y 22" monitor on my desktop. |
res.body and some other misc changes pushed |
You can build a Rex table vertically with add_row(), except I don't think there's an example... something you have to play with. But yeah, it's no biggie... using a Rex table is just a suggestion, not a requirement :-) The reporting part -- I believe whatever is shown on the screen should be saved. Looks like you're already doing that with store_loot(), so you're good on that part :-) |
Anything else need to happen to get this pushed into trunk? |
Yeah, sorry. I'll definitely work on it this week. Kinda stuck with something else right now :-( |
Working on this pull request right now (code reviewing). Is there a reason why you prefer send_request_raw() over send_request_cgi()?? I cannot tell how you're taking advantage of send_request_raw(); and I don't see why you need a hardcoded user-agent when send_request_cgi() already has a default one (and it's customizable). |
Tbh, I wasn't sure of the difference between the two. |
Ah ok, no problem then. send_request_cgi() has more stuff predefined (like user-agent, ctype, automatically calculate content length, etc)... it's more CGI compatible. send_request_raw() is more basic. You can check out request_cgi() vs request_raw() in Rex::Proto::Http::Client to see the difference :-) |
Also, I'll add this in the description because it's part of CorpWatch's API Terms of Use: |
Backported net-ssh ask_passphrase functionality MSP-10038
Made changes from some things hdm went over on IRC