Merge branch 'be-openssl-fd-reset-fix-v2'

Two issues:
- dirty_shutdown for openssl 1.1
- BEV_CTRL_SET_FD for bufferevent_openssl didn't reset state

* be-openssl-fd-reset-fix-v2:
  Fix dirty_shutdown for openssl 1.1
  Fix reusing bufferevent_openssl after fd was reseted (i.e. on new connection)
  test/https: fix ssl dirty bypass for https_simple
  test/https: cover multiple request over the same connection
  test/http: sanity check for http_request_empty_done()

Reported-by: liutao74748@163.com
ML: http://archives.seul.org/libevent/users/Nov-2016/msg00041.html

bufferevent_openssl.c

@@ -323,6 +323,8 @@ struct bufferevent_openssl {
 
 	/* Are we currently connecting, accepting, or doing IO? */
 	unsigned state : 2;
+	/* If we reset fd, we sould reset state too */
+	unsigned old_state : 2;
 };
 
 static int be_openssl_enable(struct bufferevent *, short);
@@ -502,7 +504,7 @@ conn_closed(struct bufferevent_openssl *bev_ssl, int when, int errcode, int ret)
 		break;
 	case SSL_ERROR_SYSCALL:
 		/* IO error; possibly a dirty shutdown. */
-		if (ret == 0 && ERR_peek_error() == 0)
+		if ((ret == 0 || ret == -1) && ERR_peek_error() == 0)
 			dirty_shutdown = 1;
 		break;
 	case SSL_ERROR_SSL:
@@ -1004,12 +1006,6 @@ set_open_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
 		return (r1<0 || r2<0) ? -1 : 0;
 	}
 }
-static int
-set_open_callbacks_auto(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
-{
-	fd = be_openssl_auto_fd(bev_ssl, fd);
-	return set_open_callbacks(bev_ssl, fd);
-}
 
 static int
 do_handshake(struct bufferevent_openssl *bev_ssl)
@@ -1111,13 +1107,6 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
 	}
 }
 
-static int
-set_handshake_callbacks_auto(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
-{
-	fd = be_openssl_auto_fd(bev_ssl, fd);
-	return set_handshake_callbacks(bev_ssl, fd);
-}
-
 int
 bufferevent_ssl_renegotiate(struct bufferevent *bev)
 {
@@ -1127,7 +1116,7 @@ bufferevent_ssl_renegotiate(struct bufferevent *bev)
 	if (SSL_renegotiate(bev_ssl->ssl) < 0)
 		return -1;
 	bev_ssl->state = BUFFEREVENT_SSL_CONNECTING;
-	if (set_handshake_callbacks_auto(bev_ssl, -1) < 0)
+	if (set_handshake_callbacks(bev_ssl, be_openssl_auto_fd(bev_ssl, -1)) < 0)
 		return -1;
 	if (!bev_ssl->underlying)
 		return do_handshake(bev_ssl);
@@ -1262,6 +1251,34 @@ be_openssl_flush(struct bufferevent *bufev,
 	return 0;
 }
 
+int
+be_openssl_set_fd(struct bufferevent_openssl *bev_ssl,
+    enum bufferevent_ssl_state state, int fd)
+{
+	bev_ssl->state = state;
+
+	switch (state) {
+	case BUFFEREVENT_SSL_ACCEPTING:
+		SSL_set_accept_state(bev_ssl->ssl);
+		if (set_handshake_callbacks(bev_ssl, fd) < 0)
+			return -1;
+		break;
+	case BUFFEREVENT_SSL_CONNECTING:
+		SSL_set_connect_state(bev_ssl->ssl);
+		if (set_handshake_callbacks(bev_ssl, fd) < 0)
+			return -1;
+		break;
+	case BUFFEREVENT_SSL_OPEN:
+		if (set_open_callbacks(bev_ssl, fd) < 0)
+			return -1;
+		break;
+	default:
+		return -1;
+	}
+
+	return 0;
+}
+
 static int
 be_openssl_ctrl(struct bufferevent *bev,
     enum bufferevent_ctrl_op op, union bufferevent_ctrl_data *data)
@@ -1276,11 +1293,8 @@ be_openssl_ctrl(struct bufferevent *bev,
 			bio = BIO_new_socket(data->fd, 0);
 			SSL_set_bio(bev_ssl->ssl, bio, bio);
 		}
-		if (bev_ssl->state == BUFFEREVENT_SSL_OPEN && data->fd >= 0)
+
-			return set_open_callbacks(bev_ssl, data->fd);
+		return be_openssl_set_fd(bev_ssl, bev_ssl->old_state, data->fd);
-		else {
-			return set_handshake_callbacks(bev_ssl, data->fd);
-		}
 	case BEV_CTRL_GET_FD:
 		data->fd = event_get_fd(&bev->ev_read);
 		return 0;
@@ -1344,29 +1358,14 @@ bufferevent_openssl_new_impl(struct event_base *base,
 		bufferevent_incref_(underlying);
 	}
 
-	bev_ssl->state = state;
+	bev_ssl->old_state = state;
 	bev_ssl->last_write = -1;
 
 	init_bio_counts(bev_ssl);
 
-	switch (state) {
+	fd = be_openssl_auto_fd(bev_ssl, fd);
-	case BUFFEREVENT_SSL_ACCEPTING:
+	if (be_openssl_set_fd(bev_ssl, state, fd))
-		SSL_set_accept_state(bev_ssl->ssl);
-		if (set_handshake_callbacks_auto(bev_ssl, fd) < 0)
-			goto err;
-		break;
-	case BUFFEREVENT_SSL_CONNECTING:
-		SSL_set_connect_state(bev_ssl->ssl);
-		if (set_handshake_callbacks_auto(bev_ssl, fd) < 0)
-			goto err;
-		break;
-	case BUFFEREVENT_SSL_OPEN:
-		if (set_open_callbacks_auto(bev_ssl, fd) < 0)
-			goto err;
-		break;
-	default:
 		goto err;
-	}
 
 	if (underlying) {
 		bufferevent_setwatermark(underlying, EV_READ, 0, 0);

test/regress_http.c

@@ -1004,12 +1004,21 @@ static void http_request_empty_done(struct evhttp_request *, void *);
 
 static void
 http_connection_test_(struct basic_test_data *data, int persistent,
-	const char *address, struct evdns_base *dnsbase, int ipv6, int family)
+	const char *address, struct evdns_base *dnsbase, int ipv6, int family,
+	int ssl)
 {
 	ev_uint16_t port = 0;
 	struct evhttp_connection *evcon = NULL;
 	struct evhttp_request *req = NULL;
-	struct evhttp *http = http_setup(&port, data->base, ipv6);
+	struct evhttp *http;
+
+	int mask = 0;
+	if (ipv6)
+		mask |= HTTP_BIND_IPV6;
+	if (ssl)
+		mask |= HTTP_BIND_SSL;
+
+	http = http_setup(&port, data->base, mask);
 
 	test_ok = 0;
 	if (!http && ipv6) {
@@ -1017,7 +1026,21 @@ http_connection_test_(struct basic_test_data *data, int persistent,
 	}
 	tt_assert(http);
 
-	evcon = evhttp_connection_base_new(data->base, dnsbase, address, port);
+	if (ssl) {
+#ifdef EVENT__HAVE_OPENSSL
+		SSL *ssl = SSL_new(get_ssl_ctx());
+		struct bufferevent *bev = bufferevent_openssl_socket_new(
+			data->base, -1, ssl,
+			BUFFEREVENT_SSL_CONNECTING, BEV_OPT_DEFER_CALLBACKS);
+		bufferevent_openssl_set_allow_dirty_shutdown(bev, 1);
+
+		evcon = evhttp_connection_base_bufferevent_new(data->base, dnsbase, bev, address, port);
+#else
+		tt_skip();
+#endif
+	} else {
+		evcon = evhttp_connection_base_new(data->base, dnsbase, address, port);
+	}
 	tt_assert(evcon);
 	evhttp_connection_set_family(evcon, family);
 
@@ -1093,12 +1116,12 @@ http_connection_test_(struct basic_test_data *data, int persistent,
 static void
 http_connection_test(void *arg)
 {
-	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_UNSPEC);
+	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_UNSPEC, 0);
 }
 static void
 http_persist_connection_test(void *arg)
 {
-	http_connection_test_(arg, 1, "127.0.0.1", NULL, 0, AF_UNSPEC);
+	http_connection_test_(arg, 1, "127.0.0.1", NULL, 0, AF_UNSPEC, 0);
 }
 
 static struct regress_dns_server_table search_table[] = {
@@ -1540,6 +1563,11 @@ http_request_done(struct evhttp_request *req, void *arg)
 {
 	const char *what = arg;
 
+	if (!req) {
+		fprintf(stderr, "FAILED\n");
+		exit(1);
+	}
+
 	if (evhttp_request_get_response_code(req) != HTTP_OK) {
 		fprintf(stderr, "FAILED\n");
 		exit(1);
@@ -1743,6 +1771,11 @@ http_virtual_host_test(void *arg)
 static void
 http_request_empty_done(struct evhttp_request *req, void *arg)
 {
+	if (!req) {
+		fprintf(stderr, "FAILED\n");
+		exit(1);
+	}
+
 	if (evhttp_request_get_response_code(req) != HTTP_OK) {
 		fprintf(stderr, "FAILED\n");
 		exit(1);
@@ -3574,6 +3607,10 @@ http_simple_test_impl(void *arg, int ssl, int dirty)
 	test_ok = 0;
 
 	bev = create_bev(data->base, -1, ssl);
+#ifdef EVENT__HAVE_OPENSSL
+	bufferevent_openssl_set_allow_dirty_shutdown(bev, dirty);
+#endif
+
 	evcon = evhttp_connection_base_bufferevent_new(
 		data->base, NULL, bev, "127.0.0.1", hs.port);
 	tt_assert(evcon);
@@ -4289,7 +4326,7 @@ http_ipv6_for_domain_test_impl(void *arg, int family)
 	evdns_base_nameserver_ip_add(dns_base, address);
 
 	http_connection_test_(arg, 0 /* not persistent */, "localhost", dns_base,
-		1 /* ipv6 */, family);
+		1 /* ipv6 */, family, 0);
 
  end:
 	if (dns_base)
@@ -4367,12 +4404,12 @@ http_get_addr_test(void *arg)
 static void
 http_set_family_test(void *arg)
 {
-	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_UNSPEC);
+	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_UNSPEC, 0);
 }
 static void
 http_set_family_ipv4_test(void *arg)
 {
-	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_INET);
+	http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_INET, 0);
 }
 static void
 http_set_family_ipv6_test(void *arg)
@@ -4500,6 +4537,10 @@ static void https_connection_fail_test(void *arg)
 { return http_connection_fail_test_impl(arg, 1); }
 static void https_write_during_read_test(void *arg)
 { return http_write_during_read_test_impl(arg, 1); }
+static void https_connection_test(void *arg)
+{ return http_connection_test_(arg, 0, "127.0.0.1", NULL, 0, AF_UNSPEC, 1); }
+static void https_persist_connection_test(void *arg)
+{ return http_connection_test_(arg, 1, "127.0.0.1", NULL, 0, AF_UNSPEC, 1); }
 #endif
 
 struct testcase_t http_testcases[] = {
@@ -4590,6 +4631,8 @@ struct testcase_t http_testcases[] = {
 	HTTPS(stream_out),
 	HTTPS(connection_fail),
 	HTTPS(write_during_read),
+	HTTPS(connection),
+	HTTPS(persist_connection),
 #endif
 
 	END_OF_TESTCASES