SSL: explicitly clear the session staging buffer

Since 3d7304b52, sessions are serialized into the static buffer
ngx_ssl_session_buffer, so the last cached, resumed, or proxied session
remains in memory, master secret included, until overwritten. Clear the
buffer on all exits that write to it.

While here, the session id length check is moved before serialization,
which removes an exit path with a populated buffer and a needless
i2d_SSL_SESSION() call for sessions that are not cached.

Note that sessions are still kept in shared memory until evicted.

Assisted-by: opencode:claude-opus-5
Closes: https://github.com/nginx/nginx/issues/1736

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
index 670dfefb035876e3e95608f0cd39e9233bbebe9d..8ba0122d4f0f3bc91190fe5289c0b4734ffceea5 100644 (file)
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -4458,6 +4458,14 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess)
 
 #endif
 
+    session_id = (u_char *) SSL_SESSION_get_id(sess, &session_id_length);
+
+    /* do not cache sessions with too long session id */
+
+    if (session_id_length > 32) {
+        return 0;
+    }
+
     len = i2d_SSL_SESSION(sess, NULL);
 
     /* do not cache too big session */
@@ -4469,14 +4477,6 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess)
     p = ngx_ssl_session_buffer;
     i2d_SSL_SESSION(sess, &p);
 
-    session_id = (u_char *) SSL_SESSION_get_id(sess, &session_id_length);
-
-    /* do not cache sessions with too long session id */
-
-    if (session_id_length > 32) {
-        return 0;
-    }
-
     c = ngx_ssl_get_connection(ssl_conn);
 
     ssl_ctx = c->ssl->session_ctx;
@@ -4551,6 +4551,8 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_conn, ngx_ssl_session_t *sess)
 
     ngx_shmtx_unlock(&shpool->mutex);
 
+    ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
     return 0;
 
 failed:
@@ -4561,6 +4563,8 @@ failed:
 
     ngx_shmtx_unlock(&shpool->mutex);
 
+    ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
     if (cache->fail_time != ngx_time()) {
         cache->fail_time = ngx_time();
         ngx_log_error(NGX_LOG_WARN, c->log, 0,
@@ -4643,6 +4647,8 @@ ngx_ssl_get_cached_session(ngx_ssl_conn_t *ssl_conn,
                 p = ngx_ssl_session_buffer;
                 sess = d2i_SSL_SESSION(NULL, &p, slen);
 
+                ngx_explicit_memzero(ngx_ssl_session_buffer, slen);
+
                 return sess;
             }
 

diff --git a/src/http/ngx_http_upstream_round_robin.c b/src/http/ngx_http_upstream_round_robin.c
index 8f9bcfcf05b2e7c468f56c4b71151d38f0190276..2e60f1a3736b2f348497ed06a67d3d01cbf03d03 100644 (file)
--- a/src/http/ngx_http_upstream_round_robin.c
+++ b/src/http/ngx_http_upstream_round_robin.c
@@ -1142,6 +1142,8 @@ ngx_http_upstream_set_round_robin_peer_session(ngx_peer_connection_t *pc,
         p = ngx_ssl_session_buffer;
         ssl_session = d2i_SSL_SESSION(NULL, &p, len);
 
+        ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
         rc = ngx_ssl_set_session(pc->connection, ssl_session);
 
         ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0,
@@ -1224,6 +1226,9 @@ ngx_http_upstream_save_round_robin_peer_session(ngx_peer_connection_t *pc,
 
                 ngx_http_upstream_rr_peer_unlock(peers, peer);
                 ngx_http_upstream_rr_peers_unlock(peers);
+
+                ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
                 return;
             }
 
@@ -1235,6 +1240,8 @@ ngx_http_upstream_save_round_robin_peer_session(ngx_peer_connection_t *pc,
         ngx_http_upstream_rr_peer_unlock(peers, peer);
         ngx_http_upstream_rr_peers_unlock(peers);
 
+        ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
         return;
     }
 #endif

diff --git a/src/stream/ngx_stream_upstream_round_robin.c b/src/stream/ngx_stream_upstream_round_robin.c
index b4ea9a0af8cda75edfce0e7a990aebc58fae10fa..2c8e541992d848454e8a88e89ee4d95499517dbf 100644 (file)
--- a/src/stream/ngx_stream_upstream_round_robin.c
+++ b/src/stream/ngx_stream_upstream_round_robin.c
@@ -973,6 +973,8 @@ ngx_stream_upstream_set_round_robin_peer_session(ngx_peer_connection_t *pc,
         p = ngx_ssl_session_buffer;
         ssl_session = d2i_SSL_SESSION(NULL, &p, len);
 
+        ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
         rc = ngx_ssl_set_session(pc->connection, ssl_session);
 
         ngx_log_debug1(NGX_LOG_DEBUG_STREAM, pc->log, 0,
@@ -1055,6 +1057,9 @@ ngx_stream_upstream_save_round_robin_peer_session(ngx_peer_connection_t *pc,
 
                 ngx_stream_upstream_rr_peer_unlock(peers, peer);
                 ngx_stream_upstream_rr_peers_unlock(peers);
+
+                ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
                 return;
             }
 
@@ -1066,6 +1071,8 @@ ngx_stream_upstream_save_round_robin_peer_session(ngx_peer_connection_t *pc,
         ngx_stream_upstream_rr_peer_unlock(peers, peer);
         ngx_stream_upstream_rr_peers_unlock(peers);
 
+        ngx_explicit_memzero(ngx_ssl_session_buffer, len);
+
         return;
     }
 #endif