From 8db6134c75aae08da059ad229528f5771d2a2cdc Mon Sep 17 00:00:00 2001
From: Eric Rosenberg <eric_rosenberg@apple.com>
Date: Mon, 28 Nov 2022 22:23:37 +0000
Subject: [PATCH] bound session cache

When establishing new TLS sessions, servers may send multiple session
tickets (RFC8446 4.6.1). hyper-boring caches tickets without placing a
limit on how many tickets are cached. This leads to unbounded growth of
hyper-boring's cache and leaves clients vulnerable to malicious servers
who might send many session tickets to exhaust a client's available
memory.

This change bounds the cache to a default of 8 tickets.
---
 hyper-boring/src/cache.rs | 18 +++++++++++++-----
 hyper-boring/src/lib.rs   | 20 +++++++++++++-------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/hyper-boring/src/cache.rs b/hyper-boring/src/cache.rs
index 7983d321..185c3dc4 100644
--- a/hyper-boring/src/cache.rs
+++ b/hyper-boring/src/cache.rs
@@ -40,24 +40,32 @@ impl Borrow<[u8]> for HashSession {
 pub struct SessionCache {
     sessions: HashMap<SessionKey, LinkedHashSet<HashSession>>,
     reverse: HashMap<HashSession, SessionKey>,
+    /// Maximum capacity of LinkedHashSet per SessionKey
+    per_key_session_capacity: usize,
 }
 
 impl SessionCache {
-    pub fn new() -> SessionCache {
+    pub fn with_capacity(per_key_session_capacity: usize) -> SessionCache {
         SessionCache {
             sessions: HashMap::new(),
             reverse: HashMap::new(),
+            per_key_session_capacity,
         }
     }
 
     pub fn insert(&mut self, key: SessionKey, session: SslSession) {
         let session = HashSession(session);
 
-        self.sessions
-            .entry(key.clone())
-            .or_default()
-            .insert(session.clone());
+        let sessions = self.sessions.entry(key.clone()).or_default();
 
+        // if sessions exceed capacity, discard oldest
+        if sessions.len() >= self.per_key_session_capacity {
+            if let Some(hash) = sessions.pop_front() {
+                self.reverse.remove(&hash);
+            }
+        }
+
+        sessions.insert(session.clone());
         self.reverse.insert(session, key);
     }
 
diff --git a/hyper-boring/src/lib.rs b/hyper-boring/src/lib.rs
index 6420019d..bbd60b77 100644
--- a/hyper-boring/src/lib.rs
+++ b/hyper-boring/src/lib.rs
@@ -102,8 +102,19 @@ impl HttpsLayer {
     /// Creates a new `HttpsLayer`.
     ///
     /// The session cache configuration of `ssl` will be overwritten.
-    pub fn with_connector(mut ssl: SslConnectorBuilder) -> Result<HttpsLayer, ErrorStack> {
-        let cache = Arc::new(Mutex::new(SessionCache::new()));
+    pub fn with_connector(ssl: SslConnectorBuilder) -> Result<HttpsLayer, ErrorStack> {
+        Self::with_connector_and_capacity(ssl, 8)
+    }
+
+    /// Creates a new `HttpsLayer` with session capacity.
+    ///
+    /// The session cache configuration of `ssl` will be overwritten. Session capacity is per
+    /// session key (domain).
+    pub fn with_connector_and_capacity(
+        mut ssl: SslConnectorBuilder,
+        capacity: usize,
+    ) -> Result<HttpsLayer, ErrorStack> {
+        let cache = Arc::new(Mutex::new(SessionCache::with_capacity(capacity)));
 
         ssl.set_session_cache_mode(SslSessionCacheMode::CLIENT);
 
@@ -116,11 +127,6 @@ impl HttpsLayer {
             }
         });
 
-        ssl.set_remove_session_callback({
-            let cache = cache.clone();
-            move |_, session| cache.lock().remove(session)
-        });
-
         Ok(HttpsLayer {
             inner: Inner {
                 ssl: ssl.build(),