From b56d340d39281f99e8925cc9273c11b29ba1a6c0 Mon Sep 17 00:00:00 2001 From: Tony Cheneau Date: Thu, 28 Jun 2018 13:34:32 +0200 Subject: [PATCH 2/3] When executing a program (with exec=), properly redirect stderr to the logging facility. We have some "exec= program" that display their debug information on the stderr. The standard error output (stderr) of the program was lost when using background mode. So we added a new "foreground = logstderr" configuration option. Also, we make changes to redirect the executed program stderr in the internal stunnel logging system in foreground mode as well. In practice, we print through the stunnel s_log() function, so that we can have the PID of the child that executed the program (instead of just having the stderr output with no context information). --- doc/stunnel.8.in | 32 +++++++++++-------- doc/stunnel.html.in | 4 ++- doc/stunnel.pod.in | 5 ++- src/client.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/options.c | 8 +++-- src/prototypes.h | 2 +- 6 files changed, 120 insertions(+), 19 deletions(-) diff --git a/doc/stunnel.8.in b/doc/stunnel.8.in index 26f2693..29586c8 100644 --- a/doc/stunnel.8.in +++ b/doc/stunnel.8.in @@ -1,4 +1,4 @@ -.\" Automatically generated by Pod::Man 4.07 (Pod::Simple 3.32) +.\" Automatically generated by Pod::Man 2.28 (Pod::Simple 3.29) .\" .\" Standard preamble: .\" ======================================================================== @@ -46,7 +46,7 @@ .ie \n(.g .ds Aq \(aq .el .ds Aq ' .\" -.\" If the F register is >0, we'll generate index entries on stderr for +.\" If the F register is turned on, we'll generate index entries on stderr for .\" titles (.TH), headers (.SH), subsections (.SS), items (.Ip), and index .\" entries marked with X<> in POD. Of course, you'll have to process the .\" output yourself in some meaningful fashion. @@ -54,16 +54,20 @@ .\" Avoid warning from groff about undefined register 'F'. .de IX .. -.if !\nF .nr F 0 -.if \nF>0 \{\ -. de IX -. tm Index:\\$1\t\\n%\t"\\$2" +.nr rF 0 +.if \n(.g .if rF .nr rF 1 +.if (\n(rF:(\n(.g==0)) \{ +. if \nF \{ +. de IX +. tm Index:\\$1\t\\n%\t"\\$2" .. -. if !\nF==2 \{\ -. nr % 0 -. nr F 2 +. if !\nF==2 \{ +. nr % 0 +. nr F 2 +. \} . \} .\} +.rr rF .\" ======================================================================== .\" .IX Title "stunnel 8" @@ -237,8 +241,8 @@ This option allows you to disable entering \s-1FIPS\s0 mode if \fBstunnel\fR was with \s-1FIPS 140\-2\s0 support. .Sp default: no (since version 5.00) -.IP "\fBforeground\fR = yes | quiet | no (Unix only)" 4 -.IX Item "foreground = yes | quiet | no (Unix only)" +.IP "\fBforeground\fR = yes | quiet | logstderr | no (Unix only)" 4 +.IX Item "foreground = yes | quiet | logstderr | no (Unix only)" foreground mode .Sp Stay in foreground (don't fork). @@ -246,6 +250,9 @@ Stay in foreground (don't fork). With the \fIyes\fR parameter it also logs to stderr in addition to the destinations specified with \fIsyslog\fR and \fIoutput\fR. .Sp +With the \fIlogstderr\fR parameter, the program goes in background mode. In +addition, it also logs \fIstderr\fR using the configured logging facility. +.Sp default: background in daemon mode .IP "\fBiconActive\fR = \s-1ICON_FILE \s0(\s-1GUI\s0 only)" 4 .IX Item "iconActive = ICON_FILE (GUI only)" @@ -1388,7 +1395,8 @@ certificate file, which disables generating temporary \s-1DH\s0 parameters: .Ve .SH "FILES" .IX Header "FILES" -.IP "\fI\f(CI@sysconfdir\fI@/stunnel/stunnel.conf\fR" 4 +.ie n .IP "\fI\fI@sysconfdir\fI@/stunnel/stunnel.conf\fR" 4 +.el .IP "\fI\f(CI@sysconfdir\fI@/stunnel/stunnel.conf\fR" 4 .IX Item "@sysconfdir@/stunnel/stunnel.conf" \&\fBstunnel\fR configuration file .SH "BUGS" diff --git a/doc/stunnel.html.in b/doc/stunnel.html.in index 858d0d6..a553b48 100644 --- a/doc/stunnel.html.in +++ b/doc/stunnel.html.in @@ -293,7 +293,7 @@

default: no (since version 5.00)

-
foreground = yes | quiet | no (Unix only)
+
foreground = yes | quiet | logstderr | no (Unix only)

foreground mode

@@ -302,6 +302,8 @@

With the yes parameter it also logs to stderr in addition to the destinations specified with syslog and output.

+

With the logstderr parameter, the program goes in background mode. In addition, it also logs stderr using the configured logging facility.

+

default: background in daemon mode

diff --git a/doc/stunnel.pod.in b/doc/stunnel.pod.in index e640d49..02b25a7 100644 --- a/doc/stunnel.pod.in +++ b/doc/stunnel.pod.in @@ -235,7 +235,7 @@ with FIPS 140-2 support. default: no (since version 5.00) -=item B = yes | quiet | no (Unix only) +=item B = yes | quiet | logstderr | no (Unix only) foreground mode @@ -244,6 +244,9 @@ Stay in foreground (don't fork). With the I parameter it also logs to stderr in addition to the destinations specified with I and I. +With the I parameter, the program goes in background mode. In +addition, it also logs I using the configured logging facility. + default: background in daemon mode =item B = ICON_FILE (GUI only) diff --git a/src/client.c b/src/client.c index d188620..4e6336d 100644 --- a/src/client.c +++ b/src/client.c @@ -312,6 +312,13 @@ NOEXPORT void client_run(CLI *c) { c->local_rfd.fd=c->local_wfd.fd=INVALID_SOCKET; } + /* cleanup the stderr file descriptor */ + if (c->local_efd.fd) { + close(c->local_efd.fd); + s_log(LOG_DEBUG, "Stderr descriptor (FD=%ld) closed", + (long)c->local_efd.fd); + } + #ifdef USE_FORK /* display child return code if it managed to arrive on time */ /* otherwise it will be retrieved by the init process and ignored */ @@ -635,7 +642,8 @@ NOEXPORT void transfer(CLI *c) { int read_wants_read=0, read_wants_write=0; int write_wants_read=0, write_wants_write=0; /* actual conditions on file descriptors */ - int sock_can_rd, sock_can_wr, ssl_can_rd, ssl_can_wr; + int sock_can_rd, sock_can_wr, ssl_can_rd, ssl_can_wr, stderr_can_rd = 0; + #ifdef USE_WIN32 unsigned long bytes; #else @@ -667,6 +675,8 @@ NOEXPORT void transfer(CLI *c) { s_poll_add(c->fds, c->ssl_wfd->fd, 0, read_wants_write || write_wants_write || shutdown_wants_write); } + if (c->local_efd.fd != 0) /* poll the program stderr */ + s_poll_add(c->fds, c->local_efd.fd, 1, 0); /****************************** wait for an event */ err=s_poll_wait(c->fds, @@ -700,6 +710,8 @@ NOEXPORT void transfer(CLI *c) { sock_can_wr=s_poll_canwrite(c->fds, c->sock_wfd->fd); ssl_can_rd=s_poll_canread(c->fds, c->ssl_rfd->fd); ssl_can_wr=s_poll_canwrite(c->fds, c->ssl_wfd->fd); + if (c->local_efd.fd != 0) + stderr_can_rd = s_poll_canread(c->fds, c->local_efd.fd); /****************************** identify exceptions */ if(c->sock_rfd->fd==c->sock_wfd->fd) { @@ -855,6 +867,62 @@ NOEXPORT void transfer(CLI *c) { } } + /****************************** read from stderr */ + if (stderr_can_rd) { + int i = 0, eof = 0; + ssize_t numread = 0; + char * buf = str_alloc(65536); + + while (!eof) { + i = 0; + memset(buf, 0, 65536); + /* the following call will actually print the stderr only if log_level==DEBUG */ + while (i < 65536 - 1) { + errno = 0; + numread = read(c->local_efd.fd, buf + i, 1); + + if (numread < 0 && errno == EINTR) /* interrupted */ + continue; + + if ((numread <0 && (errno == EAGAIN || errno == EWOULDBLOCK)) || + numread == 0) { /* reached EOF */ + eof = 1; + if (s_poll_hup(c->fds, c->local_efd.fd)) { + stderr_can_rd = 0; + s_log(LOG_DEBUG, "Local stderr file descriptor (FD=%ld) closed", + (long) c->local_efd.fd); + close(c->local_efd.fd); /* to be on the safe side */ + c->local_efd.fd = 0; + break; + } +#if 0 + if (i==0) s_log(LOG_ERR, "transfer: stderr handling routine had no data to read (%s)!", strerror(errno)); +#endif + break; + } + + if (numread < 0) { /* unhandled error */ + s_log(LOG_ERR, "transfer: unable to read stderr from the exec program (%s)", strerror(errno)); + close(c->local_efd.fd); + throw_exception(c, 1); + } + + if (buf[i] == '\n') { + buf[i] = 0; /* '\n' are not printed by s_log */ + ++i; /* make sure i!=0 so that line with only a '\n' will get printed */ + break; + } + ++i; + } + + if (i) { + watchdog=0; /* reset the watchdog */ + s_log(LOG_INFO, " --> %s", buf); + } + } + str_free(buf); + } + /****************************** update *_wants_* based on new *_ptr */ /* this update is also required for SSL_pending() to be used */ read_wants_read|=!(SSL_get_shutdown(c->ssl)&SSL_RECEIVED_SHUTDOWN) @@ -1069,6 +1137,8 @@ NOEXPORT void transfer(CLI *c) { (SSL_get_shutdown(c->ssl) & SSL_SENT_SHUTDOWN) ? "Y" : "n"); s_log(LOG_ERR, "sock_can_rd=%s, sock_can_wr=%s", sock_can_rd ? "Y" : "n", sock_can_wr ? "Y" : "n"); + s_log(LOG_ERR, "stderr_can_rd=%s", + stderr_can_rd ? "Y" : "n"); s_log(LOG_ERR, "ssl_can_rd=%s, ssl_can_wr=%s", ssl_can_rd ? "Y" : "n", ssl_can_wr ? "Y" : "n"); s_log(LOG_ERR, "read_wants_read=%s, read_wants_write=%s", @@ -1264,7 +1334,8 @@ NOEXPORT SOCKET connect_local(CLI *c) { /* spawn local process */ extern char **environ; NOEXPORT SOCKET connect_local(CLI *c) { /* spawn local process */ - int fd[2], pid; + int fd[2], fd_err[2], pid; + char **env; #ifdef HAVE_PTHREAD_SIGMASK sigset_t newmask; @@ -1279,7 +1350,11 @@ NOEXPORT SOCKET connect_local(CLI *c) { /* spawn local process */ } else if(make_sockets(fd)) throw_exception(c, 1); + if(make_sockets(fd_err)) + throw_exception(c, 1); + set_nonblock(fd[1], 0); /* switch back to the blocking mode */ + set_nonblock(fd_err[1], 0); /* switch back to the blocking mode */ env=env_alloc(c); pid=fork(); @@ -1298,8 +1373,14 @@ NOEXPORT SOCKET connect_local(CLI *c) { /* spawn local process */ /* dup2() does not copy FD_CLOEXEC flag */ dup2(fd[1], 0); dup2(fd[1], 1); + if(!global_options.option.log_stderr) dup2(fd[1], 2); + else { + dup2(fd_err[1], 2); + closesocket(fd_err[1]); + } + closesocket(fd[1]); /* not really needed due to FD_CLOEXEC */ #ifdef HAVE_PTHREAD_SIGMASK sigemptyset(&newmask); @@ -1316,6 +1397,9 @@ NOEXPORT SOCKET connect_local(CLI *c) { /* spawn local process */ _exit(1); /* failed, but there is no way to report an error here */ default: /* parent */ closesocket(fd[1]); + closesocket(fd_err[1]); + if (global_options.option.log_stderr) + c->local_efd.fd = fd_err[0]; env_free(env); s_log(LOG_INFO, "Local mode child started (PID=%lu)", c->pid); return fd[0]; diff --git a/src/options.c b/src/options.c index b62ea5e..9413cc6 100644 --- a/src/options.c +++ b/src/options.c @@ -870,11 +870,15 @@ NOEXPORT char *parse_global_option(CMD cmd, char *opt, char *arg) { } else if(!strcasecmp(arg, "quiet")) { new_global_options.option.foreground=1; new_global_options.option.log_stderr=0; + } else if(!strcasecmp(arg, "logstderr")) { + new_global_options.option.foreground=0; + new_global_options.option.log_stderr=1; } else if(!strcasecmp(arg, "no")) { new_global_options.option.foreground=0; new_global_options.option.log_stderr=0; } else - return "The argument needs to be either 'yes', 'quiet' or 'no'"; + return "The argument needs to be either 'yes', 'quiet', 'logstderr' or 'no'"; + return NULL; /* OK */ case CMD_END: break; @@ -885,7 +889,7 @@ NOEXPORT char *parse_global_option(CMD cmd, char *opt, char *arg) { case CMD_DEFAULT: break; case CMD_HELP: - s_log(LOG_NOTICE, "%-22s = yes|quiet|no foreground mode (don't fork, log to stderr)", + s_log(LOG_NOTICE, "%-22s = yes|quiet|logstderr|no foreground mode (don't fork, log to stderr)", "foreground"); break; } diff --git a/src/prototypes.h b/src/prototypes.h index befff2a..8cb6f33 100644 --- a/src/prototypes.h +++ b/src/prototypes.h @@ -395,7 +395,7 @@ typedef struct { SOCKADDR_UNION *bind_addr; /* address to bind() the socket */ SOCKADDR_LIST connect_addr; /* either copied or resolved dynamically */ unsigned idx; /* actually connected address in connect_addr */ - FD local_rfd, local_wfd; /* read and write local descriptors */ + FD local_rfd, local_wfd, local_efd; /* read and write local descriptors */ FD remote_fd; /* remote file descriptor */ unsigned long pid; /* PID of the local process */ SOCKET fd; /* temporary file descriptor */ -- 2.7.4