Re: [PATCH 2/5] http: correct curl version check for CURLOPT_PINNEDPUBLICKEY

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Wed, Sep 08, 2021 at 05:31:53PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> In aeff8a61216 (http: implement public key pinning, 2016-02-15) a
>> dependency and warning() was added if curl older than 7.44.0 was used,
>> but the relevant code depended on CURLOPT_PINNEDPUBLICKEY, introduced
>> in 7.39.0.
>
> According to the manpage for CURLOPT_PINNEDPUBLICKEY, it looks like
> support for various formats and implementations was phased in. In
> particular, 7.44.0 picked up sha256 support (I guess for a fingerprint?
> I've never used this feature) for most major implementations.
>
> But in terms of compiling, all we care about is that the constant is
> there. So I think the cutoff point you found is what we want. Presumably
> when the file format isn't supported we'd get some error, though it's
> not clear if that would come during the actual curl_*_perform(), or if
> we should be checking the curl_easy_setopt() result.

If we were evaluating a patch to add support for pinnedpublickey
afresh back in, say, 2017, perhaps we cared enough about the
distinction between 7.39 and 7.44 (Nov 2014 and Aug 2015,
respectively), but I'd say cut-off at 7.44 for this, once it is
written and committed in our codebase, is good enough for us.

If the code originally had cut-off at 7.39 and we were raising the
floor to 7.44 with "sha256 weren't usable before that version" as
the justification, it would be a totally different situation and it
may be worth the code change, but I am not sure if going backwards
is worth it.

So, I dunno.

Thanks.