Open redsand opened 1 year ago
Thanks for an awesome and massive pull request. We'll get testing this on our end prior to merge. thank you!
Thanks for an awesome and massive pull request. We'll get testing this on our end prior to merge. thank you!
Hit me up with any questions. There's opportunity to improve the valid credential detection. Also noticed that the new screenshot file for authentication verification can be overwritten and needs to be checked/incremented. will sort this out now.
I've not read through the code so this may already be in place, but it would be good if this was optional and disabled by default.
I think going from just taking a screenshot is a significant enough change that people won't be expecting it and in some rare cases it may cause people problems.
Just checking in, don't want you to think we forgot about this. We're all at BlackHat/Defcon this week and next so we're fairly tied up, but this is still on top of our list as to review and merge in.
Also, I do agree that we should ensure that the active checking is disabled by default, but can be enabled by a command line flag (if that's not how it is currently designed)
This feature is now disabled by default. To enable, use --enable-validation
Thanks everyone for the feedback. Request has been updated and am currently testing it internally with solid success. Have a great time in vegas, have a beer for me. <3
Bump
Any updates or feedback?
Sorry, this was not forgotten, this is just a giant merge in (with awesome functionality). Ideally Q1 is the best time for us to look to merge this.
Checking in on this.
Hey @redsand, thanks for checking in. I imagine Chris is swamped right now.
Now that we finally have the setup script rewrite dealt with, @Relkci and I will be finding the time to go through this. I don't want to make promises we can't keep so I can't give you an exact timeline, but we've started looking through your changes and you can expect some feedback from us in the near future.
Thanks for your contribution and your patience. 😄
updated to resolve conflicts
The recent change to signatures.txt breaks functionality. Please note that the username and password is a new field at the end separated by |
Hi,
This request contains several new features. Firstly we needed to make an attempt to validate the credentials provided, to further validate the results from the scan. This allows for more concrete actionable items for review and remediation. In order to do this, additional structure and formatting was required in the signatures.txt file. An additional credentials field has been added to the end, after the description field. Each entry is seperated by (username)/(password) and multiples can be added by separating them with a ';' such as: admin/admin;admin/123;user/456
Additionally, screenshots are captured to help validate default credential detection during review and has been included in the generated report.
Finally, a feature to detect remote hosts based upon the combination of Server and WWW-Authenticate headers. This is necessary for detecting and attempting default credentials from hosts that use WWW-Authenticate realms and has been tested against two remote hosts. Additional signatures and growth is required.