On Fri, Apr 4, 2014 at 7:48 PM, Jeff Trawick <traw...@gmail.com> wrote:
> On Tue, Feb 18, 2014 at 3:50 PM, Scott Deboy <sde...@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 <sde...@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/