stream-ssl: Use OPENSSL_free() to free memory from X509_NAME_oneline().

[sliver-openvswitch.git] / lib / stream-ssl.c

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 2bc9d1a..a633587 100644 (file)
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -30,12 +30,14 @@
 #include <sys/fcntl.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include "coverage.h"
 #include "dynamic-string.h"
 #include "leak-checker.h"
 #include "ofpbuf.h"
 #include "openflow/openflow.h"
 #include "packets.h"
 #include "poll-loop.h"
+#include "shash.h"
 #include "socket-util.h"
 #include "util.h"
 #include "stream-provider.h"
@@ -133,6 +135,20 @@ struct ssl_stream
 /* SSL context created by ssl_init(). */
 static SSL_CTX *ctx;
 
+/* Maps from stream target (e.g. "127.0.0.1:1234") to SSL_SESSION *.  The
+ * sessions are those from the last SSL connection to the given target.
+ * OpenSSL caches server-side sessions internally, so this cache is only used
+ * for client connections.
+ *
+ * The stream_ssl module owns a reference to each of the sessions in this
+ * table, so they must be freed with SSL_SESSION_free() when they are no
+ * longer needed. */
+static struct shash client_sessions = SHASH_INITIALIZER(&client_sessions);
+
+/* Maximum number of client sessions to cache.  Ordinarily I'd expect that one
+ * session would be sufficient but this should cover it. */
+#define MAX_CLIENT_SESSION_CACHE 16
+
 struct ssl_config_file {
     bool read;                  /* Whether the file was successfully read. */
     char *file_name;            /* Configured file name, if any. */
@@ -416,6 +432,70 @@ do_ca_cert_bootstrap(struct stream *stream)
     return EPROTO;
 }
 
+static void
+ssl_delete_session(struct shash_node *node)
+{
+    SSL_SESSION *session = node->data;
+    SSL_SESSION_free(session);
+    shash_delete(&client_sessions, node);
+}
+
+/* Find and free any previously cached session for 'stream''s target. */
+static void
+ssl_flush_session(struct stream *stream)
+{
+    struct shash_node *node;
+
+    node = shash_find(&client_sessions, stream_get_name(stream));
+    if (node) {
+        ssl_delete_session(node);
+    }
+}
+
+/* Add 'stream''s session to the cache for its target, so that it will be
+ * reused for future SSL connections to the same target. */
+static void
+ssl_cache_session(struct stream *stream)
+{
+    struct ssl_stream *sslv = ssl_stream_cast(stream);
+    SSL_SESSION *session;
+
+    /* Statistics. */
+    COVERAGE_INC(ssl_session);
+    if (SSL_session_reused(sslv->ssl)) {
+        COVERAGE_INC(ssl_session_reused);
+    }
+
+    /* Get session from stream. */
+    session = SSL_get1_session(sslv->ssl);
+    if (session) {
+        SSL_SESSION *old_session;
+
+        old_session = shash_replace(&client_sessions, stream_get_name(stream),
+                                    session);
+        if (old_session) {
+            /* Free the session that we replaced.  (We might actually have
+             * session == old_session, but either way we have to free it to
+             * avoid leaking a reference.) */
+            SSL_SESSION_free(old_session);
+        } else if (shash_count(&client_sessions) > MAX_CLIENT_SESSION_CACHE) {
+            for (;;) {
+                struct shash_node *node = shash_random_node(&client_sessions);
+                if (node->data != session) {
+                    ssl_delete_session(node);
+                    break;
+                }
+            }
+        }
+    } else {
+        /* There is no new session.  This doesn't really make sense because
+         * this function is only called upon successful connection and there
+         * should always be a new session in that case.  But I don't trust
+         * OpenSSL so I'd rather handle this case anyway. */
+        ssl_flush_session(stream);
+    }
+}
+
 static int
 ssl_connect(struct stream *stream)
 {
@@ -439,6 +519,15 @@ ssl_connect(struct stream *stream)
                                 MSG_PEEK);
         }
 
+        /* Grab SSL session information from the cache. */
+        if (sslv->type == CLIENT) {
+            SSL_SESSION *session = shash_find_data(&client_sessions,
+                                                   stream_get_name(stream));
+            if (session) {
+                SSL_set_session(sslv->ssl, session);
+            }
+        }
+
         retval = (sslv->type == CLIENT
                    ? SSL_connect(sslv->ssl) : SSL_accept(sslv->ssl));
         if (retval != 1) {
@@ -447,6 +536,18 @@ ssl_connect(struct stream *stream)
                 return EAGAIN;
             } else {
                 int unused;
+
+                if (sslv->type == CLIENT) {
+                    /* Delete any cached session for this stream's target.
+                     * Otherwise a single error causes recurring errors that
+                     * don't resolve until the SSL client or server is
+                     * restarted.  (It can take dozens of reused connections to
+                     * see this behavior, so this is difficult to test.)  If we
+                     * delete the session on the first error, though, the error
+                     * only occurs once and then resolves itself. */
+                    ssl_flush_session(stream);
+                }
+
                 interpret_ssl_error((sslv->type == CLIENT ? "SSL_connect"
                                      : "SSL_accept"), retval, error, &unused);
                 shutdown(sslv->fd, SHUT_RDWR);
@@ -471,6 +572,9 @@ ssl_connect(struct stream *stream)
             VLOG_ERR("rejecting SSL connection during bootstrap race window");
             return EPROTO;
         } else {
+            if (sslv->type == CLIENT) {
+                ssl_cache_session(stream);
+            }
             return 0;
         }
     }
@@ -1158,7 +1262,7 @@ log_ca_cert(const char *file_name, X509 *cert)
     subject = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0);
     VLOG_INFO("Trusting CA cert from %s (%s) (fingerprint %s)", file_name,
               subject ? subject : "<out of memory>", ds_cstr(&fp));
-    free(subject);
+    OPENSSL_free(subject);
     ds_destroy(&fp);
 }