From 38878d474eeb29530e6079d37041ebdeaf4b22bb Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Mon, 16 Dec 2024 12:18:26 +0100 Subject: [PATCH] http: report errors when curl_easy_setopt fails improve macro usage and debug/error printouts Signed-off-by: Hans Zandbelt --- ChangeLog | 4 +- src/http.c | 142 ++++++++++++++++++++++++++--------------------------- 2 files changed, 73 insertions(+), 73 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8570a2d4..ca7a4c84 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,8 @@ +12/16/2024 +- http: report errors when curl_easy_setopt fails and improve macro usage + 12/15/2024 - add Coverity Github action -- http: report errors when curl_easy_setopt fails for CURLOPT_SSL_OPTIONS 12/13/2024 - address warnings from static code analysis tool Coverity diff --git a/src/http.c b/src/http.c index b547c992..3342ab23 100644 --- a/src/http.c +++ b/src/http.c @@ -605,69 +605,66 @@ char *oidc_http_form_encoded_data(request_rec *r, const apr_table_t *params) { return data; } +/* + * call curl_easy_setopt with error checking and reporting + */ +#define OIDC_HTTP_CURL_SETOPT_PARMS(r, curl, code, option, ...) \ + code = curl_easy_setopt(curl, option, __VA_ARGS__); \ + if (code != CURLE_OK) \ + oidc_error(r, "curl_easy_setopt(%s) failed with: %s", #option, curl_easy_strerror(code)) + +#define OIDC_HTTP_CURL_SETOPT(...) OIDC_HTTP_CURL_SETOPT_PARMS(r, curl, code, __VA_ARGS__) + /* * set libcurl SSL options */ -#define OIDC_CURLOPT_SSL_OPTIONS "CURLOPT_SSL_OPTIONS" +#define OIDC_CURLOPT_SSL_OPTIONS_ENV_VAR_NAME "CURLOPT_SSL_OPTIONS" -#define OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, option, key, val) \ - if (_oidc_strstr(env_var_value, option) != NULL) { \ - oidc_debug(r, "curl_easy_setopt (%d) %s (%d)", key, option, val); \ - code = curl_easy_setopt(curl, key, val); \ - if (code != CURLE_OK) \ - oidc_error(r, "curl_easy_setopt for '%s' failed with: %s", option, curl_easy_strerror(code)); \ +#define OIDC_HTTP_CURL_SETOPT_SSL(option, value) \ + if (_oidc_strstr(env_var_value, #value) != NULL) { \ + oidc_debug(r, "curl_easy_setopt(%s): %s (%d)", #option, #value, value); \ + OIDC_HTTP_CURL_SETOPT(option, value); \ } static void oidc_http_set_curl_ssl_options(request_rec *r, CURL *curl) { + // NB: the variable names r, curl, code and env_var_value are used in the OIDC_HTTP_CURL_SETOPT_SSL macro const char *env_var_value = NULL; CURLcode code = CURLE_OK; if (r->subprocess_env != NULL) - env_var_value = apr_table_get(r->subprocess_env, OIDC_CURLOPT_SSL_OPTIONS); + env_var_value = apr_table_get(r->subprocess_env, OIDC_CURLOPT_SSL_OPTIONS_ENV_VAR_NAME); if (env_var_value == NULL) return; - oidc_debug(r, "SSL options environment variable %s=%s found", OIDC_CURLOPT_SSL_OPTIONS, env_var_value); + oidc_debug(r, "SSL options environment variable %s=%s found", OIDC_CURLOPT_SSL_OPTIONS_ENV_VAR_NAME, + env_var_value); #if LIBCURL_VERSION_NUM >= 0x071900 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURLSSLOPT_ALLOW_BEAST", CURLOPT_SSL_OPTIONS, - CURLSSLOPT_ALLOW_BEAST) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSL_OPTIONS, CURLSSLOPT_ALLOW_BEAST); #endif #if LIBCURL_VERSION_NUM >= 0x072c00 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURLSSLOPT_NO_REVOKE", CURLOPT_SSL_OPTIONS, - CURLSSLOPT_NO_REVOKE) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE) #endif #if LIBCURL_VERSION_NUM >= 0x074400 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURLSSLOPT_NO_PARTIALCHAIN", CURLOPT_SSL_OPTIONS, - CURLSSLOPT_NO_PARTIALCHAIN) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_PARTIALCHAIN) #endif #if LIBCURL_VERSION_NUM >= 0x074600 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURLSSLOPT_REVOKE_BEST_EFFORT", CURLOPT_SSL_OPTIONS, - CURLSSLOPT_REVOKE_BEST_EFFORT) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSL_OPTIONS, CURLSSLOPT_REVOKE_BEST_EFFORT) #endif #if LIBCURL_VERSION_NUM >= 0x074700 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURLSSLOPT_NATIVE_CA", CURLOPT_SSL_OPTIONS, - CURLSSLOPT_NATIVE_CA) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSL_OPTIONS, CURLSSLOPT_NATIVE_CA) #endif #if LIBCURL_VERSION_NUM >= 0x072200 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_TLSv1_0", CURLOPT_SSLVERSION, - CURL_SSLVERSION_TLSv1_0) - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_TLSv1_1", CURLOPT_SSLVERSION, - CURL_SSLVERSION_TLSv1_1) - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_TLSv1_2", CURLOPT_SSLVERSION, - CURL_SSLVERSION_TLSv1_2) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_0) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_1) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_2) #endif #if LIBCURL_VERSION_NUM >= 0x073400 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_TLSv1_3", CURLOPT_SSLVERSION, - CURL_SSLVERSION_TLSv1_3) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_3) #endif #if LIBCURL_VERSION_NUM >= 0x073600 - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_MAX_TLSv1_0", CURLOPT_SSLVERSION, - CURL_SSLVERSION_MAX_TLSv1_0) - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_MAX_TLSv1_1", CURLOPT_SSLVERSION, - CURL_SSLVERSION_MAX_TLSv1_1) - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_MAX_TLSv1_2", CURLOPT_SSLVERSION, - CURL_SSLVERSION_MAX_TLSv1_2) - OIDC_HTTP_SET_CURL_OPTION(r, curl, code, env_var_value, "CURL_SSLVERSION_MAX_TLSv1_3", CURLOPT_SSLVERSION, - CURL_SSLVERSION_MAX_TLSv1_3) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_MAX_TLSv1_0) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_MAX_TLSv1_1) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_MAX_TLSv1_2) + OIDC_HTTP_CURL_SETOPT_SSL(CURLOPT_SSLVERSION, CURL_SSLVERSION_MAX_TLSv1_3) #endif } @@ -707,10 +704,12 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char const apr_array_header_t *pass_cookies, const char *ssl_cert, const char *ssl_key, const char *ssl_key_pwd) { + // NB: the variable names r, curl, and code are used in the OIDC_HTTP_CURL_SETOPT macro + CURL *curl = NULL; + CURLcode code = CURLE_OK; char curl_err[CURL_ERROR_SIZE]; oidc_curl_resp_data_ctx_t d_buf = {r, NULL, 0}; oidc_curl_resp_hdr_ctx_t h_buf = {r, response_hdrs}; - CURL *curl = NULL; struct curl_slist *h_list = NULL; int i = 0; CURLcode res = CURLE_OK; @@ -739,43 +738,43 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char curl_err[0] = 0; /* some of these are not really required */ - curl_easy_setopt(curl, CURLOPT_HEADER, 0L); - curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1L); - curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L); - curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, curl_err); - curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); - curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 5L); + OIDC_HTTP_CURL_SETOPT(CURLOPT_HEADER, 0L); + OIDC_HTTP_CURL_SETOPT(CURLOPT_NOPROGRESS, 1L); + OIDC_HTTP_CURL_SETOPT(CURLOPT_NOSIGNAL, 1L); + OIDC_HTTP_CURL_SETOPT(CURLOPT_ERRORBUFFER, curl_err); + OIDC_HTTP_CURL_SETOPT(CURLOPT_FOLLOWLOCATION, 1L); + OIDC_HTTP_CURL_SETOPT(CURLOPT_MAXREDIRS, 5L); /* set the timeouts */ - curl_easy_setopt(curl, CURLOPT_TIMEOUT, http_timeout->request_timeout); - curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, http_timeout->connect_timeout); + OIDC_HTTP_CURL_SETOPT(CURLOPT_TIMEOUT, http_timeout->request_timeout); + OIDC_HTTP_CURL_SETOPT(CURLOPT_CONNECTTIMEOUT, http_timeout->connect_timeout); /* setup the buffer where the response data will be written to */ - curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, oidc_http_response_data); - curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&d_buf); + OIDC_HTTP_CURL_SETOPT(CURLOPT_WRITEFUNCTION, oidc_http_response_data); + OIDC_HTTP_CURL_SETOPT(CURLOPT_WRITEDATA, (void *)&d_buf); /* setup the buffer where the response headers will be written to */ - curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, oidc_http_response_header); - curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&h_buf); + OIDC_HTTP_CURL_SETOPT(CURLOPT_HEADERFUNCTION, oidc_http_response_header); + OIDC_HTTP_CURL_SETOPT(CURLOPT_HEADERDATA, (void *)&h_buf); #ifndef LIBCURL_NO_CURLPROTO #if LIBCURL_VERSION_NUM >= 0x075500 - curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS_STR, "http,https"); - curl_easy_setopt(curl, CURLOPT_PROTOCOLS_STR, "http,https"); + OIDC_HTTP_CURL_SETOPT(CURLOPT_REDIR_PROTOCOLS_STR, "http,https"); + OIDC_HTTP_CURL_SETOPT(CURLOPT_PROTOCOLS_STR, "http,https"); #else - curl_easy_setopt(curl, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); - curl_easy_setopt(curl, CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); + OIDC_HTTP_CURL_SETOPT(CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); + OIDC_HTTP_CURL_SETOPT(CURLOPT_PROTOCOLS, CURLPROTO_HTTP | CURLPROTO_HTTPS); #endif #endif /* set the options for validating the SSL server certificate that the remote site presents */ - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, (ssl_validate_server != FALSE ? 1L : 0L)); - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, (ssl_validate_server != FALSE ? 2L : 0L)); + OIDC_HTTP_CURL_SETOPT(CURLOPT_SSL_VERIFYPEER, (ssl_validate_server != FALSE ? 1L : 0L)); + OIDC_HTTP_CURL_SETOPT(CURLOPT_SSL_VERIFYHOST, (ssl_validate_server != FALSE ? 2L : 0L)); oidc_http_set_curl_ssl_options(r, curl); if (oidc_cfg_ca_bundle_path_get(c) != NULL) - curl_easy_setopt(curl, CURLOPT_CAINFO, oidc_cfg_ca_bundle_path_get(c)); + OIDC_HTTP_CURL_SETOPT(CURLOPT_CAINFO, oidc_cfg_ca_bundle_path_get(c)); #ifdef WIN32 else { @@ -785,7 +784,7 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char retval[0] = '\0'; buflen = SearchPath(NULL, "curl-ca-bundle.crt", NULL, MAX_PATH + 1, retval, &ptr); if (buflen > 0) - curl_easy_setopt(curl, CURLOPT_CAINFO, retval); + OIDC_HTTP_CURL_SETOPT(CURLOPT_CAINFO, retval); else oidc_warn(r, "no curl-ca-bundle.crt file found in path"); free(retval); @@ -796,7 +795,7 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char const char *s_useragent = oidc_http_user_agent(r); if ((s_useragent != NULL) && (_oidc_strcmp(s_useragent, "") != 0)) { oidc_debug(r, "set HTTP request header User-Agent to: %s", s_useragent); - curl_easy_setopt(curl, CURLOPT_USERAGENT, s_useragent); + OIDC_HTTP_CURL_SETOPT(CURLOPT_USERAGENT, s_useragent); } /* set the local interface if defined */ @@ -804,8 +803,7 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char if ((s_interface != NULL) && (_oidc_strcmp(s_interface, "") != 0)) { #if LIBCURL_VERSION_NUM >= 0x073000 oidc_debug(r, "set local interface to: %s", s_interface); - if (curl_easy_setopt(curl, CURLOPT_INTERFACE, s_interface) != CURLE_OK) - oidc_warn(r, "could not set local interface to: %s", s_interface); + OIDC_HTTP_CURL_SETOPT(CURLOPT_INTERFACE, s_interface); #else oidc_warn( r, "local interface is configured to %s, but the cURL version in use does not support setting this", @@ -815,11 +813,11 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char /* set optional outgoing proxy for the local network */ if (outgoing_proxy->host_port) { - curl_easy_setopt(curl, CURLOPT_PROXY, outgoing_proxy->host_port); + OIDC_HTTP_CURL_SETOPT(CURLOPT_PROXY, outgoing_proxy->host_port); if (outgoing_proxy->username_password) - curl_easy_setopt(curl, CURLOPT_PROXYUSERPWD, outgoing_proxy->username_password); + OIDC_HTTP_CURL_SETOPT(CURLOPT_PROXYUSERPWD, outgoing_proxy->username_password); if (outgoing_proxy->auth_type != OIDC_CONFIG_POS_INT_UNSET) - curl_easy_setopt(curl, CURLOPT_PROXYAUTH, outgoing_proxy->auth_type); + OIDC_HTTP_CURL_SETOPT(CURLOPT_PROXYAUTH, outgoing_proxy->auth_type); } /* see if we need to add token in the Bearer/DPoP Authorization header */ @@ -830,22 +828,22 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char /* see if we need to perform HTTP basic authentication to the remote site */ if (basic_auth != NULL) { - curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_BASIC); - curl_easy_setopt(curl, CURLOPT_USERPWD, basic_auth); + OIDC_HTTP_CURL_SETOPT(CURLOPT_HTTPAUTH, CURLAUTH_BASIC); + OIDC_HTTP_CURL_SETOPT(CURLOPT_USERPWD, basic_auth); } if (ssl_cert != NULL) - curl_easy_setopt(curl, CURLOPT_SSLCERT, ssl_cert); + OIDC_HTTP_CURL_SETOPT(CURLOPT_SSLCERT, ssl_cert); if (ssl_key != NULL) - curl_easy_setopt(curl, CURLOPT_SSLKEY, ssl_key); + OIDC_HTTP_CURL_SETOPT(CURLOPT_SSLKEY, ssl_key); if (ssl_key_pwd != NULL) - curl_easy_setopt(curl, CURLOPT_KEYPASSWD, ssl_key_pwd); + OIDC_HTTP_CURL_SETOPT(CURLOPT_KEYPASSWD, ssl_key_pwd); if (data != NULL) { /* set POST data */ - curl_easy_setopt(curl, CURLOPT_POSTFIELDS, data); + OIDC_HTTP_CURL_SETOPT(CURLOPT_POSTFIELDS, data); /* set HTTP method to POST */ - curl_easy_setopt(curl, CURLOPT_POST, 1); + OIDC_HTTP_CURL_SETOPT(CURLOPT_POST, 1); } if (content_type != NULL) { @@ -868,7 +866,7 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char /* see if we need to add any custom headers */ if (h_list != NULL) - curl_easy_setopt(curl, CURLOPT_HTTPHEADER, h_list); + OIDC_HTTP_CURL_SETOPT(CURLOPT_HTTPHEADER, h_list); if (pass_cookies != NULL) { /* gather cookies that we need to pass on from the incoming request */ @@ -887,12 +885,12 @@ static apr_byte_t oidc_http_request(request_rec *r, const char *url, const char /* see if we need to pass any cookies */ if (cookie_string != NULL) { oidc_debug(r, "passing browser cookies on backend call: %s", cookie_string); - curl_easy_setopt(curl, CURLOPT_COOKIE, cookie_string); + OIDC_HTTP_CURL_SETOPT(CURLOPT_COOKIE, cookie_string); } } /* set the target URL */ - curl_easy_setopt(curl, CURLOPT_URL, url); + OIDC_HTTP_CURL_SETOPT(CURLOPT_URL, url); /* call it and record the result */ for (i = 0; i <= http_timeout->retries; i++) {