2015-08-13 18:01 GMT+02:00 Torsten Bögershausen <tboegi@xxxxxx>: > (need to drop Eric from cc-list, no DNS from web.de) > > On 2015-08-13 17.28, Elia Pinto wrote: >> Teach git about a new option, "http.sslVersion", which permits one to >> specify the SSL version to use when negotiating SSL connections. The >> setting can be overridden by the GIT_SSL_VERSION environment >> variable. >> >> Signed-off-by: Elia Pinto <gitter.spiros@xxxxxxxxx> >> --- >> This is the third version of the patch. The changes compared to the previous version are: >> >> - Eliminated the unnecessary blank (Junio) >> - Place a structure to associate mnemonic names with the curl enum constant (Junio) >> - Eliminated the invocation to curl_easy_setopt to set the default SSL value. Also removed the static global variable. >> (Junio) >> - Slight correction in config.txt (Eric) >> >> Documentation/config.txt | 22 ++++++++++++++++++++++ >> contrib/completion/git-completion.bash | 1 + >> http.c | 32 +++++++++++++++++++++++++++++++- >> 3 files changed, 54 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> index 315f271..b23b01a 100644 >> --- a/Documentation/config.txt >> +++ b/Documentation/config.txt >> @@ -1595,6 +1595,28 @@ http.saveCookies:: >> If set, store cookies received during requests to the file specified by >> http.cookieFile. Has no effect if http.cookieFile is unset. >> >> +http.sslVersion:: > should this be https.sslVersion ? > (http doesn't use ssl) > >> + The SSL version to use when negotiating an SSL connection, if you >> + want to force the default. The available and default version depend on >> + whether libcurl was built against NSS or OpenSSL and the particular configuration >> + of the crypto library in use. Internally this sets the 'CURLOPT_SSL_VERSION' >> + option; see the libcurl documentation for more details on the format >> + of this option and for the ssl version supported. Actually the possible values >> + of this option are: >> + >> + - sslv2 >> + - sslv3 >> + - tlsv1 >> + - tlsv1.0 >> + - tlsv1.1 >> + - tlsv1.2 >> + > from > https://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_1.0.2C_2.0_and_3.0 > sslv2 and sslv3 are deprecated. > Should there be a motivation in the commit message why we want to support them ? They are those provided by the documentation (TLS in particular). We let the underlying library to say what is deprecated or not. In this case the call fail. > > >> ++ >> +Can be overridden by the 'GIT_SSL_VERSION' environment variable. >> +To force git to use libcurl's default ssl version and ignore any >> +explicit http.sslversion option, set 'GIT_SSL_VERSION' to the >> +empty string. >> + >> http.sslCipherList:: >> A list of SSL ciphers to use when negotiating an SSL connection. >> The available ciphers depend on whether libcurl was built against >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash >> index c97c648..6e9359c 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -2118,6 +2118,7 @@ _git_config () >> http.postBuffer >> http.proxy >> http.sslCipherList >> + http.sslVersion >> http.sslCAInfo >> http.sslCAPath >> http.sslCert >> diff --git a/http.c b/http.c >> index e9c6fdd..d5fecd6 100644 >> --- a/http.c >> +++ b/http.c >> @@ -37,6 +37,21 @@ static int curl_ssl_verify = -1; >> static int curl_ssl_try; >> static const char *ssl_cert; >> static const char *ssl_cipherlist; >> +static const char *ssl_version; >> +static struct { >> + const char *name; >> + long ssl_version; >> + } sslversions[] = { >> + { "sslv2", CURL_SSLVERSION_SSLv2 }, >> + { "sslv3", CURL_SSLVERSION_TLSv1 }, >> + { "tlsv1", CURL_SSLVERSION_TLSv1 }, >> +#if LIBCURL_VERSION_NUM >= 0x072200 >> + { "tlsv1.0", CURL_SSLVERSION_TLSv1_0 }, >> + { "tlsv1.1", CURL_SSLVERSION_TLSv1_1 }, >> + { "tlsv1.2", CURL_SSLVERSION_TLSv1_2 }, >> +#endif >> + { NULL } >> +}; >> #if LIBCURL_VERSION_NUM >= 0x070903 >> static const char *ssl_key; >> #endif >> @@ -190,6 +205,8 @@ static int http_options(const char *var, const char *value, void *cb) >> } >> if (!strcmp("http.sslcipherlist", var)) >> return git_config_string(&ssl_cipherlist, var, value); >> + if (!strcmp("http.sslversion", var)) >> + return git_config_string(&ssl_version, var, value); >> if (!strcmp("http.sslcert", var)) >> return git_config_string(&ssl_cert, var, value); >> #if LIBCURL_VERSION_NUM >= 0x070903 >> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void) >> if (http_proactive_auth) >> init_curl_http_auth(result); >> >> + if (getenv("GIT_SSL_VERSION")) >> + ssl_version = getenv("GIT_SSL_VERSION"); >> + > Minor nit to shorten: > if (ssl_version && *ssl_version) { > >> + int i; >> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) { > I think Git-style is not to have ' ' before/after ')' /'(' > for (i = 0; i < ARRAY_SIZE(sslversions); i++) > >> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) { >> + curl_easy_setopt(result, CURLOPT_SSLVERSION, >> + sslversions[i].ssl_version); > This is what my man page says: > CURLcode curl_easy_setopt(CURL *handle, CURLoption option, parameter); > [] > > RETURN VALUE > CURLE_OK (zero) means that the option was set properly... > Should the return value checked (and we die() if we fail ? It is not strictly necessary. If it fails the other curl call fail, try to use sslv2 for example (libcurl deprecated nss dunno) Thanks ! > >> + break; >> + } >> + if ( i == ARRAY_SIZE(sslversions) ) warning("unsupported ssl version %s: using default", >> + ssl_version); > Should we die() here to make things very clear to the user ? > >> + } >> + >> if (getenv("GIT_SSL_CIPHER_LIST")) >> ssl_cipherlist = getenv("GIT_SSL_CIPHER_LIST"); >> - >> if (ssl_cipherlist != NULL && *ssl_cipherlist) >> curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST, >> ssl_cipherlist); >> > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html