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/sbearssl/deps-lib/sbearssl | 2 +- src/sbearssl/sbearssl-internal.h | 2 +- src/sbearssl/sbearssl_clean_tls_and_spawn.c | 21 ---------- src/sbearssl/sbearssl_prep_spawn_drop.c | 35 +++++++++++++++++ src/sbearssl/sbearssl_run.c | 60 ++++++++++++++++++++++------- src/sbearssl/sbearssl_s6tlsc.c | 29 +++++--------- src/sbearssl/sbearssl_s6tlsd.c | 34 +++++++--------- 7 files changed, 106 insertions(+), 77 deletions(-) delete mode 100644 src/sbearssl/sbearssl_clean_tls_and_spawn.c create mode 100644 src/sbearssl/sbearssl_prep_spawn_drop.c (limited to 'src/sbearssl') diff --git a/src/sbearssl/deps-lib/sbearssl b/src/sbearssl/deps-lib/sbearssl index 4945ad8..c0acfb6 100644 --- a/src/sbearssl/deps-lib/sbearssl +++ b/src/sbearssl/deps-lib/sbearssl @@ -1,5 +1,5 @@ sbearssl_append.o -sbearssl_clean_tls_and_spawn.o +sbearssl_prep_spawn_drop.o sbearssl_cert_from.o sbearssl_cert_readbigpem.o sbearssl_cert_readfile.o diff --git a/src/sbearssl/sbearssl-internal.h b/src/sbearssl/sbearssl-internal.h index df3e3e5..cbf355b 100644 --- a/src/sbearssl/sbearssl-internal.h +++ b/src/sbearssl/sbearssl-internal.h @@ -19,6 +19,6 @@ struct sbearssl_strallocerr_s extern void sbearssl_append (void *, void const *, size_t) ; extern int sbearssl_pem_push (br_pem_decoder_context *, char const *, size_t, sbearssl_pemobject *, genalloc *, sbearssl_strallocerr *, int *) ; -extern pid_t sbearssl_clean_tls_and_spawn (char const *const *, char const *const *, int *, uint32_t) ; +extern pid_t sbearssl_prep_spawn_drop (char const *const *, char const *const *, int *, uid_t, gid_t, uint32_t) ; #endif diff --git a/src/sbearssl/sbearssl_clean_tls_and_spawn.c b/src/sbearssl/sbearssl_clean_tls_and_spawn.c deleted file mode 100644 index 258db90..0000000 --- a/src/sbearssl/sbearssl_clean_tls_and_spawn.c +++ /dev/null @@ -1,21 +0,0 @@ -/* ISC license. */ - -#include -#include -#include -#include "sbearssl-internal.h" - -pid_t sbearssl_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/sbearssl/sbearssl_prep_spawn_drop.c b/src/sbearssl/sbearssl_prep_spawn_drop.c new file mode 100644 index 0000000..1ca7eb8 --- /dev/null +++ b/src/sbearssl/sbearssl_prep_spawn_drop.c @@ -0,0 +1,35 @@ +/* ISC license. */ + +#include +#include +#include +#include +#include +#include +#include "sbearssl-internal.h" + +pid_t sbearssl_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/sbearssl/sbearssl_run.c b/src/sbearssl/sbearssl_run.c index ca4a79e..26a8bd3 100644 --- a/src/sbearssl/sbearssl_run.c +++ b/src/sbearssl/sbearssl_run.c @@ -1,7 +1,6 @@ /* ISC license. */ #include -#include #include #include #include @@ -12,13 +11,33 @@ #include #include #include +#include #include -int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, uint32_t options, tain_t const *tto) +static inline void handle_signals (pid_t pid, int *e) { - iopause_fd x[4] ; + 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 sbearssl_run (br_ssl_engine_context *ctx, int *fds, pid_t pid, unsigned int verbosity, uint32_t options, tain_t const *tto) +{ + iopause_fd x[5] = { { .fd = fds[4], .events = IOPAUSE_READ } } ; unsigned int xindex[4] ; int markedforflush = 0 ; + int e = -1 ; if (ndelay_on(fds[2]) < 0 || ndelay_on(fds[3]) < 0) strerr_diefu1sys(111, "set fds non-blocking") ; @@ -28,11 +47,16 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, for (;;) { tain_t deadline ; - unsigned int j = 0 ; + unsigned int j = 1 ; unsigned int state = br_ssl_engine_current_state(ctx) ; int r ; - if (state & BR_SSL_CLOSED) break ; + if (state & BR_SSL_CLOSED) + { + r = br_ssl_engine_last_error(ctx) ; + if (r) strerr_diefu2x(98, "establish or maintain SSL connection to peer: ", sbearssl_error_str(r)) ; + break ; + } tain_add_g(&deadline, fds[0] >= 0 && fds[2] >= 0 && state & (BR_SSL_SENDAPP | BR_SSL_RECVREC) ? tto : &tain_infinite_relative) ; @@ -42,30 +66,30 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, x[j].events = IOPAUSE_READ ; xindex[0] = j++ ; } - else xindex[0] = 4 ; + else xindex[0] = 5 ; if (fds[1] >= 0 && state & BR_SSL_RECVAPP) { x[j].fd = fds[1] ; x[j].events = IOPAUSE_WRITE ; xindex[1] = j++ ; } - else xindex[1] = 4 ; + else xindex[1] = 5 ; if (fds[2] >= 0 && state & BR_SSL_RECVREC) { x[j].fd = fds[2] ; x[j].events = IOPAUSE_READ ; xindex[2] = j++ ; } - else xindex[2] = 4 ; + else xindex[2] = 5 ; if (fds[3] >= 0 && state & BR_SSL_SENDREC) { x[j].fd = fds[3] ; x[j].events = IOPAUSE_WRITE ; xindex[3] = j++ ; } - else xindex[3] = 4 ; + else xindex[3] = 5 ; - if (!j) break ; + if (j == 1) break ; r = iopause_g(x, j, &deadline) ; if (r < 0) strerr_diefu1sys(111, "iopause") ; else if (!r) @@ -77,6 +101,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, fd_close(fds[3]) ; fds[3] = -1 ; } else br_ssl_engine_close(ctx) ; + if (e >= 0) break ; continue ; } @@ -85,6 +110,11 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, x[j].revents |= IOPAUSE_READ | IOPAUSE_WRITE ; + /* Signal */ + + if (x[0].revents & IOPAUSE_READ) handle_signals(pid, &e) ; + + /* Flush to local */ if (state & BR_SSL_RECVAPP && x[xindex[1]].events & x[xindex[1]].revents & IOPAUSE_WRITE) @@ -111,7 +141,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, /* Flush to remote */ - if (state & BR_SSL_SENDREC && x[xindex[3]].events & x[xindex[3]].revents & IOPAUSE_WRITE) + if (state & BR_SSL_SENDREC && xindex[3] < 5 && x[xindex[3]].events & x[xindex[3]].revents & IOPAUSE_WRITE) { size_t len ; unsigned char const *s = br_ssl_engine_sendrec_buf(ctx, &len) ; @@ -132,6 +162,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, fd_close(fds[3]) ; fds[3] = -1 ; } else br_ssl_engine_close(ctx) ; + if (e >= 0) break ; } state = br_ssl_engine_current_state(ctx) ; } @@ -140,7 +171,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, /* Fill from local */ - if (state & BR_SSL_SENDAPP && x[xindex[0]].events & IOPAUSE_READ && (markedforflush || x[xindex[0]].revents & IOPAUSE_READ)) + if (state & BR_SSL_SENDAPP && xindex[0] < 5 && x[xindex[0]].events & IOPAUSE_READ && (markedforflush || x[xindex[0]].revents & IOPAUSE_READ)) { size_t len ; unsigned char *s = br_ssl_engine_sendapp_buf(ctx, &len) ; @@ -160,6 +191,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, fd_close(fds[3]) ; fds[3] = -1 ; } else br_ssl_engine_close(ctx) ; + if (e >= 0) break ; } } } @@ -179,7 +211,7 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, /* Fill from remote */ - if (state & BR_SSL_RECVREC && x[xindex[2]].events & x[xindex[2]].revents & IOPAUSE_READ) + if (state & BR_SSL_RECVREC && xindex[2] < 5 && x[xindex[2]].events & x[xindex[2]].revents & IOPAUSE_READ) { size_t len ; unsigned char *s = br_ssl_engine_recvrec_buf(ctx, &len) ; @@ -204,5 +236,5 @@ int sbearssl_run (br_ssl_engine_context *ctx, int *fds, unsigned int verbosity, 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 br_ssl_engine_last_error(ctx) ; + return e ; } diff --git a/src/sbearssl/sbearssl_s6tlsc.c b/src/sbearssl/sbearssl_s6tlsc.c index e01e25e..267d79c 100644 --- a/src/sbearssl/sbearssl_s6tlsc.c +++ b/src/sbearssl/sbearssl_s6tlsc.c @@ -14,10 +14,11 @@ int sbearssl_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] } ; stralloc storage = STRALLOC_ZERO ; genalloc tas = GENALLOC_ZERO ; size_t talen ; + pid_t pid ; if (preoptions & 1) strerr_dief1x(100, "client certificates are not supported yet") ; @@ -44,31 +45,26 @@ int sbearssl_s6tlsc (char const *const *argv, char const *const *envp, tain_t co strerr_dief2x(96, "no trust anchor found in ", x) ; } + if (!random_init()) strerr_diefu1sys(111, "initialize random generator") ; + + pid = sbearssl_prep_spawn_drop(argv, envp, fds, uid, gid, !!(preoptions & 2)) ; + { unsigned char buf[BR_SSL_BUFSIZE_BIDI] ; br_x509_minimal_context xc ; br_ssl_client_context cc ; br_x509_trust_anchor btas[talen] ; size_t i = talen ; - pid_t pid ; + int wstat ; stralloc_shrink(&storage) ; while (i--) sbearssl_ta_to(genalloc_s(sbearssl_ta, &tas) + i, btas + i, storage.s) ; genalloc_free(sbearssl_ta, &tas) ; - br_ssl_client_init_full(&cc, &xc, btas, talen) ; - - if (!random_init()) - strerr_diefu1sys(111, "initialize random generator") ; random_string((char *)buf, 32) ; br_ssl_engine_inject_entropy(&cc.eng, buf, 32) ; random_finish() ; - - pid = sbearssl_clean_tls_and_spawn(argv, envp, fds, !!(preoptions & 2)) ; - if (gid && setgid(gid) < 0) strerr_diefu1sys(111, "setgid") ; - if (uid && setuid(uid) < 0) strerr_diefu1sys(111, "setuid") ; - br_ssl_engine_set_buffer(&cc.eng, buf, sizeof(buf), 1) ; if (!br_ssl_client_reset(&cc, servername, 0)) strerr_diefu2x(97, "reset client context: ", sbearssl_error_str(br_ssl_engine_last_error(&cc.eng))) ; @@ -76,13 +72,8 @@ int sbearssl_s6tlsc (char const *const *argv, char const *const *envp, tain_t co if (!sbearssl_x509_minimal_set_tain(&xc, &STAMP)) strerr_diefu1sys(111, "initialize validation time") ; - { - int wstat ; - int r = sbearssl_run(&cc.eng, fds, verbosity, options, tto) ; - if (r < 0) strerr_diefu1sys(111, "run SSL engine") ; - else if (r) strerr_diefu2x(98, "establish or maintain SSL connection to peer: ", sbearssl_error_str(r)) ; - if (wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; - return wait_estatus(wstat) ; - } + wstat = sbearssl_run(&cc.eng, 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/sbearssl/sbearssl_s6tlsd.c b/src/sbearssl/sbearssl_s6tlsd.c index dd7f52a..6c3d163 100644 --- a/src/sbearssl/sbearssl_s6tlsd.c +++ b/src/sbearssl/sbearssl_s6tlsd.c @@ -14,10 +14,12 @@ int sbearssl_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[5] = { 0, 1, 0, 1 } ; stralloc storage = STRALLOC_ZERO ; sbearssl_skey skey ; genalloc certs = GENALLOC_ZERO ; size_t chainlen ; + pid_t pid ; if (preoptions & 1) strerr_dief1x(100, "client certificates are not supported yet") ; @@ -44,14 +46,17 @@ int sbearssl_s6tlsd (char const *const *argv, char const *const *envp, tain_t co strerr_diefu2x(96, "find a certificate in ", x) ; } + if (!random_init()) strerr_diefu1sys(111, "initialize random generator") ; + + pid = sbearssl_prep_spawn_drop(argv, envp, fds, uid, gid, !!(preoptions & 2)) ; + { - int fds[4] = { 0, 1, 0, 1 } ; unsigned char buf[BR_SSL_BUFSIZE_BIDI] ; br_ssl_server_context sc ; union br_skey_u key ; br_x509_certificate chain[chainlen] ; size_t i = chainlen ; - pid_t pid ; + int wstat ; stralloc_shrink(&storage) ; while (i--) @@ -83,17 +88,6 @@ int sbearssl_s6tlsd (char const *const *argv, char const *const *envp, tain_t co strerr_dief1x(96, "unsupported private key type") ; } - if (!random_init()) - strerr_diefu1sys(111, "initialize random generator") ; - random_string((char *)buf, 32) ; - br_ssl_engine_inject_entropy(&sc.eng, buf, 32) ; - random_finish() ; - - pid = sbearssl_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") ; - { uint32_t flags = BR_OPT_ENFORCE_SERVER_PREFERENCES | BR_OPT_NO_RENEGOTIATION ; if (preoptions & 1) @@ -104,17 +98,15 @@ int sbearssl_s6tlsd (char const *const *argv, char const *const *envp, tain_t co br_ssl_engine_add_flags(&sc.eng, flags) ; } + random_string((char *)buf, 32) ; + br_ssl_engine_inject_entropy(&sc.eng, buf, 32) ; + random_finish() ; br_ssl_engine_set_buffer(&sc.eng, buf, sizeof(buf), 1) ; br_ssl_server_reset(&sc) ; tain_now_g() ; - { - int wstat ; - int r = sbearssl_run(&sc.eng, fds, verbosity, options, tto) ; - if (r < 0) strerr_diefu1sys(111, "run SSL engine") ; - else if (r) strerr_diefu2x(98, "establish or maintain SSL connection to peer: ", sbearssl_error_str(r)) ; - if (wait_pid(pid, &wstat) < 0) strerr_diefu1sys(111, "wait_pid") ; - return wait_estatus(wstat) ; - } + wstat = sbearssl_run(&sc.eng, 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