(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.spi...@gmail.com>
> ---
> 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 ?


> ++
> +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 ?

> +                             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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to