On Fri, Nov 06, 2009 at 12:00:06AM +0000, Joe Orton wrote: > On Thu, Nov 05, 2009 at 09:31:00PM +0000, Joe Orton wrote: > > * we can detect in mod_ssl when the client is renegotiating by using the > > callback installed using SSL_CTX_set_info_callback(), in conjunction > > with suitable flags in the SSLConnRec to detect the cases where this is > > either a server-initiated renegotiation or the initial handshake on the > > connection. > > Here is a very rough first hack (for discussion/testing purposes only!):
A second hack, slightly less rough hack: Index: ssl_private.h =================================================================== --- ssl_private.h (revision 832979) +++ ssl_private.h (working copy) @@ -335,6 +335,20 @@ int is_proxy; int disabled; int non_ssl_request; + + /* Track the handshake/renegotiation state for the connection so + * that all client-initiated renegotiations can be rejected, as a + * partial fix for CVE-2009-3555. */ + enum { + RENEG_INIT = 0, /* Before initial handshake */ + RENEG_REJECT, /* After initial handshake; any client-initiated + * renegotiation should be rejected */ + RENEG_ALLOW, /* A server-initated renegotiation is taking + * place (as dictated by configuration) */ + RENEG_ABORT /* Renegotiation initiated by client, abort the + * connection */ + } reneg_state; + server_rec *server; } SSLConnRec; @@ -618,7 +632,7 @@ int ssl_callback_NewSessionCacheEntry(SSL *, SSL_SESSION *); SSL_SESSION *ssl_callback_GetSessionCacheEntry(SSL *, unsigned char *, int, int *); void ssl_callback_DelSessionCacheEntry(SSL_CTX *, SSL_SESSION *); -void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE, int, int); +void ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE, int, int); #ifndef OPENSSL_NO_TLSEXT int ssl_callback_ServerNameIndication(SSL *, int *, modssl_ctx_t *); #endif Index: ssl_engine_init.c =================================================================== --- ssl_engine_init.c (revision 832979) +++ ssl_engine_init.c (working copy) @@ -520,10 +520,7 @@ SSL_CTX_set_tmp_rsa_callback(ctx, ssl_callback_TmpRSA); SSL_CTX_set_tmp_dh_callback(ctx, ssl_callback_TmpDH); - if (s->loglevel >= APLOG_DEBUG) { - /* this callback only logs if LogLevel >= info */ - SSL_CTX_set_info_callback(ctx, ssl_callback_LogTracingState); - } + SSL_CTX_set_info_callback(ctx, ssl_callback_Info); } static void ssl_init_ctx_verify(server_rec *s, Index: ssl_engine_io.c =================================================================== --- ssl_engine_io.c (revision 832979) +++ ssl_engine_io.c (working copy) @@ -103,6 +103,7 @@ ap_filter_t *pInputFilter; ap_filter_t *pOutputFilter; int nobuffer; /* non-zero to prevent buffering */ + SSLConnRec *config; } ssl_filter_ctx_t; typedef struct { @@ -193,7 +194,13 @@ static int bio_filter_out_write(BIO *bio, const char *in, int inl) { bio_filter_out_ctx_t *outctx = (bio_filter_out_ctx_t *)(bio->ptr); - + + /* Abort early if the client has initiated a renegotiation. */ + if (outctx->filter_ctx->config->reneg_state == RENEG_ABORT) { + outctx->rc = APR_ECONNABORTED; + return -1; + } + /* when handshaking we'll have a small number of bytes. * max size SSL will pass us here is about 16k. * (16413 bytes to be exact) @@ -476,6 +483,12 @@ if (!in) return 0; + /* Abort early if the client has initiated a renegotiation. */ + if (inctx->filter_ctx->config->reneg_state == RENEG_ABORT) { + inctx->rc = APR_ECONNABORTED; + return -1; + } + /* In theory, OpenSSL should flush as necessary, but it is known * not to do so correctly in some cases; see PR 46952. * @@ -1699,6 +1712,8 @@ filter_ctx = apr_palloc(c->pool, sizeof(ssl_filter_ctx_t)); + filter_ctx->config = myConnConfig(c); + filter_ctx->nobuffer = 0; filter_ctx->pOutputFilter = ap_add_output_filter(ssl_io_filter, filter_ctx, r, c); Index: ssl_engine_kernel.c =================================================================== --- ssl_engine_kernel.c (revision 832979) +++ ssl_engine_kernel.c (working copy) @@ -733,6 +733,10 @@ (unsigned char *)&id, sizeof(id)); + /* Toggle the renegotiation state to allow the new + * handshake to proceed. */ + sslconn->reneg_state = RENEG_ALLOW; + SSL_renegotiate(ssl); SSL_do_handshake(ssl); @@ -754,6 +758,8 @@ SSL_set_state(ssl, SSL_ST_ACCEPT); SSL_do_handshake(ssl); + sslconn->reneg_state = RENEG_REJECT; + if (SSL_get_state(ssl) != SSL_ST_OK) { ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "Re-negotiation handshake failed: " @@ -1814,76 +1820,55 @@ return; } -/* - * This callback function is executed while OpenSSL processes the - * SSL handshake and does SSL record layer stuff. We use it to - * trace OpenSSL's processing in out SSL logfile. - */ -void ssl_callback_LogTracingState(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc) +/* Dump debugginfo trace to the log file. */ +static void log_tracing_state(MODSSL_INFO_CB_ARG_TYPE ssl, server_rec *s, + conn_rec *c, int where, int rc) { - conn_rec *c; - server_rec *s; - SSLSrvConfigRec *sc; - /* - * find corresponding server + * create the various trace messages */ - if (!(c = (conn_rec *)SSL_get_app_data((SSL *)ssl))) { - return; + if (where & SSL_CB_HANDSHAKE_START) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Handshake: start", SSL_LIBRARY_NAME); } - - s = mySrvFromConn(c); - if (!(sc = mySrvConfig(s))) { - return; + else if (where & SSL_CB_HANDSHAKE_DONE) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Handshake: done", SSL_LIBRARY_NAME); } - - /* - * create the various trace messages - */ - if (s->loglevel >= APLOG_DEBUG) { - if (where & SSL_CB_HANDSHAKE_START) { + else if (where & SSL_CB_LOOP) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Loop: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_READ) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Read: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_WRITE) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Write: %s", + SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); + } + else if (where & SSL_CB_ALERT) { + char *str = (where & SSL_CB_READ) ? "read" : "write"; + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, + "%s: Alert: %s:%s:%s", + SSL_LIBRARY_NAME, str, + SSL_alert_type_string_long(rc), + SSL_alert_desc_string_long(rc)); + } + else if (where & SSL_CB_EXIT) { + if (rc == 0) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Handshake: start", SSL_LIBRARY_NAME); - } - else if (where & SSL_CB_HANDSHAKE_DONE) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Handshake: done", SSL_LIBRARY_NAME); - } - else if (where & SSL_CB_LOOP) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Loop: %s", + "%s: Exit: failed in %s", SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); } - else if (where & SSL_CB_READ) { + else if (rc < 0) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Read: %s", + "%s: Exit: error in %s", SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); } - else if (where & SSL_CB_WRITE) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Write: %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - else if (where & SSL_CB_ALERT) { - char *str = (where & SSL_CB_READ) ? "read" : "write"; - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Alert: %s:%s:%s", - SSL_LIBRARY_NAME, str, - SSL_alert_type_string_long(rc), - SSL_alert_desc_string_long(rc)); - } - else if (where & SSL_CB_EXIT) { - if (rc == 0) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Exit: failed in %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - else if (rc < 0) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, - "%s: Exit: error in %s", - SSL_LIBRARY_NAME, SSL_state_string_long(ssl)); - } - } } /* @@ -1903,6 +1888,60 @@ } } +/* + * This callback function is executed while OpenSSL processes the SSL + * handshake and does SSL record layer stuff. It's used to trap + * client-initiated renegotiations, and for dumping everything to the + * log. + */ +void ssl_callback_Info(MODSSL_INFO_CB_ARG_TYPE ssl, int where, int rc) +{ + conn_rec *c; + server_rec *s; + SSLSrvConfigRec *sc; + SSLConnRec *scr; + + /* + * find corresponding server + */ + if (!(c = (conn_rec *)SSL_get_app_data((SSL *)ssl))) { + return; + } + + scr = myConnConfig(c); + if (!scr) { + return; + } + + /* If the reneg state is to reject renegotiations, check the SSL + * state machine and move to ABORT if a Client Hello is being + * read. */ + if ((where & SSL_CB_ACCEPT_LOOP) && scr->reneg_state == RENEG_REJECT) { + int state = SSL_get_state(ssl); + + if (state == SSL3_ST_SR_CLNT_HELLO_A + || state == SSL23_ST_SR_CLNT_HELLO_A) { + scr->reneg_state = RENEG_ABORT; + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, + "rejecting client initiated renegotiation"); + } + } + /* If the first handshake is complete, change state to reject any + * subsequent client-initated renegotiation. */ + else if (where & SSL_CB_HANDSHAKE_DONE && scr->reneg_state == RENEG_INIT) { + scr->reneg_state = RENEG_REJECT; + } + + s = mySrvFromConn(c); + if (!(sc = mySrvConfig(s))) { + return; + } + + if (s->loglevel >= APLOG_DEBUG) { + log_tracing_state(ssl, s, c, where, rc); + } +} + #ifndef OPENSSL_NO_TLSEXT /* * This callback function is executed when OpenSSL encounters an extended