Re: svn commit: r1200040 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

2011-11-20 Thread Kaspar Brand
On 15.11.2011 17:14, Paul Querna wrote:
> Maybe explaining it as such is easier to understand:
> 
> SSLTicketKeyFile basically gives you a list of possible decryption keys.
> 
> SSLTicketKeyDefault picks which one to use for encryption.  If
> SSLTicketKeyDefault is not set, the first added decryption key is
> used.

I see. What I don't completely understand yet, however, is the need /
use case for keeping multiple decryption keys around per
SSLSrvConfigRec. When switching to a new key (with a reload/restart),
session tickets encrypted with the previous keys should no longer get
decrypted - otherwise those sessions effectively become perpetual... or
am I overlooking something?

I.e., could we just drop the SSLTicketKeyDefault directive and remove
the "keyname" part from SSLTicketKeyFile, so that there's simply one
ticket key (file) per SSLSrvConfigRec? That would make the configuration
simpler, IMO.

>>>  Changes with Apache 2.3.16
>>>
>>> +  *) mod_ssl: Add support for RFC 5077 TLS Session tickets.
>>> + [Paul Querna]
>>
>> This is somewhat misleading, I think. Session tickets are supported in
>> mod_ssl as soon as you compile it against OpenSSL 0.9.8f or later (they
>> default to on in OpenSSL, SSL_OP_NO_TICKET would have to be set
>> otherwise). What your patch adds, OTOH, is allowing explicit control of
>> the ticket encryption/decryption keys.
> 
> Sorry, this is correct.   Its not adding support to them in a single
> cluster, its making them configurable / controlable by the user --
> OpenSSL by default does generate random keys, but in a cluster of
> servers this makes session tickets basically useless.

Ok, then I it would be good to reword the entry in the CHANGES file a bit.

> for someone running a single httpd, if we can ensure that the random
> session ticket is generated once and the same across all children
> processes (which I am unsure it is?), then that would be better.

This is the case, yes. OpenSSL generates the keys in SSL_CTX_new (which
is called by mod_ssl at startup time). ssl3_send_newsession_ticket()
then uses these ticket keys by grabbing the SSL_CTX's initial_ctx -
unless you have an OpenSSL release between 0.9.8f and 0.9.8l (see also
http://cvs.openssl.org/chngview?cn=18770).

> For anyone running multiple httpd instances however, this is a very
> helpful feature and reduces the need for an external session cache
> store in memcache or such.

I agree, but then we should also document this feature.

Kaspar


Re: svn commit: r1200040 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

2011-11-15 Thread Paul Querna
On Sun, Nov 13, 2011 at 2:34 AM, Kaspar Brand  wrote:
> On 10.11.2011 00:37, pque...@apache.org wrote:
>> Author: pquerna
>> Date: Wed Nov  9 23:37:37 2011
>> New Revision: 1200040
>>
>> URL: http://svn.apache.org/viewvc?rev=1200040&view=rev
>> Log:
>> Add support for RFC 5077 TLS Session tickets.  This adds two new directives:
>>
>> * SSLTicketKeyFile: To store the private information for the encryption of 
>> the ticket.
>> * SSLTicketKeyDefault To set the default, otherwise the first listed token 
>> is used.  This enables key rotation across servers.
>
> It's not completely clear to me how these two directives interact - what
> does "first listed token" relate to? Can multiple SSLTicketKeyFile
> directives appear within a VirtualHost?


Yes.

Maybe explaining it as such is easier to understand:

SSLTicketKeyFile basically gives you a list of possible decryption keys.

SSLTicketKeyDefault picks which one to use for encryption.  If
SSLTicketKeyDefault is not set, the first added decryption key is
used.


>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Nov  9 23:37:37 2011
>> @@ -1,6 +1,9 @@
>>                                                           -*- coding: utf-8 
>> -*-
>>  Changes with Apache 2.3.16
>>
>> +  *) mod_ssl: Add support for RFC 5077 TLS Session tickets.
>> +     [Paul Querna]
>
> This is somewhat misleading, I think. Session tickets are supported in
> mod_ssl as soon as you compile it against OpenSSL 0.9.8f or later (they
> default to on in OpenSSL, SSL_OP_NO_TICKET would have to be set
> otherwise). What your patch adds, OTOH, is allowing explicit control of
> the ticket encryption/decryption keys.

Sorry, this is correct.   Its not adding support to them in a single
cluster, its making them configurable / controlable by the user --
OpenSSL by default does generate random keys, but in a cluster of
servers this makes session tickets basically useless.

>> Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1200040&r1=1200039&r2=1200040&view=diff
>> ==
>> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Wed Nov  9 23:37:37 2011
>> @@ -79,6 +79,14 @@ static const command_rec ssl_config_cmds
>>      SSL_CMD_SRV(FIPS, FLAG,
>>                  "Enable FIPS-140 mode "
>>                  "(`on', `off')")
>> +#ifdef HAVE_TLSEXT_TICKETS
>> +    SSL_CMD_SRV(TicketKeyFile, TAKE2,
>> +                "Key file to use for encrypting and decrypting the client 
>> ticket (RFC 5077) "
>> +                "(keyname '/path/to/file')")
>
> I suggest to add some info about the contents of these files (like "48
> random bytes in binary format"). Also, the documentation of this
> directive should encourage users to regularly change these keys.
>
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1200040&r1=1200039&r2=1200040&view=diff
>> ==
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Wed Nov  9 23:37:37 
>> 2011
>> @@ -200,6 +200,12 @@ static SSLSrvConfigRec *ssl_config_serve
>>      sc->fips                   = UNSET;
>>  #endif
>>
>> +#ifdef HAVE_TLSEXT_TICKETS
>> +    sc->default_ticket_name = NULL;
>> +    sc->default_ticket = NULL;
>> +    sc->tickets = apr_array_make(p, 4, sizeof(modssl_ticket_t*));
>
> Maybe a stupid question, but I don't (yet) see the reason for using an
> array with four elements... could you perhaps shed some more light on this?

APR array make pre-allocates this much space.  If you use more than 4
elements, it needs to do another allocation.  If you use less than 4,
it will only allocate memory here.  It is not a limit.

>> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=1200040&r1=1200039&r2=1200040&view=diff
>> ==
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Wed Nov  9 23:37:37 
>> 2011
>> @@ -2067,3 +2067,94 @@ static int ssl_find_vhost(void *serverna
>>      return 0;
>>  }
>>  #endif
>> +
>> +#ifdef HAVE_TLSEXT_TICKETS
>> +
>> +#ifndef tlsext_tick_md
>> +#ifdef OPENSSL_NO_SHA256
>> +#define tlsext_tick_md       EVP_sha1
>> +#else
>> +#define tlsext_tick_md       EVP_sha256
>> +#endif
>> +#endif
>
> That's something which belongs into ssl_private.h, I think.
>
>
> As a general comment, I would like to see some guidelines in the
> documentation as to when an explicit configuration of TLS session ticket

Re: svn commit: r1200040 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

2011-11-13 Thread Kaspar Brand
On 10.11.2011 00:37, pque...@apache.org wrote:
> Author: pquerna
> Date: Wed Nov  9 23:37:37 2011
> New Revision: 1200040
> 
> URL: http://svn.apache.org/viewvc?rev=1200040&view=rev
> Log:
> Add support for RFC 5077 TLS Session tickets.  This adds two new directives:
> 
> * SSLTicketKeyFile: To store the private information for the encryption of 
> the ticket.
> * SSLTicketKeyDefault To set the default, otherwise the first listed token is 
> used.  This enables key rotation across servers.

It's not completely clear to me how these two directives interact - what
does "first listed token" relate to? Can multiple SSLTicketKeyFile
directives appear within a VirtualHost?

> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Wed Nov  9 23:37:37 2011
> @@ -1,6 +1,9 @@
>   -*- coding: utf-8 
> -*-
>  Changes with Apache 2.3.16
>  
> +  *) mod_ssl: Add support for RFC 5077 TLS Session tickets.
> + [Paul Querna]

This is somewhat misleading, I think. Session tickets are supported in
mod_ssl as soon as you compile it against OpenSSL 0.9.8f or later (they
default to on in OpenSSL, SSL_OP_NO_TICKET would have to be set
otherwise). What your patch adds, OTOH, is allowing explicit control of
the ticket encryption/decryption keys.

> Modified: httpd/httpd/trunk/modules/ssl/mod_ssl.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/mod_ssl.c?rev=1200040&r1=1200039&r2=1200040&view=diff
> ==
> --- httpd/httpd/trunk/modules/ssl/mod_ssl.c (original)
> +++ httpd/httpd/trunk/modules/ssl/mod_ssl.c Wed Nov  9 23:37:37 2011
> @@ -79,6 +79,14 @@ static const command_rec ssl_config_cmds
>  SSL_CMD_SRV(FIPS, FLAG,
>  "Enable FIPS-140 mode "
>  "(`on', `off')")
> +#ifdef HAVE_TLSEXT_TICKETS
> +SSL_CMD_SRV(TicketKeyFile, TAKE2,
> +"Key file to use for encrypting and decrypting the client 
> ticket (RFC 5077) "
> +"(keyname '/path/to/file')")

I suggest to add some info about the contents of these files (like "48
random bytes in binary format"). Also, the documentation of this
directive should encourage users to regularly change these keys.

> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1200040&r1=1200039&r2=1200040&view=diff
> ==
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Wed Nov  9 23:37:37 2011
> @@ -200,6 +200,12 @@ static SSLSrvConfigRec *ssl_config_serve
>  sc->fips   = UNSET;
>  #endif
>  
> +#ifdef HAVE_TLSEXT_TICKETS
> +sc->default_ticket_name = NULL;
> +sc->default_ticket = NULL;
> +sc->tickets = apr_array_make(p, 4, sizeof(modssl_ticket_t*));

Maybe a stupid question, but I don't (yet) see the reason for using an
array with four elements... could you perhaps shed some more light on this?

> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?rev=1200040&r1=1200039&r2=1200040&view=diff
> ==
> --- httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c (original)
> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c Wed Nov  9 23:37:37 2011
> @@ -2067,3 +2067,94 @@ static int ssl_find_vhost(void *serverna
>  return 0;
>  }
>  #endif
> +
> +#ifdef HAVE_TLSEXT_TICKETS
> +
> +#ifndef tlsext_tick_md
> +#ifdef OPENSSL_NO_SHA256
> +#define tlsext_tick_md   EVP_sha1
> +#else
> +#define tlsext_tick_md   EVP_sha256
> +#endif
> +#endif

That's something which belongs into ssl_private.h, I think.


As a general comment, I would like to see some guidelines in the
documentation as to when an explicit configuration of TLS session ticket
keys really makes sense - and how to create/maintain the key files, in
this case. For a default standalone setup, people are still better off
with using OpenSSL's default of randomly generated session keys, IMO
(except that the lifetime of these keys might be too long, if httpd is
run for longer periods without any reload/restart).

Kaspar


Re: svn commit: r1200040 - in /httpd/httpd/trunk: CHANGES modules/ssl/mod_ssl.c modules/ssl/ssl_engine_config.c modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_private.h

2011-11-10 Thread Paul Querna
On Thu, Nov 10, 2011 at 12:14 AM, Rüdiger Plüm
 wrote:

> Author: pquerna
> Date: Wed Nov  9 23:37:37 2011
> New Revision: 1200040
>
> URL: http://svn.apache.org/viewvc?rev=1200040&view=rev
> Log:
> Add support for RFC 5077 TLS Session tickets.  This adds two new directives:
>
> * SSLTicketKeyFile: To store the private information for the encryption of 
> the ticket.
> * SSLTicketKeyDefault To set the default, otherwise the first listed token is 
> used.  This
> enables key rotation across servers.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/ssl/mod_ssl.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_init.c
> httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
> httpd/httpd/trunk/modules/ssl/
> ssl_private.h
.
> +const char *ssl_cmd_SSLTicketKeyFile(cmd_parms *cmd, void *dcfg, const char 
> *name, const
> char *path)
> +{
> +#ifdef HAVE_TLSEXT_TICKETS
> +apr_status_t rv;
> +apr_file_t *fp;
> +apr_size_t len;
> +char buf[TLSEXT_TICKET_KEYLEN];
> +modssl_ticket_t* ticket = NULL;
> +SSLSrvConfigRec *sc = mySrvConfig(cmd->server);
> +
> +rv = apr_file_open(&fp, path, APR_READ|APR_BINARY,
>
>
>
> Why not using ap_server_root_relative on path first?

Fixed in r1200372.


> +
> +memcpy(keyname, ticket->key_name, 16);
> +
> +RAND_pseudo_bytes(iv, EVP_MAX_IV_LENGTH);
> +
> +memcpy(iv, iv, EVP_MAX_IV_LENGTH);
>
>
> What is the purpose of this operation? Source and destination are the same.

Unneeded, No Purpose, I had an earlier version of the code when I used
a temp local buffer to generate the IV, but later just wrote directly
into the parameter with RAND_pseudo_bytes.  Removed in r1200374.


> Regards
>
> Rüdiger

Thanks again,

Paul