On 12-07-13 21:54, Dan Fandrich wrote:
> On Fri, Jul 12, 2013 at 05:33:26PM +0200, Patrick Monnerat wrote:
>> Please find a big patch in attachment:

I've taken a look at the code and I've spotted a problem in the reuse of
the variable 'i' in the function Curl_verifyhost():

The result of utf8asn1str(), the strlen(dnsname) and
Curl_cert_hostcheck() seem to each be able to reach the if (i < 0)
clause. Where in the strlen() case you would probably return another
CURL_ error which indicates the internal memory problem on the strlen()
length mismatch.

The Curl_cert_hostcheck() doesn't seem to honor the SSL_VERIFYHOST.

>> - a new module x509asn1.c implements very lightweight ASN.1 and X509
>> parsers, with functions to generate the certinfo from DER certificates.
>> These are now already used (in the patch) by the QsoSSL and GSKit SSL
>> backends, and may be easily called from other backends not implementing
>> certinfo yet.

Cool, but may I add a comment about the names of some structs and
defines which is the lack of a curl_ or CURL_ prefix. The terms used
could easily clash with another SSL backend.

It's not uncommon to use libcurl with a system native backend or using a
private build while being used in an application that also has
(depending library) linkages to another SSL stack.

A curl_/CURL_ prefix to each of these elements which could clash by name
on another backend should be a must fix before inclusion.

> I'm a bit hesitant about this part. It seems that more and more
> X.509/TLS stuff is slowly finding its way into curl itself. The ASN.1
> code especially seems to me to be the kind of thing that should be in
> a cross-platform library of some sort that curl can depend on instead.
> That kind of parsing code is the kind that's hard to get completely
> right from a security standpoint.
>
>>>> Dan

About this I would like to blame the various SSL backends that each
invented their own way of loading the X.509 structures in memory :-)

How would one create one library to rule^Wconstruct them all. They are
all fairly pragmatically compatible due to the spec, but that's about
it. There are also some conceptual difference, like PolarSSL focusing on
a minimal API (one-stop-shop connection and verification) versus OpenSSL
being pretty DIY on the verification end until recent version.

        Oscar

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html