Skip to contentSkip to conversation

Allow use of TLS 1.3

PATCH Superseded Loganaden Velvindron 3 participants 4 messages Mar 23, 2018 Documentation

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Loganaden Velvindron

author
#0 lore raw

PATCH 1/1 Allow use of TLS 1.3

Done during IETF 101 hackathon

Signed-off-by Loganaden Velvindron <[email protected]>
---
 Documentation/config.txt | 1 +
 http.c                   | 3 +++
 2 files changed, 4 insertions(+)
Documentation/config.txt +1 -0
index ce9102cea..f31d62772 100644
@@ -1957,6 +1957,7 @@ http.sslVersion::
 	- tlsv1.0
 	- tlsv1.1
 	- tlsv1.2
+	- tlsv1.3
 
 +
 Can be overridden by the `GIT_SSL_VERSION` environment variable.
http.c +3 -0
index 8c11156ae..666fe31f3 100644
@@ -61,6 +61,9 @@ static struct {
 	{ "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
 	{ "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
 	{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
+#if LIBCURL_VERSION_NUM >= 0x075200
+	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif
 #endif
 };
 #if LIBCURL_VERSION_NUM >= 0x070903
Signature · 1 line
2.16.2
#1 lore raw
Quoted 1 line · Loganaden Velvindron

Done during IETF 101 hackathon

Hi. Thanks. Let's add a meaningful commit message to this though, something like:

Add a tlsv1.3 option to http.sslVersion in addition to the existing tlsv1.[012] options. libcurl has supported this since 7.52.0.

http.c static struct { · line 66

Quoted 9 lines
http.c +3 -0
@@ -61,6 +61,9 @@ static struct {
 	{ "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
 	{ "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
 	{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
+#if LIBCURL_VERSION_NUM >= 0x075200
+	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif

I wonder if this wouldn't be better as:

    +#ifdef CURL_SSLVERSION_TLSv1_3
    +	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
    +#endif

We've been bitten before by doing version checks on libcurl code, only to find that some distros are actively backporting features, so checking the specific macros is usually better.

Quoted 3 lines
http.c +0 -0
 #endif
 };
 #if LIBCURL_VERSION_NUM >= 0x070903
author
#2 lore raw
Quoted 10 lines · Ævar Arnfjörð Bjarmason
Quoted 1 line · Loganaden Velvindron

Done during IETF 101 hackathon

Hi. Thanks. Let's add a meaningful commit message to this though, something like:

Add a tlsv1.3 option to http.sslVersion in addition to the existing tlsv1.[012] options. libcurl has supported this since 7.52.0.

Looks good to me.

Quoted 13 lines

http.c static struct { · line 66

Quoted 10 lines
http.c +3 -0
@@ -61,6 +61,9 @@ static struct {
 	{ "tlsv1.0", CURL_SSLVERSION_TLSv1_0 },
 	{ "tlsv1.1", CURL_SSLVERSION_TLSv1_1 },
 	{ "tlsv1.2", CURL_SSLVERSION_TLSv1_2 },
+#if LIBCURL_VERSION_NUM >= 0x075200
+	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
+#endif

I wonder if this wouldn't be better as:

Quoted 7 lines
    +#ifdef CURL_SSLVERSION_TLSv1_3
    +	{ "tlsv1.3", CURL_SSLVERSION_TLSv1_3 }
    +#endif

We've been bitten before by doing version checks on libcurl code, only to find that some distros are actively backporting features, so checking the specific macros is usually better.

This looks good to me as well. I will send Patch v2, with the suggestions.

Quoted 4 lines
Quoted 4 lines
http.c +0 -0
 #endif
 };
 #if LIBCURL_VERSION_NUM >= 0x070903
#3 lore raw

Hi,

Quoted 11 lines · Loganaden Velvindron
Quoted 10 lines · Ævar Arnfjörð Bjarmason
Quoted 1 line · Loganaden Velvindron

Done during IETF 101 hackathon

Hi. Thanks. Let's add a meaningful commit message to this though, something like:

Add a tlsv1.3 option to http.sslVersion in addition to the existing tlsv1.[012] options. libcurl has supported this since 7.52.0.

Can we please also add that OpenSSL 1.1.* is required (or that cURL is built with NSS or BoringSSL as the TLS backend)?

Thanks,
Johannes