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

From: Lars Kellogg-Stedman <lars@redhat.com>
Date: May 7, 2015, 16:04 UTC
Message-ID: <20150507160413.GB16334@redhat.com>
In-Reply-To: <xmqq8ud0s7sv.fsf@gitster.dls.corp.google.com>

[Apologies for the dupe; this should have been cc'd to the list]

> It is not clear to me what definition of "override" this sentence
> uses.

I was using it in what I thought was the common sense of "git will use the value in the environment variable if it exists rather than any value in the git configuration". I apologize if this wasn't clear; can you suggest how I might rephrase that?

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

So, with the current version of the patch there isn't an easy way to say, "use the defaults instead of what is in my git configuration". Setting GIT_SSL_CIPHER_LIST to an empty string would simply disable SSL.

I'll submit a new version of the patch that treats an emtpy cipher list as meaning, "do not explicitly set CURLOPT_SSL_CIPHER_LIST".

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

Yeah, I looked briefly through the tests but I didn't see any existing SSL tests and wasn't sure where to start. I'm open to suggestions on this front.

-- 
Lars Kellogg-Stedman <lars@redhat.com> | larsks @ {freenode,twitter,github}
Cloud Engineering / OpenStack          | http://blog.oddbit.com/