On Thu, Mar 26, 2015 at 2:22 AM, Ray Satiro via curl-library
<curl-library_at_cool.haxx.se> wrote:
> I think your original e-mail slipped through the cracks. It can be found at
> [1] if anyone is interested.

Thanks for providing a reference to the original email, that slipped my mind.

> 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.

Yes, that report seems to deal with the same issue.

Regarding Dan Fandrich's comment: when compiled with NO_FILESYSTEM,
CyaSSL has no way to verify a certificate without the ability to
obtain a handle to the context, that part is true. However, using
NO_FILESYSTEM and setting CURLOPT_SSL_VERIFYPEER to false is a
functional configuration (however ill-advised...). So the changes in
the first patch produce a sane result - you can compile with
NO_FILESYSTEM, and if VERIFYPEER (CURLOPT_SSL_VERIFYPEER) is enabled
(default), the connection will fail, as it should, but if the
application disables VERIFYPEER, the connection will complete as
expected.

So I would consider the first patch to be a bug-fix, really. A bug-fix
that removes the undefined reference, and allows cURL to attempt to
establish a connection, taking the configuration specified by the
application into account (VERIFYPEER, in this case).

> Aside from the issue covered in your patch you are
> able to use libcurl and CyaSSL NO_FILESYSTEM without any problems?

Yes. I have tested it on Linux, OSX and Windows.

>> 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?

Yes, same as above - tested on Linux, OSX and Windows.

> You'll need to adjust the documentation
> in the following two files to say it works for OpenSSL and CyaSSL.
> ...

No problem. Once we nail down the specifics, I will regenerate my
patches and include the related documentation changes. Should the
documentation for a new feature be a separate patch, or combined with
the patch that implements the feature?

> 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

That seems to make the context callback functionality only available
when using NO_FILESYSTEM. I imagine that there are other legitimate
uses for a context callback, even when a file system is present.
Additionally, as mentioned above, using NO_FILESYSTEM without a
context callback is valid.

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

If, as mentioned above, the context callback functionality really
should exist *only* when NO_FILESYSTEM is enabled, then I would agree
with you. Otherwise, this block is needed to catch when NO_FILESYSTEM
is true, no context callback has been specific *and* VERIFYPEER is
true (no possible way to verify without a certificate), and does so
with the least possible interruption to the overall flow.

As for combining the patches, I can do that if you like. I only kept
them separate as, I would consider the first to be a bug-fix, and the
second to be a feature implementation.

Please let me know your thoughts. I will provide patches (or a single
patch, please confirm) for whatever changes are deemed necessary.
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2015-03-26