proto_tls/wss: fix crashes when dumping the openssl error stack

This commit serializes the execution of openssl's connect/accept/read/write
operations in order to prevent adding/removing entries concurrently to/from
the openssl error stack. If the performance penalty is deemed too high, the
NO_SSL_GLOBAL_LOCK compilation flag can be used to disable this behavior and
retain the risk of crashes.

Reported in OpenSIPS#2362
Credits to Alexey Vasilyev for helping troubleshoot this.

Author: rvlad-patrascu

Makefile.openssl

@@ -18,3 +18,7 @@ else
 			-L$(LOCALBASE)/lib64 -L$(LOCALBASE)/ssl/lib64 \
 			-lssl -lcrypto
 endif
+
+# enable this flag to increase performance by not serializing openssl
+# connect/accept/read/write operations, at the cost of possible crashes
+#DEFS+= -DNO_SSL_GLOBAL_LOCK

modules/proto_tls/proto_tls.c

@@ -139,20 +139,26 @@ static mi_response_t *tls_trace_mi_1(const mi_params_t *params,
 
 trace_dest t_dst;
 
+#ifndef NO_SSL_GLOBAL_LOCK
+gen_lock_t *ssl_global_lock;
+#else
+#define ssl_global_lock NULL
+#endif
+
 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_handshake_tout, tls_send_tout, t_dst);
+			tls_handshake_tout, tls_send_tout, t_dst, ssl_global_lock);
 	lock_release(&c->write_lock);
 	return ret;
 }
 
 static int tls_write_on_socket(struct tcp_connection* c, int fd,
-		char *buf, int len)
+		char *buf, int len, gen_lock_t *ssl_global_lock)
 {
 	int n;
 
@@ -168,7 +174,7 @@ static int tls_write_on_socket(struct tcp_connection* c, int fd,
 				goto release;
 			}
 
-			n = tls_write(c, fd, buf, len, NULL);
+			n = tls_write(c, fd, buf, len, NULL, ssl_global_lock);
 			if (n >= 0 && len - n) {
 				/* if could not write entire buffer, delay it */
 				n = tcp_async_add_chunk(c, buf + n, len - n, 0);
@@ -178,7 +184,7 @@ static int tls_write_on_socket(struct tcp_connection* c, int fd,
 		}
 	} else {
 		n = tls_blocking_write(c, fd, buf, len,
-				tls_handshake_tout, tls_send_tout, t_dst);
+				tls_handshake_tout, tls_send_tout, t_dst, ssl_global_lock);
 	}
 release:
 	lock_release(&c->write_lock);
@@ -328,6 +334,14 @@ static int mod_init(void)
 				sroutes->request, RT_NO);
 	}
 
+	#ifndef NO_SSL_GLOBAL_LOCK
+	ssl_global_lock = lock_alloc();
+	if (!ssl_global_lock || !lock_init(ssl_global_lock)) {
+		LM_ERR("could not initialize openssl lock!\n");
+		return -1;
+	}
+	#endif
+
 	return 0;
 }
 
@@ -337,6 +351,11 @@ static int mod_init(void)
  */
 static void mod_destroy(void)
 {
+	#ifndef NO_SSL_GLOBAL_LOCK
+	lock_destroy(ssl_global_lock);
+	lock_dealloc(ssl_global_lock);
+	#endif
+
 	/* library destroy */
 	ERR_free_strings();
 	/*SSL_free_comp_methods(); - this function is not on std. openssl*/
@@ -440,7 +459,7 @@ static void proto_tls_conn_clean(struct tcp_connection* c)
 		c->proto_data = NULL;
 	}
 
-	tls_conn_clean(c, &tls_mgm_api);
+	tls_conn_clean(c, ssl_global_lock, &tls_mgm_api);
 }
 
 
@@ -537,7 +556,8 @@ static int proto_tls_send(struct socket_info* send_sock,
 			/* we connect under lock to make sure no one else is reading our
 			 * connect status */
 			tls_update_fd(c, fd);
-			n = tls_async_connect(c, fd, tls_async_handshake_connect_timeout, t_dst);
+			n = tls_async_connect(c, fd, tls_async_handshake_connect_timeout, t_dst,
+				ssl_global_lock);
 			lock_release(&c->write_lock);
 			if (n<0) {
 				LM_ERR("failed async TLS connect\n");
@@ -581,7 +601,7 @@ static int proto_tls_send(struct socket_info* send_sock,
 send_it:
 	LM_DBG("sending via fd %d...\n",fd);
 
-	rlen = tls_write_on_socket(c, fd, buf, len);
+	rlen = tls_write_on_socket(c, fd, buf, len, ssl_global_lock);
 	tcp_conn_set_lifetime( c, tcp_con_lifetime);
 
 	LM_DBG("after write: c=%p n=%d fd=%d\n",c, rlen, fd);
@@ -635,7 +655,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, tls_handshake_tout, t_dst);
+	ret=tls_fix_read_conn(con, tls_handshake_tout, t_dst, ssl_global_lock);
 	if (ret < 0) {
 		LM_ERR("failed to do pre-tls handshake!\n");
 		return -1;
@@ -677,7 +697,7 @@ static int tls_read_req(struct tcp_connection* con, int* bytes_read)
 		if (req->parsed<req->pos){
 			bytes=0;
 		}else{
-			bytes=tls_read(con,req);
+			bytes=tls_read(con,req, ssl_global_lock);
 			if (bytes<0) {
 				LM_ERR("failed to read \n");
 				goto error;
@@ -740,7 +760,8 @@ static int tls_async_write(struct tcp_connection* con, int fd)
 	struct tcp_async_chunk *chunk;
 	SSL *ssl = (SSL *)con->extra_data;
 
-	err = tls_fix_read_conn_unlocked(con, fd, tls_handshake_tout, t_dst);
+	err = tls_fix_read_conn_unlocked(con, fd, tls_handshake_tout, t_dst,
+		ssl_global_lock);
 	if (err < 0) {
 		LM_ERR("failed to do pre-tls handshake!\n");
 		return -1;
@@ -753,15 +774,28 @@ static int tls_async_write(struct tcp_connection* con, int fd)
 	while ((chunk = tcp_async_get_chunk(con)) != NULL) {
 		LM_DBG("Trying to send %d bytes from chunk %p in conn %p - %d %d \n",
 				chunk->len, chunk, con, chunk->ticks, get_ticks());
+
+		#ifndef NO_SSL_GLOBAL_LOCK
+		lock_get(ssl_global_lock);
+		#endif
+
 		n=SSL_write(con->extra_data, chunk->buf, chunk->len);
 		if (n<0) {
 			err = SSL_get_error(ssl, n);
 			switch (err) {
 				case SSL_ERROR_ZERO_RETURN:
+					#ifndef NO_SSL_GLOBAL_LOCK
+					lock_release(ssl_global_lock);
+					#endif
+
 					LM_DBG("connection closed cleanly\n");
 					return -1;
 				case SSL_ERROR_WANT_READ:
 				case SSL_ERROR_WANT_WRITE:
+					#ifndef NO_SSL_GLOBAL_LOCK
+					lock_release(ssl_global_lock);
+					#endif
+
 					LM_DBG("Can't finish to write chunk %p on conn %p\n",
 							chunk,con);
 					/* report back we have more writting to be done */
@@ -770,6 +804,11 @@ static int tls_async_write(struct tcp_connection* con, int fd)
 					LM_ERR("Error occurred while sending async chunk %d:%d (%s)\n",
 							err, errno,strerror(errno));
 					tls_print_errstack();
+
+					#ifndef NO_SSL_GLOBAL_LOCK
+					lock_release(ssl_global_lock);
+					#endif
+
 					/* report the conn as broken */
 					return -1;
 			}

modules/proto_wss/proto_wss.c

@@ -73,11 +73,17 @@ static str wss_resource = str_init("/");
 static int wss_raw_writev(struct tcp_connection *c, int fd,
 		const struct iovec *iov, int iovcnt, int tout);
 
+#ifndef NO_SSL_GLOBAL_LOCK
+gen_lock_t *ssl_global_lock;
+#else
+#define ssl_global_lock NULL
+#endif
+
 #define _ws_common_module "wss"
 #define _ws_common_tcp_current_req tcp_current_req
 #define _ws_common_current_req wss_current_req
 #define _ws_common_max_msg_chunks wss_max_msg_chunks
-#define _ws_common_read tls_read
+#define _ws_common_read(c, r) tls_read((c), (r), ssl_global_lock)
 #define _ws_common_writev wss_raw_writev
 #define _ws_common_read_tout wss_hs_read_tout
 /*
@@ -105,6 +111,7 @@ static int trace_filter_route_id = -1;
 /**/
 
 static int mod_init(void);
+static void mod_destroy(void);
 static int proto_wss_init(struct proto_info *pi);
 static int proto_wss_init_listener(struct socket_info *si);
 static int proto_wss_send(struct socket_info* send_sock,
@@ -180,7 +187,7 @@ struct module_exports exports = {
 	0,          /* module pre-initialization function */
 	mod_init,   /* module initialization function */
 	0,          /* response function */
-	0,          /* destroy function */
+	mod_destroy,/* destroy function */
 	0,          /* per-child init function */
 	0           /* reload confirm function */
 };
@@ -255,11 +262,24 @@ static int mod_init(void)
 				sroutes->request, RT_NO);
 	}
 
-
+	#ifndef NO_SSL_GLOBAL_LOCK
+	ssl_global_lock = lock_alloc();
+	if (!ssl_global_lock || !lock_init(ssl_global_lock)) {
+		LM_ERR("could not initialize openssl lock!\n");
+		return -1;
+	}
+	#endif
 
 	return 0;
 }
 
+static void mod_destroy(void)
+{
+	#ifndef NO_SSL_GLOBAL_LOCK
+	lock_destroy(ssl_global_lock);
+	lock_dealloc(ssl_global_lock);
+	#endif
+}
 
 static int wss_conn_init(struct tcp_connection* c)
 {
@@ -323,7 +343,7 @@ static void ws_conn_clean(struct tcp_connection* c)
 
 	}
 
-	tls_conn_clean(c, &tls_mgm_api);
+	tls_conn_clean(c, ssl_global_lock, &tls_mgm_api);
 }
 
 
@@ -491,7 +511,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, t_dst) < 0) {
+	if (tls_fix_read_conn(con, 0, t_dst, ssl_global_lock) < 0) {
 		LM_ERR("cannot fix read connection\n");
 		if ( (d=con->proto_data) && d->dest && d->tprot ) {
 			if ( d->message ) {
@@ -561,7 +581,7 @@ static int wss_raw_writev(struct tcp_connection *c, int fd,
 	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,
-				wss_hs_tls_tout, wss_send_tout, t_dst);
+				wss_hs_tls_tout, wss_send_tout, t_dst, ssl_global_lock);
 		if (n < 0) {
 			ret = -1;
 			goto end;
@@ -584,7 +604,7 @@ static int wss_raw_writev(struct tcp_connection *c, int fd,
 	}
 	lock_get(&c->write_lock);
 	n = tls_blocking_write(c, fd, buf, n,
-				wss_hs_tls_tout, wss_send_tout, t_dst);
+				wss_hs_tls_tout, wss_send_tout, t_dst, ssl_global_lock);
 #endif /* TLS_DONT_WRITE_FRAGMENTS */
 
 end: