Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: ncat - UNIX-domain sockets support
From: Tomas Hozza <thozza () redhat com>
Date: Thu, 4 Oct 2012 10:16:00 -0400 (EDT)

Hi David.

Thank you very much for reviewing my patches. I did some refactoring
and added/changed thinks you've requested.

This code looks quite solid, thanks for it. I have two requests for
this
patch:
 1. Reduce code duplication in ncat_listen.c.

I removed duplicated sections of code and used existing functions
ncat_listen_stream and ncat_listen_dgram.

 2. Add tests for local sockets in test/ncat-test.pl.

I added three tests for UNIX domain sockets implementation in Ncat.

nsock_connect_unixsock_internal should be static. Also, it looks like
more common code can be moved into nsock_connect_internal.

I removed the nsock_connect_unixsock_internal and one other function for
making sockets and moved more common code to the existing functions.
 
ncat_listen_unixsock_stream and ncat_listen_unixsock_dgram look like
mostly copy-pastes from ncat_listen_stream and ncat_listen_dgram. We
can't accept that for reasons of maintenance. These need to be
integrated in the existing loops; this may mean building new
supporting
abstractions.

They are gone! I used existing functions.

Is there any reason not to handle both SOCK_STREAM and SOCK_DGRAM
Unix
domain sockets in the same loop? I don't know if the same reasoning
applies as for separating ncat_listen_stream and ncat_listen_dgram.
(I'm
not sure why ncat_listen_stream and ncat_listen_dgram are separate
either.)

As stated earlier, I removed ncat_listen_stream and ncat_listen_dgram.
SOCK_STREAM and SOCK_DGRAM sockets cannot be handled in the same loop,
because STREAM sockets are with connection while the DGRAM sockets are
connectionless.

-      nsock_trace(ms, "Callback: %s %s %sfor EID %li [%s:%d]",
-                  nse_type2str(nse->type),
nse_status2str(nse->status), errstr,
-                  nse->id, inet_ntop_ez(&nsi->peer, nsi->peerlen),
nsi_peerport(nsi));
+#if HAVE_SYS_UN_H
+      if (nsi->peer.ss_family == AF_UNIX)
+        nsock_trace(ms, "Callback: %s %s %sfor EID %li [%s]",
+                    nse_type2str(nse->type),
nse_status2str(nse->status), errstr,
+                    nse->id, get_unixsock_path(&nsi->peer));
+      else
+#endif
+        nsock_trace(ms, "Callback: %s %s %sfor EID %li [%s:%d]",
+                    nse_type2str(nse->type),
nse_status2str(nse->status), errstr,
+                    nse->id, inet_ntop_ez(&nsi->peer,
nsi->peerlen), nsi_peerport(nsi));

Here and in many other places it looks like you need an abstraction
around inet_ntop_ez and get_unixsock_path rather than duplicating the
nsock_trace call.

I moved the abstraction to new get_hostaddr_string() function.

+        if (o.chat)
+            bye("Chat option not supported when using UNIX-domain
sockets.");
+#ifdef HAVE_OPENSSL
+        if (o.ssl)
+            bye("SSL option not supported when using UNIX-domain
sockets.");
+#endif
+        if (o.nodns)
+            bye("Not using DNS option with UNIX-domain sockets
doesn't make sense.");
+        if (o.broker)
+            bye("Connection brokering not supported when using
UNIX-domain sockets.");

Do all these things intrinsically not work with Unix domain sockets?
Or
is it just a side effect of not sharing the same loop. I see no
reason
to forbid o.nodns: that option may have effects beyond just looking
up a
connect target.

Well, to be honest I'm not sure if they won't work with UNIX sockets.
I removed some of them and included only ones I think they won't work.

It would be great if you (David) or other people would have a look on
my new patches and give me some feedback.

Thanks!

Regards,
Tomas Hozza

Attachment: 0001-Implementation-of-UNIX-domain-sockets-for-Nsock.patch
Description:

Attachment: 0002-Implementation-of-UNIX-domain-sockets-for-Ncat.patch
Description:

Attachment: 0003-Tests-for-UNIX-sockets-implementation-in-Ncat.patch
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]
AlienVault