On 01/07 09:31, Nicholas Marriott wrote:
> On Thu, Jan 06, 2011 at 03:32:17PM -0800, Jeremy Evans wrote:
> > This patch adds unix datagram socket support to nc(1). It's basically
> > the same patch I sent last June (see
> > http://marc.info/?l=openbsd-tech&m=127627296925965&w=2), but updated
> > for -current.
> >
> > Tested on amd64. Doesn't appear to cause any regressions to existing
> > support, tested with unix stream and IP stream and datagram sockets.
> > Looking for OKs.
> >
> > Jeremy
>
> Hmm, ISTR I meant to look at this ages ago but it got lost, sorry.
>
> So you are overloading -u to mean UDP without -l and datagram with -l?
> I guess this makes sense, but we need man page changes?
If you mean -U instead of -l, yes. -u without -U is IP UDP, -u with -U
is unix datagram. The man page doesn't say you can't use -u and -U
together, and it doesn't say that -U means unix stream sockets, though
that is currently all that -U supports. However, I think that making
some clarifications to the man page would helpful.
Responses inline and new diff at the end.
> > Index: netcat.c
> > ===================================================================
> > RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> > retrieving revision 1.98
> > diff -u -p -r1.98 netcat.c
> > --- netcat.c 3 Jul 2010 04:44:51 -0000 1.98
> > +++ netcat.c 6 Jan 2011 21:48:04 -0000
> > @@ -89,6 +89,7 @@ u_int rtableid;
> > int timeout = -1;
> > int family = AF_UNSPEC;
> > char *portlist[PORT_MAX+1];
> > +char *unix_dg_tmp_socket;
> >
> > void atelnet(int, unsigned char *, unsigned int);
> > void build_ports(char *);
> > @@ -99,6 +100,7 @@ int remote_connect(const char *, const c
> > int socks_connect(const char *, const char *, struct addrinfo,
> > const char *, const char *, struct addrinfo, int, const char *);
> > int udptest(int);
> > +int unix_bind(char *);
> > int unix_connect(char *);
> > int unix_listen(char *);
> > void set_common_sockopts(int);
> > @@ -241,8 +243,6 @@ main(int argc, char *argv[])
> >
> > /* Cruft to make sure options are clean, and used properly. */
> > if (argv[0] && !argv[1] && family == AF_UNIX) {
> > - if (uflag)
> > - errx(1, "cannot use -u and -U");
> > host = argv[0];
> > uport = NULL;
> > } else if (argv[0] && !argv[1]) {
> > @@ -265,6 +265,18 @@ main(int argc, char *argv[])
> > if (!lflag && kflag)
> > errx(1, "must use -l with -k");
> >
> > + /* Get name of temporary socket for unix datagram client */
> > + if ((family == AF_UNIX) && uflag && !lflag) {
> > + if(pflag) {
> > + unix_dg_tmp_socket = pflag;
> > + } else {
> > + if((unix_dg_tmp_socket = (char *)malloc(19)) == NULL)
> > + errx(1, "not enough memory");
>
> Style nit: space between if and (.
OK. My diff was bad about that, so I fixed the other cases as well.
> > + strlcpy(unix_dg_tmp_socket, "/tmp/nc.XXXXXXXXXX", 19);
> > + mktemp(unix_dg_tmp_socket);
>
> What if this fails?
You're right, a failure of mktemp should definitely be checked.
> > + }
> > + }
> > +
> > /* Initialize addrinfo structure. */
> > if (family != AF_UNIX) {
> > memset(&hints, 0, sizeof(struct addrinfo));
> > @@ -307,8 +319,12 @@ main(int argc, char *argv[])
> > int connfd;
> > ret = 0;
> >
> > - if (family == AF_UNIX)
> > - s = unix_listen(host);
> > + if (family == AF_UNIX) {
> > + if(uflag)
> > + s = unix_bind(host);
> > + else
> > + s = unix_listen(host);
> > + }
> >
> > /* Allow only one connection at a time, but stay alive. */
> > for (;;) {
> > @@ -337,17 +353,19 @@ main(int argc, char *argv[])
> > if (rv < 0)
> > err(1, "connect");
> >
> > - connfd = s;
> > + readwrite(s);
> > } else {
> > len = sizeof(cliaddr);
> > connfd = accept(s, (struct sockaddr *)&cliaddr,
> > &len);
> > + readwrite(connfd);
> > + close(connfd);
> > }
> >
> > - readwrite(connfd);
> > - close(connfd);
> > if (family != AF_UNIX)
> > close(s);
> > + else if (uflag)
> > + connect(s, NULL, 0);
>
> Likewise.
Correct, this should be checked as well.
> >
> > if (!kflag)
> > break;
> > @@ -361,6 +379,8 @@ main(int argc, char *argv[])
> > } else
> > ret = 1;
> >
> > + if(uflag)
> > + unlink(unix_dg_tmp_socket);
>
> Shouldn't this have the same condition as above?
The condition when the temp data socket is created is:
(family == AF_UNIX) && uflag && !lflag
The code for this section is (lines in new diff):
if (lflag) { // line 319
} else if (family == AF_UNIX) { // line 376
if (uflag) // line 385
} // line 389
So this does have the same conditions as above.
Here's the new diff, containing the following changes from the previous
diff:
1) Spaces between if and (
2) Switch from using -p to -s to specify source socket when using
-U and -u.
3) Error checking for mktemp and connect.
4) man page changes
Jeremy
Index: atomicio.c
===================================================================
RCS file: /cvs/src/usr.bin/nc/atomicio.c,v
retrieving revision 1.9
diff -u -p -r1.9 atomicio.c
--- atomicio.c 7 Sep 2007 14:50:44 -0000 1.9
+++ atomicio.c 6 Jan 2011 21:48:04 -0000
@@ -53,7 +53,7 @@ atomicio(ssize_t (*f) (int, void *, size
case -1:
if (errno == EINTR)
continue;
- if (errno == EAGAIN) {
+ if ((errno == EAGAIN) || (errno == ENOBUFS)) {
(void)poll(&pfd, 1, -1);
continue;
}
Index: nc.1
===================================================================
RCS file: /cvs/src/usr.bin/nc/nc.1,v
retrieving revision 1.55
diff -u -p -r1.55 nc.1
--- nc.1 25 Jul 2010 07:51:39 -0000 1.55
+++ nc.1 7 Jan 2011 16:39:07 -0000
@@ -155,6 +155,10 @@ assigns them.
Enables the RFC 2385 TCP MD5 signature option.
.It Fl s Ar source_ip_address
Specifies the IP of the interface which is used to send the packets.
+For
+.Ux Ns -domain
+datagram sockets, specifies the local temporary socket file
+to create and use so that datagrams can be received.
It is an error to use this option in conjunction with the
.Fl l
option.
@@ -179,6 +183,9 @@ Specifies to use
sockets.
.It Fl u
Use UDP instead of the default option of TCP.
+For
+.Ux Ns -domain
+sockets, use a datagram socket instead of a stream socket.
.It Fl V Ar rtable
Set the routing table to be used.
The default is 0.
Index: netcat.c
===================================================================
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.98
diff -u -p -r1.98 netcat.c
--- netcat.c 3 Jul 2010 04:44:51 -0000 1.98
+++ netcat.c 7 Jan 2011 16:41:59 -0000
@@ -89,6 +89,7 @@ u_int rtableid;
int timeout = -1;
int family = AF_UNSPEC;
char *portlist[PORT_MAX+1];
+char *unix_dg_tmp_socket;
void atelnet(int, unsigned char *, unsigned int);
void build_ports(char *);
@@ -99,6 +100,7 @@ int remote_connect(const char *, const c
int socks_connect(const char *, const char *, struct addrinfo,
const char *, const char *, struct addrinfo, int, const char *);
int udptest(int);
+int unix_bind(char *);
int unix_connect(char *);
int unix_listen(char *);
void set_common_sockopts(int);
@@ -241,8 +243,6 @@ main(int argc, char *argv[])
/* Cruft to make sure options are clean, and used properly. */
if (argv[0] && !argv[1] && family == AF_UNIX) {
- if (uflag)
- errx(1, "cannot use -u and -U");
host = argv[0];
uport = NULL;
} else if (argv[0] && !argv[1]) {
@@ -265,6 +265,19 @@ main(int argc, char *argv[])
if (!lflag && kflag)
errx(1, "must use -l with -k");
+ /* Get name of temporary socket for unix datagram client */
+ if ((family == AF_UNIX) && uflag && !lflag) {
+ if (sflag) {
+ unix_dg_tmp_socket = sflag;
+ } else {
+ if ((unix_dg_tmp_socket = (char *)malloc(19)) == NULL)
+ errx(1, "not enough memory");
+ strlcpy(unix_dg_tmp_socket, "/tmp/nc.XXXXXXXXXX", 19);
+ if (mktemp(unix_dg_tmp_socket) == NULL)
+ err(1, "mktemp");
+ }
+ }
+
/* Initialize addrinfo structure. */
if (family != AF_UNIX) {
memset(&hints, 0, sizeof(struct addrinfo));
@@ -307,8 +320,12 @@ main(int argc, char *argv[])
int connfd;
ret = 0;
- if (family == AF_UNIX)
- s = unix_listen(host);
+ if (family == AF_UNIX) {
+ if (uflag)
+ s = unix_bind(host);
+ else
+ s = unix_listen(host);
+ }
/* Allow only one connection at a time, but stay alive. */
for (;;) {
@@ -337,17 +354,21 @@ main(int argc, char *argv[])
if (rv < 0)
err(1, "connect");
- connfd = s;
+ readwrite(s);
} else {
len = sizeof(cliaddr);
connfd = accept(s, (struct sockaddr *)&cliaddr,
&len);
+ readwrite(connfd);
+ close(connfd);
}
- readwrite(connfd);
- close(connfd);
if (family != AF_UNIX)
close(s);
+ else if (uflag) {
+ if (connect(s, NULL, 0) < 0)
+ err(1, "connect");
+ }
if (!kflag)
break;
@@ -361,6 +382,8 @@ main(int argc, char *argv[])
} else
ret = 1;
+ if (uflag)
+ unlink(unix_dg_tmp_socket);
exit(ret);
} else {
@@ -421,18 +444,19 @@ main(int argc, char *argv[])
}
/*
- * unix_connect()
- * Returns a socket connected to a local unix socket. Returns -1 on failure.
+ * unix_bind()
+ * Returns a unix socket bound to the given path
*/
int
-unix_connect(char *path)
+unix_bind(char *path)
{
struct sockaddr_un sun;
int s;
- if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0)
+ /* Create unix domain socket. */
+ if ((s = socket(AF_UNIX, uflag ? SOCK_DGRAM : SOCK_STREAM,
+ 0)) < 0)
return (-1);
- (void)fcntl(s, F_SETFD, 1);
memset(&sun, 0, sizeof(struct sockaddr_un));
sun.sun_family = AF_UNIX;
@@ -443,27 +467,32 @@ unix_connect(char *path)
errno = ENAMETOOLONG;
return (-1);
}
- if (connect(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) < 0) {
+
+ if (bind(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) < 0) {
close(s);
return (-1);
}
return (s);
-
}
/*
- * unix_listen()
- * Create a unix domain socket, and listen on it.
+ * unix_connect()
+ * Returns a socket connected to a local unix socket. Returns -1 on failure.
*/
int
-unix_listen(char *path)
+unix_connect(char *path)
{
struct sockaddr_un sun;
int s;
- /* Create unix domain socket. */
- if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0)
- return (-1);
+ if (uflag) {
+ if ((s = unix_bind(unix_dg_tmp_socket)) < 0)
+ return (-1);
+ } else {
+ if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0)
+ return (-1);
+ }
+ (void)fcntl(s, F_SETFD, 1);
memset(&sun, 0, sizeof(struct sockaddr_un));
sun.sun_family = AF_UNIX;
@@ -474,11 +503,24 @@ unix_listen(char *path)
errno = ENAMETOOLONG;
return (-1);
}
-
- if (bind(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) < 0) {
+ if (connect(s, (struct sockaddr *)&sun, SUN_LEN(&sun)) < 0) {
close(s);
return (-1);
}
+ return (s);
+
+}
+
+/*
+ * unix_listen()
+ * Create a unix domain socket, and listen on it.
+ */
+int
+unix_listen(char *path)
+{
+ int s;
+ if ((s = unix_bind(path)) < 0)
+ return (-1);
if (listen(s, 5) < 0) {
close(s);
