bug#28948: feh does encounter certificate errors with valid certificates

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]

Ricardo Wurmus <rekado@elephly.net> writes:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> ng0 <ng0@infotropique.org> writes:
>>
>>> feh https://i.imgur.com/263enxT.jpg
>>> feh opens image
>>>
>>> Problem:
>>> user@abyayala ~/src/guix/guix$ feh https://i.imgur.com/263enxT.jpg
>>> feh WARNING: open url: server certificate verification failed. CAfile: none CRLfile: none
>>> feh WARNING: https://i.imgur.com/263enxT.jpg - File does not exist
>>> feh: No loadable images specified.
>>> See 'man feh' for detailed usage information
>>>
>>> nss etc are in my profile, no problem with other curl based applications.
>>
>> The attached patch should fix the problem.  Can you try it?
>
> We’ve done something similar in r-curl IIRC.  I wonder if we should just
> patch libcurl, so that all users of libcurl would benefit from this change.

IIRC the reason it's not supported in libcurl is because getenv() is not
thread-safe, whereas libcurl is designed to be.

>
>> +diff --git a/src/imlib.c b/src/imlib.c
>> +index dfb79aa..82a9865 100644
>> +--- a/src/imlib.c
>> ++++ b/src/imlib.c
>> +@@ -429,6 +429,10 @@ static char *feh_http_load_image(char *url)
>> + 			if (opt.insecure_ssl) {
>> + 				curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0);
>> + 				curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);
>> ++			} else {
>> ++				// Allow the user to specify custom CA certificates.
>> ++				curl_easy_setopt(curl, CURLOPT_CAINFO,
>> ++						getenv("CURL_CA_BUNDLE"));
>> + 			}
>
> Is it safe to pass the empty string to curl_easy_setopt, in case
> CURL_CA_BUNDLE is unset?  Do we need to check the value first or can we
> pass it without checking?

getenv() returns NULL if the variable is unset.  I'm not sure if it
would reset the default on other distros, but it makes no difference for
Guix since libcurl does not have a default CA bundle and handles NULL
here gracefully.

I submitted it upstream in hope of getting feedback/testing there, but
it was simply merged as-is: <https://github.com/derf/feh/pull/340>

I do agree that it's rather crude, will try to improve it a bit.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]