[sr-dev] git:master: tls: new parameter 'renegotiation' to enable/ disable client renegotiation

Module: sip-router Branch: master Commit: 5731486ac973324f25b091367a573d710f4fe71f URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5731486… Author: Daniel-Constantin Mierla &lt;miconda(a)gmail.com&gt; Committer: Daniel-Constantin Mierla &lt;miconda(a)gmail.com&gt; Date: Sat Dec 17 22:22:04 2011 +0100 tls: new parameter 'renegotiation' to enable/disable client renegotiation - default is 0 (renegotiation disabled), to protect against SSL renegotiation attack - can be enabled by setting it to 1 --- modules/tls/tls_domain.c | 36 ++++++++++++++++++++++++++++++++++++ modules/tls/tls_mod.c | 3 +++ modules/tls/tls_mod.h | 2 ++ modules/tls/tls_server.c | 32 ++++++++++++++++++++++++-------- modules/tls/tls_server.h | 4 +++- 5 files changed, 68 insertions(+), 9 deletions(-) diff --git a/modules/tls/tls_domain.c b/modules/tls/tls_domain.c index a61149d..d353b81 100644 --- a/modules/tls/tls_domain.c +++ b/modules/tls/tls_domain.c @@ -591,6 +591,40 @@ static int set_verification(tls_domain_t* d) } +/* This callback function is executed when libssl processes the SSL + * handshake and does SSL record layer stuff. It's used to trap + * client-initiated renegotiations. + */ + +static void sr_ssl_ctx_info_callback(const SSL *ssl, int event, int ret) +{ + struct tls_extra_data* data = 0; + int tls_dbg; + + if (event & SSL_CB_HANDSHAKE_START) { + tls_dbg = cfg_get(tls, tls_cfg, debug); + LOG(tls_dbg, "SSL handshake started\n"); + if(data==0) + data = (struct tls_extra_data*)SSL_get_app_data(ssl); + if(data->flags & F_TLS_CON_HANDSHAKED) { + LOG(tls_dbg, "SSL renegotiation initiated by client\n"); + data->flags |= F_TLS_CON_RENEGOTIATION; + } + } + if (event & SSL_CB_HANDSHAKE_DONE) { + tls_dbg = cfg_get(tls, tls_cfg, debug); + if(data==0) + data = (struct tls_extra_data*)SSL_get_app_data(ssl); + LOG(tls_dbg, "SSL handshake done\n"); + /* CVE-2009-3555 - disable renegotiation */ + if (ssl->s3) { + LOG(tls_dbg, "SSL disable renegotiation\n"); + ssl->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS; + } + data->flags |= F_TLS_CON_HANDSHAKED; + } +} + /** * @brief Configure generic SSL parameters * @param d domain @@ -635,6 +669,8 @@ static int set_ssl_options(tls_domain_t* d) #endif for(i = 0; i < procs_no; i++) { SSL_CTX_set_options(d->ctx[i], options); + if(sr_tls_renegotiation==0) + SSL_CTX_set_info_callback(d->ctx[i], sr_ssl_ctx_info_callback); } return 0; } diff --git a/modules/tls/tls_mod.c b/modules/tls/tls_mod.c index 2f7d626..e0c05d8 100644 --- a/modules/tls/tls_mod.c +++ b/modules/tls/tls_mod.c @@ -175,6 +175,8 @@ tls_domains_cfg_t** tls_domains_cfg = NULL; gen_lock_t* tls_domains_cfg_lock = NULL; +int sr_tls_renegotiation = 0; + /* * Exported functions */ @@ -218,6 +220,7 @@ static param_export_t params[] = { {"tls_force_run", PARAM_INT, &default_tls_cfg.force_run}, {"low_mem_threshold1", PARAM_INT, &default_tls_cfg.low_mem_threshold1}, {"low_mem_threshold2", PARAM_INT, &default_tls_cfg.low_mem_threshold2}, + {"renegotiation", PARAM_INT, &sr_tls_renegotiation}, {0, 0, 0} }; diff --git a/modules/tls/tls_mod.h b/modules/tls/tls_mod.h index a19e9aa..462e5d0 100644 --- a/modules/tls/tls_mod.h +++ b/modules/tls/tls_mod.h @@ -53,4 +53,6 @@ extern tls_domain_t srv_defaults; extern str tls_domains_cfg_file; +extern int sr_tls_renegotiation; + #endif /* _TLS_MOD_H */ diff --git a/modules/tls/tls_server.c b/modules/tls/tls_server.c index 5ccf545..e3d0a61 100644 --- a/modules/tls/tls_server.c +++ b/modules/tls/tls_server.c @@ -209,6 +209,10 @@ static int tls_complete_init(struct tcp_connection* c) #endif SSL_set_bio(data->ssl, data->rwbio, data->rwbio); c->extra_data = data; + + /* link the extra data struct inside ssl connection*/ + SSL_set_app_data(data->ssl, data); + return 0; error: @@ -908,6 +912,7 @@ int tls_read_f(struct tcp_connection* c, int* flags) int n, flush_flags; char* err_src; int x; + int tls_dbg; TLS_RD_TRACE("(%p, %p (%d)) start (%s -> %s:%d*)\n", c, flags, *flags, @@ -1092,15 +1097,26 @@ continue_ssl_read: * In the later case, this whole function should be called again * once there is more output space (set RD_CONN_REPEAT_READ). */ - if (unlikely(n <= 0)) { - ssl_error = SSL_get_error(ssl, n); - err_src = "TLS read:"; - /* errors handled below, outside the lock */ + + if (unlikely(tls_c->flags & F_TLS_CON_RENEGOTIATION)) { + /* Fix CVE-2009-3555 - disable renegotiation if started by client + * - simulate SSL EOF to force close connection*/ + tls_dbg = cfg_get(tls, tls_cfg, debug); + LOG(tls_dbg, "Reading on a renegotiation of connection (n:%d) (%d)\n", + n, SSL_get_error(ssl, n)); + err_src = "TLS R-N read:"; + ssl_error = SSL_ERROR_ZERO_RETURN; } else { - ssl_error = SSL_ERROR_NONE; - r->pos += n; - ssl_read += n; - bytes_free -=n; + if (unlikely(n <= 0)) { + ssl_error = SSL_get_error(ssl, n); + err_src = "TLS read:"; + /* errors handled below, outside the lock */ + } else { + ssl_error = SSL_ERROR_NONE; + r->pos += n; + ssl_read += n; + bytes_free -=n; + } } TLS_RD_TRACE("(%p, %p) SSL_read() => %d (err=%d) ssl_read=%d" " *flags=%d tls_c->flags=%d\n", diff --git a/modules/tls/tls_server.h b/modules/tls/tls_server.h index 62047d5..4b00d06 100644 --- a/modules/tls/tls_server.h +++ b/modules/tls/tls_server.h @@ -49,7 +49,9 @@ struct tls_rd_buf { }; /* tls conn flags */ -#define F_TLS_CON_WR_WANTS_RD 1 /* write wants read */ +#define F_TLS_CON_WR_WANTS_RD 1 /* write wants read */ +#define F_TLS_CON_HANDSHAKED 2 /* connection is handshaked */ +#define F_TLS_CON_RENEGOTIATION 4 /* renegotiation by clinet */ struct tls_extra_data { tls_domains_cfg_t* cfg; /* Configuration used for this connection */