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 2a1b9c00026925f12405ffdecc861d79681e5064..b88a78aabe05afd699e14be29339b0741034e109 100644 (file)
--- 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 1ae4f61131dbb25d281bfe32accfaaa9c73134d9..91fbf4ab06852df5be50cbc703896dcd8c447f31 100644 (file)
--- 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 348bec39cbc77f43491035abf12acaf0d5a53cdd..84ed78ad2104d549dc1f5045ac522659b15a1ab2 100644 (file)
--- 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 */