From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 30 Oct 2009 21:08:29 +0000 (+0000)
Subject: Keep openssl errors associated with the right bufferevent object.
X-Git-Tag: release-2.0.3-alpha~63
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=516452b71a4450f7201bc1a6bd31878c68a81e57;p=libevent

Keep openssl errors associated with the right bufferevent object.

OpenSSL has a per-thread error stack, and really doesn't like you
leaving errors on the stack.  Rather than discard the errors or force
the user to handle them, this patch pulls them off the openssl stack
and puts them on a stack associated with the bufferevent_openssl.  If
the user leaves them on the stack then, it won't affect any other
connections.

This bug was found by Roman Puls.  Thanks!

svn:r1481
---

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index 2a1b9c00..b88a78aa 100644
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -89,7 +89,7 @@ print_err(int val)
 	int err;
 	printf("Error was %d\n", val);
 
-	while ((err = ERR_get_error())) {
+	while ((err = ERR_get_error()))x {
 		const char *msg = (const char*)ERR_reason_error_string(err);
 		const char *lib = (const char*)ERR_lib_error_string(err);
 		const char *func = (const char*)ERR_func_error_string(err);
@@ -296,6 +296,9 @@ struct bufferevent_openssl {
 	 * and we need to try it again with this many bytes. */
 	ev_ssize_t last_write;
 
+#define NUM_ERRORS 3
+	ev_uint32_t errors[NUM_ERRORS];
+
 	/* When we next get available space, we should say "read" instead of
 	   "write". This can happen if there's a renegotiation during a read
 	   operation. */
@@ -306,6 +309,8 @@ struct bufferevent_openssl {
 	unsigned allow_dirty_shutdown : 1;
 	/* XXXX */
 	unsigned fd_is_set : 1;
+	/* XXX */
+	unsigned n_errors : 2;
 
 	/* Are we currently connecting, accepting, or doing IO? */
 	unsigned state : 2;
@@ -344,6 +349,19 @@ upcast(struct bufferevent *bev)
 	return bev_o;
 }
 
+static inline void
+put_error(struct bufferevent_openssl *bev_ssl, unsigned long err)
+{
+	if (bev_ssl->n_errors == NUM_ERRORS)
+		return;
+	/* The error type according to openssl is "unsigned long", but
+	   openssl never uses more than 32 bits of it.  It _can't_ use more
+	   than 32 bits of it, since it needs to report errors on systems
+	   where long is only 32 bits.
+	 */
+	bev_ssl->errors[bev_ssl->n_errors++] = (uint32_t) err;
+}
+
 /* Have the base communications channel (either the underlying bufferevent or
  * ev_read and ev_write) start reading.  Take the read-blocked-on-write flag
  * into account. */
@@ -456,6 +474,7 @@ conn_closed(struct bufferevent_openssl *bev_ssl, int errcode, int ret)
 {
 	int event = BEV_EVENT_ERROR;
 	int dirty_shutdown = 0;
+	unsigned long err;
 
 	switch (errcode) {
 	case SSL_ERROR_ZERO_RETURN:
@@ -467,7 +486,7 @@ conn_closed(struct bufferevent_openssl *bev_ssl, int errcode, int ret)
 		break;
 	case SSL_ERROR_SYSCALL:
 		/* IO error; possibly a dirty shutdown. */
-		if (ret == 0 && ERR_get_error() == 0)
+		if (ret == 0 && ERR_peek_error() == 0)
 			dirty_shutdown = 1;
 		break;
 	case SSL_ERROR_SSL:
@@ -486,6 +505,10 @@ conn_closed(struct bufferevent_openssl *bev_ssl, int errcode, int ret)
 		event_errx(1, "Unexpected OpenSSL error code %d", errcode);
 	}
 
+	while ((err = ERR_get_error())) {
+		put_error(bev_ssl, err);
+	}
+
 	if (dirty_shutdown && bev_ssl->allow_dirty_shutdown)
 		event = BEV_EVENT_EOF;
 
@@ -1179,3 +1202,17 @@ bufferevent_openssl_socket_new(struct event_base *base,
 	return bufferevent_openssl_new_impl(
 		base, NULL, fd, ssl, state, options);
 }
+
+unsigned long
+bufferevent_get_openssl_error(struct bufferevent *bev)
+{
+	unsigned long err = 0;
+	struct bufferevent_openssl *bev_ssl;
+	BEV_LOCK(bev);
+	bev_ssl = upcast(bev);
+	if (bev_ssl && bev_ssl->n_errors) {
+		err = bev_ssl->errors[--bev_ssl->n_errors];
+	}
+	BEV_UNLOCK(bev);
+	return err;
+}
diff --git a/include/event2/bufferevent_ssl.h b/include/event2/bufferevent_ssl.h
index 1ae4f611..91fbf4ab 100644
--- a/include/event2/bufferevent_ssl.h
+++ b/include/event2/bufferevent_ssl.h
@@ -67,6 +67,8 @@ bufferevent_openssl_get_ssl(struct bufferevent *bufev);
 
 int bufferevent_ssl_renegotiate(struct bufferevent *bev);
 
+unsigned long bufferevent_get_openssl_error(struct bufferevent *bev);
+
 #endif
 
 #ifdef __cplusplus
diff --git a/sample/le-proxy.c b/sample/le-proxy.c
index 348bec39..84ed78ad 100644
--- a/sample/le-proxy.c
+++ b/sample/le-proxy.c
@@ -87,8 +87,21 @@ eventcb(struct bufferevent *bev, short what, void *ctx)
 	struct bufferevent *partner = ctx;
 
 	if (what & (BEV_EVENT_EOF|BEV_EVENT_ERROR)) {
-		if (what & BEV_EVENT_ERROR)
-			perror("maybe an error");
+		if (what & BEV_EVENT_ERROR) {
+			unsigned long err;
+			while ((err = (bufferevent_get_openssl_error(bev)))) {
+				const char *msg = (const char*)
+				    ERR_reason_error_string(err);
+				const char *lib = (const char*)
+				    ERR_lib_error_string(err);
+				const char *func = (const char*)
+				    ERR_func_error_string(err);
+				fprintf(stderr,
+				    "%s in %s %s\n", msg, lib, func);
+			}
+			if (errno)
+				perror("connection error");
+		}
 
 		if (partner) {
 			/* Flush all pending data */