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

From: Junio C Hamano <gitster@pobox.com>
Date: May 7, 2015, 15:53 UTC
Message-ID: <xmqq8ud0s7sv.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1431008210-673-1-git-send-email-lars@redhat.com>

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

Show 38 quoted lines

> 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.

Show 36 quoted lines

> 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())