Re: [PATCH] add support for specifying an SSL cipher list

[Junio C Hamano <gitster@pobox.com>]

Lars Kellogg-Stedman <lars@redhat.com> writes:

> Teach git about a new option, "http.sslCipherList", which permits one to
> specify a list of ciphers to use when negotiating SSL connections.  The
> setting can be overwridden by the GIT_SSL_CIPHER_LIST environment
> variable.
>
> Signed-off-by: Lars Kellogg-Stedman <lars@redhat.com>
> ---
>
> I was recently helping someone diagnose the following error when
> trying to clone a remote repository:
>
>   fatal: unable to access 'https://example.org/': Cannot communicate
>   securely with peer: no common encryption algorithm(s).
>
> This happens when the remote server and the default libcurl
> configuration do not share any ciphers in common.  In this particular
> case the solution was to add 'ecdhe_ecdsa_aes_128_gcm_sha_256' to the
> list of ciphers via CURLOPT_SSL_CIPHER_LIST.  This patch permits one
> to make such a configuration change in git.
>
>  Documentation/config.txt |  7 +++++++
>  http.c                   | 11 +++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 2e5ceaf..b17985c 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1560,6 +1560,13 @@ 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.sslCipherList::
> +  A list of SSL ciphers to use when negotiating an SSL connection.
> +  The available ciphers depend on whether libcurl was built against
> +  NSS or OpenSSL and the particular configuration of the crypto
> +  library in use.  Can be overwridden by the 'GIT_SSL_CIPHER_LIST'
> +  environment variable.

It is not clear to me what definition of "override" this sentence
uses.  If you set something to this configuration variable, and if
you want to revert the list back to whatever cURL uses by default,
what exact value should I set GIT_SSL_CIPHER_LIST to?  Do I have to
find out the list of cipher suites cURL uses by default from the doc
and list them all in the correct order, or can I merely set it to an
empty string, i.e.

	$ GIT_SSL_CIPHER_LIST= git fetch ...

or what?

I also wonder if this feature is something we would want a test or
two to protect against future changes accidentally breaking it, but
I do not offhand know how hard it would be to come up with a
reasonable test.

Thanks.

> diff --git a/http.c b/http.c
> index 4b179f6..8077f8d 100644
> --- a/http.c
> +++ b/http.c
> @@ -36,6 +36,7 @@ char curl_errorstr[CURL_ERROR_SIZE];
>  static int curl_ssl_verify = -1;
>  static int curl_ssl_try;
>  static const char *ssl_cert;
> +static const char *ssl_cipherlist;
>  #if LIBCURL_VERSION_NUM >= 0x070903
>  static const char *ssl_key;
>  #endif
> @@ -187,6 +188,9 @@ static int http_options(const char *var, const char *value, void *cb)
>  		curl_ssl_verify = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp("http.sslcipherlist", var)) {
> +		return git_config_string(&ssl_cipherlist, var, value);
> +	}
>  	if (!strcmp("http.sslcert", var))
>  		return git_config_string(&ssl_cert, var, value);
>  #if LIBCURL_VERSION_NUM >= 0x070903
> @@ -361,6 +365,13 @@ static CURL *get_curl_handle(void)
>  	if (http_proactive_auth)
>  		init_curl_http_auth(result);
>  
> +	if (getenv("GIT_SSL_CIPHER_LIST"))
> +		ssl_cipherlist = getenv("GIT_SSL_CIPHER_LIST");
> +
> +	if (ssl_cipherlist != NULL)
> +		curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,
> +				ssl_cipherlist);
> +
>  	if (ssl_cert != NULL)
>  		curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
>  	if (has_cert_password())