Bug#828608: xmltooling: FTBFS with openssl 1.1.0

[Kurt Roeckx]

On Wed, Nov 09, 2016 at 07:13:58PM +0100, Ferenc Wágner wrote:
>             struct curl_tlssessioninfo* tlsinfo = nullptr;
>             CURLcode infocode = curl_easy_getinfo(ctx->m_handle, CURLINFO_TLS_SSL_PTR, &tlsinfo);
>             if (infocode == CURLE_OK && tlsinfo && tlsinfo->backend == CURLSSLBACKEND_OPENSSL && tlsinfo->internals) {
>                 SSL* ssl = reinterpret_cast<SSL*>(tlsinfo->internals);
>                 const SSL_CIPHER* cipher = SSL_get_current_cipher(ssl);
>                 log.debug("SSL version: %s, cipher: %s", SSL_get_version(ssl), cipher ? SSL_CIPHER_get_name(cipher) : "unknown");

Can I just say this is really ugly code? It's called "internal",
you really have no business of touching this. And that just for
some debug log.

> xmltooling::xml_ssl_ctx_callback(CURL* curl, SSL_CTX* ssl_ctx, void* userptr)
> {

Since libcurl seems to be exposing this functionality, I think you
need to use the same version of openssl than libcurl is using.
libcurl really shouldn't have been exposing this.

>     CURLSOAPTransport* conf = reinterpret_cast<CURLSOAPTransport*>(userptr);
> 
>     // Default flags manually disable SSLv2 and SSLv3 so we're not dependent on libcurl
>     // to do it. Also disable the ticket option where implemented, since this breaks a
>     // variety of servers. Newer libcurl also does this for us.
> #ifdef SSL_OP_NO_TICKET
>     SSL_CTX_set_options(ssl_ctx, conf->m_openssl_ops|SSL_OP_NO_TICKET);
> #else
>     SSL_CTX_set_options(ssl_ctx, conf->m_openssl_ops);
> #endif

I guess this is from the day that there were servers that where
tls extension intolerant. They all really should support this now.
But libcurl just ignores the ticket anyway, but does support the
session id, so it's actually good that curl currently sets this
itself. But curl should really add support for session tickets
instead.

> #ifndef XMLTOOLING_NO_XMLSEC
>     if (conf->m_cred)
>         conf->m_cred->attach(ssl_ctx);
> 
>     if (conf->m_trustEngine) {
>         SSL_CTX_set_verify(ssl_ctx,SSL_VERIFY_PEER,nullptr);

Curl should be doing this by default, and actually has a setting
for this ...

> #if (OPENSSL_VERSION_NUMBER >= 0x00907000L)
>         // With 0.9.7, we can pass a callback argument directly.
>         SSL_CTX_set_cert_verify_callback(ssl_ctx,verify_callback,userptr);

Is there a reason it implements it's own certificate verification
rather than using curl's? I guess curl didn't do this always.

> #else
>         // With 0.9.6, there's no argument, so we're going to use a really embarrassing hack and
>         // stuff the argument in the depth property where it will get copied to the context object
>         // that's handed to the callback.
>         SSL_CTX_set_cert_verify_callback(ssl_ctx,reinterpret_cast<int (*)()>(verify_callback),nullptr);
>         SSL_CTX_set_verify_depth(ssl_ctx,reinterpret_cast<int>(userptr));

Code that still supports 0.9.6? Can I suggest you rip that?

It at least looks to me you could just remove all code that is
related to libssl and let curl do it for you. But it's of course
still using libcrypto for other things.


Kurt