rustls · ctz · Sep 30, 2018 · Sep 25, 2018 · Sep 29, 2018
diff --git a/README.md b/README.md
@@ -20,6 +20,10 @@ Rustls is currently in development and hence unstable.  [Here's what I'm working
   - Fix a bug in rustls::Stream for non-blocking transports.
   - Move TLS1.3 support from draft 28 to final RFC8446 version.
   - Don't offer (eg) TLS1.3 if no TLS1.3 suites are configured.
+  - Support stateful resumption in TLS1.3.  Stateless resumption
+    was previously supported, but is not the default configuration.
+  - *Breaking API change*: `generate()` removed from `StoresServerSessions` trait.
+  - *Breaking API change*: `take()` added to `StoresServerSessions` trait.
 * 0.13.1 (2018-08-17):
   - Fix a bug in rustls::Stream for non-blocking transports
     (backport).

diff --git a/examples/internal/bogo_shim.rs b/examples/internal/bogo_shim.rs
@@ -297,6 +297,8 @@ fn make_server_cfg(opts: &Options) -> Arc<rustls::ServerConfig> {
 
     if opts.tickets {
         cfg.ticketer = rustls::Ticketer::new();
+    } else if opts.resumes == 0 {
+        cfg.set_persistence(Arc::new(rustls::NoServerSessionStorage {}));
     }
 
     if !opts.protocols.is_empty() {

diff --git a/src/client/hs.rs b/src/client/hs.rs
@@ -1009,7 +1009,8 @@ impl State for ExpectTLS13EncryptedExtensions {
         }
 
         if self.handshake.resuming_session.is_some() {
-            if sess.common.early_traffic {
+            let was_early_traffic = sess.common.early_traffic;
+            if was_early_traffic {
                 if exts.early_data_extension_offered() {
                     sess.early_data.accepted();
                 } else {
@@ -1018,7 +1019,7 @@ impl State for ExpectTLS13EncryptedExtensions {
                 }
             }
 
-            if !sess.common.early_traffic {
+            if was_early_traffic && !sess.common.early_traffic {
                 // If no early traffic, set the encryption key for handshakes
                 let suite = sess.common.get_suite_assert();
                 let write_key = sess.common.get_key_schedule()

diff --git a/src/server/handy.rs b/src/server/handy.rs
@@ -1,6 +1,4 @@
 use msgs::enums::SignatureScheme;
-use msgs::handshake::SessionID;
-use rand;
 use sign;
 use key;
 use webpki;
@@ -14,15 +12,15 @@ use std::sync::{Arc, Mutex};
 pub struct NoServerSessionStorage {}
 
 impl server::StoresServerSessions for NoServerSessionStorage {
-    fn generate(&self) -> SessionID {
-        SessionID::empty()
-    }
     fn put(&self, _id: Vec<u8>, _sec: Vec<u8>) -> bool {
         false
     }
     fn get(&self, _id: &[u8]) -> Option<Vec<u8>> {
         None
     }
+    fn take(&self, _id: &[u8]) -> Option<Vec<u8>> {
+        None
+    }
 }
 
 /// An implementor of `StoresServerSessions` that stores everything
@@ -54,12 +52,6 @@ impl ServerSessionMemoryCache {
 }
 
 impl server::StoresServerSessions for ServerSessionMemoryCache {
-    fn generate(&self) -> SessionID {
-        let mut v = [0u8; 32];
-        rand::fill_random(&mut v);
-        SessionID::new(&v)
-    }
-
     fn put(&self, key: Vec<u8>, value: Vec<u8>) -> bool {
         self.cache.lock()
             .unwrap()
@@ -73,6 +65,12 @@ impl server::StoresServerSessions for ServerSessionMemoryCache {
             .unwrap()
             .get(key).cloned()
     }
+
+    fn take(&self, key: &[u8]) -> Option<Vec<u8>> {
+        self.cache.lock()
+            .unwrap()
+            .remove(key)
+    }
 }
 
 /// Something which never produces tickets.
@@ -193,14 +191,6 @@ mod test {
     use super::*;
     use StoresServerSessions;
 
-    #[test]
-    fn test_noserversessionstorage_yields_no_sessid() {
-        let c = NoServerSessionStorage {};
-        assert_eq!(c.generate(), SessionID::empty());
-        assert_eq!(c.generate().len(), 0);
-        assert!(c.generate().is_empty());
-    }
-
     #[test]
     fn test_noserversessionstorage_drops_put() {
         let c = NoServerSessionStorage {};
@@ -216,12 +206,6 @@ mod test {
         assert_eq!(c.get(&[0x02]), None);
     }
 
-    #[test]
-    fn test_serversessionmemorycache_yields_sessid() {
-        let c = ServerSessionMemoryCache::new(4);
-        assert_eq!(c.generate().len(), 32);
-    }
-
     #[test]
     fn test_serversessionmemorycache_accepts_put() {
         let c = ServerSessionMemoryCache::new(4);

diff --git a/src/server/hs.rs b/src/server/hs.rs
@@ -768,6 +768,22 @@ impl ExpectClientHello {
         sess.common.send_msg(m, false);
     }
 
+    fn attempt_tls13_ticket_decryption(&mut self,
+                                       sess: &mut ServerSessionImpl,
+                                       ticket: &[u8]) -> Option<persist::ServerSessionValue> {
+        if sess.config.ticketer.enabled() {
+            sess.config
+                .ticketer
+                .decrypt(ticket)
+                .and_then(|plain| persist::ServerSessionValue::read_bytes(&plain))
+        } else {
+            sess.config
+                .session_storage
+                .take(ticket)
+                .and_then(|plain| persist::ServerSessionValue::read_bytes(&plain))
+        }
+    }
+
     fn start_resumption(mut self,
                         sess: &mut ServerSessionImpl,
                         client_hello: &ClientHelloPayload,
@@ -881,10 +897,7 @@ impl ExpectClientHello {
             }
 
             for (i, psk_id) in psk_offer.identities.iter().enumerate() {
-                let maybe_resume = sess.config
-                    .ticketer
-                    .decrypt(&psk_id.identity.0)
-                    .and_then(|plain| persist::ServerSessionValue::read_bytes(&plain));
+                let maybe_resume = self.attempt_tls13_ticket_decryption(sess, &psk_id.identity.0);
 
                 if !can_resume(sess, &self.handshake, &maybe_resume) {
                     continue;
@@ -1129,10 +1142,9 @@ impl State for ExpectClientHello {
         // If we're not offered a ticket or a potential session ID,
         // allocate a session ID.
         if self.handshake.session_id.is_empty() && !ticket_received {
-            let sessid = sess.config
-                .session_storage
-                .generate();
-            self.handshake.session_id = sessid;
+            let mut bytes = [0u8; 32];
+            rand::fill_random(&mut bytes);
+            self.handshake.session_id = SessionID::new(&bytes);
         }
 
         // Perhaps resume?  If we received a ticket, the sessionid
@@ -1672,11 +1684,8 @@ impl ExpectTLS13Finished {
         })
     }
 
-    fn emit_ticket_tls13(&mut self, sess: &mut ServerSessionImpl) {
-        if !self.send_ticket {
-            return;
-        }
-
+    fn emit_stateless_ticket_tls13(&mut self, sess: &mut ServerSessionImpl) {
+        debug_assert!(self.send_ticket);
         let nonce = rand::random_vec(32);
         let plain = get_server_session_value_tls13(&self.handshake, sess, &nonce)
             .get_encoding();
@@ -1701,10 +1710,38 @@ impl ExpectTLS13Finished {
             }),
         };
 
-        trace!("sending new ticket {:?}", m);
+        trace!("sending new stateless ticket {:?}", m);
         self.handshake.transcript.add_message(&m);
         sess.common.send_msg(m, true);
     }
+
+    fn emit_stateful_ticket_tls13(&mut self, sess: &mut ServerSessionImpl) {
+        debug_assert!(self.send_ticket);
+        let nonce = rand::random_vec(32);
+        let id = rand::random_vec(32);
+        let plain = get_server_session_value_tls13(&self.handshake, sess, &nonce)
+            .get_encoding();
+
+        if sess.config.session_storage.put(id.clone(), plain) {
+            let stateful_lifetime = 24 * 60 * 60; // this is a bit of a punt
+            let age_add = rand::random_u32();
+            let payload = NewSessionTicketPayloadTLS13::new(stateful_lifetime, age_add, nonce, id);
+            let m = Message {
+                typ: ContentType::Handshake,
+                version: ProtocolVersion::TLSv1_3,
+                payload: MessagePayload::Handshake(HandshakeMessagePayload {
+                    typ: HandshakeType::NewSessionTicket,
+                    payload: HandshakePayload::NewSessionTicketTLS13(payload),
+                }),
+            };
+
+            trace!("sending new stateful ticket {:?}", m);
+            self.handshake.transcript.add_message(&m);
+            sess.common.send_msg(m, true);
+        } else {
+            trace!("resumption not available; not issuing ticket");
+        }
+    }
 }
 
 impl State for ExpectTLS13Finished {
@@ -1749,8 +1786,12 @@ impl State for ExpectTLS13Finished {
             .get_mut_key_schedule()
             .current_client_traffic_secret = read_key;
 
-        if sess.config.ticketer.enabled() {
-            self.emit_ticket_tls13(sess);
+        if self.send_ticket {
+            if sess.config.ticketer.enabled() {
+                self.emit_stateless_ticket_tls13(sess);
+            } else {
+                self.emit_stateful_ticket_tls13(sess);
+            }
         }
 
         sess.common.we_now_encrypting();

diff --git a/src/server/mod.rs b/src/server/mod.rs
@@ -3,7 +3,7 @@ use keylog::{KeyLog, NoKeyLog};
 use suites::{SupportedCipherSuite, ALL_CIPHERSUITES};
 use msgs::enums::{ContentType, SignatureScheme};
 use msgs::enums::{AlertDescription, HandshakeType, ProtocolVersion};
-use msgs::handshake::{ServerExtension, SessionID};
+use msgs::handshake::ServerExtension;
 use msgs::message::Message;
 use error::TLSError;
 use sign;
@@ -21,29 +21,37 @@ mod hs;
 mod common;
 pub mod handy;
 
-/// A trait for the ability to generate Session IDs, and store
-/// server session data. The keys and values are opaque.
+/// A trait for the ability to store server session data.
+///
+/// The keys and values are opaque.
 ///
 /// Both the keys and values should be treated as
 /// **highly sensitive data**, containing enough key material
-/// to break all security of the corresponding session.
+/// to break all security of the corresponding sessions.
+///
+/// Implementations can be lossy (in other words, forgetting
+/// key/value pairs) without any negative security consequences.
+///
+/// However, note that `take` **must** reliably delete a returned
+/// value.  If it does not, there may be security consequences.
 ///
-/// `put` is a mutating operation; this isn't expressed
+/// `put` and `take` are mutating operations; this isn't expressed
 /// in the type system to allow implementations freedom in
 /// how to achieve interior mutability.  `Mutex` is a common
 /// choice.
 pub trait StoresServerSessions : Send + Sync {
-    /// Generate a session ID.
-    fn generate(&self) -> SessionID;
-
-    /// Store session secrets encoded in `value` against key `id`,
-    /// overwrites any existing value against `id`.  Returns `true`
+    /// Store session secrets encoded in `value` against `key`,
+    /// overwrites any existing value against `key`.  Returns `true`
     /// if the value was stored.
     fn put(&self, key: Vec<u8>, value: Vec<u8>) -> bool;
 
-    /// Find a session with the given `id`.  Return it, or None
+    /// Find a value with the given `key`.  Return it, or None
     /// if it doesn't exist.
     fn get(&self, key: &[u8]) -> Option<Vec<u8>>;
+
+    /// Find a value with the given `key`.  Return it and delete it;
+    /// or None if it doesn't exist.
+    fn take(&self, key: &[u8]) -> Option<Vec<u8>>;
 }
 
 /// A trait for the ability to encrypt and decrypt tickets.