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

Portscan #6

Merged
merged 9 commits into from Jul 6, 2013

Conversation

Projects
None yet
3 participants
@webstersprodigy
Contributor

webstersprodigy commented Jun 10, 2013

This is a portscan module I've found useful. For example, if I'm host A and can't reach host C, but have code execution on b1,b2,b3...bn, I can run this portscan module using remote powershell on b1...bn to see if any can reach host C.

I'm new to powershell, so suggestions are welcome. I have one C# type that is probably not strictly necessary (I use it for async callbacks), but debugging I couldn't get it working with strictly powershell.

@webstersprodigy

This comment has been minimized.

Contributor

webstersprodigy commented Jun 10, 2013

Just noticed that right now this only works with powershell version 3. I'm not sure all the problems it has with version 2. I'll update later today.

@webstersprodigy

This comment has been minimized.

Contributor

webstersprodigy commented Jun 10, 2013

Set-StrictMode -Version 2.0 works, but 2.0 still doesn't, am trying to debug now

@sunnyc7

This comment has been minimized.

sunnyc7 commented Jun 11, 2013

Would suggest looking into the code here by Jeff Graves.
Multi-threaded port scanner, scans in CIDR.
TCP+UDP
http://www.orcsweb.com/blog/jeff/multithreaded-powershell-port-scanner/

I am not sure if version 2.0 compatibility is an issue, as long as you are using System.Net.

@webstersprodigy

This comment has been minimized.

Contributor

webstersprodigy commented Jun 11, 2013

@sunnyc7 interesting link - I gave it a whirl and it's not bad. It does do UDP and that is cool - probably worth integrating. Here are some things where it has disadvantages

  • Way slower. There's not good async threading, so for any decent sized number of ports it's fairly unusable. Also, there's no ping scan. With invoke-portscan, you can scan top thousand ports on a /24 in about two minutes. This threading piece took me a while - with his you'd probably have to let run overnight.
  • Less options for use on the command line - you have to copy to config files, they have to be in a certain place etc. It's not as flexible.
  • No powershell 2.0 support - which I do think is important (at least for me). If I want to run this on N hosts, then I need it to actually run on 2.0 or 3.0.
  • A bunch of small things, like his breaks with ipv6 cidr, probably more
@mattifestation

This comment has been minimized.

Contributor

mattifestation commented Jun 11, 2013

Thanks a lot for your contribution, Rich! Before I can merge this into PowerSploit though, I have a few recommendations. Let me know what your thoughts are:

  1. Please refer to the PowerSploit style guide (https://github.com/mattifestation/PowerSploit#script-style-guide). Rule number one states that 'though shalt not write to the host/console'. All functions that output meaningful information should output proper objects. That way, they can be piped to other cmdlets/functions. As you may or may not be aware, this is tricky when running in a callback. The when running in the callback function, you're kind of in a bastardized state where a lot of the functionality of PowerShell breaks. To rectify this, the best thing to do is to output all of your objects to a global array. Then you can output proper objects upon completion of the port scan.

  2. I'm glad you went with async IO for a port scan. This is the obvious choice as we wouldn't want to block the main thread. Calling Start-Job, in my opinion defeats the purpose of proper multithreading though since Start-Job fires up a child process. I would recommend that you use a runspace pool instead.

  3. Parameters in PowerShell should be self-describing. While I appreciate you replicating the 'feel' of nmap, it doesn't translate too well to PowerShell. For example, instead of -xPorts, call it -ExcludedPorts. Remember, you can always create an alias for a parameter if you want to maintain the unix-y 'feel'.

  4. HelpMessage is not a replacement for .PARAMETER in comment-based help. This will become apparent if you run Get-Help Invoke-Portscan -Full. You shoudn't have to read the source code to understand a function and it's parameters. You should be able to get everything you need from Get-Help.

  5. I would take out the CIDR parsing for IPv6 since it has a PSv3 dependency.

  6. Parameters that accept a comma-separated list should accept an array instead. For example, instead of '[String] $Ports', consider using '[UInt16[]] $Ports'. What's nice about this is that it provides parameter validation for free since a UInt16 can only accept 0-65535. Also, consider using validation attributes - ValidateRange, ValidateSet, ValidatePattern (which would be perfect for validating CIDR ranges), ValidateScript, etc. http://msdn.microsoft.com/en-us/library/windows/desktop/ms714836(v=vs.85).aspx

That's all for now.

Thanks again, Matt

@webstersprodigy

This comment has been minimized.

Contributor

webstersprodigy commented Jun 11, 2013

Thanks for the feedback - I agree with all of it. I don't have tons of time to work on this over the next 1.5 weeks, but I can help make these changes when I have spare cycles (or of course feel free to make whatever changes earlier)

@mattifestation

This comment has been minimized.

Contributor

mattifestation commented Jun 11, 2013

Thanks for understanding. I'm patient. Take your time. :D

webstersprodigy added some commits Jun 18, 2013

Addressed mattifestation feedback
See #6 (comment)

1) I like this feedback a lot and took it.

2) I tried going thread only but it got messed up with very large scans. Eventually,
I didn't think it was worth the amount of effort to make it reliable with only threads

3) Tried to do this

4) Did this

5) I like the idea in general and I took this one place (top-ports), but not for the two
examples you gave. The reasoning is, I want people to be able to specify various options
and arrays aren't that flexible. For example, I want people to specify a port list like
"80,90,8080-8090". Similar with CIDR, since that's one option, but they could also be
specifying hostnames e.g. "google.com,192.168.1.1/24,10.0.0.1"

@mattifestation mattifestation merged commit 98510d8 into PowerShellMafia:master Jul 6, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment