On Sat, Sep 05, 2015 at 01:00:54PM -0600, Bob Beck wrote:
> OK to put it in and have others turdshine along?
I think some things should be fixed before.
$ mandoc -Tlint nc.1
mandoc: nc.1:105:64: WARNING: whitespace at end of input line
mandoc: nc.1:188:69: WARNING: whitespace at end of input line
mandoc: nc.1:190:17: WARNING: whitespace at end of input line
mandoc: nc.1:210:16: WARNING: whitespace at end of input line
mandoc: nc.1:212:14: WARNING: whitespace at end of input line
$ WARNINGS=yes make
cc -O2 -pipe -Wall -Wpointer-arith -Wuninitialized -Wstrict-prototypes
-Wmissing-prototypes -Wunused -Wsign-compare -Wshadow
-Wdeclaration-after-statement -c netcat.c
netcat.c: In function 'main':
netcat.c:593: warning: declaration of 'i' shadows a previous local
netcat.c:534: warning: shadowed declaration is here
re-wrap the long lines
> @@ -98,6 +98,11 @@ to use IPv4 addresses only.
> Forces
> .Nm
> to use IPv6 addresses only.
> +.It Fl c
> +If using a tcp socket to connect or listen, use TLS. Illegal if not using
> TCP sockets.
> +.It Fl C Ar certificate_filename
> +Specifies the filename from which the public part of the TLS
> +certificate is loaded, in pem format. Illegal if not using TLS.
> .It Fl D
> Enable debugging on the socket.
> .It Fl d
C should be before c
> @@ -132,6 +137,9 @@ Forces
> to stay listening for another connection after its current connection
> is completed.
> It is an error to use this option without the
> +.It Fl K Ar key_filename
> +Specifies the filename from which the private key for the TLS certificate
> +is loaded in pem format. Illegal if not using TLS.
> .Fl l
> option.
> When used together with the
This flag is in the middle of another.
> @@ -176,6 +184,11 @@ option.
> Specifies that source and/or destination ports should be chosen randomly
> instead of sequentially within a range or in the order that the system
> assigns them.
> +.It Fl R Ar CA_filename
> +Specifies the filename from which the root CA bundle for Certificate
> +verification is loaded in pem format. Illegal if not using TLS.
> +Default value is
> +.Pa /etc/ssl/cert.pem .
> .It Fl S
> Enables the RFC 2385 TCP MD5 signature option.
> .It Fl s Ar source
R should be before r
> @@ -95,6 +102,13 @@ int Sflag; /* TCP
> MD5 signature opti
> int Tflag = -1; /* IP Type of Service */
> int rtableid = -1;
>
> +int usetls; /* use TLS */
> +char *Cflag; /* Public cert file */
> +char *Kflag; /* Private key file */
> +char *Rflag = DEFAULT_CA_FILE; /* Root CA file */
> +int cachanged; /* Using non-default CA file */
> +int TLSopt; /* TLS options */
Don't mix tab and spaces after the type.
> @@ -145,7 +162,7 @@ main(int argc, char *argv[])
> signal(SIGPIPE, SIG_IGN);
>
> while ((ch = getopt(argc, argv,
> - "46DdFhI:i:klNnO:P:p:rSs:tT:UuV:vw:X:x:z")) != -1) {
> + "46DcC:dFhI:i:kK:lNnO:P:p:rSs:tT:UuV:vw:X:x:z")) != -1) {
Upper case before lower case.
The R is missing.
> @@ -183,6 +203,9 @@ main(int argc, char *argv[])
> case 'k':
> kflag = 1;
> break;
> + case 'K':
> + Kflag = optarg;
> + break;
Upper case before lower case.
> @@ -195,12 +218,19 @@ main(int argc, char *argv[])
> case 'P':
> Pflag = optarg;
> break;
> + case 'C':
> + Cflag = optarg;
> + break;
Move the C up.
> case 'p':
> pflag = optarg;
> break;
> case 'r':
> rflag = 1;
> break;
> + case 'R':
> + cachanged = 1;
> + Rflag = optarg;
> + break;
Upper case before lower case.
Keeping the same order everywhere is important to check that all
list are equal.
> @@ -347,6 +391,25 @@ main(int argc, char *argv[])
> proxyhints.ai_flags |= AI_NUMERICHOST;
> }
>
> + if (usetls) {
> + if (tls_init() == -1)
> + errx(1, "unable to initialize tls");
> + if ((tlsc = tls_config_new()) == NULL)
> + errx(1, "unable allocate tls config");
> + if (Cflag && (tls_config_set_cert_file(tlsc, Cflag) == -1))
> + errx(1, "unable to set TLS certificate file %s", Cflag);
> + if (Kflag && (tls_config_set_key_file(tlsc, Kflag) == -1))
> + errx(1, "unable to set TLS key file %s", Kflag);
When you set Rflag here to DEFAULT_CA_FILE if it is NULL, you can
get rid of the additional variable cachanged.
> + if (Rflag && (tls_config_set_ca_file(tlsc, Rflag) == -1))
> + errx(1, "unable to set Root CA file %s", Rflag);
> @@ -405,11 +469,44 @@ main(int argc, char *argv[])
> }
> if (vflag)
> report_connect((struct sockaddr
> *)&cliaddr, len);
> -
> - readwrite(connfd);
> + if (usetls) {
> + int i;
> + if ((ctx = tls_server()) == NULL)
> + errx(1, "tls client creation
> failed");
> + if (tls_configure(ctx, tlsc) == -1)
> + errx(1, "tls configuration
> failed (%s)",
> + tls_error(ctx));
> + do {
> + i = tls_accept_socket(ctx,
> &cctx, connfd);
> + if (i == -1) {
> + warn ("tls connection
> failed (%s)",
> + tls_error(ctx));
> + cctx = NULL;
You may leak an allocated cctx here. I think you have to free it.
I don't like this part of tls_accept_socket(), it should be changed
there. tls_accept_socket() should take a context created before.
> + }
> + } while (i == TLS_READ_AGAIN || i ==
> TLS_WRITE_AGAIN);
> + }
> + if (usetls && cctx)
> + readwrite(connfd, cctx);
> + if (!usetls)
> + readwrite(connfd, NULL);
> + if (ctx) {
> + int i;
> + do {
> + i = tls_close(ctx);
> + } while (i == TLS_READ_AGAIN || i ==
> TLS_WRITE_AGAIN);
> + tls_free(ctx);
> + ctx = NULL;
> + }
> + if (cctx) {
> + int i;
> + do {
> + i = tls_close(cctx);
> + } while (i == TLS_READ_AGAIN || i ==
> TLS_WRITE_AGAIN);
> + tls_free(cctx);
> + cctx = NULL;
> + }
> close(connfd);
This is a double close, tls_close() should not close(2) the fd.
> @@ -772,6 +889,7 @@ readwrite(int net_fd)
> pfd[POLL_STDOUT].fd = stdout_fd;
> pfd[POLL_STDOUT].events = 0;
>
> +
> while (1) {
> /* both inputs are gone, buffers are empty, we are done */
> if (pfd[POLL_STDIN].fd == -1 && pfd[POLL_NETIN].fd == -1
Useless chunk.
> @@ -922,17 +1040,27 @@ readwrite(int net_fd)
> }
>
> ssize_t
> -drainbuf(int fd, unsigned char *buf, size_t *bufpos)
> +drainbuf(int fd, unsigned char *buf, size_t *bufpos, struct tls *tls)
Call all struct tls pointer ctx for consistency.
> {
> ssize_t n;
> ssize_t adjust;
> + size_t t;
>
> - n = write(fd, buf, *bufpos);
> - /* don't treat EAGAIN, EINTR as error */
> - if (n == -1 && (errno == EAGAIN || errno == EINTR))
> - n = -2;
> - if (n <= 0)
> - return n;
> + if (tls) {
> + n = tls_write(tls, buf, *bufpos, &t);
> + if (n == TLS_READ_AGAIN || n == TLS_WRITE_AGAIN)
> + n = -2;
This does not set the poll(2) read/write flags correctly.
> + if (n < 0)
> + return n;
> + n = t;
Return 0 if t is 0 to avoid the adjust buffer code below. I would
recommend that the n returned by tls_write() works like the n from
write(2) to make this consistent. It should be ssize_t and contain
the length.
> + } else {
> + n = write(fd, buf, *bufpos);
> + /* don't treat EAGAIN, EINTR as error */
> + if (n == -1 && (errno == EAGAIN || errno == EINTR))
> + n = -2;
> + if (n <= 0)
> + return n;
> + }
> /* adjust buffer */
> adjust = *bufpos - n;
> if (adjust > 0)
