f87aaedc9bcdbda819ff0e01e942b5e74c477829 galt Thu Feb 3 02:12:18 2022 -0800 Fixing multi-threading issues by removing setenv usage in https.c and common.c. refs #28844 diff --git src/lib/https.c src/lib/https.c index 96836db..37e17dc 100644 --- src/lib/https.c +++ src/lib/https.c @@ -5,30 +5,34 @@ #include "openssl/ssl.h" #include "openssl/err.h" #include <sys/socket.h> #include <unistd.h> #include <pthread.h> #include <signal.h> #include "common.h" #include "internet.h" #include "errAbort.h" #include "hash.h" #include "net.h" +char *https_cert_check = NULL; +char *https_cert_check_depth = NULL; +char *https_cert_check_verbose = NULL; +char *https_cert_check_domain_exceptions = NULL; // For use with callback. Set a variable into the connection itself, // and then use that during the callback. struct myData { char *hostName; }; int myDataIndex = -1; static pthread_mutex_t *mutexes = NULL; static unsigned long openssl_id_callback(void) { return ((unsigned long)pthread_self()); @@ -62,38 +66,54 @@ BIO *sbio; // ssl bio }; static void xerrno(char *msg) { fprintf(stderr, "%s : %s\n", strerror(errno), msg); fflush(stderr); } static void xerr(char *msg) { fprintf(stderr, "%s\n", msg); fflush(stderr); } void initDomainWhiteListHash(); // forward declaration +char *mySetenv(char *setting, char *defaultValue) +/* avoid real setenv which causes problems in multi-threaded programs */ +{ +char *thisSetting = getenv(setting); +if (thisSetting) + return cloneString(thisSetting); +else + return cloneString(defaultValue); +} + void openSslInit() /* do only once */ { static boolean done = FALSE; static pthread_mutex_t osiMutex = PTHREAD_MUTEX_INITIALIZER; pthread_mutex_lock( &osiMutex ); if (!done) { + // setenv here for thread-safety + https_cert_check = mySetenv("https_cert_check", "log"); // DEFAULT certificate check is log. + https_cert_check_depth = mySetenv("https_cert_check_depth", "9"); // DEFAULT depth check level is 9. + https_cert_check_verbose = mySetenv("https_cert_check_verbose", "off"); // DEFAULT verbose is off. + https_cert_check_domain_exceptions = mySetenv("https_cert_check_domain_exceptions", ""); // DEFAULT space separated list is empty string. + SSL_library_init(); ERR_load_crypto_strings(); ERR_load_SSL_strings(); OpenSSL_add_all_algorithms(); openssl_pthread_setup(); myDataIndex = SSL_get_ex_new_index(0, "myDataIndex", NULL, NULL, NULL); initDomainWhiteListHash(); done = TRUE; } pthread_mutex_unlock( &osiMutex ); } void *netConnectHttpsThread(void *threadParam) /* use a thread to run socket back to user */ @@ -301,81 +321,81 @@ /* * Catch a too long certificate chain. The depth limit set using * SSL_CTX_set_verify_depth() is by purpose set to "limit+1" so * that whenever the "depth>verify_depth" condition is met, we * have violated the limit and want to log this error condition. * We must do it here, because the CHAIN_TOO_LONG error would not * be found explicitly; only errors introduced by cutting off the * additional certificates would be logged. */ ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); myData = SSL_get_ex_data(ssl, myDataIndex); -if (depth > atoi(getenv("https_cert_check_depth"))) +if (depth > atoi(https_cert_check_depth)) { preverify_ok = 0; err = X509_V_ERR_CERT_CHAIN_TOO_LONG; X509_STORE_CTX_set_error(ctx, err); } -if (sameString(getenv("https_cert_check_verbose"), "on")) +if (sameString(https_cert_check_verbose, "on")) { fprintf(stderr,"depth=%d:%s\n", depth, buf); } if (!preverify_ok) { if (getenv("SCRIPT_NAME")) // CGI mode { fprintf(stderr, "verify error:num=%d:%s:depth=%d:%s hostName=%s CGI=%s\n", err, X509_verify_cert_error_string(err), depth, buf, myData->hostName, getenv("SCRIPT_NAME")); } - if (!sameString(getenv("https_cert_check"), "log")) + if (!sameString(https_cert_check, "log")) { char *cn = strstr(buf, "/CN="); if (cn) cn+=4; // strlen /CN= if (sameString(cn, myData->hostName)) warn("%s on %s", X509_verify_cert_error_string(err), cn); else warn("%s on %s (%s)", X509_verify_cert_error_string(err), cn, myData->hostName); } } /* err contains the last verification error. */ if (!preverify_ok && (err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT)) { X509_NAME_oneline(X509_get_issuer_name(cert), buf, 256); fprintf(stderr, "issuer= %s\n", buf); } -if (sameString(getenv("https_cert_check"), "warn") || sameString(getenv("https_cert_check"), "log")) +if (sameString(https_cert_check, "warn") || sameString(https_cert_check, "log")) return 1; else return preverify_ok; } struct hash *domainWhiteList = NULL; void initDomainWhiteListHash() /* Initialize once, has all the old existing domains * for which cert checking is skipped since they are not compatible (yet) with openssl.*/ { domainWhiteList = hashNew(8); // whitelisted domain exceptions set in hg.conf // space separated list. -char *dmwl = cloneString(getenv("https_cert_check_domain_exceptions")); +char *dmwl = cloneString(https_cert_check_domain_exceptions); int wordCount = chopByWhite(dmwl, NULL, 0); if (wordCount > 0) { char **words; AllocArray(words, wordCount); chopByWhite(dmwl, words, wordCount); int w; for(w=0; w < wordCount; w++) { hashStoreName(domainWhiteList, words[w]); } freeMem(words); } freez(&dmwl); @@ -498,94 +518,86 @@ safef(wildHost, sizeof wildHost, "*%s", dot); result = hashLookup(domainWhiteList, wildHost); } } return result; } int netConnectHttps(char *hostName, int port, boolean noProxy) /* Return socket for https connection with server or -1 if error. */ { int fd=0; // https_cert_check env var can be abort warn or none. -setenv("https_cert_check", "log", 0); // DEFAULT certificate check is log. - -setenv("https_cert_check_depth", "9", 0); // DEFAULT depth check level is 9. - -setenv("https_cert_check_verbose", "off", 0); // DEFAULT verbose is off. - -setenv("https_cert_check_domain_exceptions", "", 0); // DEFAULT space separated list is empty string. - char *proxyUrl = getenv("https_proxy"); if (noProxy) proxyUrl = NULL; char *connectHost; int connectPort; BIO *fbio=NULL; // file descriptor bio BIO *sbio=NULL; // ssl bio SSL_CTX *ctx; SSL *ssl; openSslInit(); ctx = SSL_CTX_new(SSLv23_client_method()); fd_set readfds; fd_set writefds; int err; struct timeval tv; struct myData myData; boolean doSetMyData = FALSE; -if (!sameString(getenv("https_cert_check"), "none")) +if (!sameString(https_cert_check, "none")) { if (checkIfInHashWithWildCard(hostName)) { // old existing domains which are not (yet) compatible with openssl. if (getenv("SCRIPT_NAME")) // CGI mode { fprintf(stderr, "domain %s cert check skipped because it is white-listed as an exception.\n", hostName); } } else { // verify peer cert of the server. // Set TRUSTED_FIRST for openssl 1.0 // Fixes common issue openssl 1.0 had with with LetsEncrypt certs in the Fall of 2021. X509_STORE_set_flags(SSL_CTX_get_cert_store(ctx), X509_V_FLAG_TRUSTED_FIRST); // This flag causes intermediate certificates in the trust store to be treated as trust-anchors, in the same way as the self-signed root CA certificates. // This makes it possible to trust certificates issued by an intermediate CA without having to trust its ancestor root CA. // GNU-TLS uses it, and openssl probably will do it in the future. // Currently this does not fix any of our known issues with users servers certs. // X509_STORE_set_flags(SSL_CTX_get_cert_store(ctx), X509_V_FLAG_PARTIAL_CHAIN); // verify_callback gets called once per certificate returned by the server. SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, verify_callback); /* * Let the verify_callback catch the verify_depth error so that we get * an appropriate error in the logfile. */ - SSL_CTX_set_verify_depth(ctx, atoi(getenv("https_cert_check_depth")) + 1); + SSL_CTX_set_verify_depth(ctx, atoi(https_cert_check_depth) + 1); // VITAL FOR PROPER VERIFICATION OF CERTS if (!SSL_CTX_set_default_verify_paths(ctx)) { warn("SSL set default verify paths failed"); } // add the hostName to the structure and set it here, making it available during callback. myData.hostName = hostName; doSetMyData = TRUE; } } // Don't want any retries since we are non-blocking bio now