{"thread":{"id":"40080","subject":"[PATCH v3] http: add support for specifying the SSL version","startedAt":"2015-08-13T15:28:51Z","lastAt":"2015-08-14T19:51:29Z","messageCount":13,"participants":["Elia Pinto","Eric Sunshine","Torsten Bögershausen","Ilari Liusvaara","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"267984","messageId":"1439479731-16018-1-git-send-email-gitter.spiros@gmail.com","threadId":"40080","inReplyTo":null,"subject":"[PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-13T15:28:51Z","receivedAt":"2015-08-13T15:28:51Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Teach git about a new option, \"http.sslVersion\", which permits one to\nspecify the SSL version to use when negotiating SSL connections. The\nsetting can be overridden by the GIT_SSL_VERSION environment\nvariable.\n\nSigned-off-by: Elia Pinto \n---\nThis is the third version of the patch. The changes compared to the previous version are:\n\n- Eliminated the unnecessary blank (Junio)\n- Place a structure to associate mnemonic names with the curl enum constant (Junio)\n- Eliminated the invocation to curl_easy_setopt to set the default SSL value. Also removed the static global variable.\n (Junio)\n- Slight correction in config.txt (Eric)\n\n Documentation/config.txt | 22 ++++++++++++++++++++++\n contrib/completion/git-completion.bash | 1 +\n http.c | 32 +++++++++++++++++++++++++++++++-\n 3 files changed, 54 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 315f271..b23b01a 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1595,6 +1595,28 @@ http.saveCookies::\n \tIf set, store cookies received during requests to the file specified by\n \thttp.cookieFile. Has no effect if http.cookieFile is unset.\n \n+http.sslVersion::\n+\tThe SSL version to use when negotiating an SSL connection, if you\n+\twant to force the default. The available and default version depend on\n+\twhether libcurl was built against NSS or OpenSSL and the particular configuration\n+\tof the crypto library in use. Internally this sets the 'CURLOPT_SSL_VERSION'\n+\toption; see the libcurl documentation for more details on the format\n+\tof this option and for the ssl version supported. Actually the possible values\n+\tof this option are:\n+\n+\t- sslv2\n+\t- sslv3\n+\t- tlsv1\n+\t- tlsv1.0\n+\t- tlsv1.1\n+\t- tlsv1.2\n+\n++\n+Can be overridden by the 'GIT_SSL_VERSION' environment variable.\n+To force git to use libcurl's default ssl version and ignore any\n+explicit http.sslversion option, set 'GIT_SSL_VERSION' to the\n+empty string.\n+\n http.sslCipherList::\n A list of SSL ciphers to use when negotiating an SSL connection.\n The available ciphers depend on whether libcurl was built against\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex c97c648..6e9359c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2118,6 +2118,7 @@ _git_config ()\n \t\thttp.postBuffer\n \t\thttp.proxy\n \t\thttp.sslCipherList\n+\t\thttp.sslVersion\n \t\thttp.sslCAInfo\n \t\thttp.sslCAPath\n \t\thttp.sslCert\ndiff --git a/http.c b/http.c\nindex e9c6fdd..d5fecd6 100644\n--- a/http.c\n+++ b/http.c\n@@ -37,6 +37,21 @@ static int curl_ssl_verify = -1;\n static int curl_ssl_try;\n static const char *ssl_cert;\n static const char *ssl_cipherlist;\n+static const char *ssl_version;\n+static struct {\n+\tconst char *name;\n+\tlong ssl_version;\n+\t} sslversions[] = {\n+\t\t{ \"sslv2\", CURL_SSLVERSION_SSLv2 },\n+\t\t{ \"sslv3\", CURL_SSLVERSION_TLSv1 },\n+\t\t{ \"tlsv1\", CURL_SSLVERSION_TLSv1 },\n+#if LIBCURL_VERSION_NUM >= 0x072200\n+\t\t{ \"tlsv1.0\", CURL_SSLVERSION_TLSv1_0 },\n+\t\t{ \"tlsv1.1\", CURL_SSLVERSION_TLSv1_1 },\n+\t\t{ \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n+#endif\n+\t\t{ NULL }\n+};\n #if LIBCURL_VERSION_NUM >= 0x070903\n static const char *ssl_key;\n #endif\n@@ -190,6 +205,8 @@ static int http_options(const char *var, const char *value, void *cb)\n \t}\n \tif (!strcmp(\"http.sslcipherlist\", var))\n \t\treturn git_config_string(&ssl_cipherlist, var, value);\n+\tif (!strcmp(\"http.sslversion\", var))\n+\t\treturn git_config_string(&ssl_version, var, value);\n \tif (!strcmp(\"http.sslcert\", var))\n \t\treturn git_config_string(&ssl_cert, var, value);\n #if LIBCURL_VERSION_NUM >= 0x070903\n@@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n \tif (http_proactive_auth)\n \t\tinit_curl_http_auth(result);\n \n+\tif (getenv(\"GIT_SSL_VERSION\"))\n+\t\tssl_version = getenv(\"GIT_SSL_VERSION\");\n+\tif (ssl_version != NULL && *ssl_version) {\n+\t\tint i;\n+\t\tfor ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n+\t\t\tif (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n+\t\t\t\tcurl_easy_setopt(result, CURLOPT_SSLVERSION,\n+\t\t\t\t\tsslversions[i].ssl_version);\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif ( i == ARRAY_SIZE(sslversions) ) warning(\"unsupported ssl version %s: using default\",\n+\t\t\t\t\t\t\tssl_version);\n+\t}\n+\n \tif (getenv(\"GIT_SSL_CIPHER_LIST\"))\n \t\tssl_cipherlist = getenv(\"GIT_SSL_CIPHER_LIST\");\n-\n \tif (ssl_cipherlist != NULL && *ssl_cipherlist)\n \t\tcurl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,\n \t\t\t\tssl_cipherlist);\n-- \n2.5.0.234.gefc8a62.dirty\n"},{"id":"267986","messageId":"CAPig+cTug2Q3v1K5r76fhJ6OQY9V1e6MbiXQBGQJD51TCOGW=A@mail.gmail.com","threadId":"40080","inReplyTo":"1439479731-16018-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-13T15:47:01Z","receivedAt":"2015-08-13T15:47:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 13, 2015 at 11:28 AM, Elia Pinto wrote:\n> Teach git about a new option, \"http.sslVersion\", which permits one to\n> specify the SSL version to use when negotiating SSL connections. The\n> setting can be overridden by the GIT_SSL_VERSION environment\n> variable.\n>\n> Signed-off-by: Elia Pinto \n> ---\n> This is the third version of the patch. The changes compared to the previous version are:\n\nLooks better. A few comments below...\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c97c648..6e9359c 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n> if (http_proactive_auth)\n> init_curl_http_auth(result);\n>\n> + if (getenv(\"GIT_SSL_VERSION\"))\n> + ssl_version = getenv(\"GIT_SSL_VERSION\");\n> + if (ssl_version != NULL && *ssl_version) {\n> + int i;\n> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n\nThis sort of loop is normally either handled by indexing up to a limit\n(ARRAY_SIZE, in this case) or by iterating until hitting a sentinel\n(NULL, in this case). It is redundant to use both, as this code does.\nThe former (using ARRAY_SIZE) is typically employed when you know the\nnumber of items upfront, such as when the item list is local and\ncompiled in; the latter (NULL sentinel) is typically used when\nreceiving an item list as an argument to a function where you don't\nknow the item count upfront (and the item count is not passed to the\nfunction as a separate argument).\n\nIn this case, the item list is local and its size is known to the\ncompiler, so that suggests using ARRAY_SIZE, and dropping the NULL\nsentinel.\n\nStyle aside: This 'if' statement is very wide and likely should be\nwrapped over multiple lines (trying to keep the code within an\n80-column limit).\n\n> + curl_easy_setopt(result, CURLOPT_SSLVERSION,\n> + sslversions[i].ssl_version);\n> + break;\n> + }\n> + if ( i == ARRAY_SIZE(sslversions) ) warning(\"unsupported ssl version %s: using default\",\n> + ssl_version);\n\nStyle:\nDrop spaces inside 'if' parentheses.\nPlace warning() on its own line.\n\n> + }\n> +\n> if (getenv(\"GIT_SSL_CIPHER_LIST\"))\n> ssl_cipherlist = getenv(\"GIT_SSL_CIPHER_LIST\");\n> -\n> if (ssl_cipherlist != NULL && *ssl_cipherlist)\n> curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,\n> ssl_cipherlist);\n> --\n> 2.5.0.234.gefc8a62.dirty\n"},{"id":"267987","messageId":"CA+EOSBkSkvvBQDpxL_ygj+2haMk1U7T00-Xmxn8iyXcnV6RN5Q@mail.gmail.com","threadId":"40080","inReplyTo":"CAPig+cTug2Q3v1K5r76fhJ6OQY9V1e6MbiXQBGQJD51TCOGW=A@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-13T15:58:03Z","receivedAt":"2015-08-13T15:58:03Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-08-13 17:47 GMT+02:00 Eric Sunshine :\n> On Thu, Aug 13, 2015 at 11:28 AM, Elia Pinto wrote:\n>> Teach git about a new option, \"http.sslVersion\", which permits one to\n>> specify the SSL version to use when negotiating SSL connections. The\n>> setting can be overridden by the GIT_SSL_VERSION environment\n>> variable.\n>>\n>> Signed-off-by: Elia Pinto \n>> ---\n>> This is the third version of the patch. The changes compared to the previous version are:\n>\n> Looks better. A few comments below...\n>\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index c97c648..6e9359c 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n>> if (http_proactive_auth)\n>> init_curl_http_auth(result);\n>>\n>> + if (getenv(\"GIT_SSL_VERSION\"))\n>> + ssl_version = getenv(\"GIT_SSL_VERSION\");\n>> + if (ssl_version != NULL && *ssl_version) {\n>> + int i;\n>> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n>> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n>\n> This sort of loop is normally either handled by indexing up to a limit\n> (ARRAY_SIZE, in this case) or by iterating until hitting a sentinel\n> (NULL, in this case). It is redundant to use both, as this code does.\nI do not think. sslversions[i].name can be null, see how the structure\nis initialized. No ?\n\nThe other your observations written below are ok for me, but i will\nwait for your answer on this before you send another revision. Thank\nyou very much.\n\n> The former (using ARRAY_SIZE) is typically employed when you know the\n> number of items upfront, such as when the item list is local and\n> compiled in; the latter (NULL sentinel) is typically used when\n> receiving an item list as an argument to a function where you don't\n> know the item count upfront (and the item count is not passed to the\n> function as a separate argument).\n>\n> In this case, the item list is local and its size is known to the\n> compiler, so that suggests using ARRAY_SIZE, and dropping the NULL\n> sentinel.\n>\n> Style aside: This 'if' statement is very wide and likely should be\n> wrapped over multiple lines (trying to keep the code within an\n> 80-column limit).\nok.\n>\n>> + curl_easy_setopt(result, CURLOPT_SSLVERSION,\n>> + sslversions[i].ssl_version);\n>> + break;\n>> + }\n>> + if ( i == ARRAY_SIZE(sslversions) ) warning(\"unsupported ssl version %s: using default\",\n>> + ssl_version);\n>\n> Style:\n> Drop spaces inside 'if' parentheses.\n> Place warning() on its own line.\nok.\n\n\n>\n>> + }\n>> +\n>> if (getenv(\"GIT_SSL_CIPHER_LIST\"))\n>> ssl_cipherlist = getenv(\"GIT_SSL_CIPHER_LIST\");\n>> -\n>> if (ssl_cipherlist != NULL && *ssl_cipherlist)\n>> curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,\n>> ssl_cipherlist);\n>> --\n>> 2.5.0.234.gefc8a62.dirty\n"},{"id":"267988","messageId":"55CCBF6F.3070808@web.de","threadId":"40080","inReplyTo":"1439479731-16018-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-08-13T16:01:51Z","receivedAt":"2015-08-13T16:01:51Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"(need to drop Eric from cc-list, no DNS from web.de)\n\nOn 2015-08-13 17.28, Elia Pinto wrote:\n> Teach git about a new option, \"http.sslVersion\", which permits one to\n> specify the SSL version to use when negotiating SSL connections. The\n> setting can be overridden by the GIT_SSL_VERSION environment\n> variable.\n> \n> Signed-off-by: Elia Pinto \n> ---\n> This is the third version of the patch. The changes compared to the previous version are:\n> \n> - Eliminated the unnecessary blank (Junio)\n> - Place a structure to associate mnemonic names with the curl enum constant (Junio)\n> - Eliminated the invocation to curl_easy_setopt to set the default SSL value. Also removed the static global variable.\n> (Junio)\n> - Slight correction in config.txt (Eric)\n> \n> Documentation/config.txt | 22 ++++++++++++++++++++++\n> contrib/completion/git-completion.bash | 1 +\n> http.c | 32 +++++++++++++++++++++++++++++++-\n> 3 files changed, 54 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 315f271..b23b01a 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1595,6 +1595,28 @@ http.saveCookies::\n> \tIf set, store cookies received during requests to the file specified by\n> \thttp.cookieFile. Has no effect if http.cookieFile is unset.\n> \n> +http.sslVersion::\nshould this be https.sslVersion ?\n(http doesn't use ssl)\n\n> +\tThe SSL version to use when negotiating an SSL connection, if you\n> +\twant to force the default. The available and default version depend on\n> +\twhether libcurl was built against NSS or OpenSSL and the particular configuration\n> +\tof the crypto library in use. Internally this sets the 'CURLOPT_SSL_VERSION'\n> +\toption; see the libcurl documentation for more details on the format\n> +\tof this option and for the ssl version supported. Actually the possible values\n> +\tof this option are:\n> +\n> +\t- sslv2\n> +\t- sslv3\n> +\t- tlsv1\n> +\t- tlsv1.0\n> +\t- tlsv1.1\n> +\t- tlsv1.2\n> +\nfrom\nhttps://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_1.0.2C_2.0_and_3.0\nsslv2 and sslv3 are deprecated.\nShould there be a motivation in the commit message why we want to support them ?\n\n\n> ++\n> +Can be overridden by the 'GIT_SSL_VERSION' environment variable.\n> +To force git to use libcurl's default ssl version and ignore any\n> +explicit http.sslversion option, set 'GIT_SSL_VERSION' to the\n> +empty string.\n> +\n> http.sslCipherList::\n> A list of SSL ciphers to use when negotiating an SSL connection.\n> The available ciphers depend on whether libcurl was built against\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index c97c648..6e9359c 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2118,6 +2118,7 @@ _git_config ()\n> \t\thttp.postBuffer\n> \t\thttp.proxy\n> \t\thttp.sslCipherList\n> +\t\thttp.sslVersion\n> \t\thttp.sslCAInfo\n> \t\thttp.sslCAPath\n> \t\thttp.sslCert\n> diff --git a/http.c b/http.c\n> index e9c6fdd..d5fecd6 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -37,6 +37,21 @@ static int curl_ssl_verify = -1;\n> static int curl_ssl_try;\n> static const char *ssl_cert;\n> static const char *ssl_cipherlist;\n> +static const char *ssl_version;\n> +static struct {\n> +\tconst char *name;\n> +\tlong ssl_version;\n> +\t} sslversions[] = {\n> +\t\t{ \"sslv2\", CURL_SSLVERSION_SSLv2 },\n> +\t\t{ \"sslv3\", CURL_SSLVERSION_TLSv1 },\n> +\t\t{ \"tlsv1\", CURL_SSLVERSION_TLSv1 },\n> +#if LIBCURL_VERSION_NUM >= 0x072200\n> +\t\t{ \"tlsv1.0\", CURL_SSLVERSION_TLSv1_0 },\n> +\t\t{ \"tlsv1.1\", CURL_SSLVERSION_TLSv1_1 },\n> +\t\t{ \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n> +#endif\n> +\t\t{ NULL }\n> +};\n> #if LIBCURL_VERSION_NUM >= 0x070903\n> static const char *ssl_key;\n> #endif\n> @@ -190,6 +205,8 @@ static int http_options(const char *var, const char *value, void *cb)\n> \t}\n> \tif (!strcmp(\"http.sslcipherlist\", var))\n> \t\treturn git_config_string(&ssl_cipherlist, var, value);\n> +\tif (!strcmp(\"http.sslversion\", var))\n> +\t\treturn git_config_string(&ssl_version, var, value);\n> \tif (!strcmp(\"http.sslcert\", var))\n> \t\treturn git_config_string(&ssl_cert, var, value);\n> #if LIBCURL_VERSION_NUM >= 0x070903\n> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n> \tif (http_proactive_auth)\n> \t\tinit_curl_http_auth(result);\n> \n> +\tif (getenv(\"GIT_SSL_VERSION\"))\n> +\t\tssl_version = getenv(\"GIT_SSL_VERSION\");\n> +\t\nMinor nit to shorten:\nif (ssl_version && *ssl_version) {\n\n> +\t\tint i;\n> +\t\tfor ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\nI think Git-style is not to have ' ' before/after ')' /'('\nfor (i = 0; i < ARRAY_SIZE(sslversions); i++)\n\n> +\t\t\tif (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n> +\t\t\t\tcurl_easy_setopt(result, CURLOPT_SSLVERSION,\n> +\t\t\t\t\tsslversions[i].ssl_version);\nThis is what my man page says:\n CURLcode curl_easy_setopt(CURL *handle, CURLoption option, parameter);\n[]\n\nRETURN VALUE\n CURLE_OK (zero) means that the option was set properly...\nShould the return value checked (and we die() if we fail ?\n\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\tif ( i == ARRAY_SIZE(sslversions) ) warning(\"unsupported ssl version %s: using default\",\n> +\t\t\t\t\t\t\tssl_version);\nShould we die() here to make things very clear to the user ?\n\n> +\t}\n> +\n> \tif (getenv(\"GIT_SSL_CIPHER_LIST\"))\n> \t\tssl_cipherlist = getenv(\"GIT_SSL_CIPHER_LIST\");\n> -\n> \tif (ssl_cipherlist != NULL && *ssl_cipherlist)\n> \t\tcurl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,\n> \t\t\t\tssl_cipherlist);\n> \n"},{"id":"267989","messageId":"CA+EOSBkzU=6pKkqYdGqRRcbbudTJkRwcXxswP+zMshVrZaM_mw@mail.gmail.com","threadId":"40080","inReplyTo":"55CCBF6F.3070808@web.de","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-13T16:10:48Z","receivedAt":"2015-08-13T16:10:48Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-08-13 18:01 GMT+02:00 Torsten Bögershausen :\n> (need to drop Eric from cc-list, no DNS from web.de)\n>\n> On 2015-08-13 17.28, Elia Pinto wrote:\n>> Teach git about a new option, \"http.sslVersion\", which permits one to\n>> specify the SSL version to use when negotiating SSL connections. The\n>> setting can be overridden by the GIT_SSL_VERSION environment\n>> variable.\n>>\n>> Signed-off-by: Elia Pinto \n>> ---\n>> This is the third version of the patch. The changes compared to the previous version are:\n>>\n>> - Eliminated the unnecessary blank (Junio)\n>> - Place a structure to associate mnemonic names with the curl enum constant (Junio)\n>> - Eliminated the invocation to curl_easy_setopt to set the default SSL value. Also removed the static global variable.\n>> (Junio)\n>> - Slight correction in config.txt (Eric)\n>>\n>> Documentation/config.txt | 22 ++++++++++++++++++++++\n>> contrib/completion/git-completion.bash | 1 +\n>> http.c | 32 +++++++++++++++++++++++++++++++-\n>> 3 files changed, 54 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 315f271..b23b01a 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1595,6 +1595,28 @@ http.saveCookies::\n>> If set, store cookies received during requests to the file specified by\n>> http.cookieFile. Has no effect if http.cookieFile is unset.\n>>\n>> +http.sslVersion::\n> should this be https.sslVersion ?\n> (http doesn't use ssl)\n>\n>> + The SSL version to use when negotiating an SSL connection, if you\n>> + want to force the default. The available and default version depend on\n>> + whether libcurl was built against NSS or OpenSSL and the particular configuration\n>> + of the crypto library in use. Internally this sets the 'CURLOPT_SSL_VERSION'\n>> + option; see the libcurl documentation for more details on the format\n>> + of this option and for the ssl version supported. Actually the possible values\n>> + of this option are:\n>> +\n>> + - sslv2\n>> + - sslv3\n>> + - tlsv1\n>> + - tlsv1.0\n>> + - tlsv1.1\n>> + - tlsv1.2\n>> +\n> from\n> https://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_1.0.2C_2.0_and_3.0\n> sslv2 and sslv3 are deprecated.\n> Should there be a motivation in the commit message why we want to support them ?\nThey are those provided by the documentation (TLS in particular). We\nlet the underlying library to say what is deprecated or not. In this\ncase the call fail.\n>\n>\n>> ++\n>> +Can be overridden by the 'GIT_SSL_VERSION' environment variable.\n>> +To force git to use libcurl's default ssl version and ignore any\n>> +explicit http.sslversion option, set 'GIT_SSL_VERSION' to the\n>> +empty string.\n>> +\n>> http.sslCipherList::\n>> A list of SSL ciphers to use when negotiating an SSL connection.\n>> The available ciphers depend on whether libcurl was built against\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index c97c648..6e9359c 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -2118,6 +2118,7 @@ _git_config ()\n>> http.postBuffer\n>> http.proxy\n>> http.sslCipherList\n>> + http.sslVersion\n>> http.sslCAInfo\n>> http.sslCAPath\n>> http.sslCert\n>> diff --git a/http.c b/http.c\n>> index e9c6fdd..d5fecd6 100644\n>> --- a/http.c\n>> +++ b/http.c\n>> @@ -37,6 +37,21 @@ static int curl_ssl_verify = -1;\n>> static int curl_ssl_try;\n>> static const char *ssl_cert;\n>> static const char *ssl_cipherlist;\n>> +static const char *ssl_version;\n>> +static struct {\n>> + const char *name;\n>> + long ssl_version;\n>> + } sslversions[] = {\n>> + { \"sslv2\", CURL_SSLVERSION_SSLv2 },\n>> + { \"sslv3\", CURL_SSLVERSION_TLSv1 },\n>> + { \"tlsv1\", CURL_SSLVERSION_TLSv1 },\n>> +#if LIBCURL_VERSION_NUM >= 0x072200\n>> + { \"tlsv1.0\", CURL_SSLVERSION_TLSv1_0 },\n>> + { \"tlsv1.1\", CURL_SSLVERSION_TLSv1_1 },\n>> + { \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n>> +#endif\n>> + { NULL }\n>> +};\n>> #if LIBCURL_VERSION_NUM >= 0x070903\n>> static const char *ssl_key;\n>> #endif\n>> @@ -190,6 +205,8 @@ static int http_options(const char *var, const char *value, void *cb)\n>> }\n>> if (!strcmp(\"http.sslcipherlist\", var))\n>> return git_config_string(&ssl_cipherlist, var, value);\n>> + if (!strcmp(\"http.sslversion\", var))\n>> + return git_config_string(&ssl_version, var, value);\n>> if (!strcmp(\"http.sslcert\", var))\n>> return git_config_string(&ssl_cert, var, value);\n>> #if LIBCURL_VERSION_NUM >= 0x070903\n>> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n>> if (http_proactive_auth)\n>> init_curl_http_auth(result);\n>>\n>> + if (getenv(\"GIT_SSL_VERSION\"))\n>> + ssl_version = getenv(\"GIT_SSL_VERSION\");\n>> +\n> Minor nit to shorten:\n> if (ssl_version && *ssl_version) {\n>\n>> + int i;\n>> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n> I think Git-style is not to have ' ' before/after ')' /'('\n> for (i = 0; i < ARRAY_SIZE(sslversions); i++)\n>\n>> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n>> + curl_easy_setopt(result, CURLOPT_SSLVERSION,\n>> + sslversions[i].ssl_version);\n> This is what my man page says:\n> CURLcode curl_easy_setopt(CURL *handle, CURLoption option, parameter);\n> []\n>\n> RETURN VALUE\n> CURLE_OK (zero) means that the option was set properly...\n> Should the return value checked (and we die() if we fail ?\nIt is not strictly necessary. If it fails the other curl call fail,\ntry to use sslv2 for example (libcurl deprecated nss dunno)\n\nThanks !\n\n>\n>> + break;\n>> + }\n>> + if ( i == ARRAY_SIZE(sslversions) ) warning(\"unsupported ssl version %s: using default\",\n>> + ssl_version);\n> Should we die() here to make things very clear to the user ?\n>\n>> + }\n>> +\n>> if (getenv(\"GIT_SSL_CIPHER_LIST\"))\n>> ssl_cipherlist = getenv(\"GIT_SSL_CIPHER_LIST\");\n>> -\n>> if (ssl_cipherlist != NULL && *ssl_cipherlist)\n>> curl_easy_setopt(result, CURLOPT_SSL_CIPHER_LIST,\n>> ssl_cipherlist);\n>>\n>\n"},{"id":"267990","messageId":"CAPig+cSC2a07RYioQ4+sw=pujFW8=sv_d5vv=XiayuSg7FBcHw@mail.gmail.com","threadId":"40080","inReplyTo":"CA+EOSBkSkvvBQDpxL_ygj+2haMk1U7T00-Xmxn8iyXcnV6RN5Q@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-13T16:11:30Z","receivedAt":"2015-08-13T16:11:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 13, 2015 at 11:58 AM, Elia Pinto wrote:\n> 2015-08-13 17:47 GMT+02:00 Eric Sunshine :\n>> On Thu, Aug 13, 2015 at 11:28 AM, Elia Pinto wrote:\n>>> Teach git about a new option, \"http.sslVersion\", which permits one to\n>>> specify the SSL version to use when negotiating SSL connections. The\n>>> setting can be overridden by the GIT_SSL_VERSION environment\n>>> variable.\n>>>\n>>> Signed-off-by: Elia Pinto \n>>> ---\n>>> This is the third version of the patch. The changes compared to the previous version are:\n>>\n>> Looks better. A few comments below...\n>>\n>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>>> index c97c648..6e9359c 100644\n>>> --- a/contrib/completion/git-completion.bash\n>>> +++ b/contrib/completion/git-completion.bash\n>>> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n>>> if (http_proactive_auth)\n>>> init_curl_http_auth(result);\n>>>\n>>> + if (getenv(\"GIT_SSL_VERSION\"))\n>>> + ssl_version = getenv(\"GIT_SSL_VERSION\");\n>>> + if (ssl_version != NULL && *ssl_version) {\n>>> + int i;\n>>> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n>>> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n>>\n>> This sort of loop is normally either handled by indexing up to a limit\n>> (ARRAY_SIZE, in this case) or by iterating until hitting a sentinel\n>> (NULL, in this case). It is redundant to use both, as this code does.\n> I do not think. sslversions[i].name can be null, see how the structure\n> is initialized. No ?\n\nThe initialization:\n\n static struct {\n const char *name;\n long ssl_version;\n } sslversions[] = {\n { \"sslv2\", CURL_SSLVERSION_SSLv2 },\n ...\n { \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n { NULL }\n };\n\nterminates the list with a NULL sentinel entry, which does indeed set\nsslversions[i].name to NULL. When you know the item count ahead of\ntime (as you do in this case), this sort of end-of-list sentinel is\nredundant, and complicates the code unnecessarily. For instance, the\n'sslversions[i].name != NULL' expression in the 'if':\n\n if (sslversions[i].name != NULL && *sslversions[i].name ...\n\nis an unwanted complication. In fact, the '*sslversions[i].name'\nexpression is also unnecessary.\n"},{"id":"267991","messageId":"CA+EOSBkOGzyOB-NRGTNm0b==OZH7eB=sZaGa0mRa4798_v-EHQ@mail.gmail.com","threadId":"40080","inReplyTo":"CAPig+cSC2a07RYioQ4+sw=pujFW8=sv_d5vv=XiayuSg7FBcHw@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-13T16:15:59Z","receivedAt":"2015-08-13T16:15:59Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-08-13 18:11 GMT+02:00 Eric Sunshine :\n> On Thu, Aug 13, 2015 at 11:58 AM, Elia Pinto wrote:\n>> 2015-08-13 17:47 GMT+02:00 Eric Sunshine :\n>>> On Thu, Aug 13, 2015 at 11:28 AM, Elia Pinto wrote:\n>>>> Teach git about a new option, \"http.sslVersion\", which permits one to\n>>>> specify the SSL version to use when negotiating SSL connections. The\n>>>> setting can be overridden by the GIT_SSL_VERSION environment\n>>>> variable.\n>>>>\n>>>> Signed-off-by: Elia Pinto \n>>>> ---\n>>>> This is the third version of the patch. The changes compared to the previous version are:\n>>>\n>>> Looks better. A few comments below...\n>>>\n>>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>>>> index c97c648..6e9359c 100644\n>>>> --- a/contrib/completion/git-completion.bash\n>>>> +++ b/contrib/completion/git-completion.bash\n>>>> @@ -364,9 +381,22 @@ static CURL *get_curl_handle(void)\n>>>> if (http_proactive_auth)\n>>>> init_curl_http_auth(result);\n>>>>\n>>>> + if (getenv(\"GIT_SSL_VERSION\"))\n>>>> + ssl_version = getenv(\"GIT_SSL_VERSION\");\n>>>> + if (ssl_version != NULL && *ssl_version) {\n>>>> + int i;\n>>>> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n>>>> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n>>>\n>>> This sort of loop is normally either handled by indexing up to a limit\n>>> (ARRAY_SIZE, in this case) or by iterating until hitting a sentinel\n>>> (NULL, in this case). It is redundant to use both, as this code does.\n>> I do not think. sslversions[i].name can be null, see how the structure\n>> is initialized. No ?\n>\n> The initialization:\n>\n> static struct {\n> const char *name;\n> long ssl_version;\n> } sslversions[] = {\n> { \"sslv2\", CURL_SSLVERSION_SSLv2 },\n> ...\n> { \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n> { NULL }\n> };\n>\n> terminates the list with a NULL sentinel entry, which does indeed set\n> sslversions[i].name to NULL. When you know the item count ahead of\n> time (as you do in this case), this sort of end-of-list sentinel is\n> redundant, and complicates the code unnecessarily. For instance, the\n> 'sslversions[i].name != NULL' expression in the 'if':\n>\n> if (sslversions[i].name != NULL && *sslversions[i].name ...\n>\n> is an unwanted complication. In fact, the '*sslversions[i].name'\n> expression is also unnecessary.\nI agree. But this is what suggested me Junio: =). What do I have to do ?\nIt becomes difficult to keep everyone happy: =)\n\nJunio ?\n\nThanks\n"},{"id":"267992","messageId":"20150813162454.GA18545@LK-Perkele-VII","threadId":"40080","inReplyTo":"CA+EOSBkzU=6pKkqYdGqRRcbbudTJkRwcXxswP+zMshVrZaM_mw@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2015-08-13T16:24:54Z","receivedAt":"2015-08-13T16:24:54Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Thu, Aug 13, 2015 at 06:10:48PM +0200, Elia Pinto wrote:\n> 2015-08-13 18:01 GMT+02:00 Torsten Bögershausen :\n> >> +\n> > from\n> > https://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_1.0.2C_2.0_and_3.0\n> > sslv2 and sslv3 are deprecated.\n> > Should there be a motivation in the commit message why we want to support them ?\n> They are those provided by the documentation (TLS in particular). We\n> let the underlying library to say what is deprecated or not. In this\n> case the call fail.\n\nThe statement from the relevant SDO is much stronger than \"deprecated\",\nit is \"not to be used under any cirmumstances\".\n\nOption like this looks only useful for connecting to really broken\nservers, damn security.\n\nIt could be useful for connecting to buggy servers after TLS 1.3\ncomes out and is implemented, as there are lots of servers (IIRC, on\norder of 10%) that can't deal with TLS 1.3 properly (but very few, IIRC\n<<0.1%, that can't deal with TLS 1.2 correctly[1]).\n\nAlso, is this option settable globally for all HTTP servers? One\ndefinitely does not want that to be possible. Configurations like\nthis need to be per-server if they exist at all.\n\n\n\n[1] Where correctly includes secure downnegotiation, as TLS\nis intended to do when faced with version mismatch.\n\n\n-Ilari\n"},{"id":"267993","messageId":"CA+EOSB=i6orXCLC4ZyqO5uhH7JPH_6DXHB0yFeXnH77oHgARew@mail.gmail.com","threadId":"40080","inReplyTo":"20150813162454.GA18545@LK-Perkele-VII","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-13T16:33:26Z","receivedAt":"2015-08-13T16:33:26Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-08-13 18:24 GMT+02:00 Ilari Liusvaara :\n> On Thu, Aug 13, 2015 at 06:10:48PM +0200, Elia Pinto wrote:\n>> 2015-08-13 18:01 GMT+02:00 Torsten Bögershausen :\n>> >> +\n>> > from\n>> > https://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_1.0.2C_2.0_and_3.0\n>> > sslv2 and sslv3 are deprecated.\n>> > Should there be a motivation in the commit message why we want to support them ?\n>> They are those provided by the documentation (TLS in particular). We\n>> let the underlying library to say what is deprecated or not. In this\n>> case the call fail.\n>\n> The statement from the relevant SDO is much stronger than \"deprecated\",\n> it is \"not to be used under any cirmumstances\".\n>\n> Option like this looks only useful for connecting to really broken\n> servers, damn security.\nI know very well this topic.\nhttps://securitypitfalls.wordpress.com/2015/07/29/july-2015-scan-results/\nI prefer that the decision is from the libray not us.\n\n\n>\n> It could be useful for connecting to buggy servers after TLS 1.3\n> comes out and is implemented, as there are lots of servers (IIRC, on\n> order of 10%) that can't deal with TLS 1.3 properly (but very few, IIRC\n> <<0.1%, that can't deal with TLS 1.2 correctly[1]).\n>\n> Also, is this option settable globally for all HTTP servers? One\n> definitely does not want that to be possible. Configurations like\n> this need to be per-server if they exist at all.\n>\n>\n>\n> [1] Where correctly includes secure downnegotiation, as TLS\n> is intended to do when faced with version mismatch.\n>\n>\n> -Ilari\n"},{"id":"267994","messageId":"CAPig+cQj4-4tnZv1JkUZdGHzgL=x2f6Zg7JeYn5bBgp991WNhg@mail.gmail.com","threadId":"40080","inReplyTo":"CA+EOSBkOGzyOB-NRGTNm0b==OZH7eB=sZaGa0mRa4798_v-EHQ@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-13T16:37:49Z","receivedAt":"2015-08-13T16:37:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 13, 2015 at 12:15 PM, Elia Pinto wrote:\n> 2015-08-13 18:11 GMT+02:00 Eric Sunshine :\n>> On Thu, Aug 13, 2015 at 11:58 AM, Elia Pinto wrote:\n>>> 2015-08-13 17:47 GMT+02:00 Eric Sunshine :\n>>>>> + if (ssl_version != NULL && *ssl_version) {\n>>>>> + int i;\n>>>>> + for ( i = 0; i < ARRAY_SIZE(sslversions); i++ ) {\n>>>>> + if (sslversions[i].name != NULL && *sslversions[i].name && !strcmp(ssl_version,sslversions[i].name)) {\n>>>>\n>>>> This sort of loop is normally either handled by indexing up to a limit\n>>>> (ARRAY_SIZE, in this case) or by iterating until hitting a sentinel\n>>>> (NULL, in this case). It is redundant to use both, as this code does.\n>>> I do not think. sslversions[i].name can be null, see how the structure\n>>> is initialized. No ?\n>>\n>> The initialization:\n>>\n>> static struct {\n>> const char *name;\n>> long ssl_version;\n>> } sslversions[] = {\n>> { \"sslv2\", CURL_SSLVERSION_SSLv2 },\n>> ...\n>> { \"tlsv1.2\", CURL_SSLVERSION_TLSv1_2 },\n>> { NULL }\n>> };\n>>\n>> terminates the list with a NULL sentinel entry, which does indeed set\n>> sslversions[i].name to NULL. When you know the item count ahead of\n>> time (as you do in this case), this sort of end-of-list sentinel is\n>> redundant, and complicates the code unnecessarily. For instance, the\n>> 'sslversions[i].name != NULL' expression in the 'if':\n>>\n>> if (sslversions[i].name != NULL && *sslversions[i].name ...\n>>\n>> is an unwanted complication. In fact, the '*sslversions[i].name'\n>> expression is also unnecessary.\n> I agree. But this is what suggested me Junio: =). What do I have to do ?\n> It becomes difficult to keep everyone happy: =)\n\nYou're referring to [1] in which Junio's example table initialization\nhad the NULL sentinel. That approach is fine, and my earlier comment:\n\n This sort of loop is normally either handled by indexing up to a\n limit (ARRAY_SIZE, in this case) or by iterating until hitting a\n sentinel (NULL, in this case). It is redundant to use both...\n\nwasn't saying that you shouldn't use the NULL sentinel. It said only\nthat you should choose one approach rather than complicating the code\nunnecessarily by mixing the two.\n\nSo, your loop can either look like this, if you use the NULL sentinel:\n\n struct ssl_map *p = sslversions;\n while (p->name) {\n if (!strcmp(ssl_version, p->name))\n ...\n }\n\nor like this, if you use ARRAY_SIZE:\n\n for (i = 0; i < ARRAY_SIZE(sslversions); i++) {\n if (!strcmp(ssl_version, sslversions[i].name))\n ...\n }\n\nEach loop form is valid, and (other than the fact that the compiler\nknows the array size, thus slightly favoring the ARRAY_SIZE form) the\nchoice of which of the above two forms to use isn't that important,\nand you can choose whichever you like, but please do choose one of the\nabove two. If you feel that Junio would be happier with the\nNULL-sentinel form, then go with that.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/275773\n"},{"id":"267996","messageId":"CAPig+cQZczCShRKeaQ=UP02OmUG5D2-ZCtaEO0qbm=LQ4m=ctw@mail.gmail.com","threadId":"40080","inReplyTo":"CAPig+cQj4-4tnZv1JkUZdGHzgL=x2f6Zg7JeYn5bBgp991WNhg@mail.gmail.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-13T16:49:16Z","receivedAt":"2015-08-13T16:49:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 13, 2015 at 12:37 PM, Eric Sunshine wrote:\n> So, your loop can either look like this, if you use the NULL sentinel:\n>\n> struct ssl_map *p = sslversions;\n> while (p->name) {\n> if (!strcmp(ssl_version, p->name))\n> ...\n> }\n\nThat's not quite correct. 'p' needs to be incremented, of course, so:\n\n struct ssl_map *p;\n for (p = sslversions; p->name; p++) {\n if (!strcmp(ssl_version, p->name))\n ...\n }\n\nwould be nicely idiomatic.\n"},{"id":"268048","messageId":"xmqqlhddiy5a.fsf@gitster.dls.corp.google.com","threadId":"40080","inReplyTo":"55CCBF6F.3070808@web.de","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-14T17:21:37Z","receivedAt":"2015-08-14T17:21:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 315f271..b23b01a 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -1595,6 +1595,28 @@ http.saveCookies::\n>> \tIf set, store cookies received during requests to the file specified by\n>> \thttp.cookieFile. Has no effect if http.cookieFile is unset.\n>> \n>> +http.sslVersion::\n> should this be https.sslVersion ?\n> (http doesn't use ssl)\n\nBut there are sslCipherList, etc., already present, and more\nimportantly, I think you want http.proxy to apply even if you happen\nto be talking http over SSL.\n\nMore importantly, given that we have the \"limited to this URL\"\nmechanism \"http..\" that overrides \"http.\",\nintroducing \"https.sslWhatEver\" would force people to have two\nconfiguration sections for no real benefit, other than silencing\npedants that want to say \"these things should be defined only for\nhttps\".\n\n>> + if (sslversions[i].name != NULL && *sslversions[i].name &&\n>> !strcmp(ssl_version,sslversions[i].name)) {\n>> +\t\t\t\tcurl_easy_setopt(result, CURLOPT_SSLVERSION,\n>> +\t\t\t\t\tsslversions[i].ssl_version);\n> This is what my man page says:\n> CURLcode curl_easy_setopt(CURL *handle, CURLoption option, parameter);\n> []\n>\n> RETURN VALUE\n> CURLE_OK (zero) means that the option was set properly...\n> Should the return value checked (and we die() if we fail ?\n\nProbably. Do we check status from other calls to setopt?\n"},{"id":"268065","messageId":"CA+EOSBmqVo8LsOLjzc6vLV1YFT2t=57f-GM7DC8Na6Ggi2anUQ@mail.gmail.com","threadId":"40080","inReplyTo":"xmqqlhddiy5a.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] http: add support for specifying the SSL version","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2015-08-14T19:51:29Z","receivedAt":"2015-08-14T19:51:29Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"2015-08-14 19:21 GMT+02:00 Junio C Hamano :\n> Torsten Bögershausen writes:\n>\n>>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>>> index 315f271..b23b01a 100644\n>>> --- a/Documentation/config.txt\n>>> +++ b/Documentation/config.txt\n>>> @@ -1595,6 +1595,28 @@ http.saveCookies::\n>>> If set, store cookies received during requests to the file specified by\n>>> http.cookieFile. Has no effect if http.cookieFile is unset.\n>>>\n>>> +http.sslVersion::\n>> should this be https.sslVersion ?\n>> (http doesn't use ssl)\n>\n> But there are sslCipherList, etc., already present, and more\n> importantly, I think you want http.proxy to apply even if you happen\n> to be talking http over SSL.\n>\n> More importantly, given that we have the \"limited to this URL\"\n> mechanism \"http..\" that overrides \"http.\",\n> introducing \"https.sslWhatEver\" would force people to have two\n> configuration sections for no real benefit, other than silencing\n> pedants that want to say \"these things should be defined only for\n> https\".\n>\n>>> + if (sslversions[i].name != NULL && *sslversions[i].name &&\n>>> !strcmp(ssl_version,sslversions[i].name)) {\n>>> + curl_easy_setopt(result, CURLOPT_SSLVERSION,\n>>> + sslversions[i].ssl_version);\n>> This is what my man page says:\n>> CURLcode curl_easy_setopt(CURL *handle, CURLoption option, parameter);\n>> []\n>>\n>> RETURN VALUE\n>> CURLE_OK (zero) means that the option was set properly...\n>> Should the return value checked (and we die() if we fail ?\n>\n> Probably. Do we check status from other calls to setopt?\nNo. In this case anyway is not important i think: we already check if\nthe version is accepted by curl, and if it is deprecated ( sslv2 for\neample) we have an error in any case. refs\nhttp://curl.haxx.se/libcurl/c/CURLOPT_SSLVERSION.html\n\nBest Regards\n"}]}