cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] cyassl: remove undefined reference to CyaSSL_no_filesystem_verify & add support for CURLOPT_SSL_CTX_FUNCTION

From: Ray Satiro via curl-library <curl-library_at_cool.haxx.se>
Date: Thu, 26 Mar 2015 02:22:33 -0400

On 3/25/2015 12:10 PM, Kyle L. Huff wrote:
> On Sun, Feb 22, 2015 at 3:05 PM, Kyle L. Huff <kyle.huff_at_curetheitch.com> wrote:
>> The attached patches remove the reference to
>> "CyaSSL_no_filesystem_verify", and enables CURLOPT_SSL_CTX_FUNCTION
>> when using CyaSSL.
> I would like to gauge if there is any interest in implementing the
> above mentioned changes in cURL.
>
> I realize that this is a very low priority, and I am just trying to
> identify if I will need to maintain local patches within my projects
> own build system.
>
> My argument(s) for inclusion -
>
> Patch 1 -
> The changes in the first patch only affect those using CyaSSL and
> NO_FILESYSTEM, so it probably only matters to a very small number of
> users - maybe only me. Given this very small impact, my only arguments
> for patch 1 are -
>
> 1.) The original code (appears to me) to be incorrect (undefined references)
> 2.) Portability when using CyaSSL (systems without a filesystem)

I think your original e-mail slipped through the cracks. It can be found
at [1] if anyone is interested.

It appears this issue has come up before, I found a 2011 bug thread at
[2]. Refer to Dan Fandrich's comment about libcurl requiring a CyaSSL
compiled without NO_FILESYSTEM. Aside from the issue covered in your
patch you are able to use libcurl and CyaSSL NO_FILESYSTEM without any
problems?

> Patch 2 -
> This implements context callback functionality when using CyaSSL,
> which has a potentially wider audience/impact, and my arguments are -
>
> 1.) Consistency with the usage of other SSL libraries
> (CURLOPT_SSL_CTX_FUNCTION)
> 2.) Versatility

This seems like a good idea and it looks like you are not the first [3].
Did you test it, does it work for you? You'll need to adjust the
documentation in the following two files to say it works for OpenSSL and
CyaSSL.

docs/libcurl/opts/CURLOPT_SSL_CTX_DATA.3
docs/libcurl/opts/CURLOPT_SSL_CTX_FUNCTION.3

If a NO_FILESYSTEM really does require a CTX function (just my guess)
then I would do it differently. What I would do is combine your two
patches into one and redo it so you get rid of the
CyaSSL_no_filesystem_verify in the #else block and replace it with
something like this:

#ifndef NO_FILESYSTEM
   existing stuff is here
#else
   if(!data->set.ssl.fsslctx) {
     failf(data,"SSL: CyaSSL no-filesystem requires
CURLOPT_SSL_CTX_FUNCTION");
     return CURLE_SSL_CONNECT_ERROR;
   }
#endif

and the block that calls the callback is fine just get rid of that extra
NO_FILESYSTEM check now since it's covered above.

> None of those are really very strong arguments for inclusion, the
> entire matter is rather insignificant. Again, I am just trying to plan
> accordingly.

It's not insignificant, thanks for the follow up.

[1]: http://curl.haxx.se/mail/lib-2015-02/0172.html
[2]: http://sourceforge.net/p/curl/bugs/1071
[3]:
http://blog.steffenl.com/2013/02/ssl-context-function-with-curl-and.html
[4]: http://curl.haxx.se/dev/contribute.html

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2015-03-26