Re: [PATCH] Allow use of TLS 1.3

From: Loganaden Velvindron <logan@hackers.mu>
Date: Mar 23, 2018, 18:39 UTC
Message-ID: <20180323183950.GA15994@voidlinux>
In-Reply-To: <87in9my6y3.fsf@evledraar.gmail.com>

On Fri, Mar 23, 2018 at 07:37:08PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 10 quoted lines

> 
> On Fri, Mar 23 2018, Loganaden Velvindron wrote:
> 
> > 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.

Show 20 quoted lines

> 
> > --- a/http.c
> > +++ b/http.c
> > @@ -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.

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

> 
> >  #endif
> >  };
> >  #if LIBCURL_VERSION_NUM >= 0x070903