Remote Desktop Protocol #97
Conversation
|
I don't understand the root cause of it failing to complete a successful build after adding @microsoft Remote Desktop Protocol support. I suspect that it's a simple bug that I can fix though I'd appreciate help in its identification. |
There was a problem hiding this comment.
Thanks, there are a few issues with this PR which I've commented on.
In Summary: We should avoid duplicating code for the same protocol on different ports. Taking into account #46 all protocols should probably take port number as a parameter so we can rearrange and inject new ports to try at a higher level.
| @@ -87,6 +91,7 @@ func (os *OnionScan) Scan(hiddenService string, out chan *report.OnionScanReport | |||
|
|
|||
| report := report.NewOnionScanReport(hiddenService) | |||
|
|
|||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
Thank you for your help @s-rah! I didn't understand that prior to deciding to commit it but I understand it's due to a conflict. I'll amend it. I presume this is the problem causing it to fail to build? If it isn't then I'll redo it all again prior to commit it although perhaps I should also set up automated builds to be certain it's successful. I also changed to default branch that I commit source code to so I'm using the preferred branch. I hadn't committed source code to another branch before and I found it quite confusing!
| report.WebDetected = true | ||
| wps := new(spider.OnionSpider) | ||
| wps.Crawl(report.HiddenService, osc, report) | ||
| } |
There was a problem hiding this comment.
Let's not repeat a bunch of code when all that changes is the port - let's make port number a configurable parameter instead
There was a problem hiding this comment.
I agree. I'm embarrassed that I opted to duplicate the source code rather than just implement it as a parameter instead because it's such a basic skill and agnostic to programming languages! I'll redo it. I guess I perhaps should start to add useful comments in the source code too.
| if conn != nil { | ||
| conn.Close() | ||
| } | ||
| } |
There was a problem hiding this comment.
Same as above, let's not repeat a bunch of code, let's make port number configurable instead.
| @@ -33,4 +33,21 @@ func (sps *TLSProtocolScanner) ScanProtocol(hiddenService string, osc *config.On | |||
| if conn != nil { | |||
| conn.Close() | |||
| } | |||
| osc.LogInfo(fmt.Sprintf("Checking %s TLS(8443)\n", hiddenService)) | |||
| Info interface{} `json:"info"` | ||
| } | ||
|
|
||
| func (osr *OnionScanReport) AddProtocolInfo(protocolType string, protocolPort uint, protocolInfo interface{}) { |
There was a problem hiding this comment.
This hasn't been finalized yet, and commits shouldn't introduce it or rely on it.
|
@s-rah I applied the changes as suggested but it's still not resulting in a successful build as I should expect and I'm so confused! |
|
@sainslie You can see the broken builds at https://travis-ci.org/s-rah/onionscan/builds/169658253 - you are redeclaring some variables which is causing the code to not compile. - You should be able to see this when building locally. |
|
@s-rah I just outright deleted the intended additional alternate ports until I can add it as a parameter but in the meantime it at least adds @microsoft Remote Desktop Protocol to the list of protocols that are supported in addition to @RealVNC. I'm excited about the graphic aspects that are being added too. It's going to be much better to understand the results! |
|
Thanks @sainslie, this looks good. Can you squash the commits down to a single commit? - there is a lot of cruft currently in commit log :) |
I added basic support for @microsoft Remote Desktop Protocol detection although I intend to add additional support for it to extract richer data from detected instances as @s-rah has done for the other popular @RealVNC protocol. I hope it can be used for identification of specific iterations of it based upon the extracted data. I also added additional support for alternate ports to other popular supported protocols as it's a common technique to use alternate ports.