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
