On Tue, 31 Mar 2009, Sajith T S wrote:

> I have some patches to make curl work with CyaSSL.

Thanks. They look like a good start. I'll offer my first review comments
below.

> It is not perfect either, things are likely broken or dysfunctional. But I
> figured that posting them anyway and asking for help is better than putting
> it off forever. When I started this work, I took and patched curl 7.18.2;
> but curl CVS seems to have moved from there since, especially the SSL
> interfaces.

Indeed. We converted to the generic internal SSL interface shortly after
7.18.2... The upside is of course that the new way is much better (less work)
to use when introducing support for new SSL libraries with libcurl!

> * Certificate verification is broken; you have to run "curl -k" for now.
> According to documentation, CyaSSL takes a different route to certificate
> checking, but I have not figured out that yet. Please help there as well.

Uh, where is these docs that explain this?

> * I have tested these specific set of patches only with with 32-bit x86,
> Linux and GCC. It was written originally for a small MIPS box running
> Linux, I do not have access to those right now. If folks running other
> embedded systems architecture are interested, help is much welcomed.

So what have you proven to work? You mention that you don't verify peers and
some sites don't work. How much _does_ work?

I gave the cyassl code a quick glance just now. It seems it isn't thread-safe,
do you know if that's so?

> * Regarding the copyright notice: can we simply copy and paste it from
> elsewhere in curl? cyassl.c is heavily based on example source code budled
> with cyassl, so I think cyassl authors also have to be credited.

We'll of course credit all contributors. But what is perhaps more important is
that there must be no actual ownership claims from those authors as then that
piece of code would be GPL licensed (as that's what cyassl is licensed under)
and I don't want GPL licensed code in libcurl.

> Clearly this is not the highest quality patches I should be offering, all
> feedback/help/review are much appreciated.

Some nits that immediately struck me:

A) why does the code set the socket to non-blocking when it already should
    always and unconditionally be non-blocking? (seems like a plain copy and
    paste from the cyassl example code that isn't needed)

B) the function claims non-blocking connect but is in fact (stupidly) looping
    until it has connected, which is limited libcurl a bit since it actually
    supports for example OpenSSL doing non-blocking handshaking.

C) The configure.ac patch _removes_ some things that it shouldn't and
    instead introduces the old way of dealing with poll...

Given my previous track record with yassl[*] (which is even claimed to be more
complete) I'm not personally going to spend a lot of time on this work until
someone tells me with some faith that this is actually a good and useful SSL
library. But I'm willing to commit patches that look good.

[*] = http://daniel.haxx.se/blog/2008/11/17/what-is-this-yassl-really/

-- 
  / daniel.haxx.se