curl / Mailing Lists / curl-library / Single Mail

curl-library

Re: [Patch 1/3 v2] nss: map CURL_SSLVERSION_DEFAULT to NSS default

From: Kamil Dudka <kdudka_at_redhat.com>
Date: Thu, 03 Nov 2016 10:18:38 +0100

On Wednesday, November 02, 2016 16:46:00 Ray Satiro wrote:
> On 11/2/2016 4:49 AM, Kamil Dudka wrote:
> >> This looks better but since that api function is only available since
> >>
> >> >3.14 apparently [1] does libcurl still build with versions older than
> >> >that
> >
> > It does not since the following commit:
> >
> > https://github.com/curl/curl/commit/curl-7_33_0-220-g30e7e75
>
> Ok. A few more things. You could squash #3 into #2

I see really no benefit in squashing those commits. #2 works on its own.
Getting a squashed patch out of two subsequent commits is trivial but not
vice versa.

> they're both basically support TLSv1.3. And:
>
> packages/OS400/curl.inc.in:
>
> d CURL_SSLVERSION_TLSv1_3...
> d c 7

I was not even aware that we (manually) maintain such a file :) Added.

> after CURL_SSLVERSION_TLSv1_2.
>
> src/tool_setopt.c:
>
> NV(CURL_SSLVERSION_TLSv1_3),
>
> after CURL_SSLVERSION_TLSv1_2.

Good catch. Added.

> also I notice you didn't touch cyassl and openssl ?

Yes because they work as expected without patching. Ideally, all TLS
backends should be ready for adding new TLS versions without any changes
unless they implement the new versions.

Patch #1 fixes the NSS backend to be ready for future introduction of
new TLS versions. It is IMO bad practice to treat unknown TLS versions
as default TLS version. I am not sure where this design anti-pattern
came from.

Thanks for review! I will repost the patchset shortly...

Kamil
-------------------------------------------------------------------
List admin: https://cool.haxx.se/list/listinfo/curl-library
Etiquette: https://curl.haxx.se/mail/etiquette.html
Received on 2016-11-03