Return-Path: X-Original-To: apmail-httpd-dev-archive@www.apache.org Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C1AEB1128B for ; Wed, 9 Apr 2014 16:25:38 +0000 (UTC) Received: (qmail 59566 invoked by uid 500); 9 Apr 2014 16:25:26 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 58713 invoked by uid 500); 9 Apr 2014 16:25:23 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 58676 invoked by uid 99); 9 Apr 2014 16:25:19 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 Apr 2014 16:25:19 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of trawick@gmail.com designates 209.85.217.182 as permitted sender) Received: from [209.85.217.182] (HELO mail-lb0-f182.google.com) (209.85.217.182) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 09 Apr 2014 16:25:13 +0000 Received: by mail-lb0-f182.google.com with SMTP id n15so1324774lbi.41 for ; Wed, 09 Apr 2014 09:24:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=epbrmbeX8NfAMo5kQPcuLz3jOKjy8p46Yk05rJpNW4E=; b=zZaeNzngKlRjx+I4us+Emp42TTnP5As3Sss2CGJUskaeZvuG/8iqFx5EX/gzu3bwov mBWpKRWUw+A2psU5ac0mi/rODzBv6n09zcRUFUpomrN81Q+UE4xogTpsl0g4QgiGCyYf otv5WBHGLxrSBgf8wIBQ3JqxZjTpWYWwOQNHmm9O0IXKeG2l1VOXiP03kGQh6VHif9lf O5cHI2yfzmR+Qd1qCiJlyw1KqBaqhcxLHg1y4UX63VK8aWxTw3Zh7UNsf1cKHMPkbOVF vJ4Xhky9lbrjNLPrsmp7kF1EHHkMxewJ3S6F4yakVfD2vG1OOCqXHkg+H/X7c85K5/77 9c4A== MIME-Version: 1.0 X-Received: by 10.152.87.102 with SMTP id w6mr1945758laz.46.1397060691207; Wed, 09 Apr 2014 09:24:51 -0700 (PDT) Received: by 10.114.170.132 with HTTP; Wed, 9 Apr 2014 09:24:51 -0700 (PDT) In-Reply-To: References: <3B42C4E4-59AF-49F1-B261-6722A0918F94@secondstryke.com> Date: Wed, 9 Apr 2014 10:24:51 -0600 Message-ID: Subject: Re: [PATCH 55467] - Updates to mod_ssl to support TLS hello extensions and TLS supplemental data From: Jeff Trawick To: Apache HTTP Server Development List Content-Type: multipart/alternative; boundary=001a11c3501a3aac8d04f69e8a8a X-Virus-Checked: Checked by ClamAV on apache.org --001a11c3501a3aac8d04f69e8a8a Content-Type: text/plain; charset=ISO-8859-1 On Fri, Apr 4, 2014 at 7:48 PM, Jeff Trawick wrote: > On Tue, Feb 18, 2014 at 3:50 PM, Scott Deboy 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 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/ --001a11c3501a3aac8d04f69e8a8a Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On F= ri, Apr 4, 2014 at 7:48 PM, Jeff Trawick <trawick@gmail.com>= wrote:


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_custo= m_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. &= nbsp;As long as any modules that need to participate use your APIs, the dup= licate registration is handled.  However, the memory allocation error = (or any other future error) is not handled.

In the optional function the module passes userda= ta (userdata which must be known at initialization, which severely limits i= ts usefulness).  During the handshake every module that handles any TL= S extension via this API will be called, and all but one must ignore the ca= ll 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 (receivi= ng or generating) must get the conn_rec (which is easy since mod_ssl retrie= ves the conn_rec in the call from OpenSSL.  Given the conn_rec, it doe= sn't seem important that the module call also receives its init-time us= erdata, but it doesn't hurt anything.

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

Does that make sense so far?

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

--/--


static int ssl_ct_ssl_init_ctx(server_rec *s, apr_= pool_t *p, int is_proxy, SSL_CTX *ssl_ctx)
{
  &nb= sp; ct_callback_info *cbi =3D apr_pcalloc(p, sizeof *cbi);
 =   ct_server_config *sconf =3D ap_get_module_config(s->module_confi= g,
                    =                      = ;          &ssl_ct_module);

    cbi->s =3D s;

   = ; if (is_proxy && sconf->proxy_awareness !=3D PROXY_OBLIVIOUS) {=
        /* _cli_ =3D "client" */
        if (!SSL_CTX_set_custom_cli_ext(ssl_ctx, CT_= EXTENSION_TYPE,
              =                      = ;     client_extension_callback_1,
     =                     &nbs= p;             client_extension_callback_2, c= bi)) {
            /* error, abort startu= p */
        }
    }
    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 c= onfiguration.

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 supplem= ental data messages has recently been added to the OpenSSL GitHub master br= anch.
>
> I’ve submitted a patch to mod_ssl which allows third-party modul= es to send and receive TLS hello extensions and TLS supplemental data via o= ptional hooks and functions.
>
> The patch can be found here: https://issues.apache.org/b= ugzilla/show_bug.cgi?id=3D55467
>
> I’m happy to update the patch based on feedback.
>
> Thanks much,
>
> Scott Deboy
>




--
Born in Ros= well... married an alien...
http://emptyhammock.com/




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

--001a11c3501a3aac8d04f69e8a8a--