Skip to content

Commit

Permalink
proto_tls: fix crash when tracing an outgoing connection
Browse files Browse the repository at this point in the history 
This was caused by attempting to send tracing data that was not ready yet.

Thanks to Razvan for reporting and help in debugging.

(cherry picked from commit fabe43e1ad8e6f974c9177cb6676d1a386120e45)
  • Loading branch information
@rvlad-patrascu
rvlad-patrascu committed Aug 14, 2018
1 parent ded1729 commit 9ab114d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 32 deletions.
30 changes: 5 additions & 25 deletions modules/proto_tls/proto_tls.c
View file
Expand Up @@ -112,13 +112,15 @@ static void tls_report(int type, unsigned long long conn_id, int conn_flags,
static struct mi_root* tls_trace_mi(struct mi_root* cmd, void* param );


trace_dest t_dst;

static int w_tls_blocking_write(struct tcp_connection *c, int fd, const char *buf,
size_t len)
{
int ret;

lock_get(&c->write_lock);
ret = tls_blocking_write(c, fd, buf, len, &tls_mgm_api);
ret = tls_blocking_write(c, fd, buf, len, &tls_mgm_api, t_dst);
lock_release(&c->write_lock);
return ret;
}
Expand All @@ -137,7 +139,6 @@ static struct tcp_req tls_current_req;
#define TLS_TRACE_PROTO "proto_hep"

static str trace_destination_name = {NULL, 0};
trace_dest t_dst;
trace_proto_t tprot;

/* module tracing parameters */
Expand Down Expand Up @@ -423,8 +424,6 @@ static int proto_tls_send(struct socket_info* send_sock,
int port;
int fd, n;

struct tls_data* data;

if (to){
su2ip_addr(&ip, to);
port=su_getport(to);
Expand Down Expand Up @@ -466,29 +465,10 @@ static int proto_tls_send(struct socket_info* send_sock,
}

send_it:
/* if there is pending tracing data on a connection startet by us
* (connected) -> flush it
* As this is a write op, we look only for connected conns, not to conflict
* with accepted conns (flushed on read op) */
if ( (c->flags&F_CONN_ACCEPTED)==0 && c->proto_flags & F_TLS_TRACE_READY ) {
data = c->proto_data;
/* send the message if set from tls_mgm */
if ( data->message ) {
send_trace_message( data->message, t_dst);
data->message = NULL;
}

/* don't allow future traces for this connection */
data->tprot = 0;
data->dest = 0;

c->proto_flags &= ~( F_TLS_TRACE_READY );
}

LM_DBG("sending via fd %d...\n",fd);

lock_get(&c->write_lock);
n = tls_blocking_write(c, fd, buf, len, &tls_mgm_api);
n = tls_blocking_write(c, fd, buf, len, &tls_mgm_api, t_dst);
lock_release(&c->write_lock);
tcp_conn_set_lifetime( c, tcp_con_lifetime);

Expand Down Expand Up @@ -537,7 +517,7 @@ static int tls_read_req(struct tcp_connection* con, int* bytes_read)
}

/* do this trick in order to trace whether if it's an error or not */
ret=tls_fix_read_conn(con);
ret=tls_fix_read_conn(con, t_dst);

/* if there is pending tracing data on an accepted connection, flush it
* As this is a read op, we look only for accepted conns, not to conflict
Expand Down
4 changes: 2 additions & 2 deletions modules/proto_wss/proto_wss.c
View file
Expand Up @@ -542,7 +542,7 @@ static int wss_read_req(struct tcp_connection* con, int* bytes_read)
struct ws_data* d;

/* we need to fix the SSL connection before doing anything */
if (tls_fix_read_conn(con) < 0) {
if (tls_fix_read_conn(con, t_dst) < 0) {
LM_ERR("cannot fix read connection\n");
if ( (d=con->proto_data) && d->dest && d->tprot ) {
if ( d->message ) {
Expand Down Expand Up @@ -611,7 +611,7 @@ static int wss_raw_writev(struct tcp_connection *c, int fd,
#ifndef TLS_DONT_WRITE_FRAGMENTS
lock_get(&c->write_lock);
for (i = 0; i < iovcnt; i++) {
n = tls_blocking_write(c, fd, iov[i].iov_base, iov[i].iov_len, &tls_mgm_api);
n = tls_blocking_write(c, fd, iov[i].iov_base, iov[i].iov_len, &tls_mgm_api, t_dst);
if (n < 0) {
ret = -1;
goto end;
Expand Down
35 changes: 30 additions & 5 deletions modules/tls_mgm/tls_conn_server.h
View file
Expand Up @@ -368,11 +368,30 @@ static int tls_accept(struct tcp_connection *c, short *poll_events)
return -1;
}

void tls_send_trace_data(struct tcp_connection *c, trace_dest t_dst) {
struct tls_data* data;

if ( (c->flags&F_CONN_ACCEPTED)==0 && c->proto_flags & F_TLS_TRACE_READY ) {
data = c->proto_data;

/* send the message if set from tls_mgm */
if ( data->message ) {
send_trace_message( data->message, t_dst);
data->message = NULL;
}

/* don't allow future traces for this connection */
data->tprot = 0;
data->dest = 0;

c->proto_flags &= ~( F_TLS_TRACE_READY );
}
}

/*
* wrapper around SSL_connect, returns 0 on success, -1 on error
*/
static int tls_connect(struct tcp_connection *c, short *poll_events)
static int tls_connect(struct tcp_connection *c, short *poll_events, trace_dest t_dst)
{
int ret, err;
SSL *ssl;
Expand All @@ -394,6 +413,8 @@ static int tls_connect(struct tcp_connection *c, short *poll_events)
trace_tls( c, ssl, TRANS_TRACE_CONNECTED,
TRANS_TRACE_SUCCESS, &CONNECT_FAIL);

tls_send_trace_data(c, t_dst);

c->proto_flags &= ~F_TLS_DO_CONNECT;
LM_DBG("new TLS connection to %s:%d using %s %s %d\n",
ip_addr2a(&c->rcv.src_ip), c->rcv.src_port,
Expand Down Expand Up @@ -433,6 +454,8 @@ static int tls_connect(struct tcp_connection *c, short *poll_events)
trace_tls( c, ssl, TRANS_TRACE_CONNECTED,
TRANS_TRACE_FAILURE, &CONNECT_FAIL);

tls_send_trace_data(c, t_dst);

c->state = S_CONN_BAD;
return -1;
case SSL_ERROR_WANT_READ:
Expand Down Expand Up @@ -463,6 +486,8 @@ static int tls_connect(struct tcp_connection *c, short *poll_events)
tls_err_s.s = tls_err_buf;
trace_tls( c, ssl, TRANS_TRACE_CONNECTED,
TRANS_TRACE_FAILURE, &tls_err_s);

tls_send_trace_data(c, t_dst);
} else {
tls_print_errstack();
}
Expand All @@ -482,7 +507,7 @@ static int tls_connect(struct tcp_connection *c, short *poll_events)
* does not transit a connection into S_CONN_OK then tcp layer would not
* call tcp_read
*/
static int tls_fix_read_conn(struct tcp_connection *c)
static int tls_fix_read_conn(struct tcp_connection *c, trace_dest t_dst)
{
/*
* no lock acquired
Expand All @@ -505,7 +530,7 @@ static int tls_fix_read_conn(struct tcp_connection *c)
} else if ( c->proto_flags & F_TLS_DO_CONNECT ) {
ret = tls_update_fd(c, c->fd);
if (!ret)
ret = tls_connect(c, NULL);
ret = tls_connect(c, NULL, t_dst);
}

lock_release(&c->write_lock);
Expand Down Expand Up @@ -570,7 +595,7 @@ static int tls_write(struct tcp_connection *c, int fd, const void *buf,
* fixme: probably does not work correctly
*/
static int tls_blocking_write(struct tcp_connection *c, int fd, const char *buf,
size_t len, struct tls_mgm_binds *api)
size_t len, struct tls_mgm_binds *api, trace_dest t_dst)
{
#define MAX_SSL_RETRIES 32
int written, n;
Expand Down Expand Up @@ -599,7 +624,7 @@ static int tls_blocking_write(struct tcp_connection *c, int fd, const char *buf,
goto error;
timeout = api->get_handshake_timeout();
} else if ( c->proto_flags & F_TLS_DO_CONNECT ) {
if (tls_connect(c, &(pf.events)) < 0)
if (tls_connect(c, &(pf.events), t_dst) < 0)
goto error;
timeout = api->get_handshake_timeout();
} else {
Expand Down

0 comments on commit 9ab114d

Please sign in to comment.