From 19ed13c7538b72325a247310a0dfe6543728adc1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Jun 2014 22:55:54 +0900 Subject: [PATCH 1/7] Fix off-by-one error when computing padding --- lib/nghttp2_session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 4d7e9c80..9c429603 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -4158,7 +4158,7 @@ static ssize_t inbound_frame_compute_pad(nghttp2_inbound_frame *iframe) DEBUGF(fprintf(stderr, "recv: padlen=%zu\n", padlen)); /* We cannot use iframe->frame.hd.length because of CONTINUATION */ - if(padlen - (padlen > 255) - 1 > iframe->payloadleft) { + if(padlen - (padlen > 256) - 1 > iframe->payloadleft) { return -1; } From 14b818efc89d3e968189e7756bafc76ad16e4beb Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 21:02:40 +0900 Subject: [PATCH 2/7] nghttpx: Use std::unique_ptr instead of char* --- src/shrpx.cc | 98 ++++++++++++++++++++------------------ src/shrpx_config.cc | 57 ++++++++++++---------- src/shrpx_config.h | 42 ++++++++-------- src/shrpx_http2_session.cc | 23 +++++---- src/shrpx_ssl.cc | 44 +++++++++-------- 5 files changed, 141 insertions(+), 123 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index c4a96deb..73ddbb01 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -144,14 +144,15 @@ evconnlistener* create_evlistener(ListenHandler *handler, int family) hints.ai_flags |= AI_ADDRCONFIG; #endif // AI_ADDRCONFIG - auto node = strcmp("*", get_config()->host) == 0 ? NULL : get_config()->host; + auto node = strcmp("*", get_config()->host.get()) == 0 ? + nullptr : get_config()->host.get(); addrinfo *res, *rp; r = getaddrinfo(node, service.c_str(), &hints, &res); if(r != 0) { if(LOG_ENABLED(INFO)) { LOG(INFO) << "Unable to get IPv" << (family == AF_INET ? "4" : "6") - << " address for " << get_config()->host << ": " + << " address for " << get_config()->host.get() << ": " << gai_strerror(r); } return NULL; @@ -240,11 +241,12 @@ void drop_privileges() namespace { void save_pid() { - std::ofstream out(get_config()->pid_file, std::ios::binary); + std::ofstream out(get_config()->pid_file.get(), std::ios::binary); out << getpid() << "\n"; out.close(); if(!out) { - LOG(ERROR) << "Could not save PID to file " << get_config()->pid_file; + LOG(ERROR) << "Could not save PID to file " + << get_config()->pid_file.get(); exit(EXIT_FAILURE); } } @@ -288,7 +290,7 @@ int event_loop() auto evlistener4 = create_evlistener(listener_handler, AF_INET); if(!evlistener6 && !evlistener4) { LOG(FATAL) << "Failed to listen on address " - << get_config()->host << ", port " << get_config()->port; + << get_config()->host.get() << ", port " << get_config()->port; exit(EXIT_FAILURE); } @@ -347,11 +349,11 @@ void fill_default_config() mod_config()->daemon = false; mod_config()->server_name = "nghttpx nghttp2/" NGHTTP2_VERSION; - set_config_str(&mod_config()->host, "*"); + mod_config()->host = strcopy("*"); mod_config()->port = 3000; - mod_config()->private_key_file = 0; - mod_config()->private_key_passwd = 0; - mod_config()->cert_file = 0; + mod_config()->private_key_file = nullptr; + mod_config()->private_key_passwd = nullptr; + mod_config()->cert_file = nullptr; // Read timeout for HTTP2 upstream connection mod_config()->http2_upstream_read_timeout.tv_sec = 180; @@ -388,9 +390,9 @@ void fill_default_config() mod_config()->upstream_no_tls = false; mod_config()->downstream_no_tls = false; - set_config_str(&mod_config()->downstream_host, "127.0.0.1"); + mod_config()->downstream_host = strcopy("127.0.0.1"); mod_config()->downstream_port = 80; - mod_config()->downstream_hostport = 0; + mod_config()->downstream_hostport = nullptr; mod_config()->downstream_addrlen = 0; mod_config()->num_worker = 1; @@ -398,13 +400,13 @@ void fill_default_config() mod_config()->add_x_forwarded_for = false; mod_config()->no_via = false; mod_config()->accesslog = false; - set_config_str(&mod_config()->conf_path, "/etc/nghttpx/nghttpx.conf"); + mod_config()->conf_path = strcopy("/etc/nghttpx/nghttpx.conf"); mod_config()->syslog = false; mod_config()->syslog_facility = LOG_DAEMON; mod_config()->use_syslog = false; // Default accept() backlog mod_config()->backlog = -1; - mod_config()->ciphers = 0; + mod_config()->ciphers = nullptr; mod_config()->honor_cipher_order = false; mod_config()->http2_proxy = false; mod_config()->http2_bridge = false; @@ -412,16 +414,16 @@ void fill_default_config() mod_config()->client = false; mod_config()->client_mode = false; mod_config()->insecure = false; - mod_config()->cacert = 0; - mod_config()->pid_file = 0; + mod_config()->cacert = nullptr; + mod_config()->pid_file = nullptr; mod_config()->uid = 0; mod_config()->gid = 0; mod_config()->backend_ipv4 = false; mod_config()->backend_ipv6 = false; mod_config()->tty = isatty(fileno(stderr)); mod_config()->cert_tree = 0; - mod_config()->downstream_http_proxy_userinfo = 0; - mod_config()->downstream_http_proxy_host = 0; + mod_config()->downstream_http_proxy_userinfo = nullptr; + mod_config()->downstream_http_proxy_host = nullptr; mod_config()->downstream_http_proxy_port = 0; mod_config()->downstream_http_proxy_addrlen = 0; mod_config()->rate_limit_cfg = nullptr; @@ -497,14 +499,14 @@ Connections: -b, --backend= Set backend host and port. Default: ')" - << get_config()->downstream_host << "," + << get_config()->downstream_host.get() << "," << get_config()->downstream_port << R"(' -f, --frontend= Set frontend host and port. If is '*', it assumes all addresses including both IPv4 and IPv6. Default: ')" - << get_config()->host << "," << get_config()->port << R"(' + << get_config()->host.get() << "," << get_config()->port << R"(' --backlog= Set listen backlog size. If -1 is given, libevent will choose suitable value. Default: )" @@ -798,7 +800,7 @@ Misc: intended to be used to drop root privileges. --conf= Load configuration from . Default: )" - << get_config()->conf_path << R"( + << get_config()->conf_path.get() << R"( -v, --version Print version and exit. -h, --help Print this help and exit.)" << std::endl; @@ -978,7 +980,7 @@ int main(int argc, char **argv) break; case 12: // --conf - set_config_str(&mod_config()->conf_path, optarg); + mod_config()->conf_path = strcopy(optarg); break; case 13: // --syslog @@ -1166,10 +1168,10 @@ int main(int argc, char **argv) nghttp2::ssl::LibsslGlobalLock(); #endif // NOTHREADS - if(conf_exists(get_config()->conf_path)) { - if(load_config(get_config()->conf_path) == -1) { + if(conf_exists(get_config()->conf_path.get())) { + if(load_config(get_config()->conf_path.get()) == -1) { LOG(FATAL) << "Failed to load configuration from " - << get_config()->conf_path; + << get_config()->conf_path.get(); exit(EXIT_FAILURE); } } @@ -1211,13 +1213,13 @@ int main(int argc, char **argv) if(get_config()->cert_file && get_config()->private_key_file) { mod_config()->default_ssl_ctx = - ssl::create_ssl_context(get_config()->private_key_file, - get_config()->cert_file); + ssl::create_ssl_context(get_config()->private_key_file.get(), + get_config()->cert_file.get()); if(get_config()->cert_tree) { - if(ssl::cert_lookup_tree_add_cert_from_file(get_config()->cert_tree, - get_config()->default_ssl_ctx, - get_config()->cert_file) - == -1) { + if(ssl::cert_lookup_tree_add_cert_from_file + (get_config()->cert_tree, + get_config()->default_ssl_ctx, + get_config()->cert_file.get()) == -1) { LOG(FATAL) << "Failed to parse command-line argument."; exit(EXIT_FAILURE); } @@ -1256,31 +1258,33 @@ int main(int argc, char **argv) } bool downstream_ipv6_addr = - is_ipv6_numeric_addr(get_config()->downstream_host); + is_ipv6_numeric_addr(get_config()->downstream_host.get()); - std::string hostport; + { + std::string hostport; - if(downstream_ipv6_addr) { - hostport += "["; + if(downstream_ipv6_addr) { + hostport += "["; + } + + hostport += get_config()->downstream_host.get(); + + if(downstream_ipv6_addr) { + hostport += "]"; + } + + hostport += ":"; + hostport += util::utos(get_config()->downstream_port); + + mod_config()->downstream_hostport = strcopy(hostport); } - hostport += get_config()->downstream_host; - - if(downstream_ipv6_addr) { - hostport += "]"; - } - - hostport += ":"; - hostport += util::utos(get_config()->downstream_port); - - set_config_str(&mod_config()->downstream_hostport, hostport.c_str()); - if(LOG_ENABLED(INFO)) { LOG(INFO) << "Resolving backend address"; } if(resolve_hostname(&mod_config()->downstream_addr, &mod_config()->downstream_addrlen, - get_config()->downstream_host, + get_config()->downstream_host.get(), get_config()->downstream_port, get_config()->backend_ipv4 ? AF_INET : (get_config()->backend_ipv6 ? @@ -1294,7 +1298,7 @@ int main(int argc, char **argv) } if(resolve_hostname(&mod_config()->downstream_http_proxy_addr, &mod_config()->downstream_http_proxy_addrlen, - get_config()->downstream_http_proxy_host, + get_config()->downstream_http_proxy_host.get(), get_config()->downstream_http_proxy_port, AF_UNSPEC) == -1) { exit(EXIT_FAILURE); diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 0c6e125b..3c0bbe1f 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -222,12 +222,20 @@ std::string read_passwd_from_file(const char *filename) return line; } -void set_config_str(char **destp, const char *val) +std::unique_ptr strcopy(const char *val) { - if(*destp) { - free(*destp); - } - *destp = strdup(val); + auto len = strlen(val); + auto res = util::make_unique(len + 1); + memcpy(res.get(), val, len + 1); + return res; +} + +std::unique_ptr strcopy(const std::string& val) +{ + auto len = val.size(); + auto res = util::make_unique(len + 1); + memcpy(res.get(), val.c_str(), len + 1); + return res; } std::unique_ptr parse_config_str_list(size_t *outlen, const char *s) @@ -275,17 +283,17 @@ int parse_config(const char *opt, const char *optarg) if(util::strieq(opt, SHRPX_OPT_BACKEND)) { if(split_host_port(host, sizeof(host), &port, optarg) == -1) { return -1; - } else { - set_config_str(&mod_config()->downstream_host, host); - mod_config()->downstream_port = port; } + + mod_config()->downstream_host = strcopy(host); + mod_config()->downstream_port = port; } else if(util::strieq(opt, SHRPX_OPT_FRONTEND)) { if(split_host_port(host, sizeof(host), &port, optarg) == -1) { return -1; - } else { - set_config_str(&mod_config()->host, host); - mod_config()->port = port; } + + mod_config()->host = strcopy(host); + mod_config()->port = port; } else if(util::strieq(opt, SHRPX_OPT_WORKERS)) { mod_config()->num_worker = strtol(optarg, nullptr, 10); } else if(util::strieq(opt, SHRPX_OPT_HTTP2_MAX_CONCURRENT_STREAMS)) { @@ -374,9 +382,9 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_BACKEND_NO_TLS)) { mod_config()->downstream_no_tls = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_BACKEND_TLS_SNI_FIELD)) { - set_config_str(&mod_config()->backend_tls_sni_name, optarg); + mod_config()->backend_tls_sni_name = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_PID_FILE)) { - set_config_str(&mod_config()->pid_file, optarg); + mod_config()->pid_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_USER)) { auto pwd = getpwnam(optarg); if(!pwd) { @@ -387,18 +395,18 @@ int parse_config(const char *opt, const char *optarg) mod_config()->uid = pwd->pw_uid; mod_config()->gid = pwd->pw_gid; } else if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_FILE)) { - set_config_str(&mod_config()->private_key_file, optarg); + mod_config()->private_key_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_PASSWD_FILE)) { auto passwd = read_passwd_from_file(optarg); if (passwd.empty()) { LOG(ERROR) << "Couldn't read key file's passwd from " << optarg; return -1; } - set_config_str(&mod_config()->private_key_passwd, passwd.c_str()); + mod_config()->private_key_passwd = strcopy(passwd); } else if(util::strieq(opt, SHRPX_OPT_CERTIFICATE_FILE)) { - set_config_str(&mod_config()->cert_file, optarg); + mod_config()->cert_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_DH_PARAM_FILE)) { - set_config_str(&mod_config()->dh_param_file, optarg); + mod_config()->dh_param_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_SUBCERT)) { // Private Key file and certificate file separated by ':'. const char *sp = strchr(optarg, ':'); @@ -419,7 +427,7 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_BACKLOG)) { mod_config()->backlog = strtol(optarg, nullptr, 10); } else if(util::strieq(opt, SHRPX_OPT_CIPHERS)) { - set_config_str(&mod_config()->ciphers, optarg); + mod_config()->ciphers = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_HONOR_CIPHER_ORDER)) { mod_config()->honor_cipher_order = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_CLIENT)) { @@ -427,7 +435,7 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_INSECURE)) { mod_config()->insecure = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_CACERT)) { - set_config_str(&mod_config()->cacert, optarg); + mod_config()->cacert = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV4)) { mod_config()->backend_ipv4 = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV6)) { @@ -445,13 +453,12 @@ int parse_config(const char *opt, const char *optarg) // userinfo component is empty string. if(!val.empty()) { val = util::percentDecode(val.begin(), val.end()); - set_config_str(&mod_config()->downstream_http_proxy_userinfo, - val.c_str()); + mod_config()->downstream_http_proxy_userinfo = strcopy(val); } } if(u.field_set & UF_HOST) { http2::copy_url_component(val, &u, UF_HOST, optarg); - set_config_str(&mod_config()->downstream_http_proxy_host, val.c_str()); + mod_config()->downstream_http_proxy_host = strcopy(val); } else { LOG(ERROR) << "backend-http-proxy-uri does not contain hostname"; return -1; @@ -493,11 +500,11 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT)) { mod_config()->verify_client = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT_CACERT)) { - set_config_str(&mod_config()->verify_client_cacert, optarg); + mod_config()->verify_client_cacert = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_CLIENT_PRIVATE_KEY_FILE)) { - set_config_str(&mod_config()->client_private_key_file, optarg); + mod_config()->client_private_key_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_CLIENT_CERT_FILE)) { - set_config_str(&mod_config()->client_cert_file, optarg); + mod_config()->client_cert_file = strcopy(optarg); } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_DUMP_REQUEST_HEADER)) { auto f = open_file_for_write(optarg); if(f == NULL) { diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 0e26db12..2d556cf0 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -162,25 +162,25 @@ struct Config { timeval downstream_read_timeout; timeval downstream_write_timeout; timeval downstream_idle_read_timeout; - char *host; - char *private_key_file; - char *private_key_passwd; - char *cert_file; - char *dh_param_file; + std::unique_ptr host; + std::unique_ptr private_key_file; + std::unique_ptr private_key_passwd; + std::unique_ptr cert_file; + std::unique_ptr dh_param_file; SSL_CTX *default_ssl_ctx; ssl::CertLookupTree *cert_tree; const char *server_name; - char *downstream_host; - char *downstream_hostport; - char *backend_tls_sni_name; - char *pid_file; - char *conf_path; - char *ciphers; - char *cacert; + std::unique_ptr downstream_host; + std::unique_ptr downstream_hostport; + std::unique_ptr backend_tls_sni_name; + std::unique_ptr pid_file; + std::unique_ptr conf_path; + std::unique_ptr ciphers; + std::unique_ptr cacert; // userinfo in http proxy URI, not percent-encoded form - char *downstream_http_proxy_userinfo; + std::unique_ptr downstream_http_proxy_userinfo; // host in http proxy URI - char *downstream_http_proxy_host; + std::unique_ptr downstream_http_proxy_host; // Rate limit configuration per connection ev_token_bucket_cfg *rate_limit_cfg; // Rate limit configuration per worker (thread) @@ -194,9 +194,9 @@ struct Config { char **tls_proto_list; // Path to file containing CA certificate solely used for client // certificate validation - char *verify_client_cacert; - char *client_private_key_file; - char *client_cert_file; + std::unique_ptr verify_client_cacert; + std::unique_ptr client_private_key_file; + std::unique_ptr client_cert_file; FILE *http2_upstream_dump_request_header; FILE *http2_upstream_dump_response_header; nghttp2_option *http2_option; @@ -293,9 +293,11 @@ std::unique_ptr parse_config_str_list(size_t *outlen, const char *s); // allowed. This function returns pair of NAME and VALUE. std::pair parse_header(const char *optarg); -// Copies NULL-terminated string |val| to |*destp|. If |*destp| is not -// NULL, it is freed before copying. -void set_config_str(char **destp, const char *val); +// Returns a copy of NULL-terminated string |val|. +std::unique_ptr strcopy(const char *val); + +// Returns a copy of val.c_str(). +std::unique_ptr strcopy(const std::string& val); // Returns string for syslog |facility|. const char* str_syslog_facility(int facility); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 42167028..d44ec3ee 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -338,15 +338,16 @@ void proxy_eventcb(bufferevent *bev, short events, void *ptr) SSLOG(INFO, http2session) << "Connected to the proxy"; } std::string req = "CONNECT "; - req += get_config()->downstream_hostport; + req += get_config()->downstream_hostport.get(); req += " HTTP/1.1\r\nHost: "; - req += get_config()->downstream_host; + req += get_config()->downstream_host.get(); req += "\r\n"; if(get_config()->downstream_http_proxy_userinfo) { req += "Proxy-Authorization: Basic "; - size_t len = strlen(get_config()->downstream_http_proxy_userinfo); - req += base64::encode(get_config()->downstream_http_proxy_userinfo, - get_config()->downstream_http_proxy_userinfo+len); + size_t len = strlen(get_config()->downstream_http_proxy_userinfo.get()); + req += base64::encode + (get_config()->downstream_http_proxy_userinfo.get(), + get_config()->downstream_http_proxy_userinfo.get() + len); req += "\r\n"; } req += "\r\n"; @@ -393,7 +394,8 @@ int Http2Session::initiate_connection() if(get_config()->downstream_http_proxy_host && state_ == DISCONNECTED) { if(LOG_ENABLED(INFO)) { SSLOG(INFO, this) << "Connecting to the proxy " - << get_config()->downstream_http_proxy_host << ":" + << get_config()->downstream_http_proxy_host.get() + << ":" << get_config()->downstream_http_proxy_port; } bev_ = bufferevent_socket_new(evbase_, -1, BEV_OPT_DEFER_CALLBACKS); @@ -414,7 +416,8 @@ int Http2Session::initiate_connection() get_config()->downstream_http_proxy_addrlen); if(rv != 0) { SSLOG(ERROR, this) << "Failed to connect to the proxy " - << get_config()->downstream_http_proxy_host << ":" + << get_config()->downstream_http_proxy_host.get() + << ":" << get_config()->downstream_http_proxy_port; return SHRPX_ERR_NETWORK; } @@ -442,13 +445,13 @@ int Http2Session::initiate_connection() const char *sni_name = nullptr; if ( get_config()->backend_tls_sni_name ) { - sni_name = get_config()->backend_tls_sni_name; + sni_name = get_config()->backend_tls_sni_name.get(); } else { - sni_name = get_config()->downstream_host; + sni_name = get_config()->downstream_host.get(); } - if(!util::numeric_host(sni_name)) { + if(sni_name && !util::numeric_host(sni_name)) { // TLS extensions: SNI. There is no documentation about the return // code for this function (actually this is macro wrapping SSL_ctrl // at the time of this writing). diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index a53c6493..7a3f2118 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -107,13 +107,13 @@ namespace { int ssl_pem_passwd_cb(char *buf, int size, int rwflag, void *user_data) { auto config = static_cast(user_data); - int len = (int)strlen(config->private_key_passwd); + int len = (int)strlen(config->private_key_passwd.get()); if (size < len + 1) { LOG(ERROR) << "ssl_pem_passwd_cb: buf is too small " << size; return 0; } // Copy string including last '\0'. - memcpy(buf, config->private_key_passwd, len+1); + memcpy(buf, config->private_key_passwd.get(), len + 1); return len; } } // namespace @@ -252,7 +252,7 @@ SSL_CTX* create_ssl_context(const char *private_key_file, const char *ciphers; if(get_config()->ciphers) { - ciphers = get_config()->ciphers; + ciphers = get_config()->ciphers.get(); // If ciphers are given, honor its order unconditionally SSL_CTX_set_options(ssl_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE); } else { @@ -291,7 +291,7 @@ SSL_CTX* create_ssl_context(const char *private_key_file, if(get_config()->dh_param_file) { // Read DH parameters from file - auto bio = BIO_new_file(get_config()->dh_param_file, "r"); + auto bio = BIO_new_file(get_config()->dh_param_file.get(), "r"); if(bio == nullptr) { LOG(FATAL) << "BIO_new_file() failed: " << ERR_error_string(ERR_get_error(), nullptr); @@ -333,11 +333,11 @@ SSL_CTX* create_ssl_context(const char *private_key_file, } if(get_config()->verify_client) { if(get_config()->verify_client_cacert) { - if(SSL_CTX_load_verify_locations(ssl_ctx, - get_config()->verify_client_cacert, - nullptr) != 1) { + if(SSL_CTX_load_verify_locations + (ssl_ctx, get_config()->verify_client_cacert.get(), nullptr) != 1) { + LOG(FATAL) << "Could not load trusted ca certificates from " - << get_config()->verify_client_cacert << ": " + << get_config()->verify_client_cacert.get() << ": " << ERR_error_string(ERR_get_error(), nullptr); DIE(); } @@ -345,10 +345,11 @@ SSL_CTX* create_ssl_context(const char *private_key_file, // error even though it returns success. See // http://forum.nginx.org/read.php?29,242540 ERR_clear_error(); - auto list = SSL_load_client_CA_file(get_config()->verify_client_cacert); + auto list = SSL_load_client_CA_file + (get_config()->verify_client_cacert.get()); if(!list) { LOG(FATAL) << "Could not load ca certificates from " - << get_config()->verify_client_cacert << ": " + << get_config()->verify_client_cacert.get() << ": " << ERR_error_string(ERR_get_error(), nullptr); DIE(); } @@ -405,7 +406,7 @@ SSL_CTX* create_ssl_client_context() const char *ciphers; if(get_config()->ciphers) { - ciphers = get_config()->ciphers; + ciphers = get_config()->ciphers.get(); } else { ciphers = "HIGH:!aNULL:!eNULL"; } @@ -425,10 +426,11 @@ SSL_CTX* create_ssl_client_context() } if(get_config()->cacert) { - if(SSL_CTX_load_verify_locations(ssl_ctx, get_config()->cacert, nullptr) - != 1) { + if(SSL_CTX_load_verify_locations + (ssl_ctx, get_config()->cacert.get(), nullptr) != 1) { + LOG(FATAL) << "Could not load trusted ca certificates from " - << get_config()->cacert << ": " + << get_config()->cacert.get() << ": " << ERR_error_string(ERR_get_error(), nullptr); DIE(); } @@ -436,20 +438,20 @@ SSL_CTX* create_ssl_client_context() if(get_config()->client_private_key_file) { if(SSL_CTX_use_PrivateKey_file(ssl_ctx, - get_config()->client_private_key_file, + get_config()->client_private_key_file.get(), SSL_FILETYPE_PEM) != 1) { LOG(FATAL) << "Could not load client private key from " - << get_config()->client_private_key_file << ": " + << get_config()->client_private_key_file.get() << ": " << ERR_error_string(ERR_get_error(), nullptr); DIE(); } } if(get_config()->client_cert_file) { - if(SSL_CTX_use_certificate_chain_file(ssl_ctx, - get_config()->client_cert_file) - != 1) { + if(SSL_CTX_use_certificate_chain_file + (ssl_ctx, get_config()->client_cert_file.get()) != 1) { + LOG(FATAL) << "Could not load client certificate from " - << get_config()->client_cert_file << ": " + << get_config()->client_cert_file.get() << ": " << ERR_error_string(ERR_get_error(), nullptr); DIE(); } @@ -686,7 +688,7 @@ int check_cert(SSL *ssl) std::vector dns_names; std::vector ip_addrs; get_altnames(cert, dns_names, ip_addrs, common_name); - if(verify_hostname(get_config()->downstream_host, + if(verify_hostname(get_config()->downstream_host.get(), &get_config()->downstream_addr, get_config()->downstream_addrlen, dns_names, ip_addrs, common_name) != 0) { From 1f58be423d01f1cdf5ea58ca19d162a108f49d46 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 21:05:36 +0900 Subject: [PATCH 3/7] nghttpx: Use nullptr instead of 0 --- src/shrpx.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 73ddbb01..8924d017 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -421,7 +421,7 @@ void fill_default_config() mod_config()->backend_ipv4 = false; mod_config()->backend_ipv6 = false; mod_config()->tty = isatty(fileno(stderr)); - mod_config()->cert_tree = 0; + mod_config()->cert_tree = nullptr; mod_config()->downstream_http_proxy_userinfo = nullptr; mod_config()->downstream_http_proxy_host = nullptr; mod_config()->downstream_http_proxy_port = 0; From 0fd5b2aa32ff0b1898c74241f0a49ed04f47ceae Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 22:52:27 +0900 Subject: [PATCH 4/7] nghttpx: Use std::vector for tls_proto_list and npn_list Now SSL/TLS option mask to disable particular SSL/TLS protocol versions are pre-calculated and stored in Config. --- src/shrpx.cc | 15 +++++---- src/shrpx_client_handler.cc | 1 - src/shrpx_config.cc | 40 ++++++++++++++---------- src/shrpx_config.h | 31 ++++++++++--------- src/shrpx_config_test.cc | 26 +++++++++------- src/shrpx_ssl.cc | 61 +++++++++++++++++-------------------- src/shrpx_ssl.h | 14 ++++++--- 7 files changed, 102 insertions(+), 86 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index 8924d017..4ff3c0d8 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -435,7 +435,6 @@ void fill_default_config() mod_config()->worker_read_burst = 0; mod_config()->worker_write_rate = 0; mod_config()->worker_write_burst = 0; - mod_config()->npn_list = nullptr; mod_config()->verify_client = false; mod_config()->verify_client_cacert = nullptr; mod_config()->client_private_key_file = nullptr; @@ -452,6 +451,8 @@ void fill_default_config() (mod_config()->http2_option, 1); nghttp2_option_set_no_auto_connection_window_update (mod_config()->http2_option, 1); + + mod_config()->tls_proto_mask = 0; } } // namespace @@ -1188,15 +1189,17 @@ int main(int argc, char **argv) } } - if(!get_config()->npn_list) { - mod_config()->npn_list = parse_config_str_list(&mod_config()->npn_list_len, - DEFAULT_NPN_LIST).release(); + if(get_config()->npn_list.empty()) { + mod_config()->npn_list = parse_config_str_list(DEFAULT_NPN_LIST); } - if(!get_config()->tls_proto_list) { + if(get_config()->tls_proto_list.empty()) { mod_config()->tls_proto_list = parse_config_str_list - (&mod_config()->tls_proto_list_len, DEFAULT_TLS_PROTO_LIST).release(); + (DEFAULT_TLS_PROTO_LIST); } + mod_config()->tls_proto_mask = + ssl::create_tls_proto_mask(get_config()->tls_proto_list); + if(!get_config()->subcerts.empty()) { mod_config()->cert_tree = ssl::cert_lookup_tree_new(); } diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index d7a11760..d4f97f09 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -384,7 +384,6 @@ int ClientHandler::validate_next_proto() CLOG(INFO, this) << "The negotiated next protocol: " << proto; } if(!ssl::in_proto_list(get_config()->npn_list, - get_config()->npn_list_len, next_proto, next_proto_len)) { break; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 3c0bbe1f..11ceda1c 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -238,12 +238,12 @@ std::unique_ptr strcopy(const std::string& val) return res; } -std::unique_ptr parse_config_str_list(size_t *outlen, const char *s) +std::vector parse_config_str_list(const char *s) { size_t len = 1; for(const char *first = s, *p = nullptr; (p = strchr(first, ',')); ++len, first = p + 1); - auto list = util::make_unique(len); + auto list = std::vector(len); auto first = strdup(s); len = 0; for(;;) { @@ -256,10 +256,20 @@ std::unique_ptr parse_config_str_list(size_t *outlen, const char *s) first = p + 1; } list[len++] = first; - *outlen = len; + return list; } +void clear_config_str_list(std::vector& list) +{ + if(list.empty()) { + return; + } + + free(list[0]); + list.clear(); +} + std::pair parse_header(const char *optarg) { // We skip possible ":" at the start of optarg. @@ -490,13 +500,13 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_BURST)) { mod_config()->worker_write_burst = strtoul(optarg, nullptr, 10); } else if(util::strieq(opt, SHRPX_OPT_NPN_LIST)) { - delete [] mod_config()->npn_list; - mod_config()->npn_list = parse_config_str_list(&mod_config()->npn_list_len, - optarg).release(); + clear_config_str_list(mod_config()->npn_list); + + mod_config()->npn_list = parse_config_str_list(optarg); } else if(util::strieq(opt, SHRPX_OPT_TLS_PROTO_LIST)) { - delete [] mod_config()->tls_proto_list; - mod_config()->tls_proto_list = parse_config_str_list - (&mod_config()->tls_proto_list_len, optarg).release(); + clear_config_str_list(mod_config()->tls_proto_list); + + mod_config()->tls_proto_list = parse_config_str_list(optarg); } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT)) { mod_config()->verify_client = util::strieq(optarg, "yes"); } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT_CACERT)) { @@ -524,17 +534,15 @@ int parse_config(const char *opt, const char *optarg) } else if(util::strieq(opt, SHRPX_OPT_PADDING)) { mod_config()->padding = strtoul(optarg, nullptr, 10); } else if(util::strieq(opt, SHRPX_OPT_ALTSVC)) { - size_t len; + auto tokens = parse_config_str_list(optarg); - auto tokens = parse_config_str_list(&len, optarg); - - if(len < 2) { + if(tokens.size() < 2) { // Requires at least protocol_id and port LOG(ERROR) << "altsvc: too few parameters: " << optarg; return -1; } - if(len > 4) { + if(tokens.size() > 4) { // We only need protocol_id, port, host and origin LOG(ERROR) << "altsvc: too many parameters: " << optarg; return -1; @@ -555,11 +563,11 @@ int parse_config(const char *opt, const char *optarg) altsvc.protocol_id = tokens[0]; altsvc.protocol_id_len = strlen(altsvc.protocol_id); - if(len > 2) { + if(tokens.size() > 2) { altsvc.host = tokens[2]; altsvc.host_len = strlen(altsvc.host); - if(len > 3) { + if(tokens.size() > 3) { altsvc.origin = tokens[3]; altsvc.origin_len = strlen(altsvc.origin); } diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 2d556cf0..06243978 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -188,10 +188,10 @@ struct Config { // list of supported NPN/ALPN protocol strings in the order of // preference. The each element of this list is a NULL-terminated // string. - char **npn_list; + std::vector npn_list; // list of supported SSL/TLS protocol strings. The each element of // this list is a NULL-terminated string. - char **tls_proto_list; + std::vector tls_proto_list; // Path to file containing CA certificate solely used for client // certificate validation std::unique_ptr verify_client_cacert; @@ -217,11 +217,10 @@ struct Config { size_t worker_read_burst; size_t worker_write_rate; size_t worker_write_burst; - // The number of elements in npn_list - size_t npn_list_len; - // The number of elements in tls_proto_list - size_t tls_proto_list_len; size_t padding; + // Bit mask to disable SSL/TLS protocol versions. This will be + // passed to SSL_CTX_set_options(). + long int tls_proto_mask; // downstream protocol; this will be determined by given options. shrpx_proto downstream_proto; int syslog_facility; @@ -278,14 +277,18 @@ std::string read_passwd_from_file(const char *filename); // Parses comma delimited strings in |s| and returns the array of // pointers, each element points to the each substring in |s|. The -// number of elements are stored in |*outlen|. The |s| must be comma -// delimited list of strings. The strings must be delimited by a -// single comma and any white spaces around it are treated as a part -// of protocol strings. This function may modify |s| and the caller -// must leave it as is after this call. This function copies |s| and -// first element in the return value points to it. It is caller's -// responsibility to deallocate its memory. -std::unique_ptr parse_config_str_list(size_t *outlen, const char *s); +// |s| must be comma delimited list of strings. The strings must be +// delimited by a single comma and any white spaces around it are +// treated as a part of protocol strings. This function may modify +// |s| and the caller must leave it as is after this call. This +// function copies |s| and first element in the return value points to +// it. It is caller's responsibility to deallocate its memory. +std::vector parse_config_str_list(const char *s); + +// Clears all elements of |list|, which is returned by +// parse_config_str_list(). If list is not empty, list[0] is freed by +// free(2). After this call, list.empty() must be true. +void clear_config_str_list(std::vector& list); // Parses header field in |optarg|. We expect header field is formed // like "NAME: VALUE". We require that NAME is non empty string. ":" diff --git a/src/shrpx_config_test.cc b/src/shrpx_config_test.cc index 37d68b6c..cfe95c4e 100644 --- a/src/shrpx_config_test.cc +++ b/src/shrpx_config_test.cc @@ -32,32 +32,36 @@ namespace shrpx { void test_shrpx_config_parse_config_str_list(void) { - size_t outlen; - auto res = parse_config_str_list(&outlen, "a"); - CU_ASSERT(1 == outlen); + auto res = parse_config_str_list("a"); + CU_ASSERT(1 == res.size()); CU_ASSERT(0 == strcmp("a", res[0])); + clear_config_str_list(res); - res = parse_config_str_list(&outlen, "a,"); - CU_ASSERT(2 == outlen); + res = parse_config_str_list("a,"); + CU_ASSERT(2 == res.size()); CU_ASSERT(0 == strcmp("a", res[0])); CU_ASSERT(0 == strcmp("", res[1])); + clear_config_str_list(res); - res = parse_config_str_list(&outlen, ",a,,"); - CU_ASSERT(4 == outlen); + res = parse_config_str_list(",a,,"); + CU_ASSERT(4 == res.size()); CU_ASSERT(0 == strcmp("", res[0])); CU_ASSERT(0 == strcmp("a", res[1])); CU_ASSERT(0 == strcmp("", res[2])); CU_ASSERT(0 == strcmp("", res[3])); + clear_config_str_list(res); - res = parse_config_str_list(&outlen, ""); - CU_ASSERT(1 == outlen); + res = parse_config_str_list(""); + CU_ASSERT(1 == res.size()); CU_ASSERT(0 == strcmp("", res[0])); + clear_config_str_list(res); - res = parse_config_str_list(&outlen, "alpha,bravo,charlie"); - CU_ASSERT(3 == outlen); + res = parse_config_str_list("alpha,bravo,charlie"); + CU_ASSERT(3 == res.size()); CU_ASSERT(0 == strcmp("alpha", res[0])); CU_ASSERT(0 == strcmp("bravo", res[1])); CU_ASSERT(0 == strcmp("charlie", res[2])); + clear_config_str_list(res); } void test_shrpx_config_parse_header(void) diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 7a3f2118..186eebeb 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -88,14 +88,14 @@ int verify_callback(int preverify_ok, X509_STORE_CTX *ctx) } // namespace namespace { -size_t set_npn_prefs(unsigned char *out, char **protos, size_t len) +size_t set_npn_prefs(unsigned char *out, const std::vector& protos) { unsigned char *ptr = out; size_t listlen = 0; - for(size_t i = 0; i < len; ++i) { - size_t plen = strlen(protos[i]); + for(auto proto : protos) { + auto plen = strlen(proto); *ptr = plen; - memcpy(ptr+1, protos[i], *ptr); + memcpy(ptr+1, proto, *ptr); ptr += *ptr+1; listlen += 1 + plen; } @@ -166,11 +166,7 @@ int alpn_select_proto_cb(SSL* ssl, // We assume that get_config()->npn_list contains ALPN protocol // identifier sorted by preference order. So we just break when we // found the first overlap. - for(auto needle_ptr = get_config()->npn_list, - end_needle_ptr = needle_ptr + get_config()->npn_list_len; - needle_ptr < end_needle_ptr; ++needle_ptr) { - - auto target_proto_id = *needle_ptr; + for(auto target_proto_id : get_config()->npn_list) { auto target_proto_len = strlen(reinterpret_cast(target_proto_id)); @@ -206,27 +202,29 @@ int alpn_select_proto_cb(SSL* ssl, #endif // OPENSSL_VERSION_NUMBER >= 0x10002000L namespace { -const char *names[] = { "TLSv1.2", "TLSv1.1", "TLSv1.0", "SSLv3" }; -const size_t namelen = sizeof(names)/sizeof(names[0]); -const long int masks[] = { SSL_OP_NO_TLSv1_2, SSL_OP_NO_TLSv1_1, - SSL_OP_NO_TLSv1, SSL_OP_NO_SSLv3 }; -long int create_tls_proto_mask(char **tls_proto_list, size_t len) +const char *tls_names[] = { "TLSv1.2", "TLSv1.1", "TLSv1.0", "SSLv3" }; +const size_t tls_namelen = sizeof(tls_names)/sizeof(tls_names[0]); +const long int tls_masks[] = { SSL_OP_NO_TLSv1_2, SSL_OP_NO_TLSv1_1, + SSL_OP_NO_TLSv1, SSL_OP_NO_SSLv3 }; +} // namespace + +long int create_tls_proto_mask(const std::vector& tls_proto_list) { long int res = 0; - for(size_t i = 0; i < namelen; ++i) { + + for(size_t i = 0; i < tls_namelen; ++i) { size_t j; - for(j = 0; j < len; ++j) { - if(strcasecmp(names[i], tls_proto_list[j]) == 0) { + for(j = 0; j < tls_proto_list.size(); ++j) { + if(util::strieq(tls_names[i], tls_proto_list[j])) { break; } } - if(j == len) { - res |= masks[i]; + if(j == tls_proto_list.size()) { + res |= tls_masks[i]; } } return res; } -} // namespace SSL_CTX* create_ssl_context(const char *private_key_file, const char *cert_file) @@ -243,8 +241,7 @@ SSL_CTX* create_ssl_context(const char *private_key_file, SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | SSL_OP_SINGLE_ECDH_USE | SSL_OP_SINGLE_DH_USE | SSL_OP_NO_TICKET | - create_tls_proto_mask(get_config()->tls_proto_list, - get_config()->tls_proto_list_len)); + get_config()->tls_proto_mask); const unsigned char sid_ctx[] = "shrpx"; SSL_CTX_set_session_id_context(ssl_ctx, sid_ctx, sizeof(sid_ctx)-1); @@ -364,8 +361,8 @@ SSL_CTX* create_ssl_context(const char *private_key_file, SSL_CTX_set_info_callback(ssl_ctx, info_callback); // NPN advertisement - auto proto_list_len = set_npn_prefs(proto_list, get_config()->npn_list, - get_config()->npn_list_len); + auto proto_list_len = set_npn_prefs(proto_list, get_config()->npn_list); + next_proto.first = proto_list; next_proto.second = proto_list_len; SSL_CTX_set_next_protos_advertised_cb(ssl_ctx, next_proto_cb, &next_proto); @@ -401,8 +398,7 @@ SSL_CTX* create_ssl_client_context() SSL_CTX_set_options(ssl_ctx, SSL_OP_ALL | SSL_OP_NO_SSLv2 | SSL_OP_NO_COMPRESSION | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | - create_tls_proto_mask(get_config()->tls_proto_list, - get_config()->tls_proto_list_len)); + get_config()->tls_proto_mask); const char *ciphers; if(get_config()->ciphers) { @@ -461,8 +457,8 @@ SSL_CTX* create_ssl_client_context() #if OPENSSL_VERSION_NUMBER >= 0x10002000L // ALPN advertisement - auto proto_list_len = set_npn_prefs(proto_list, get_config()->npn_list, - get_config()->npn_list_len); + auto proto_list_len = set_npn_prefs(proto_list, get_config()->npn_list); + next_proto.first = proto_list; next_proto.second = proto_list_len; SSL_CTX_set_alpn_protos(ssl_ctx, proto_list, proto_list[0] + 1); @@ -900,12 +896,11 @@ int cert_lookup_tree_add_cert_from_file(CertLookupTree *lt, SSL_CTX *ssl_ctx, return 0; } -bool in_proto_list(char **protos, size_t len, - const unsigned char *proto, size_t protolen) +bool in_proto_list(const std::vector& protos, + const unsigned char *needle, size_t len) { - for(size_t i = 0; i < len; ++i) { - if(strlen(protos[i]) == protolen && - memcmp(protos[i], proto, protolen) == 0) { + for(auto proto : protos) { + if(strlen(proto) == len && memcmp(proto, needle, len) == 0) { return true; } } diff --git a/src/shrpx_ssl.h b/src/shrpx_ssl.h index ac6aed87..bbd9e4cc 100644 --- a/src/shrpx_ssl.h +++ b/src/shrpx_ssl.h @@ -123,15 +123,19 @@ SSL_CTX* cert_lookup_tree_lookup(CertLookupTree *lt, const char *hostname, int cert_lookup_tree_add_cert_from_file(CertLookupTree *lt, SSL_CTX *ssl_ctx, const char *certfile); -// Returns true if |proto| which has |protolen| bytes is included in -// the protocol list |protos|, which has |len| elements. The format of -// the |protos| is the one used in Config::npn_list. -bool in_proto_list(char **protos, size_t len, - const unsigned char *proto, size_t protolen); +// Returns true if |needle| which has |len| bytes is included in the +// protocol list |protos|. +bool in_proto_list(const std::vector& protos, + const unsigned char *needle, size_t len); // Returns true if security requirement for HTTP/2 is fulfilled. bool check_http2_requirement(SSL *ssl); +// Returns SSL/TLS option mask to disable SSL/TLS protocol version not +// included in |tls_proto_list|. The returned mask can be directly +// passed to SSL_CTX_set_options(). +long int create_tls_proto_mask(const std::vector& tls_proto_list); + } // namespace ssl } // namespace shrpx From db8af31e2be099f3960e327e5117c30bf1e2e440 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 23:01:48 +0900 Subject: [PATCH 5/7] nghttpx: Code cleanup --- src/shrpx_config.cc | 393 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 320 insertions(+), 73 deletions(-) diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 11ceda1c..62d1cbf1 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -297,56 +297,128 @@ int parse_config(const char *opt, const char *optarg) mod_config()->downstream_host = strcopy(host); mod_config()->downstream_port = port; - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND)) { if(split_host_port(host, sizeof(host), &port, optarg) == -1) { return -1; } mod_config()->host = strcopy(host); mod_config()->port = port; - } else if(util::strieq(opt, SHRPX_OPT_WORKERS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WORKERS)) { mod_config()->num_worker = strtol(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_HTTP2_MAX_CONCURRENT_STREAMS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_HTTP2_MAX_CONCURRENT_STREAMS)) { mod_config()->http2_max_concurrent_streams = strtol(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_LOG_LEVEL)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_LOG_LEVEL)) { if(Log::set_severity_level_by_name(optarg) == -1) { LOG(ERROR) << "Invalid severity level: " << optarg; return -1; } - } else if(util::strieq(opt, SHRPX_OPT_DAEMON)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_DAEMON)) { mod_config()->daemon = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_HTTP2_PROXY)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_HTTP2_PROXY)) { mod_config()->http2_proxy = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_HTTP2_BRIDGE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_HTTP2_BRIDGE)) { mod_config()->http2_bridge = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_CLIENT_PROXY)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CLIENT_PROXY)) { mod_config()->client_proxy = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_ADD_X_FORWARDED_FOR)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_ADD_X_FORWARDED_FOR)) { mod_config()->add_x_forwarded_for = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_NO_VIA)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_NO_VIA)) { mod_config()->no_via = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_READ_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_READ_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->http2_upstream_read_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_READ_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_READ_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->upstream_read_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_WRITE_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_WRITE_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->upstream_write_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_READ_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_READ_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->downstream_read_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_WRITE_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_WRITE_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->downstream_write_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_ACCESSLOG)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_ACCESSLOG)) { mod_config()->accesslog = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_KEEP_ALIVE_TIMEOUT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_KEEP_ALIVE_TIMEOUT)) { timeval tv = {strtol(optarg, nullptr, 10), 0}; mod_config()->downstream_idle_read_timeout = tv; - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS) || - util::strieq(opt, SHRPX_OPT_BACKEND_HTTP2_WINDOW_BITS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS) || + util::strieq(opt, SHRPX_OPT_BACKEND_HTTP2_WINDOW_BITS)) { size_t *resp; const char *optname; if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_WINDOW_BITS)) { @@ -365,10 +437,12 @@ int parse_config(const char *opt, const char *optarg) << " specify the integer in the range [0, 30], inclusive"; return -1; } - } else if(util::strieq(opt, - SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS) || - util::strieq(opt, - SHRPX_OPT_BACKEND_HTTP2_CONNECTION_WINDOW_BITS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS) || + util::strieq(opt, SHRPX_OPT_BACKEND_HTTP2_CONNECTION_WINDOW_BITS)) { size_t *resp; const char *optname; if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_CONNECTION_WINDOW_BITS)) { @@ -387,15 +461,35 @@ int parse_config(const char *opt, const char *optarg) << " specify the integer in the range [16, 30], inclusive"; return -1; } - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_NO_TLS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_NO_TLS)) { mod_config()->upstream_no_tls = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_NO_TLS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_NO_TLS)) { mod_config()->downstream_no_tls = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_TLS_SNI_FIELD)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_TLS_SNI_FIELD)) { mod_config()->backend_tls_sni_name = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_PID_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_PID_FILE)) { mod_config()->pid_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_USER)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_USER)) { auto pwd = getpwnam(optarg); if(!pwd) { LOG(ERROR) << "--user: failed to get uid from " << optarg @@ -404,20 +498,40 @@ int parse_config(const char *opt, const char *optarg) } mod_config()->uid = pwd->pw_uid; mod_config()->gid = pwd->pw_gid; - } else if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_FILE)) { mod_config()->private_key_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_PASSWD_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_PRIVATE_KEY_PASSWD_FILE)) { auto passwd = read_passwd_from_file(optarg); if (passwd.empty()) { LOG(ERROR) << "Couldn't read key file's passwd from " << optarg; return -1; } mod_config()->private_key_passwd = strcopy(passwd); - } else if(util::strieq(opt, SHRPX_OPT_CERTIFICATE_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CERTIFICATE_FILE)) { mod_config()->cert_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_DH_PARAM_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_DH_PARAM_FILE)) { mod_config()->dh_param_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_SUBCERT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_SUBCERT)) { // Private Key file and certificate file separated by ':'. const char *sp = strchr(optarg, ':'); if(sp) { @@ -425,32 +539,76 @@ int parse_config(const char *opt, const char *optarg) // TODO Do we need private key for subcert? mod_config()->subcerts.emplace_back(keyfile, sp+1); } - } else if(util::strieq(opt, SHRPX_OPT_SYSLOG)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_SYSLOG)) { mod_config()->syslog = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_SYSLOG_FACILITY)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_SYSLOG_FACILITY)) { int facility = int_syslog_facility(optarg); if(facility == -1) { LOG(ERROR) << "Unknown syslog facility: " << optarg; return -1; } mod_config()->syslog_facility = facility; - } else if(util::strieq(opt, SHRPX_OPT_BACKLOG)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKLOG)) { mod_config()->backlog = strtol(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_CIPHERS)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CIPHERS)) { mod_config()->ciphers = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_HONOR_CIPHER_ORDER)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_HONOR_CIPHER_ORDER)) { mod_config()->honor_cipher_order = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_CLIENT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CLIENT)) { mod_config()->client = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_INSECURE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_INSECURE)) { mod_config()->insecure = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_CACERT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CACERT)) { mod_config()->cacert = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV4)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV4)) { mod_config()->backend_ipv4 = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV6)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_IPV6)) { mod_config()->backend_ipv6 = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_BACKEND_HTTP_PROXY_URI)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_BACKEND_HTTP_PROXY_URI)) { // parse URI and get hostname, port and optionally userinfo. http_parser_url u; memset(&u, 0, sizeof(u)); @@ -483,57 +641,137 @@ int parse_config(const char *opt, const char *optarg) LOG(ERROR) << "Could not parse backend-http-proxy-uri"; return -1; } - } else if(util::strieq(opt, SHRPX_OPT_READ_RATE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_READ_RATE)) { mod_config()->read_rate = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_READ_BURST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_READ_BURST)) { mod_config()->read_burst = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WRITE_RATE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WRITE_RATE)) { mod_config()->write_rate = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WRITE_BURST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WRITE_BURST)) { mod_config()->write_burst = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WORKER_READ_RATE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WORKER_READ_RATE)) { mod_config()->worker_read_rate = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WORKER_READ_BURST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WORKER_READ_BURST)) { mod_config()->worker_read_burst = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_RATE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_RATE)) { mod_config()->worker_write_rate = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_BURST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_WORKER_WRITE_BURST)) { mod_config()->worker_write_burst = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_NPN_LIST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_NPN_LIST)) { clear_config_str_list(mod_config()->npn_list); mod_config()->npn_list = parse_config_str_list(optarg); - } else if(util::strieq(opt, SHRPX_OPT_TLS_PROTO_LIST)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_TLS_PROTO_LIST)) { clear_config_str_list(mod_config()->tls_proto_list); mod_config()->tls_proto_list = parse_config_str_list(optarg); - } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT)) { mod_config()->verify_client = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT_CACERT)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_VERIFY_CLIENT_CACERT)) { mod_config()->verify_client_cacert = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_CLIENT_PRIVATE_KEY_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CLIENT_PRIVATE_KEY_FILE)) { mod_config()->client_private_key_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_CLIENT_CERT_FILE)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_CLIENT_CERT_FILE)) { mod_config()->client_cert_file = strcopy(optarg); - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_DUMP_REQUEST_HEADER)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_DUMP_REQUEST_HEADER)) { auto f = open_file_for_write(optarg); - if(f == NULL) { + if(f == nullptr) { return -1; } mod_config()->http2_upstream_dump_request_header = f; - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_DUMP_RESPONSE_HEADER)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_HTTP2_DUMP_RESPONSE_HEADER)) { auto f = open_file_for_write(optarg); - if(f == NULL) { + if(f == nullptr) { return -1; } mod_config()->http2_upstream_dump_response_header = f; - } else if(util::strieq(opt, SHRPX_OPT_HTTP2_NO_COOKIE_CRUMBLING)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_HTTP2_NO_COOKIE_CRUMBLING)) { mod_config()->http2_no_cookie_crumbling = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_FRONTEND_FRAME_DEBUG)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_FRONTEND_FRAME_DEBUG)) { mod_config()->upstream_frame_debug = util::strieq(optarg, "yes"); - } else if(util::strieq(opt, SHRPX_OPT_PADDING)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_PADDING)) { mod_config()->padding = strtoul(optarg, nullptr, 10); - } else if(util::strieq(opt, SHRPX_OPT_ALTSVC)) { + + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_ALTSVC)) { auto tokens = parse_config_str_list(optarg); if(tokens.size() < 2) { @@ -575,7 +813,10 @@ int parse_config(const char *opt, const char *optarg) mod_config()->altsvcs.push_back(std::move(altsvc)); - } else if(util::strieq(opt, SHRPX_OPT_ADD_RESPONSE_HEADER)) { + return 0; + } + + if(util::strieq(opt, SHRPX_OPT_ADD_RESPONSE_HEADER)) { auto p = parse_header(optarg); if(p.first.empty()) { LOG(ERROR) << "add-response-header: header field name is empty: " @@ -583,13 +824,19 @@ int parse_config(const char *opt, const char *optarg) return -1; } mod_config()->add_response_headers.push_back(std::move(p)); - } else if(util::strieq(opt, "conf")) { - LOG(WARNING) << "conf is ignored"; - } else { - LOG(ERROR) << "Unknown option: " << opt; - return -1; + + return 0; } - return 0; + + if(util::strieq(opt, "conf")) { + LOG(WARNING) << "conf is ignored"; + + return 0; + } + + LOG(ERROR) << "Unknown option: " << opt; + + return -1; } int load_config(const char *filename) From e665123ebebeb2942663576c34edf060a92dc3c8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 23:02:03 +0900 Subject: [PATCH 6/7] nghttpx: Use nullptr instead of NULL --- src/shrpx_config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 62d1cbf1..7a47d0a3 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -194,7 +194,7 @@ namespace { FILE* open_file_for_write(const char *filename) { auto f = fopen(filename, "wb"); - if(f == NULL) { + if(f == nullptr) { LOG(ERROR) << "Failed to open " << filename << " for writing. Cause: " << strerror(errno); } From de14c0222772456e87772f1df7ceac3b1c30de4c Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Jun 2014 23:03:26 +0900 Subject: [PATCH 7/7] nghttpx: Code cleanup --- src/shrpx_config.cc | 118 +++++++++++++++++++++++++++++--------------- 1 file changed, 78 insertions(+), 40 deletions(-) diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 7a47d0a3..7af4dca9 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -919,47 +919,85 @@ int int_syslog_facility(const char *strfacility) { if(util::strieq(strfacility, "auth")) { return LOG_AUTH; - } else if(util::strieq(strfacility, "authpriv")) { - return LOG_AUTHPRIV; - } else if(util::strieq(strfacility, "cron")) { - return LOG_CRON; - } else if(util::strieq(strfacility, "daemon")) { - return LOG_DAEMON; - } else if(util::strieq(strfacility, "ftp")) { - return LOG_FTP; - } else if(util::strieq(strfacility, "kern")) { - return LOG_KERN; - } else if(util::strieq(strfacility, "local0")) { - return LOG_LOCAL0; - } else if(util::strieq(strfacility, "local1")) { - return LOG_LOCAL1; - } else if(util::strieq(strfacility, "local2")) { - return LOG_LOCAL2; - } else if(util::strieq(strfacility, "local3")) { - return LOG_LOCAL3; - } else if(util::strieq(strfacility, "local4")) { - return LOG_LOCAL4; - } else if(util::strieq(strfacility, "local5")) { - return LOG_LOCAL5; - } else if(util::strieq(strfacility, "local6")) { - return LOG_LOCAL6; - } else if(util::strieq(strfacility, "local7")) { - return LOG_LOCAL7; - } else if(util::strieq(strfacility, "lpr")) { - return LOG_LPR; - } else if(util::strieq(strfacility, "mail")) { - return LOG_MAIL; - } else if(util::strieq(strfacility, "news")) { - return LOG_NEWS; - } else if(util::strieq(strfacility, "syslog")) { - return LOG_SYSLOG; - } else if(util::strieq(strfacility, "user")) { - return LOG_USER; - } else if(util::strieq(strfacility, "uucp")) { - return LOG_UUCP; - } else { - return -1; } + + if(util::strieq(strfacility, "authpriv")) { + return LOG_AUTHPRIV; + } + + if(util::strieq(strfacility, "cron")) { + return LOG_CRON; + } + + if(util::strieq(strfacility, "daemon")) { + return LOG_DAEMON; + } + + if(util::strieq(strfacility, "ftp")) { + return LOG_FTP; + } + + if(util::strieq(strfacility, "kern")) { + return LOG_KERN; + } + + if(util::strieq(strfacility, "local0")) { + return LOG_LOCAL0; + } + + if(util::strieq(strfacility, "local1")) { + return LOG_LOCAL1; + } + + if(util::strieq(strfacility, "local2")) { + return LOG_LOCAL2; + } + + if(util::strieq(strfacility, "local3")) { + return LOG_LOCAL3; + } + + if(util::strieq(strfacility, "local4")) { + return LOG_LOCAL4; + } + + if(util::strieq(strfacility, "local5")) { + return LOG_LOCAL5; + } + + if(util::strieq(strfacility, "local6")) { + return LOG_LOCAL6; + } + + if(util::strieq(strfacility, "local7")) { + return LOG_LOCAL7; + } + + if(util::strieq(strfacility, "lpr")) { + return LOG_LPR; + } + + if(util::strieq(strfacility, "mail")) { + return LOG_MAIL; + } + + if(util::strieq(strfacility, "news")) { + return LOG_NEWS; + } + + if(util::strieq(strfacility, "syslog")) { + return LOG_SYSLOG; + } + + if(util::strieq(strfacility, "user")) { + return LOG_USER; + } + + if(util::strieq(strfacility, "uucp")) { + return LOG_UUCP; + } + + return -1; } } // namespace shrpx