From 2c2188c09d14bfcfb37c563a46bdc2e8a110c3d8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 8 Oct 2016 15:22:11 +0900 Subject: [PATCH] nghttpx: Refactor ocsp command execution We have now generic read-only command execution in shrpx_exec.{h,cc}. --- src/CMakeLists.txt | 1 + src/Makefile.am | 1 + src/shrpx_connection_handler.cc | 87 ++------------------- src/shrpx_exec.cc | 134 ++++++++++++++++++++++++++++++++ src/shrpx_exec.h | 47 +++++++++++ 5 files changed, 188 insertions(+), 82 deletions(-) create mode 100644 src/shrpx_exec.cc create mode 100644 src/shrpx_exec.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ee96cd83..2a1836af 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -111,6 +111,7 @@ if(ENABLE_APP) shrpx_router.cc shrpx_api_downstream_connection.cc shrpx_health_monitor_downstream_connection.cc + shrpx_exec.cc ) if(HAVE_SPDYLAY) list(APPEND NGHTTPX_SRCS diff --git a/src/Makefile.am b/src/Makefile.am index cee98e85..23594e71 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -138,6 +138,7 @@ NGHTTPX_SRCS = \ shrpx_api_downstream_connection.cc shrpx_api_downstream_connection.h \ shrpx_health_monitor_downstream_connection.cc \ shrpx_health_monitor_downstream_connection.h \ + shrpx_exec.cc shrpx_exec.h \ buffer.h memchunk.h template.h allocator.h if HAVE_SPDYLAY diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index a01e1981..8d99e216 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -44,6 +44,7 @@ #include "shrpx_accept_handler.h" #include "shrpx_memcached_dispatcher.h" #include "shrpx_signal.h" +#include "shrpx_exec.h" #include "util.h" #include "template.h" @@ -506,7 +507,6 @@ void ConnectionHandler::cancel_ocsp_update() { // https://github.com/h2o/h2o int ConnectionHandler::start_ocsp_update(const char *cert_file) { int rv; - int pfd[2]; if (LOG_ENABLED(INFO)) { LOG(INFO) << "Start ocsp update for " << cert_file; @@ -520,92 +520,15 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) { get_config()->tls.ocsp.fetch_ocsp_response_file.c_str()), const_cast(cert_file), nullptr}; -#ifdef O_CLOEXEC - if (pipe2(pfd, O_CLOEXEC) == -1) { - return -1; - } -#else // !O_CLOEXEC - if (pipe(pfd) == -1) { - return -1; - } - util::make_socket_closeonexec(pfd[0]); - util::make_socket_closeonexec(pfd[1]); -#endif // !O_CLOEXEC - - auto closer = defer([&pfd]() { - if (pfd[0] != -1) { - close(pfd[0]); - } - - if (pfd[1] != -1) { - close(pfd[1]); - } - }); - - sigset_t oldset; - - rv = shrpx_signal_block_all(&oldset); + Process proc; + rv = exec_read_command(proc, argv); if (rv != 0) { - auto error = errno; - LOG(ERROR) << "Blocking all signals failed: " << strerror(error); - return -1; } - auto pid = fork(); + ocsp_.pid = proc.pid; + ocsp_.fd = proc.rfd; - if (pid == 0) { - // child process - shrpx_signal_unset_worker_proc_ign_handler(); - - rv = shrpx_signal_unblock_all(); - if (rv != 0) { - auto error = errno; - LOG(FATAL) << "Unblocking all signals failed: " << strerror(error); - - _Exit(EXIT_FAILURE); - } - - dup2(pfd[1], 1); - close(pfd[0]); - - rv = execv(argv[0], argv); - if (rv == -1) { - auto error = errno; - LOG(ERROR) << "Could not execute ocsp query command: " << argv[0] - << ", execve() faild, errno=" << error; - _Exit(EXIT_FAILURE); - } - // unreachable - } - - // parent process - if (pid == -1) { - auto error = errno; - LOG(ERROR) << "Could not execute ocsp query command for " << cert_file - << ": " << argv[0] << ", fork() failed, errno=" << error; - } - - rv = shrpx_signal_set(&oldset); - if (rv != 0) { - auto error = errno; - LOG(FATAL) << "Restoring all signals failed: " << strerror(error); - - _Exit(EXIT_FAILURE); - } - - if (pid == -1) { - return -1; - } - - close(pfd[1]); - pfd[1] = -1; - - ocsp_.pid = pid; - ocsp_.fd = pfd[0]; - pfd[0] = -1; - - util::make_socket_nonblocking(ocsp_.fd); ev_io_set(&ocsp_.rev, ocsp_.fd, EV_READ); ev_io_start(loop_, &ocsp_.rev); diff --git a/src/shrpx_exec.cc b/src/shrpx_exec.cc new file mode 100644 index 00000000..a59579bf --- /dev/null +++ b/src/shrpx_exec.cc @@ -0,0 +1,134 @@ +/* + * nghttp2 - HTTP/2 C Library + * + * Copyright (c) 2016 Tatsuhiro Tsujikawa + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#include "shrpx_exec.h" + +#include + +#include "shrpx_signal.h" +#include "util.h" +#include "template.h" + +using namespace nghttp2; + +namespace shrpx { + +// inspired by h2o_read_command function from h2o project: +// https://github.com/h2o/h2o +int exec_read_command(Process &proc, char *const argv[]) { + int rv; + int pfd[2]; + +#ifdef O_CLOEXEC + if (pipe2(pfd, O_CLOEXEC) == -1) { + return -1; + } +#else // !O_CLOEXEC + if (pipe(pfd) == -1) { + return -1; + } + util::make_socket_closeonexec(pfd[0]); + util::make_socket_closeonexec(pfd[1]); +#endif // !O_CLOEXEC + + auto closer = defer([&pfd]() { + if (pfd[0] != -1) { + close(pfd[0]); + } + + if (pfd[1] != -1) { + close(pfd[1]); + } + }); + + sigset_t oldset; + + rv = shrpx_signal_block_all(&oldset); + if (rv != 0) { + auto error = errno; + LOG(ERROR) << "Blocking all signals failed: errno=" << error; + + return -1; + } + + auto pid = fork(); + + if (pid == 0) { + // child process + shrpx_signal_unset_worker_proc_ign_handler(); + + rv = shrpx_signal_unblock_all(); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Unblocking all signals failed: errno=" << error; + + _Exit(EXIT_FAILURE); + } + + dup2(pfd[1], 1); + close(pfd[0]); + + rv = execv(argv[0], argv); + if (rv == -1) { + auto error = errno; + LOG(ERROR) << "Could not execute command: " << argv[0] + << ", execve() faild, errno=" << error; + _Exit(EXIT_FAILURE); + } + // unreachable + } + + // parent process + if (pid == -1) { + auto error = errno; + LOG(ERROR) << "Could not execute command: " << argv[0] + << ", fork() failed, errno=" << error; + } + + rv = shrpx_signal_set(&oldset); + if (rv != 0) { + auto error = errno; + LOG(FATAL) << "Restoring all signals failed: errno=" << error; + + _Exit(EXIT_FAILURE); + } + + if (pid == -1) { + return -1; + } + + close(pfd[1]); + pfd[1] = -1; + + util::make_socket_nonblocking(pfd[0]); + + proc.pid = pid; + proc.rfd = pfd[0]; + + pfd[0] = -1; + + return 0; +} + +} // namespace shrpx diff --git a/src/shrpx_exec.h b/src/shrpx_exec.h new file mode 100644 index 00000000..2d8a7701 --- /dev/null +++ b/src/shrpx_exec.h @@ -0,0 +1,47 @@ +/* + * nghttp2 - HTTP/2 C Library + * + * Copyright (c) 2016 Tatsuhiro Tsujikawa + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE + * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION + * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +#ifndef SHRPX_EXEC_H +#define SHRPX_EXEC_H + +#include "unistd.h" + +namespace shrpx { + +struct Process { + pid_t pid; + // fd to read from process + int rfd; +}; + +// Executes command |argv| after forking current process. The command +// should not expect to read from stdin. Parent process can read the +// stdout from command using proc.rfd. On success, this function +// returns 0, and process information is stored in |proc|. Otherwise, +// returns -1. +int exec_read_command(Process &proc, char *const argv[]); + +} // namespace shrpx + +#endif // SHRPX_EXEC_H