Re: [PATCH] mod_ssl: improving session caching for SNI configurations

[Joe Orton <jo...@redhat.com>]

On Mon, Nov 09, 2009 at 07:06:16PM +0100, Kaspar Brand wrote:
> Dr Stephen Henson wrote:
> > Yes that looks better. There is an alternative technique if it is easier to find
> > a "base" SSL_CTX, you can retrieve the auto generated keys using
> > SSL_CTX_get_tlsext_ticket_keys() and then copy to the new context as above.
> 
> The loop actually iterates over all contexts, so we could just remember
> the keys of the first SSL-enabled vhost, we don't have to find the
> "base" context. I.e., simply replace
> 
>   RAND_bytes(tlsext_tick_keys, tick_keys_len);
> 
> by
> 
>   SSL_CTX_get_tlsext_ticket_keys(sc->server->ssl_ctx,
>                                  tlsext_tick_keys, tick_keys_len);
> 
> I prefer the former, however, because 1) it's shorter, 2) RAND_bytes are
> cheap (aren't they? ;-) and 3) ... it would actually need another
> workaround, for OpenSSL < 0.9.8l, as I realized in the meantime: I
> should have compiled against 0.9.8k for my tests, not 0_9_8-stable -
> because this way I missed the TLXEXT_TICKET_KEYS typo :-/ In the
> attached patches (v4), I've therefore added a workaround for
> SSL_CTRL_SET_TLXEXT_TICKET_KEYS.
> 
> And back to the question whether ap_md5_binary should be used or not, I
> have now switched to apr_sha1 for the trunk version - maybe that's an
> acceptable compromise (use SHA-1 for trunk, stay with MD5 in 2.2.x, for
> backward compatibility)?

Thanks for working on this - sorry that I didn't have time to look into 
this last week.  A few questions:

1) why do we need a new config directive for session ticket support?  
I'm struggling to understand why any server admin would need/want 
control over support for session tickets.

2) I do not think we should be hacking around OpenSSL bugs as the code 
for SSL_CTX_set_tlsext_ticket_keys() is.  People using buggy versions of 
OpenSSL should upgrade to versions which are not buggy, if they are 
affected.  At most here I'd do as your original patch does, to set the 
SSL_OP_NO_TICKET flag if no session cache is enabled.  This is 
sufficient to fix the issue in question, right?

3) The MD5 hash used in the session id context is just a hash.  It has 
no cryptographic use AFAIK and MD5 is a perfectly fine hash, so I don't 
see any need to change that.

I'll commit the session-id-ctx change, that looks good.

Regards, Joe

Re: [PATCH] mod_ssl: improving session caching for SNI configurations

[Kaspar Brand <ht...@velox.ch>]

Joe Orton wrote:
> 1) why do we need a new config directive for session ticket support?  
> I'm struggling to understand why any server admin would need/want 
> control over support for session tickets.

Session tickets are a relatively new thing (RFC 4507 is from May 2006),
and I'm not sure how well current TLS implementations interoperate with
each other. For situations like these, it seems desirable to me if the
admin can control this through a directive (it's not fundamentally
different form knobs like SSLHonorCipherOrder, SSLOptions
OptRenegotiate, SSLRenegBufferSize, and SSLStrictSNIVHostCheck, IMO).

> 2) I do not think we should be hacking around OpenSSL bugs as the code 
> for SSL_CTX_set_tlsext_ticket_keys() is.  People using buggy versions of 
> OpenSSL should upgrade to versions which are not buggy, if they are 
> affected.

OpenSSL versions 0.9.8f through 0.9.8l are probably going to stay around
for quite some time, so why not do these users a favor with a few
(conditionally compiled) lines? It could just be considered a special
case of OpenSSL "feature checking", like #ifdefs for
SSL_OP_CIPHER_SERVER_PREFERENCE or HAVE_SSL_X509V3_EXT_d2i (which are
quite common in mod_ssl).

> At most here I'd do as your original patch does, to set the 
> SSL_OP_NO_TICKET flag if no session cache is enabled.  This is 
> sufficient to fix the issue in question, right?

My original patch disabled tickets only if a session cache *is*
configured (nSessionCacheMode != SSL_SCMODE_NONE), because in that case,
session tickets really make sense - there's no stateful cache, so leave
support for stateless resumption enabled.

Turning off ticket support when no cache is configured seems like a bad
idea to me - it will fix the issue for clients based on OpenSSL <=
0.9.8l, true, but at the cost of making it impossible to run mod_ssl
with stateless resumption only (which I think is a perfectly legitimate
use case).

> 3) The MD5 hash used in the session id context is just a hash.  It has 
> no cryptographic use AFAIK and MD5 is a perfectly fine hash, so I don't 
> see any need to change that.

Correct, the question is how likely a collision of two vhost_ids
(servername:port) is with MD5. Changing to SHA-1 for trunk shouldn't
hurt, however - and might obviate future questions as to why mod_ssl
(still) uses MD5 internally.

Kaspar