From dddbfab568d42e443f102d35c84432824cc59fee Mon Sep 17 00:00:00 2001 From: Laurent Bercot Date: Wed, 22 Mar 2017 21:37:30 +0000 Subject: Fix case where s6-tls[cd] would sometimes not detect an application and remain there forever with its zombie, both condemned to err in limbo for all eternity, the living and the dead, hand in hand --- src/stls/deps-lib/stls | 2 +- src/stls/stls-internal.h | 2 +- src/stls/stls_clean_tls_and_spawn.c | 21 ------------- src/stls/stls_prep_spawn_drop.c | 35 +++++++++++++++++++++ src/stls/stls_run.c | 63 ++++++++++++++++++++++++++++--------- src/stls/stls_s6tlsc.c | 20 ++++-------- src/stls/stls_s6tlsd.c | 19 ++++------- 7 files changed, 97 insertions(+), 65 deletions(-) delete mode 100644 src/stls/stls_clean_tls_and_spawn.c create mode 100644 src/stls/stls_prep_spawn_drop.c (limited to 'src/stls') diff --git a/src/stls/deps-lib/stls b/src/stls/deps-lib/stls index 03cebfa..7ecfa0b 100644 --- a/src/stls/deps-lib/stls +++ b/src/stls/deps-lib/stls @@ -1,4 +1,4 @@ -stls_clean_tls_and_spawn.o +stls_prep_spawn_drop.o stls_run.o stls_s6tlsc.o stls_s6tlsd.o diff --git a/src/stls/stls-internal.h b/src/stls/stls-internal.h index 48a119e..d5c59e7 100644 --- a/src/stls/stls-internal.h +++ b/src/stls/stls-internal.h @@ -6,6 +6,6 @@ #include #include -extern pid_t stls_clean_tls_and_spawn (char const *const *, char const *const *, int *, uint32_t) ; +extern pid_t stls_prep_spawn_drop (char const *const *, char const *const *, int *, uid_t, gid_t, uint32_t) ; #endif diff --git a/src/stls/stls_clean_tls_and_spawn.c b/src/stls/stls_clean_tls_and_spawn.c deleted file mode 100644 index 37ea619..0000000 --- a/src/stls/stls_clean_tls_and_spawn.c +++ /dev/null @@ -1,21 +0,0 @@ -/* ISC license. */ - -#include -#include -#include -#include "stls-internal.h" - -pid_t stls_clean_tls_and_spawn (char const *const *argv, char const *const *envp, int *fds, uint32_t options) -{ - if (!(options & 1)) return child_spawn2(argv[0], argv, envp, fds) ; - else - { - char const modifs[] = "CADIR\0CAFILE\0KEYFILE\0CERTFILE\0TLS_UID\0TLS_GID" ; - size_t modiflen = sizeof(modifs) ; - size_t n = env_len(envp) ; - char const *newenv[n + 7] ; - size_t newenvlen = env_merge(newenv, n+7, envp, n, modifs, modiflen) ; - if (!newenvlen) return 0 ; - return child_spawn2(argv[0], argv, newenv, fds) ; - } -} diff --git a/src/stls/stls_prep_spawn_drop.c b/src/stls/stls_prep_spawn_drop.c new file mode 100644 index 0000000..c6f4e13 --- /dev/null +++ b/src/stls/stls_prep_spawn_drop.c @@ -0,0 +1,35 @@ +/* ISC license. */ + +#include +#include +#include +#include +#include +#include +#include "stls-internal.h" + +pid_t stls_prep_spawn_drop (char const *const *argv, char const *const *envp, int *fds, uid_t uid, gid_t gid, uint32_t options) +{ + pid_t pid ; + + fds[4] = selfpipe_init() ; + if (fds[4] < 0) strerr_diefu1sys(111, "init selfpipe") ; + if (selfpipe_trap(SIGCHLD) < 0) strerr_diefu1sys(111, "trap SIGCHLD") ; + + if (!(options & 1)) pid = child_spawn2(argv[0], argv, envp, fds) ; + else + { + char const modifs[] = "CADIR\0CAFILE\0KEYFILE\0CERTFILE\0TLS_UID\0TLS_GID" ; + size_t modiflen = sizeof(modifs) ; + size_t n = env_len(envp) ; + char const *newenv[n + 7] ; + size_t newenvlen = env_merge(newenv, n+7, envp, n, modifs, modiflen) ; + if (!newenvlen) return 0 ; + pid = child_spawn2(argv[0], argv, newenv, fds) ; + } + + if (!pid) strerr_diefu2sys(111, "spawn ", argv[0]) ; + if (gid && setgid(gid) < 0) strerr_diefu1sys(111, "setgid") ; + if (uid && setuid(uid) < 0) strerr_diefu1sys(111, "setuid") ; + return pid ; +} diff --git a/src/stls/stls_run.c b/src/stls/stls_run.c index 1a035e2..3f2742d 100644 --- a/src/stls/stls_run.c +++ b/src/stls/stls_run.c @@ -114,12 +114,31 @@ static void closeit (struct tls *ctx, int *fds, int brutal) fd_close(fds[3]) ; fds[3] = -1 ; } -int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t options, tain_t const *tto) +static void handle_signals (pid_t pid, int *e) +{ + for (;;) switch (selfpipe_read()) + { + case -1 : strerr_diefu1sys(111, "read selfpipe") ; + case 0 : return ; + case SIGCHLD : + { + int wstat ; + if (wait_pid_nohang(pid, &wstat) == pid) + { + *e = wstat ; + return ; + } + } + } +} + +int stls_run (struct tls *ctx, int *fds, pid_t pid, unsigned int verbosity, uint32_t options, tain_t const *tto) { tlsbuf_t b[2] = { { .blockedonother = 0 }, { .blockedonother = 0 } } ; - iopause_fd x[4] ; + iopause_fd x[5] = { { .fd = fds[4], .events = IOPAUSE_READ } } ; unsigned int xindex[4] ; unsigned int i = 0 ; + int e = -1 ; for (; i < 2 ; i++) { @@ -133,7 +152,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option for (;;) { tain_t deadline ; - unsigned int xlen = 0 ; + unsigned int xlen = 1 ; int r ; tain_add_g(&deadline, fds[0] >= 0 && fds[2] >= 0 && buffer_isempty(&b[0].b) && buffer_isempty(&b[1].b) ? tto : &tain_infinite_relative) ; @@ -147,7 +166,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option x[xlen].events = IOPAUSE_READ ; xindex[0] = xlen++ ; } - else xindex[0] = 4 ; + else xindex[0] = 5 ; if (fds[1] >= 0 && buffer_iswritable(&b[1].b)) { @@ -155,7 +174,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option x[xlen].events = IOPAUSE_WRITE ; xindex[1] = xlen++ ; } - else xindex[1] = 4 ; + else xindex[1] = 5 ; if (fds[2] >= 0 && !b[1].blockedonother && buffer_isreadable(&b[1].b)) { @@ -163,7 +182,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option x[xlen].events = IOPAUSE_READ ; xindex[2] = xlen++ ; } - else xindex[2] = 4 ; + else xindex[2] = 5 ; if (fds[3] >= 0 && !b[0].blockedonother && buffer_iswritable(&b[0].b)) { @@ -171,9 +190,9 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option x[xlen].events = IOPAUSE_WRITE ; xindex[3] = xlen++ ; } - else xindex[3] = 4 ; + else xindex[3] = 5 ; - if (!xlen) break ; + if (xlen == 1) break ; /* poll() */ @@ -184,6 +203,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option { fd_close(fds[0]) ; fds[0] = -1 ; closeit(ctx, fds, options & 1) ; + if (e >= 0) break ; continue ; } @@ -192,9 +212,14 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option x[xlen].revents |= IOPAUSE_READ | IOPAUSE_WRITE ; + /* Signal */ + + if (x[0].revents & IOPAUSE_READ) handle_signals(pid, &e) ; + + /* Flush to local */ - if (xindex[1] < 4 && x[xindex[1]].revents & IOPAUSE_WRITE) + if (xindex[1] < 5 && x[xindex[1]].revents & IOPAUSE_WRITE) { r = buffer_flush(&b[1].b) ; if (!r && !error_isagain(errno)) @@ -217,7 +242,7 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option /* Flush to remote */ - if (xindex[3] < 4 && x[xindex[3]].revents & IOPAUSE_WRITE) + if (xindex[3] < 5 && x[xindex[3]].revents & IOPAUSE_WRITE) { r = buffer_tls_flush(ctx, b) ; if (r < 0) @@ -225,27 +250,35 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option strerr_warnwu2x("write to peer: ", tls_error(ctx)) ; fd_close(fds[0]) ; fds[0] = -1 ; } - if (r && fds[0] < 0) closeit(ctx, fds, options & 1) ; + if (r && fds[0] < 0) + { + closeit(ctx, fds, options & 1) ; + if (e >= 0) break ; + } } /* Fill from local */ - if (xindex[0] < 4 && x[xindex[0]].revents & IOPAUSE_READ) + if (xindex[0] < 5 && x[xindex[0]].revents & IOPAUSE_READ) { r = sanitize_read(buffer_fill(&b[0].b)) ; if (r < 0) { if (errno != EPIPE) strerr_warnwu1sys("read from application") ; fd_close(fds[0]) ; fds[0] = -1 ; - if (buffer_isempty(&b[0].b)) closeit(ctx, fds, options & 1) ; + if (buffer_isempty(&b[0].b)) + { + closeit(ctx, fds, options & 1) ; + if (e >= 0) break ; + } } } /* Fill from remote */ - if (xindex[2] < 4 && x[xindex[2]].revents & IOPAUSE_READ) + if (xindex[2] < 5 && x[xindex[2]].revents & IOPAUSE_READ) { r = buffer_tls_fill(ctx, b) ; if (r < 0) @@ -273,5 +306,5 @@ int stls_run (struct tls *ctx, int *fds, unsigned int verbosity, uint32_t option if (fds[0] >= 0) fd_close(fds[0]) ; if (fds[3] >= 0) fd_close(fds[3]) ; if (fds[2] >= 0) fd_close(fds[2]) ; - return 0 ; + return e ; } diff --git a/src/stls/stls_s6tlsc.c b/src/stls/stls_s6tlsc.c index a9a2c98..87a45ac 100644 --- a/src/stls/stls_s6tlsc.c +++ b/src/stls/stls_s6tlsc.c @@ -14,11 +14,12 @@ int stls_s6tlsc (char const *const *argv, char const *const *envp, tain_t const *tto, uint32_t preoptions, uint32_t options, uid_t uid, gid_t gid, unsigned int verbosity, char const *servername, int *sfd) { - int fds[4] = { sfd[0], sfd[1], sfd[0], sfd[1] } ; + int fds[5] = { sfd[0], sfd[1], sfd[0], sfd[1] } ; struct tls *ctx ; struct tls_config *cfg ; pid_t pid ; char const *x ; + int wstat ; if (tls_init() < 0) strerr_diefu1sys(111, "tls_init") ; cfg = tls_config_new() ; @@ -72,23 +73,14 @@ int stls_s6tlsc (char const *const *argv, char const *const *envp, tain_t const if (!ctx) strerr_diefu1sys(111, "tls_client") ; if (tls_configure(ctx, cfg) < 0) diectx(97, ctx, "tls_configure") ; - pid = stls_clean_tls_and_spawn(argv, envp, fds, !!(preoptions & 2)) ; - if (!pid) strerr_diefu2sys(111, "spawn ", argv[0]) ; - if (gid && setgid(gid) < 0) strerr_diefu1sys(111, "setgid") ; - if (uid && setuid(uid) < 0) strerr_diefu1sys(111, "setuid") ; + pid = stls_prep_spawn_drop(argv, envp, fds, uid, gid, !!(preoptions & 2)) ; if (tls_connect_fds(ctx, fds[2], fds[3], servername) < 0) diectx(97, ctx, "tls_connect_fds") ; tls_config_free(cfg) ; if (tls_handshake(ctx) < 0) diectx(97, ctx, "perform SSL handshake") ; - { - int wstat ; - int r = stls_run(ctx, fds, verbosity, options, tto) ; - if (r < 0) strerr_diefu1sys(111, "run SSL engine") ; - else if (r) diectx(98, ctx, "maintain SSL connection to peer") ; - tls_free(ctx) ; - if (wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; - return wait_estatus(wstat) ; - } + wstat = stls_run(ctx, fds, pid, verbosity, options, tto) ; + if (wstat < 0 && wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; + return wait_estatus(wstat) ; } diff --git a/src/stls/stls_s6tlsd.c b/src/stls/stls_s6tlsd.c index fd59d48..07446e7 100644 --- a/src/stls/stls_s6tlsd.c +++ b/src/stls/stls_s6tlsd.c @@ -15,12 +15,13 @@ int stls_s6tlsd (char const *const *argv, char const *const *envp, tain_t const *tto, uint32_t preoptions, uint32_t options, uid_t uid, gid_t gid, unsigned int verbosity) { - int fds[4] = { 0, 1, 0, 1 } ; + int fds[5] = { 0, 1, 0, 1 } ; struct tls *cctx ; struct tls *ctx ; struct tls_config *cfg ; pid_t pid ; char const *x ; + int wstat ; if (tls_init() < 0) strerr_diefu1sys(111, "tls_init") ; cfg = tls_config_new() ; @@ -76,22 +77,14 @@ int stls_s6tlsd (char const *const *argv, char const *const *envp, tain_t const if (tls_configure(ctx, cfg) < 0) diectx(97, ctx, "tls_configure") ; tls_config_free(cfg) ; - pid = stls_clean_tls_and_spawn(argv, envp, fds, !!(preoptions & 2)) ; - if (!pid) strerr_diefu2sys(111, "spawn ", argv[0]) ; - if (gid && setgid(gid) < 0) strerr_diefu1sys(111, "setgid") ; - if (uid && setuid(uid) < 0) strerr_diefu1sys(111, "setuid") ; + pid = stls_prep_spawn_drop(argv, envp, fds, uid, gid, !!(preoptions & 2)) ; if (tls_accept_fds(ctx, &cctx, fds[2], fds[3]) < 0) diectx(97, ctx, "tls_accept_fds") ; tls_free(ctx) ; if (tls_handshake(cctx) < 0) diectx(97, cctx, "perform SSL handshake") ; - { - int wstat ; - int r = stls_run(cctx, fds, verbosity, options, tto) ; - if (r < 0) strerr_diefu1sys(111, "run SSL engine") ; - else if (r) diectx(98, cctx, "maintain SSL connection to peer") ; - if (wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; - return wait_estatus(wstat) ; - } + wstat = stls_run(cctx, fds, pid, verbosity, options, tto) ; + if (wstat < 0 && wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; + return wait_estatus(wstat) ; } -- cgit v1.2.3