httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: [PATCH 55467] - Updates to mod_ssl to support TLS hello extensions and TLS supplemental data
Date Wed, 09 Apr 2014 16:24:51 GMT
On Fri, Apr 4, 2014 at 7:48 PM, Jeff Trawick <trawick@gmail.com> wrote:

> On Tue, Feb 18, 2014 at 3:50 PM, Scott Deboy <sdeboy@secondstryke.com>wrote:
>
>> Hi folks,
>>
>> I was wondering if someone would be willing/interested in reviewing the
>> patch I've attached to issue 55467.
>>
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=55467
>>
>> The patch adds hooks to mod_ssl which give third-party modules the
>> ability to send and receive custom TLS hello extensions TLS supplemental
>> data.  It also gives third-party modules the ability to trigger
>> renegotiation.  It leverages APIs recently added to OpenSSL master and
>> 1.0.2 stable branches.
>>
>> Any feedback is appreciated!
>>
>>
> Any thoughts out there on passing SSL* to the hook as void* as in the
> patch?  I've been experimenting with some hooks to enable Certificate
> Transparency in a module, and it seemed feasible to me to let mod_ssl.h own
> the job of getting the right headers included in order to specify the right
> OpenSSL datatype on the API.  Is that asking for trouble?
>
> If building with OpenSSL < 1.0.2, the affected optional hooks shouldn't be
> available.
>
> I anticipate syncing my CT code with the pieces
> for SSL_CTX_set_custom_cli_ext()/SSL_CTX_set_custom_srv_ext() and
> committing the relevant parts of your patch (not that the rest is much
> different).  Hopefully some "genuine" mod_ssl developers will render an
> opinion on placement and any other details.
>
>
I started "really" looking at the TLS extension APIs today...  Here are
some notes, hopefully based on a proper understanding of the code :)  :

The optional functions can only be called at startup -- not thread-safe, no
impact on connection anyway.  I guess this would be handled in API
documentation.

SSL_CTX_set_custom_cli_ext() and SSL_CTX_set_custom_srv_ext() fail if a
callback is already registered for the extension type or if a memory
allocation error occurs.  As long as any modules that need to participate
use your APIs, the duplicate registration is handled.  However, the memory
allocation error (or any other future error) is not handled.

In the optional function the module passes userdata (userdata which must be
known at initialization, which severely limits its usefulness).  During the
handshake every module that handles any TLS extension via this API will be
called, and all but one must ignore the call if it isn't the extension type
they registered.  The hook only gets the init-time value passed in, not the
conn_rec (from which the server_rec and all sorts of context-specific
configuration could be found).

I think some changes are appropriate:

a) the module code that actually handles the TLS extension (receiving or
generating) must get the conn_rec (which is easy since mod_ssl retrieves
the conn_rec in the call from OpenSSL.  Given the conn_rec, it doesn't seem
important that the module call also receives its init-time userdata, but it
doesn't hurt anything.

b) since only one module can register to handle a particular TLS extension,
there is no need for mod_ssl to run a hook (which could be accidentally
subverted by some other module failing a call for an extension that it
doesn't handle); instead, the init-time registration of a module's interest
in a TLS extension should provide a callback function; so OpenSSL call's
mod_ssl's callback which retrieves the conn_rec and looks up the module
callback function by TLS extension id

Does that make sense so far?

Another concern is that a module may not actually care about extensions
depending on vhost-specific configuration and/or mode.  The server-side
callbacks are simply ignored if running in a proxy, and similar for the
client-side callbacks.  As long as a module can get to its configuration
(via conn_rec presumably) in the other cases it can filter out calls that
it shouldn't do anything for.

--/--

So why not let the module handle all of this itself, without mod_ssl going
to the trouble of keeping track stuff that OpenSSL already handles?  For my
initial C-T implementation (
https://github.com/trawick/ct-httpd/blob/master/src/proto1/) I added a hook
which the ssl_ct module implements as follows:

static int ssl_ct_ssl_init_ctx(server_rec *s, apr_pool_t *p, int is_proxy,
SSL_CTX *ssl_ctx)
{
    ct_callback_info *cbi = apr_pcalloc(p, sizeof *cbi);
    ct_server_config *sconf = ap_get_module_config(s->module_config,
                                                   &ssl_ct_module);

    cbi->s = s;

    if (is_proxy && sconf->proxy_awareness != PROXY_OBLIVIOUS) {
        /* _cli_ = "client" */
        if (!SSL_CTX_set_custom_cli_ext(ssl_ctx, CT_EXTENSION_TYPE,
                                        client_extension_callback_1,
                                        client_extension_callback_2, cbi)) {
            /* error, abort startup */
        }
    }
    else if configured to do stuff in server mode {
    }

    /* do other stuff unrelated to TLS extensions */

   return OK;
}

mod_ssl calls this in ssl_init_ctx() after all of its existing
configuration.

Thoughts?


>
>
>> Thanks much,
>>
>> Scott
>>
>> On Feb 6, 2014, at 2:20 PM, Scott Deboy <sdeboy@secondstryke.com> wrote:
>>
>> > Support for sending and receiving TLS hello extensions and TLS
>> supplemental data messages has recently been added to the OpenSSL GitHub
>> master branch.
>> >
>> > I've submitted a patch to mod_ssl which allows third-party modules to
>> send and receive TLS hello extensions and TLS supplemental data via
>> optional hooks and functions.
>> >
>> > The patch can be found here:
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=55467
>> >
>> > I'm happy to update the patch based on feedback.
>> >
>> > Thanks much,
>> >
>> > Scott Deboy
>> >
>>
>>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
> http://edjective.org/
>
>


-- 
Born in Roswell... married an alien...
http://emptyhammock.com/
http://edjective.org/

Mime
View raw message