From: Emeric Brun <ebrun@haproxy.com>
Date: Tue, 6 Jun 2017 12:35:14 +0000 (+0000)
Subject: BUG/MAJOR: ssl: buffer overflow using offloaded ciphering on async engine
X-Git-Tag: v1.8-dev3~305
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b5e42a817b4b687c90ad9db12fe2bcd1652a00fe;p=thirdparty%2Fhaproxy.git

BUG/MAJOR: ssl: buffer overflow using offloaded ciphering on async engine

The Openssl's ASYNC API does'nt support moving buffers on SSL_read/write
This patch disables the ASYNC mode dynamically when the handshake
is left and re-enables it on reneg.
---

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 61376fc78b..49bfd858cc 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -1294,7 +1294,11 @@ ssl-engine <name> [algo <comma-seperated list of algorithms>]
 ssl-mode-async
   Adds SSL_MODE_ASYNC mode to the SSL context. This enables asynchronous TLS
   I/O operations if asynchronous capable SSL engines are used. The current
-  implementation supports a maximum of 32 engines.
+  implementation supports a maximum of 32 engines. The Openssl ASYNC API
+  doesn't support moving read/write buffers and is not compliant with
+  haproxy's buffer management. So the asynchronous mode is disabled on
+  read/write  operations (it is only enabled during initial and reneg
+  handshakes).
 
 tune.buffers.limit <number>
   Sets a hard limit on the number of buffers which may be allocated per process.
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c8e6b57b69..f9b236a8f7 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4757,6 +4757,15 @@ int ssl_sock_handshake(struct connection *conn, unsigned int flag)
 	}
 
 reneg_ok:
+
+#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
+	/* ASYNC engine API doesn't support moving read/write
+	 * buffers. So we disable ASYNC mode right after
+	 * the handshake to avoid buffer oveflows.
+	 */
+	if (global_ssl.async)
+		SSL_clear_mode(conn->xprt_ctx, SSL_MODE_ASYNC);
+#endif
 	/* Handshake succeeded */
 	if (!SSL_session_reused(conn->xprt_ctx)) {
 		if (objt_server(conn->target)) {
@@ -4875,6 +4884,11 @@ static int ssl_sock_to_buf(struct connection *conn, struct buffer *buf, int coun
 				/* handshake is running, and it needs to enable write */
 				conn->flags |= CO_FL_SSL_WAIT_HS;
 				__conn_sock_want_send(conn);
+#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
+				/* Async mode can be re-enabled, because we're leaving data state.*/
+				if (global_ssl.async)
+					SSL_set_mode(conn->xprt_ctx, SSL_MODE_ASYNC);
+#endif
 				break;
 			}
 			else if (ret == SSL_ERROR_WANT_READ) {
@@ -4882,18 +4896,17 @@ static int ssl_sock_to_buf(struct connection *conn, struct buffer *buf, int coun
 					/* handshake is running, and it may need to re-enable read */
 					conn->flags |= CO_FL_SSL_WAIT_HS;
 					__conn_sock_want_recv(conn);
+#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
+					/* Async mode can be re-enabled, because we're leaving data state.*/
+					if (global_ssl.async)
+						SSL_set_mode(conn->xprt_ctx, SSL_MODE_ASYNC);
+#endif
 					break;
 				}
 				/* we need to poll for retry a read later */
 				fd_cant_recv(conn->t.sock.fd);
 				break;
 			}
-#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
-			else if (ret == SSL_ERROR_WANT_ASYNC) {
-				ssl_async_process_fds(conn, conn->xprt_ctx);
-				break;
-			}
-#endif
 			/* otherwise it's a real error */
 			goto out_error;
 		}
@@ -4984,6 +4997,11 @@ static int ssl_sock_from_buf(struct connection *conn, struct buffer *buf, int fl
 					/* handshake is running, and it may need to re-enable write */
 					conn->flags |= CO_FL_SSL_WAIT_HS;
 					__conn_sock_want_send(conn);
+#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
+					/* Async mode can be re-enabled, because we're leaving data state.*/
+					if (global_ssl.async)
+						SSL_set_mode(conn->xprt_ctx, SSL_MODE_ASYNC);
+#endif
 					break;
 				}
 				/* we need to poll to retry a write later */
@@ -4994,14 +5012,13 @@ static int ssl_sock_from_buf(struct connection *conn, struct buffer *buf, int fl
 				/* handshake is running, and it needs to enable read */
 				conn->flags |= CO_FL_SSL_WAIT_HS;
 				__conn_sock_want_recv(conn);
-				break;
-			}
 #if OPENSSL_VERSION_NUMBER >= 0x1010000fL
-			else if (ret == SSL_ERROR_WANT_ASYNC) {
-				ssl_async_process_fds(conn, conn->xprt_ctx);
+				/* Async mode can be re-enabled, because we're leaving data state.*/
+				if (global_ssl.async)
+					SSL_set_mode(conn->xprt_ctx, SSL_MODE_ASYNC);
+#endif
 				break;
 			}
-#endif
 			goto out_error;
 		}
 	}