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

Adds -z option for Ncat #444

Closed
wants to merge 16 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@tremblerz
Contributor

tremblerz commented Jul 1, 2016

No description provided.

@@ -723,6 +728,24 @@ int main(int argc, char *argv[])
}
}
if (o.zerobyte) {

This comment has been minimized.

@tremblerz

tremblerz Jul 1, 2016

Contributor

@bonsaiviking Any better way to blacklist most of the incompatible options ?

@dmiller-nmap

This comment has been minimized.

dmiller-nmap commented Jul 1, 2016

This is well on its way. A few points of feedback:

  1. -z also works for SCTP and even Unix sockets, so it's not true that it "only works for TCP."
  2. Traditional netcat has a workaround for UDP that involves sending a single null byte ('\0') and treating a ECONNREFUSED as the false condition and everything else as true. It's ok if you don't implement this right away, but we should open a subsequent issue documenting the deficiency.
  3. We don't need another long option (--zero), just add it as a short option.
  4. We should suppress the "Ncat: Connection refused" message when using -z because it's usually used with scripting and the extra output will probably be objected to by someone. Netcat does not produce any output with -z.
  5. There's not a real better way to blacklist, but some of these probably don't matter. We want to blacklist ones that change the behavior, not the timing. Here's my list:
    • Any of the exec options: -c, -e, --lua-exec
    • -l, but don't explicitly check for things that only have meaning when -l is given, like -k, --chat, etc.
@@ -307,7 +307,7 @@ int main(int argc, char *argv[])
{"nsock-engine", required_argument, NULL, 0},
{"test", no_argument, NULL, 0},
{"ssl", no_argument, &o.ssl, 1},
{"zero", no_argument, NULL, 'z'},
{"z", no_argument, NULL, 'z'},

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 2, 2016

To add a short argument, you don't need to add to the long_options array, just make sure it's included in the string argument to getopt_long (which it is below).

@tremblerz

This comment has been minimized.

Contributor

tremblerz commented Jul 2, 2016

@bonsaiviking Thanks for review, I have pushed correction for this. I am little doubtful over the implementation of UDP scan, here are few reasons why -

  1. Netcat reports all those UDP ports as open which do not reply with ECONNREFUSED. Implication - Link to paste
  2. What should be the time-limit to wait for ECONNREFUSED, I am thinking to implement it by setting up the value of o.idletimeout internally.

I also think that connect_report() shouldn't be called here if o.proto==IPPROTO_UDP.

This is the LINK to capture performed for Netcat. First five frames correspond to command nc -zuv google.com 21 while next four correspond to nc -zuv google.com 80. Payload sent by them is 58(Hex) that is equivalent to "X", any particular reason for it ?

@@ -1259,6 +1279,11 @@ static void write_socket_handler(nsock_pool nsp, nsock_event evt, void *data)
ncat_assert(status == NSE_STATUS_SUCCESS);
}
if (o.zerobyte){
ncat_assert(o.proto == IPPROTO_UDP);
nsock_read(nsp, cs.sock_nsi, read_socket_handler, 1000 * 2, NULL);

This comment has been minimized.

@tremblerz

tremblerz Jul 3, 2016

Contributor

I've kept 2s as time limit here.

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 5, 2016

We shouldn't hard-code this. We should be able to use either o.conntimeout or o.idletimeout instead. I lean towards conntimeout because we're waiting for a "connection" instead of a "read," though with UDP they're essentially the same thing.

@@ -1078,6 +1078,13 @@ int ncat_connect(void)
return rc == NSOCK_LOOP_ERROR ? 1 : 0;
}
static void send_udp_null(nsock_pool nsp)
{
char *NULL_PROBE = "\0";

This comment has been minimized.

@tremblerz

tremblerz Jul 3, 2016

Contributor

@bonsaiviking Is this implementation correct ? (I am little doubtful here)

nsock_readbytes(nsp, cs.stdin_nsi, read_stdin_handler, -1, NULL, 0);
if (o.zerobyte && o.proto==IPPROTO_UDP)

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 5, 2016

Should probably skip this if --recv-only is specified.

@@ -618,6 +621,7 @@ int main(int argc, char *argv[])
" --proxy <addr[:port]> Specify address of host to proxy through\n"
" --proxy-type <type> Specify proxy type (\"http\" or \"socks4\" or \"socks5\")\n"
" --proxy-auth <auth> Authenticate with HTTP or SOCKS proxy server\n"
" -z Just to establish connection, doesn't send any payload\n"

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 5, 2016

Careful of the spacing here (6 spaces), and put this below -w. The help text for traditional and OpenBSD netcat calls this "zero-I/O mode [used for scanning]" so we should probably use something similar, though I would skip the mention of scanning. Perhaps: "Zero-I/O mode, report connection status only"

} while ($pid > 0 && $pid != $c_pid);
$pid == $c_pid or die;
$code = $? >> 8;
$code == 2 or die "Exit code was $code, not 2";

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 15, 2016

Why is exit code 2 for this? ncat/util.c has this comment for the die and bye functions:

/* Exit status 2 indicates a program error other than a network error. */

So I think we should try to have the exit code be 1 in this case.

This comment has been minimized.

@tremblerz

tremblerz Jul 15, 2016

Contributor

I am trying but have not been able to catch the reason for getting the error code as 2. Strangely, it prints error code as 1 when tried manually like ./ncat -zv localhost 5000 then echo $?

do {
$pid = waitpid($c_pid, 0);
} while ($pid > 0 && $pid != $c_pid);
$pid == $c_pid or "$pid != $c_pid";

This comment has been minimized.

@dmiller-nmap

dmiller-nmap Jul 19, 2016

Missing die on this line.

@dmiller-nmap

This comment has been minimized.

dmiller-nmap commented Jul 19, 2016

Looks great! @tremblerz commit this as soon as you fix the missing die statement on line 3149.

@nmap-bot nmap-bot closed this in d04046a Jul 19, 2016

sergeykhegay added a commit to sergeykhegay/nmap that referenced this pull request Jul 27, 2016

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