Re: [PATCH] http.c: Add config options/parsing for SSL engine vars

[Jeff King <peff@peff.net>]

On Tue, Apr 30, 2013 at 01:17:03PM -0700, Junio C Hamano wrote:

> Jerry Qassar <jqassar@gmail.com> writes:
> 
> > Curl already does support engine-based certificates (in code and
> > help).  Its problem is that a) it doesn't yet read your engine
> > defs out of OpenSSL config, and b) a bug in copying the engine
> > data, once that's patched, to the handle that calling apps use.
> 
> So once the problem (a) is fixed, if the user has OpenSSL config
> then the user doesn't need configuration from setopt() side?  That
> makes it sound like you do not need to patch us at all, but there
> must be something else going on...

My understanding is that we first have to tell curl "yes, use the
engine", and then the engine-specific OpenSSL config can be loaded by
curl. But I am just guessing from the conversation up until now; I know
nothing about ssl crypto engines.

As an aside, curl can be linked against gnutls, too. Does any of this
work with gnutls? I think we don't have to care; curl abstracts all of
that away from us, and it is up to the user to choose an engine that
matches their library versions. But it might be a good point of
reference when somebody later comes to the list and says "I followed the
documentation, but it doesn't work".

> > Errors are handled by curl (up to this point):
> >
> > 1) Setting the cert type to FOO:
> > error: not supported file type 'FOO' for certificate...
> > fatal: HTTP request failed
> >
> > 2) Setting the key type to FOO:
> > error: not supported file type for private key...
> > fatal: HTTP request failed
> >
> > 3) Setting engine type to something invalid:
> >  * SSL Engine 'pkcsfoo' not found (only with GIT_CURL_VERBOSE set)
> > error: crypto engine not set, can't load certificate...
> > fatal: HTTP request failed
> 
> Where do "error:" and "fatal:" happen in the codeflow?
>
> I am guessing that "error:" may come from these easy_setopt() calls, but
> the "fatal: HTTP request failed" come from us, much later in the
> callpath when we actually make http request.

Those are almost certainly from curl_errorstr() when we make the
info/refs http request.

> Between these two times, aren't we throwing user data at the cURL
> library and possibly over the wire to the remote side (with a SSL
> configuration that is different from what the user intended to use),
> no?

I assume that curl is smart enough not to send any data over the wire,
and that it is noticing early in the process that something is wrong and
is barfing there.

It would be nicer to notice earlier (when we are setting up the handle),
but in practice I don't think it matters. We start off all http
conversations by making a short GET, and we don't do any significant
work beforehand. So as long as curl does not do significant work before
hitting those errors internally, it probably does not matter much either
way.

-Peff