From 313121a229cab8ae019d87c535f1fd0c479cde10 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Fri, 10 Apr 2020 13:42:27 -0600 Subject: [PATCH] Use proper CURLOPT values for VERIFYHOST and VERIFYPEER re: https://github.com/Unidata/netcdf-c/issues/1684 re: e-support VZL-904142 Two issues: 1. As of libcurl 7.66, the semantics of CURLOPT_SSL_VERIFYHOST changed so that the non-zero values affects certificate processing. 2. The current library was forcing the values of VERIFYPEER and VERIFYHOST to zero instead of leaving them to the default values. Solution was first to leave the defaults in place for VERIFYPEER and VERIFYHOST as long as they are not set in .ocrc/.dodsrc file. Second, the value of HTTP.SSL.VERIFYPEER or HTTP.SSL.VERIFYHOST as set in .ocrc/.dodrc is used to set the corresponding CURLOPT flags. So for example, adding > HTTP.SSL.VERIFYHOST=2 will set the value of CURLOPT_SSL_VERIFYHOST to 2, the default. Using > HTTP.SSL.VERIFYHOST=0 will set the value of CURLOPT_SSL_VERIFYHOST to 0, which disables it. Similarly for VERIFYPEER. Finally the semantics of HTTP.SSL.VALIDATE is now equivalent to > HTTP.SSL.VERIFYPEER=1 > HTTP.SSL.VERIFYHOST=2 --- CMakeLists.txt | 7 ++++++ NUG/DAP2.dox | 10 +++++++- NUG/DAP4.dox | 10 ++++++++ RELEASE_NOTES.md | 1 + config.h.cmake.in | 3 +++ configure.ac | 14 ++++++++++++ libdap4/d4curlfunctions.c | 11 +++++++-- libdispatch/dauth.c | 48 +++++++++++++++++++++------------------ oc2/occurlfunctions.c | 14 ++++++++++-- 9 files changed, 91 insertions(+), 27 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cdd7ea5d1..f99d5984e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -836,6 +836,13 @@ CHECK_C_SOURCE_COMPILES(" #include int main() {int x = CURLOPT_TCP_KEEPALIVE;}" HAVE_CURLOPT_KEEPALIVE) +# Check to see if we have libcurl 7.66 or later +CHECK_C_SOURCE_COMPILES(" +#include +#if (LIBCURL_VERSION_MAJOR*1000 + LIBCURL_VERSION_MINOR >= 7066) + choke me +#endif" HAVE_LIBCURL_766) + # Option to Build DAP2+DAP4 Clients OPTION(ENABLE_DAP "Enable DAP2 and DAP4 Client." ON) IF(ENABLE_DAP) diff --git a/NUG/DAP2.dox b/NUG/DAP2.dox index b6b8b973f..036b87b81 100644 --- a/NUG/DAP2.dox +++ b/NUG/DAP2.dox @@ -619,9 +619,17 @@ follows. Type: String representing directory Description: Path to a directory containing trusted certificates for validating server certificates. Related CURL Flags: CURLOPT_CAPATH +1. HTTP.SSL.VERIFYPEER + Type: integer + Description: Set certificate checking on the server. + Related CURL Flags: CURLOPT_SSL_VERIFYHOST +1. HTTP.SSL.VERIFYPEER + Type: integer + Description: Set host validation for the server. + Related CURL Flags: CURLOPT_SSL_VERIFYPEER 1. HTTP.SSL.VALIDATE Type: boolean ("1"/"0") - Description: Cause the client to verify the server's presented certificate. + Description: Alias for VERIFYPEER=1 and VERIFYHOST=2 Related CURL Flags: CURLOPT_SSL_VERIFYPEER, CURLOPT_SSL_VERIFYHOST 1. HTTP.TIMEOUT Type: String ("dddddd") diff --git a/NUG/DAP4.dox b/NUG/DAP4.dox index cc770e9d6..ea9a7d487 100644 --- a/NUG/DAP4.dox +++ b/NUG/DAP4.dox @@ -198,6 +198,16 @@ follows. Description: Path to a directory containing trusted certificates for validating server certificates. Related CURL Flags: CURLOPT_CAPATH +-# HTTP.SSL.VERIFYPEER + Type: integer + Description: Set certificate checking on the server. + Related CURL Flags: CURLOPT_SSL_VERIFYHOST + +-# HTTP.SSL.VERIFYPEER + Type: integer + Description: Set host validation for the server. + Related CURL Flags: CURLOPT_SSL_VERIFYPEER + -# HTTP.SSL.VALIDATE Type: boolean ("1"/"0") Description: Cause the client to verify the server's presented certificate. diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index e6c741156..ff448d20b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release ## 4.8.0 - TBD +* [Bug Fix] Use proper CURLOPT values for VERIFYHOST and VERIFYPEER; the semantics for VERIFYHOST in particular changed. Documented in NUG/DAP2.md. See [https://github.com/Unidata/netcdf-c/issues/1684]. * [Bug Fix][cmake] Correct an issue with parallel filter test logic in CMake-based builds. * [Bug Fix] Now allow nc_inq_var_deflate()/nc_inq_var_szip() to be called for all formats, not just HDF5. Non-HDF5 files return NC_NOERR and report no compression in use. This reverts behavior that was changed in the 4.7.4 release. See [https://github.com/Unidata/netcdf-c/issues/1691]. * [Bug Fix] Compiling on a big-endian machine exposes some missing forward delcarations in dfilter.c. diff --git a/config.h.cmake.in b/config.h.cmake.in index bbeb7a346..55007d46b 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -178,6 +178,9 @@ are set when opening a binary file on Windows. */ /* Is CURLOPT_USERNAME defined */ #cmakedefine HAVE_CURLOPT_USERNAME 1 +/* Is LIBCURL version >= 7.66 */ +#cmakedefine HAVE_LIBCURL_766 1 + /* Define to 1 if you have the declaration of `isfinite', and to 0 if you don't. */ #cmakedefine HAVE_DECL_ISFINITE 1 diff --git a/configure.ac b/configure.ac index 82781217d..fddc67beb 100644 --- a/configure.ac +++ b/configure.ac @@ -750,6 +750,20 @@ AC_MSG_RESULT([${havecurloption}]) if test $havecurloption = yes; then AC_DEFINE([HAVE_CURLOPT_KEEPALIVE],[1],[Is CURLOPT_TCP_KEEPALIVE defined]) fi +# CURLOPT_VERIFYHOST semantics differ depending on version +AC_MSG_CHECKING([whether libcurl is version 7.66 or later?]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM( +[#include "curl/curl.h"], +[[ +#if LIBCURL_VERSION_NUM < 0x074200 +error "<7.66"; +#endif +]])], [libcurl766=yes], [libcurl766=no]) + +AC_MSG_RESULT([$libcurl766]) +if test x$libcurl66 = xno; then + AC_DEFINE([HAVE_LIBCURL_766],[1],[Is libcurl version 7.66 or later]) +fi CFLAGS="$SAVECFLAGS" diff --git a/libdap4/d4curlfunctions.c b/libdap4/d4curlfunctions.c index 386d0762c..948153d11 100644 --- a/libdap4/d4curlfunctions.c +++ b/libdap4/d4curlfunctions.c @@ -133,8 +133,15 @@ set_curlflag(NCD4INFO* state, int flag) case CURLOPT_SSL_VERIFYPEER: case CURLOPT_SSL_VERIFYHOST: { struct ssl* ssl = &state->auth.ssl; - CHECK(state, CURLOPT_SSL_VERIFYPEER, (OPTARG)(ssl->verifypeer?1L:0L)); - CHECK(state, CURLOPT_SSL_VERIFYHOST, (OPTARG)(ssl->verifyhost?1L:0L)); + /* VERIFYPEER == 0 => VERIFYHOST == 0 */ + /* We need to have 2 states: default and a set value */ + /* So -1 => default, >= 0 => use value; */ + if(ssl->verifypeer >= 0) + CHECK(state, CURLOPT_SSL_VERIFYPEER, (OPTARG)(ssl->verifypeer)); +#ifdef HAVE_LIBCURL_766 + if(ssl->verifyhost >= 0) + CHECK(state, CURLOPT_SSL_VERIFYHOST, (OPTARG)(ssl->verifyhost)); +#endif if(ssl->certificate) CHECK(state, CURLOPT_SSLCERT, ssl->certificate); if(ssl->key) diff --git a/libdispatch/dauth.c b/libdispatch/dauth.c index f49dbd529..94f208c45 100644 --- a/libdispatch/dauth.c +++ b/libdispatch/dauth.c @@ -30,6 +30,8 @@ See COPYRIGHT for license information. /* Define the curl flag defaults in envv style */ static const char* AUTHDEFAULTS[] = { +"HTTP.SSL.VERIFYPEER","-1", /* Use default */ +"HTTP.SSL.VERIFYHOST","-1", /* Use default */ "HTTP.TIMEOUT","1800", /*seconds */ /* Long but not infinite */ "HTTP.CONNECTTIMEOUT","50", /*seconds */ /* Long but not infinite */ NULL @@ -124,8 +126,6 @@ NC_authsetup(NCauth* auth, NCURI* uri) NC_rclookup("HTTP.PROXY.SERVER",uri_hostport)); setauthfield(auth,"HTTP.PROXY_SERVER", NC_rclookup("HTTP.PROXY_SERVER",uri_hostport)); - setauthfield(auth,"HTTP.SSL.VALIDATE", - NC_rclookup("HTTP.SSL.VALIDATE",uri_hostport)); setauthfield(auth,"HTTP.SSL.CERTIFICATE", NC_rclookup("HTTP.SSL.CERTIFICATE",uri_hostport)); setauthfield(auth,"HTTP.SSL.KEY", @@ -138,6 +138,11 @@ NC_authsetup(NCauth* auth, NCURI* uri) NC_rclookup("HTTP.SSL.CAPATH",uri_hostport)); setauthfield(auth,"HTTP.SSL.VERIFYPEER", NC_rclookup("HTTP.SSL.VERIFYPEER",uri_hostport)); + setauthfield(auth,"HTTP.SSL.VERIFYHOST", + NC_rclookup("HTTP.SSL.VERIFYHOST",uri_hostport)); + /* Alias for VERIFYHOST + VERIFYPEER */ + setauthfield(auth,"HTTP.SSL.VALIDATE", + NC_rclookup("HTTP.SSL.VALIDATE",uri_hostport)); setauthfield(auth,"HTTP.NETRC", NC_rclookup("HTTP.NETRC",uri_hostport)); @@ -255,13 +260,28 @@ setauthfield(NCauth* auth, const char* flag, const char* value) nclog(NCLOGNOTE,"HTTP.PROXY.SERVER: %s", value); #endif } + if(strcmp(flag,"HTTP.SSL.VERIFYPEER")==0) { + int v; + if((v = atol(value))) { + auth->ssl.verifypeer = v; +#ifdef D4DEBUG + nclog(NCLOGNOTE,"HTTP.SSL.VERIFYPEER: %d", v); +#endif + } + } + if(strcmp(flag,"HTTP.SSL.VERIFYHOST")==0) { + int v; + if((v = atol(value))) { + auth->ssl.verifyhost = v; +#ifdef D4DEBUG + nclog(NCLOGNOTE,"HTTP.SSL.VERIFYHOST: %d", v); +#endif + } + } if(strcmp(flag,"HTTP.SSL.VALIDATE")==0) { if(atoi(value)) { auth->ssl.verifypeer = 1; - auth->ssl.verifyhost = 1; -#ifdef D4DEBUG - nclog(NCLOGNOTE,"HTTP.SSL.VALIDATE: %ld", 1); -#endif + auth->ssl.verifyhost = 2; } } @@ -309,22 +329,6 @@ setauthfield(NCauth* auth, const char* flag, const char* value) nclog(NCLOGNOTE,"HTTP.SSL.CAPATH: %s", auth->ssl.capath); #endif } - - if(strcmp(flag,"HTTP.SSL.VERIFYPEER")==0) { - const char* s = value; - int tf = 0; - if(s == NULL || strcmp(s,"0")==0 || strcasecmp(s,"false")==0) - tf = 0; - else if(strcmp(s,"1")==0 || strcasecmp(s,"true")==0) - tf = 1; - else - tf = 1; /* default if not null */ - auth->ssl.verifypeer = tf; -#ifdef D4DEBUG - nclog(NCLOGNOTE,"HTTP.SSL.VERIFYPEER: %d", auth->ssl.verifypeer); -#endif - } - if(strcmp(flag,"HTTP.NETRC")==0) { nullfree(auth->curlflags.netrc); auth->curlflags.netrc = strdup(value); diff --git a/oc2/occurlfunctions.c b/oc2/occurlfunctions.c index ab5d95e94..3fb3c603e 100644 --- a/oc2/occurlfunctions.c +++ b/oc2/occurlfunctions.c @@ -131,8 +131,17 @@ ocset_curlflag(OCstate* state, int flag) case CURLOPT_SSL_VERIFYPEER: case CURLOPT_SSL_VERIFYHOST: { struct ssl* ssl = &state->auth.ssl; - CHECK(state, CURLOPT_SSL_VERIFYPEER, (OPTARG)(ssl->verifypeer?1L:0L)); - CHECK(state, CURLOPT_SSL_VERIFYHOST, (OPTARG)(ssl->verifyhost?1L:0L)); + /* VERIFYPEER == 0 => VERIFYHOST == 0 */ + /* We need to have 2 states: default and a set value */ + /* So -1 => default >= 0 => use value */ + if(ssl->verifypeer >= 0) { + CHECK(state, CURLOPT_SSL_VERIFYPEER, (OPTARG)(ssl->verifypeer)); + } +#ifdef HAVE_LIBCURL_766 + if(ssl->verifyhost >= 0) { + CHECK(state, CURLOPT_SSL_VERIFYHOST, (OPTARG)(ssl->verifyhost)); + } +#endif if(ssl->certificate) CHECK(state, CURLOPT_SSLCERT, ssl->certificate); if(ssl->key) @@ -213,6 +222,7 @@ ocset_flags_perlink(OCstate* state) if(stat == NC_NOERR && state->curlkeepalive.active != 0) stat = ocset_curlflag(state, CURLOPT_TCP_KEEPALIVE); #endif + return stat; }