Re: [PATCH] mod_ssl: improving session caching for SNI configurations
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
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
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
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)? Could one of the httpd committers take over and make a decision, therefore...? Help with getting this into the tree would be much appreciated - thanks! Kaspar Index: httpd-trunk/modules/ssl/ssl_private.h === --- httpd-trunk/modules/ssl/ssl_private.h (revision 833672) +++ httpd-trunk/modules/ssl/ssl_private.h (working copy) @@ -405,6 +405,9 @@ typedef struct { #if defined(HAVE_OPENSSL_ENGINE_H) defined(HAVE_ENGINE_INIT) const char *szCryptoDevice; #endif +#ifndef OPENSSL_NO_TLSEXT +ssl_enabled_t session_tickets_enabled; +#endif #ifdef HAVE_OCSP_STAPLING const ap_socache_provider_t *stapling_cache; @@ -585,6 +588,7 @@ const char *ssl_cmd_SSLUserName(cmd_par const char *ssl_cmd_SSLLogLevelDebugDump(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg); const char *ssl_cmd_SSLStrictSNIVHostCheck(cmd_parms *cmd, void *dcfg, int flag); +const char *ssl_cmd_SSLSessionTicketExtension(cmd_parms *cmd, void *cdfg, int flag); const char *ssl_cmd_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag); const char *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *); Index: httpd-trunk/modules/ssl/ssl_engine_init.c === --- httpd-trunk/modules/ssl/ssl_engine_init.c (revision 833672) +++ httpd-trunk/modules/ssl/ssl_engine_init.c (working copy) @@ -363,6 +363,15 @@ static void ssl_init_server_check(server (theoretically shouldn't happen!)); ssl_die(); } + +/* + * Session tickets (stateless resumption) + */ +if ((myModConfig(s))-session_tickets_enabled == SSL_ENABLED_FALSE) { +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + Disabling TLS session ticket support); +SSL_CTX_set_options(mctx-ssl_ctx, SSL_OP_NO_TICKET); +} } #ifndef OPENSSL_NO_TLSEXT @@ -1061,6 +1070,11 @@ void ssl_init_CheckServers(server_rec *b BOOL conflict = FALSE; +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +unsigned char *tlsext_tick_keys = NULL; +long tick_keys_len; +#endif + /* * Give out warnings when a server has HTTPS configured * for the HTTP port or vice versa @@ -1085,6 +1099,27 @@ void ssl_init_CheckServers(server_rec *b ssl_util_vhostid(p, s), DEFAULT_HTTP_PORT, DEFAULT_HTTPS_PORT); } + +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +/* we have to work around a typo in OpenSSL 0.9.8l, too */ +#define SSL_CTRL_SET_TLXEXT_TICKET_KEYS SSL_CTRL_SET_TLSEXT_TICKET_KEYS +/* + * When using OpenSSL versions 0.9.8f through 0.9.8l, configure + * the same ticket encryption parameters for every SSL_CTX (workaround + * for SNI+SessionTicket extension interoperability issue in these versions) + */ +if ((sc-enabled == SSL_ENABLED_TRUE) || +(sc-enabled == SSL_ENABLED_OPTIONAL)) { +if (!tlsext_tick_keys) { +tick_keys_len = SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, + NULL, -1); +tlsext_tick_keys = (unsigned char *)apr_palloc(p, tick_keys_len); +RAND_bytes(tlsext_tick_keys, tick_keys_len); +} +SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, + tlsext_tick_keys, tick_keys_len); +} +#endif } /* Index: httpd-trunk/modules/ssl/ssl_engine_config.c
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Kaspar Brand wrote: Does that sound reasonable? If so, I would prepare a new patch with SSL_CTX_set_tlsext_ticket_keys and the new config directive. No reactions = no objections? Would it perhaps be possible to piggyback onto Joe's reneg patch and get this also into 2.2.15...? ;-) Attached are updated patches for trunk and 2.2.x which: - fix TLS session tickets for SNI configurations when OpenSSL versions between 0.9.8f and 0.9.8l are used - add a new (global) SSLSessionTicketExtension configuration directive which allows controlling SSL_OP_NO_TICKET (defaulting to on, i.e. tickets are left enabled, but can be turned off if necessary) - include the fix for the SNI callback which makes sure that the correct session id context is set (to prevent improper session resumption). Kaspar Index: httpd-trunk/modules/ssl/ssl_private.h === --- httpd-trunk/modules/ssl/ssl_private.h (revision 833672) +++ httpd-trunk/modules/ssl/ssl_private.h (working copy) @@ -405,6 +405,9 @@ typedef struct { #if defined(HAVE_OPENSSL_ENGINE_H) defined(HAVE_ENGINE_INIT) const char *szCryptoDevice; #endif +#ifndef OPENSSL_NO_TLSEXT +ssl_enabled_t session_tickets_enabled; +#endif #ifdef HAVE_OCSP_STAPLING const ap_socache_provider_t *stapling_cache; @@ -585,6 +588,7 @@ const char *ssl_cmd_SSLUserName(cmd_par const char *ssl_cmd_SSLLogLevelDebugDump(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg); const char *ssl_cmd_SSLStrictSNIVHostCheck(cmd_parms *cmd, void *dcfg, int flag); +const char *ssl_cmd_SSLSessionTicketExtension(cmd_parms *cmd, void *cdfg, int flag); const char *ssl_cmd_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag); const char *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *); Index: httpd-trunk/modules/ssl/ssl_engine_init.c === --- httpd-trunk/modules/ssl/ssl_engine_init.c (revision 833672) +++ httpd-trunk/modules/ssl/ssl_engine_init.c (working copy) @@ -363,6 +363,15 @@ static void ssl_init_server_check(server (theoretically shouldn't happen!)); ssl_die(); } + +/* + * Session tickets (stateless resumption) + */ +if ((myModConfig(s))-session_tickets_enabled == SSL_ENABLED_FALSE) { +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + Disabling TLS session ticket support); +SSL_CTX_set_options(mctx-ssl_ctx, SSL_OP_NO_TICKET); +} } #ifndef OPENSSL_NO_TLSEXT @@ -1061,6 +1070,14 @@ void ssl_init_CheckServers(server_rec *b BOOL conflict = FALSE; +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +#define TICK_KEYS_LEN sizeof(((SSL_CTX *)0)-tlsext_tick_key_name) \ + + sizeof(((SSL_CTX *)0)-tlsext_tick_hmac_key) \ + + sizeof(((SSL_CTX *)0)-tlsext_tick_aes_key) +unsigned char tlsext_tick_keys[TICK_KEYS_LEN]; +RAND_pseudo_bytes(tlsext_tick_keys, TICK_KEYS_LEN); +#endif + /* * Give out warnings when a server has HTTPS configured * for the HTTP port or vice versa @@ -1085,6 +1102,18 @@ void ssl_init_CheckServers(server_rec *b ssl_util_vhostid(p, s), DEFAULT_HTTP_PORT, DEFAULT_HTTPS_PORT); } + +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +/* + * When using OpenSSL versions 0.9.8f through 0.9.8l, also configure + * the same key_name/hmac_key/aes_key combo for every SSL_CTX (workaround + * for SNI+SessionTicket extension interoperability issue in these versions) + */ +if ((sc-enabled == SSL_ENABLED_TRUE) || +(sc-enabled == SSL_ENABLED_OPTIONAL)) +SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, + tlsext_tick_keys, TICK_KEYS_LEN); +#endif } /* Index: httpd-trunk/modules/ssl/ssl_engine_config.c === --- httpd-trunk/modules/ssl/ssl_engine_config.c (revision 833672) +++ httpd-trunk/modules/ssl/ssl_engine_config.c (working copy) @@ -80,6 +80,9 @@ SSLModConfigRec *ssl_config_global_creat mc-stapling_mutex_file= NULL; mc-stapling_mutex = NULL; #endif +#ifndef OPENSSL_NO_TLSEXT +mc-session_tickets_enabled = SSL_ENABLED_UNSET; +#endif memset(mc-pTmpKeys, 0, sizeof(mc-pTmpKeys)); @@ -1673,6 +1676,26 @@ const char *ssl_cmd_SSLStaplingForceURL( #endif /* HAVE_OCSP_STAPLING */ +const char *ssl_cmd_SSLSessionTicketExtension(cmd_parms *cmd, void *dcfg, int flag) +{ +#ifndef OPENSSL_NO_TLSEXT +const char *err; +SSLModConfigRec *mc = myModConfig(cmd-server); + +if ((err = ap_check_cmd_context(cmd, GLOBAL_ONLY))) { +return err; +} + +
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Kaspar Brand wrote: +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +#define TICK_KEYS_LEN sizeof(((SSL_CTX *)0)-tlsext_tick_key_name) \ + + sizeof(((SSL_CTX *)0)-tlsext_tick_hmac_key) \ + + sizeof(((SSL_CTX *)0)-tlsext_tick_aes_key) +unsigned char tlsext_tick_keys[TICK_KEYS_LEN]; +RAND_pseudo_bytes(tlsext_tick_keys, TICK_KEYS_LEN); +#endif + A few comments about that: These are cryptographic keys (or at least the HMAC and AES keys are) so you should use RAND_bytes(), not RAND_pseudo_bytes(). Don't dereference the structures directly as at some point the sizes might change, the structure made opaque or a different mechanism used for storing keys (e.g. HSM support). The approved way is to call: SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, NULL, -1) which will return the combined length of all keys. Finally: +sid_ctx = ap_md5_binary(c-pool, (unsigned char*)sc-vhost_id, +sc-vhost_id_len); should we be using MD5 now if it can be avoided? Steve. -- Dr Stephen N. Henson. Senior Technical/Cryptography Advisor, Open Source Software Institute: www.oss-institute.org OpenSSL Core team: www.openssl.org
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Dr Stephen Henson wrote: A few comments about that: Thanks for the review! These are cryptographic keys (or at least the HMAC and AES keys are) so you should use RAND_bytes(), not RAND_pseudo_bytes(). Ok - when looking at ssl_lib.c:SSL_CTX_new(), I didn't realize that RAND_pseudo_bytes() is only used for tlsext_tick_key_name. Changed accordingly. Don't dereference the structures directly as at some point the sizes might change, the structure made opaque or a different mechanism used for storing keys (e.g. HSM support). I was looking at a way to determine the size at compile time, but if you think that's an unsafe method (note: it's only expected to work for 0.9.8f through 0.9.8l), then let's change it. The approved way is to call: SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, NULL, -1) which will return the combined length of all keys. Did that - does v3 of the patch (attached) look better? Is it ok to use apr_palloc here? Finally: +sid_ctx = ap_md5_binary(c-pool, (unsigned char*)sc-vhost_id, +sc-vhost_id_len); should we be using MD5 now if it can be avoided? That's what is currently used in mod_ssl.c:ssl_init_ssl_connection() - these two should be kept in sync. I'm not opposed to a change (at both places), but the question is whether now is the right time for doing that. For distcache setups e.g. I would assume that it has some unwanted consequences if two 2.2.x releases calculate the session id context in a different way... but I'm interested to hear others' opinions on this. Kaspar Index: httpd-trunk/modules/ssl/ssl_private.h === --- httpd-trunk/modules/ssl/ssl_private.h (revision 833672) +++ httpd-trunk/modules/ssl/ssl_private.h (working copy) @@ -405,6 +405,9 @@ typedef struct { #if defined(HAVE_OPENSSL_ENGINE_H) defined(HAVE_ENGINE_INIT) const char *szCryptoDevice; #endif +#ifndef OPENSSL_NO_TLSEXT +ssl_enabled_t session_tickets_enabled; +#endif #ifdef HAVE_OCSP_STAPLING const ap_socache_provider_t *stapling_cache; @@ -585,6 +588,7 @@ const char *ssl_cmd_SSLUserName(cmd_par const char *ssl_cmd_SSLLogLevelDebugDump(cmd_parms *, void *, const char *); const char *ssl_cmd_SSLRenegBufferSize(cmd_parms *cmd, void *dcfg, const char *arg); const char *ssl_cmd_SSLStrictSNIVHostCheck(cmd_parms *cmd, void *dcfg, int flag); +const char *ssl_cmd_SSLSessionTicketExtension(cmd_parms *cmd, void *cdfg, int flag); const char *ssl_cmd_SSLProxyEngine(cmd_parms *cmd, void *dcfg, int flag); const char *ssl_cmd_SSLProxyProtocol(cmd_parms *, void *, const char *); Index: httpd-trunk/modules/ssl/ssl_engine_init.c === --- httpd-trunk/modules/ssl/ssl_engine_init.c (revision 833672) +++ httpd-trunk/modules/ssl/ssl_engine_init.c (working copy) @@ -363,6 +363,15 @@ static void ssl_init_server_check(server (theoretically shouldn't happen!)); ssl_die(); } + +/* + * Session tickets (stateless resumption) + */ +if ((myModConfig(s))-session_tickets_enabled == SSL_ENABLED_FALSE) { +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + Disabling TLS session ticket support); +SSL_CTX_set_options(mctx-ssl_ctx, SSL_OP_NO_TICKET); +} } #ifndef OPENSSL_NO_TLSEXT @@ -1061,6 +1070,11 @@ void ssl_init_CheckServers(server_rec *b BOOL conflict = FALSE; +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +unsigned char *tlsext_tick_keys = NULL; +long tick_keys_len; +#endif + /* * Give out warnings when a server has HTTPS configured * for the HTTP port or vice versa @@ -1085,6 +1099,25 @@ void ssl_init_CheckServers(server_rec *b ssl_util_vhostid(p, s), DEFAULT_HTTP_PORT, DEFAULT_HTTPS_PORT); } + +#if !defined(OPENSSL_NO_TLSEXT) OPENSSL_VERSION_NUMBER 0x009080d0 +/* + * When using OpenSSL versions 0.9.8f through 0.9.8l, configure + * the same ticket encryption parameters for every SSL_CTX (workaround + * for SNI+SessionTicket extension interoperability issue in these versions) + */ +if ((sc-enabled == SSL_ENABLED_TRUE) || +(sc-enabled == SSL_ENABLED_OPTIONAL)) { +if (!tlsext_tick_keys) { +tick_keys_len = SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, + NULL, -1); +tlsext_tick_keys = (unsigned char *)apr_palloc(p, tick_keys_len); +RAND_bytes(tlsext_tick_keys, tick_keys_len); +} +SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, + tlsext_tick_keys, tick_keys_len); +} +#endif } /* Index:
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Kaspar Brand wrote: Dr Stephen Henson wrote: A few comments about that: Thanks for the review! These are cryptographic keys (or at least the HMAC and AES keys are) so you should use RAND_bytes(), not RAND_pseudo_bytes(). Ok - when looking at ssl_lib.c:SSL_CTX_new(), I didn't realize that RAND_pseudo_bytes() is only used for tlsext_tick_key_name. Changed accordingly. Don't dereference the structures directly as at some point the sizes might change, the structure made opaque or a different mechanism used for storing keys (e.g. HSM support). I was looking at a way to determine the size at compile time, but if you think that's an unsafe method (note: it's only expected to work for 0.9.8f through 0.9.8l), then let's change it. These things have a habit of persisting far longer than their expected lifetime ;-) The approved way is to call: SSL_CTX_set_tlsext_ticket_keys(sc-server-ssl_ctx, NULL, -1) which will return the combined length of all keys. Did that - does v3 of the patch (attached) look better? Is it ok to use apr_palloc here? 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. Steve. -- Dr Stephen N. Henson. Senior Technical/Cryptography Advisor, Open Source Software Institute: www.oss-institute.org OpenSSL Core team: www.openssl.org
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Thanks Kaspar. I tested your patch against 2.2 branch on to my apache-2.2.12 and found it to be working. My observations: Without SSLSessionCache it failed as usual after a long time. With SSLSessionCache it succeeded. Looking forward for this patch to get committed. Thanks to all involved in this bug fix/analysis. With regards Kamesh Jayachandran On 11/04/2009 03:05 PM, Kaspar Brand wrote: Kamesh Jayachandran wrote: Reasonable fix for this on the server side is to apply SSL_OP_NO_TICKET patch and enable SSLSessionCache. There is actually another reason why disabling TLS session tickets makes sense at the present time: with OpenSSL's current stable version (0.9.8k), session tickets only work properly for the first/default vhost. For all other vhosts, mod_ssl will fail to decrypt a previously-generated ticket, due to the order in which OpenSSL currently deals with the SNI and ticket extensions (and their callbacks). The consequence is that with 2.2.x and an SNI configuration, session caching for clients supporting TLS tickets is not working for all but the first vhost. The attached patch (for trunk, plus a backport for 2.2.x) includes two proposed changes: 1) When configuring a new SSL context (in ssl_engine_init.c:ssl_init_ctx_tls_extensions), it disables session ticket support if a server-side session cache is configured. Enabling both session tickets and a cache for stateful resumption at the same time doesn't make that much sense anyway, IMO. This change will also solve the issue with OpenSSL clients (as reported by Kamesh), provided that a server-side cache is configured. 2) In the SNI callback, it adjusts OpenSSL's session id context - which makes sure that the session can be properly resumed. (With the current mod_ssl code, this context is always tied to the first vhost, possibly resulting in incorrect resumption behavior.) The first change might later be revised, depending on how future OpenSSL versions deal with the combination of SNI + session tickets (work is underway in this area). But given the fact that OpenSSL versions between 0.9.8f and 0.9.8k will stay around for quite some time, I consider this the appropriate fix for the time being (later on, it could be #if'd based on OPENSSL_VERSION_NUMBER, or maybe even made configurable through an additional directive). Kaspar
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Kaspar Brand wrote: Kamesh Jayachandran wrote: Reasonable fix for this on the server side is to apply SSL_OP_NO_TICKET patch and enable SSLSessionCache. There is actually another reason why disabling TLS session tickets makes sense at the present time: with OpenSSL's current stable version (0.9.8k), session tickets only work properly for the first/default vhost. For all other vhosts, mod_ssl will fail to decrypt a previously-generated ticket, due to the order in which OpenSSL currently deals with the SNI and ticket extensions (and their callbacks). The consequence is that with 2.2.x and an SNI configuration, session caching for clients supporting TLS tickets is not working for all but the first vhost. The current OpenSSL (unreleased) stable code uses ticket keys from the initial ctx and not the current one. This makes session resumption with tickets and SNI work again because they all use the same keys. The equivalent can be done with previous versions of OpenSSL by generating the three ticket related keys and initializing the same ones in all SSL_CTX structures. The function macro SSL_CTX_set_tlsext_ticket_keys can be used to do this. 1) When configuring a new SSL context (in ssl_engine_init.c:ssl_init_ctx_tls_extensions), it disables session ticket support if a server-side session cache is configured. Enabling both session tickets and a cache for stateful resumption at the same time doesn't make that much sense anyway, IMO. This change will also solve the issue with OpenSSL clients (as reported by Kamesh), provided that a server-side cache is configured. I suppose if some clients support tickets and others do not then enabling both makes sense. You'd get improved performance for the equivalent cache size because ticket supporting clients would do their own caching and non-ticket clients would use normal stateful session resumption. Though as you note older versions of OpenSSL will be in use for quite a while after 0.9.8l is released. Note this should all be fixed in current unreleased OpenSSL (which will be 0.9.8l) but it needs client side as well as server side changes. Steve. -- Dr Stephen N. Henson. Senior Technical/Cryptography Advisor, Open Source Software Institute: www.oss-institute.org OpenSSL Core team: www.openssl.org
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
On 11/04/2009 10:35 AM, Kaspar Brand wrote: Kamesh Jayachandran wrote: Reasonable fix for this on the server side is to apply SSL_OP_NO_TICKET patch and enable SSLSessionCache. There is actually another reason why disabling TLS session tickets makes sense at the present time: with OpenSSL's current stable version (0.9.8k), session tickets only work properly for the first/default vhost. For all other vhosts, mod_ssl will fail to decrypt a previously-generated ticket, due to the order in which OpenSSL currently deals with the SNI and ticket extensions (and their callbacks). The consequence is that with 2.2.x and an SNI configuration, session caching for clients supporting TLS tickets is not working for all but the first vhost. The attached patch (for trunk, plus a backport for 2.2.x) includes two proposed changes: 1) When configuring a new SSL context (in ssl_engine_init.c:ssl_init_ctx_tls_extensions), it disables session ticket support if a server-side session cache is configured. Enabling both session tickets and a cache for stateful resumption at the same time doesn't make that much sense anyway, IMO. This change will also solve the issue with OpenSSL clients (as reported by Kamesh), provided that a server-side cache is configured. I guess your current patch fails on trunk since myModConfig(s))-nSessionCacheMode is no longer present in trunk 2) In the SNI callback, it adjusts OpenSSL's session id context - which makes sure that the session can be properly resumed. (With the current mod_ssl code, this context is always tied to the first vhost, possibly resulting in incorrect resumption behavior.) Can you please elaborate in more detail why this shouldn't be done when we have done renegotiations so far? Regards RĂ¼diger
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
Ruediger Pluem wrote: I guess your current patch fails on trunk since myModConfig(s))-nSessionCacheMode is no longer present in trunk Oops, you're right - my bad. I didn't compile trunk with that last change applied, obviously. For trunk, it should be if ((myModConfig(s))-sesscache_mode != SSL_SESS_CACHE_OFF) { instead. 2) In the SNI callback, it adjusts OpenSSL's session id context - which makes sure that the session can be properly resumed. (With the current mod_ssl code, this context is always tied to the first vhost, possibly resulting in incorrect resumption behavior.) Can you please elaborate in more detail why this shouldn't be done when we have done renegotiations so far? When ssl_hook_Access triggers a renegotation, it sets the session id context to a request-specific id, before calling SSL_renegotiate (to limit session reuse to this specific request). If we would overwrite the context during that renegotation (when an SNI extension is encountered and therefore the callback executed), then this coupling would get lost. Kaspar
Re: [PATCH] mod_ssl: improving session caching for SNI configurations
On 11/04/2009 05:59 PM, Kaspar Brand wrote: Ruediger Pluem wrote: 2) In the SNI callback, it adjusts OpenSSL's session id context - which makes sure that the session can be properly resumed. (With the current mod_ssl code, this context is always tied to the first vhost, possibly resulting in incorrect resumption behavior.) Can you please elaborate in more detail why this shouldn't be done when we have done renegotiations so far? When ssl_hook_Access triggers a renegotation, it sets the session id context to a request-specific id, before calling SSL_renegotiate (to limit session reuse to this specific request). If we would overwrite the context during that renegotation (when an SNI extension is encountered and therefore the callback executed), then this coupling would get lost. Thanks for explaining. Makes sense. I would like to see your comment on Steves comment regarding the usage of SSL_CTX_set_tlsext_ticket_keys. Regards RĂ¼diger