stream-ssl: Only cache SSL sessions after they shut down.

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

diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 69beab9..84c1a11 100644 (file)
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,7 +45,10 @@
 #include "timeval.h"
 #include "vlog.h"
 
-VLOG_DEFINE_THIS_MODULE(stream_ssl)
+VLOG_DEFINE_THIS_MODULE(stream_ssl);
+
+COVERAGE_DEFINE(ssl_session);
+COVERAGE_DEFINE(ssl_session_reused);
 
 /* Active SSL. */
 
@@ -385,7 +388,7 @@ do_ca_cert_bootstrap(struct stream *stream)
 
     file = fdopen(fd, "w");
     if (!file) {
-        int error = errno;
+        error = errno;
         VLOG_ERR("could not bootstrap CA cert: fdopen failed: %s",
                  strerror(error));
         unlink(ca_cert.file_name);
@@ -402,7 +405,7 @@ do_ca_cert_bootstrap(struct stream *stream)
     }
 
     if (fclose(file)) {
-        int error = errno;
+        error = errno;
         VLOG_ERR("could not bootstrap CA cert: writing %s failed: %s",
                  ca_cert.file_name, strerror(error));
         unlink(ca_cert.file_name);
@@ -460,12 +463,6 @@ 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) {
@@ -487,12 +484,6 @@ ssl_cache_session(struct stream *stream)
                 }
             }
         }
-    } 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);
     }
 }
 
@@ -572,8 +563,10 @@ 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);
+            /* Statistics. */
+            COVERAGE_INC(ssl_session);
+            if (SSL_session_reused(sslv->ssl)) {
+                COVERAGE_INC(ssl_session_reused);
             }
             return 0;
         }
@@ -595,6 +588,8 @@ ssl_close(struct stream *stream)
      * background. */
     SSL_shutdown(sslv->ssl);
 
+    ssl_cache_session(stream);
+
     /* SSL_shutdown() might have signaled an error, in which case we need to
      * flush it out of the OpenSSL error queue or the next OpenSSL operation
      * will falsely signal an error. */
@@ -921,7 +916,7 @@ pssl_accept(struct pstream *pstream, struct stream **new_streamp)
 
     new_fd = accept(pssl->fd, &sin, &sin_len);
     if (new_fd < 0) {
-        int error = errno;
+        error = errno;
         if (error != EAGAIN) {
             VLOG_DBG_RL(&rl, "accept: %s", strerror(error));
         }
@@ -988,7 +983,9 @@ do_ssl_init(void)
     SSL_library_init();
     SSL_load_error_strings();
 
-    method = TLSv1_method();
+    /* New OpenSSL changed TLSv1_method() to return a "const" pointer, so the
+     * cast is needed to avoid a warning with those newer versions. */
+    method = (SSL_METHOD *) TLSv1_method();
     if (method == NULL) {
         VLOG_ERR("TLSv1_method: %s", ERR_error_string(ERR_get_error(), NULL));
         return ENOPROTOOPT;
@@ -1006,6 +1003,17 @@ do_ssl_init(void)
     SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
                        NULL);
 
+    /* We have to set a session context ID string in 'ctx' because OpenSSL
+     * otherwise refuses to use a cached session on the server side when
+     * SSL_VERIFY_PEER is set.  And it not only refuses to use the cached
+     * session, it actually generates an error and kills the connection.
+     * According to a comment in ssl_get_prev_session() in OpenSSL's
+     * ssl/ssl_sess.c, this is intentional behavior.
+     *
+     * Any context string is OK, as long as one is set. */
+    SSL_CTX_set_session_id_context(ctx, (const unsigned char *) PACKAGE,
+                                   strlen(PACKAGE));
+
     return 0;
 }
 
@@ -1045,7 +1053,7 @@ tmp_dh_callback(SSL *ssl OVS_UNUSED, int is_export OVS_UNUSED, int keylength)
 
 /* Returns true if SSL is at least partially configured. */
 bool
-stream_ssl_is_configured(void) 
+stream_ssl_is_configured(void)
 {
     return private_key.file_name || certificate.file_name || ca_cert.file_name;
 }
@@ -1262,7 +1270,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);
 }
 
@@ -1286,7 +1294,7 @@ stream_ssl_set_ca_cert_file__(const char *file_name, bool bootstrap)
         for (i = 0; i < n_certs; i++) {
             /* SSL_CTX_add_client_CA makes a copy of the relevant data. */
             if (SSL_CTX_add_client_CA(ctx, certs[i]) != 1) {
-                VLOG_ERR("failed to add client certificate %d from %s: %s",
+                VLOG_ERR("failed to add client certificate %zu from %s: %s",
                          i, file_name,
                          ERR_error_string(ERR_get_error(), NULL));
             } else {