From 64ce9741800526fcf62151245f61af10bf269a1e Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 08:19:40 +0100
Subject: [PATCH 01/26] Don't check ciphersuite and compression in SSL session
 cache lookup

Session-ID based session resumption requires that the resumed session
is consistent with the client's ClientHello in terms of choice of
ciphersuite and choice of compression.

This check was previously assumed to be performed in the session cache
implementation, which seems wrong: The session cache should be an id-based
lookup only, and protocol specific checks should be left to Mbed TLS.

This commit
- adds an explicit ciphersuite and compression consistency check after
  the SSL session cache has been queried
- removes the ciphersuite and compression consistency check from
  Mbed TLS' session cache reference implementation.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 10 +++----
 library/ssl_srv.c   | 64 +++++++++++++++++++++++++++++++++------------
 2 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index bb5007bd1a..ce85157d2f 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -78,14 +78,12 @@ int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session )
             continue;
 #endif
 
-        if( session->ciphersuite != entry->session.ciphersuite ||
-            session->compression != entry->session.compression ||
-            session->id_len != entry->session.id_len )
-            continue;
-
-        if( memcmp( session->id, entry->session.id,
+        if( session->id_len != entry->session.id_len ||
+            memcmp( session->id, entry->session.id,
                     entry->session.id_len ) != 0 )
+        {
             continue;
+        }
 
         ret = mbedtls_ssl_session_copy( session, &entry->session );
         if( ret != 0 )
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 913a18f7e5..40ad490d8e 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2765,6 +2765,53 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl )
 }
 #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY */
 
+static void ssl_check_id_based_session_resumption( mbedtls_ssl_context *ssl )
+{
+    int ret;
+    mbedtls_ssl_session session_tmp = { 0 };
+    mbedtls_ssl_session * const session = ssl->session_negotiate;
+
+    /* Resume is 0  by default, see ssl_handshake_init().
+     * It may be already set to 1 by ssl_parse_session_ticket_ext(). */
+    if( ssl->handshake->resume == 1 )
+        return;
+    if( ssl->session_negotiate->id_len == 0 )
+        return;
+    if( ssl->conf->f_get_cache == NULL )
+        return;
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
+    if( ssl->renego_status != MBEDTLS_SSL_INITIAL_HANDSHAKE )
+        return;
+#endif
+
+    session_tmp.id_len = session->id_len;
+    memcpy( session_tmp.id, session->id, session->id_len );
+
+    ret = ssl->conf->f_get_cache( ssl->conf->p_cache,
+                                  &session_tmp );
+    if( ret != 0 )
+        goto exit;
+
+    if( session->ciphersuite != session_tmp.ciphersuite ||
+        session->compression != session_tmp.compression )
+    {
+        /* Mismatch between cached and negotiated session */
+        goto exit;
+    }
+
+    /* Move semantics */
+    /* Zeroization of session_tmp happens at the end of the function. */
+    mbedtls_ssl_session_free( session );
+    *session = session_tmp;
+
+    MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from cache" ) );
+    ssl->handshake->resume = 1;
+
+exit:
+
+    mbedtls_platform_zeroize( &session_tmp, sizeof( session_tmp ) );
+}
+
 static int ssl_write_server_hello( mbedtls_ssl_context *ssl )
 {
 #if defined(MBEDTLS_HAVE_TIME)
@@ -2835,22 +2882,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl )
 
     MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", buf + 6, 32 );
 
-    /*
-     * Resume is 0  by default, see ssl_handshake_init().
-     * It may be already set to 1 by ssl_parse_session_ticket_ext().
-     * If not, try looking up session ID in our cache.
-     */
-    if( ssl->handshake->resume == 0 &&
-#if defined(MBEDTLS_SSL_RENEGOTIATION)
-        ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE &&
-#endif
-        ssl->session_negotiate->id_len != 0 &&
-        ssl->conf->f_get_cache != NULL &&
-        ssl->conf->f_get_cache( ssl->conf->p_cache, ssl->session_negotiate ) == 0 )
-    {
-        MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from cache" ) );
-        ssl->handshake->resume = 1;
-    }
+    ssl_check_id_based_session_resumption( ssl );
 
     if( ssl->handshake->resume == 0 )
     {

From a637ff6ddd04bbe2b5728a9544da47807f44ce7c Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 08:42:48 +0100
Subject: [PATCH 02/26] Introduce typedef for SSL session cache callbacks

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl.h | 16 +++++++++++-----
 library/ssl_tls.c     |  6 +++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 41973b767f..b2d5574fc8 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -501,6 +501,7 @@ typedef enum
    MBEDTLS_SSL_TLS_PRF_SHA256
 }
 mbedtls_tls_prf_types;
+
 /**
  * \brief          Callback type: send data on the network.
  *
@@ -626,6 +627,11 @@ typedef struct mbedtls_ssl_key_cert mbedtls_ssl_key_cert;
 typedef struct mbedtls_ssl_flight_item mbedtls_ssl_flight_item;
 #endif
 
+/* TODO: Document */
+typedef int mbedtls_ssl_cache_get_t( void *data, mbedtls_ssl_session *session );
+/* TODO: Document */
+typedef int mbedtls_ssl_cache_set_t( void *data, const mbedtls_ssl_session *session );
+
 #if defined(MBEDTLS_SSL_ASYNC_PRIVATE)
 #if defined(MBEDTLS_X509_CRT_PARSE_C)
 /**
@@ -968,9 +974,9 @@ struct mbedtls_ssl_config
     void *p_rng;                    /*!< context for the RNG function       */
 
     /** Callback to retrieve a session from the cache                       */
-    int (*f_get_cache)(void *, mbedtls_ssl_session *);
+    mbedtls_ssl_cache_get_t *f_get_cache;
     /** Callback to store a session into the cache                          */
-    int (*f_set_cache)(void *, const mbedtls_ssl_session *);
+    mbedtls_ssl_cache_set_t *f_set_cache;
     void *p_cache;                  /*!< context for cache callbacks        */
 
 #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION)
@@ -2428,9 +2434,9 @@ void mbedtls_ssl_conf_handshake_timeout( mbedtls_ssl_config *conf, uint32_t min,
  * \param f_set_cache    session set callback
  */
 void mbedtls_ssl_conf_session_cache( mbedtls_ssl_config *conf,
-        void *p_cache,
-        int (*f_get_cache)(void *, mbedtls_ssl_session *),
-        int (*f_set_cache)(void *, const mbedtls_ssl_session *) );
+                                     void *p_cache,
+                                     mbedtls_ssl_cache_get_t *f_get_cache,
+                                     mbedtls_ssl_cache_set_t *f_set_cache );
 #endif /* MBEDTLS_SSL_SRV_C */
 
 #if defined(MBEDTLS_SSL_CLI_C)
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 1a04f6cb9c..67fcebfddc 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -4167,9 +4167,9 @@ void mbedtls_ssl_set_timer_cb( mbedtls_ssl_context *ssl,
 
 #if defined(MBEDTLS_SSL_SRV_C)
 void mbedtls_ssl_conf_session_cache( mbedtls_ssl_config *conf,
-        void *p_cache,
-        int (*f_get_cache)(void *, mbedtls_ssl_session *),
-        int (*f_set_cache)(void *, const mbedtls_ssl_session *) )
+                                     void *p_cache,
+                                     mbedtls_ssl_cache_get_t *f_get_cache,
+                                     mbedtls_ssl_cache_set_t *f_set_cache )
 {
     conf->p_cache = p_cache;
     conf->f_get_cache = f_get_cache;

From ccdaf6ed220bd8dd301e175864712df45043109f Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 09:26:17 +0100
Subject: [PATCH 03/26] Add session ID as explicit parameter to SSL session
 cache API

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl.h       | 10 ++++++++--
 include/mbedtls/ssl_cache.h | 25 +++++++++++++++++++------
 library/ssl_cache.c         | 19 ++++++++++++++-----
 library/ssl_srv.c           |  5 ++---
 library/ssl_tls.c           |  5 ++++-
 5 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index b2d5574fc8..89912c606f 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -628,9 +628,15 @@ typedef struct mbedtls_ssl_flight_item mbedtls_ssl_flight_item;
 #endif
 
 /* TODO: Document */
-typedef int mbedtls_ssl_cache_get_t( void *data, mbedtls_ssl_session *session );
+typedef int mbedtls_ssl_cache_get_t( void *data,
+                                     unsigned char const *session_id,
+                                     size_t session_id_len,
+                                     mbedtls_ssl_session *session );
 /* TODO: Document */
-typedef int mbedtls_ssl_cache_set_t( void *data, const mbedtls_ssl_session *session );
+typedef int mbedtls_ssl_cache_set_t( void *data,
+                                     unsigned char const *session_id,
+                                     size_t session_id_len,
+                                     const mbedtls_ssl_session *session );
 
 #if defined(MBEDTLS_SSL_ASYNC_PRIVATE)
 #if defined(MBEDTLS_X509_CRT_PARSE_C)
diff --git a/include/mbedtls/ssl_cache.h b/include/mbedtls/ssl_cache.h
index c6ef2960f4..cb55f7f96f 100644
--- a/include/mbedtls/ssl_cache.h
+++ b/include/mbedtls/ssl_cache.h
@@ -99,19 +99,32 @@ void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache );
  * \brief          Cache get callback implementation
  *                 (Thread-safe if MBEDTLS_THREADING_C is enabled)
  *
- * \param data     SSL cache context
- * \param session  session to retrieve entry for
+ * \param data            The SSL cache context to use.
+ * \param session_id      The pointer to the buffer holding the session ID
+ *                        for the session to load.
+ * \param session_id_len  The length of \p session_id in bytes.
+ * \param session         The address at which to store the session
+ *                        associated with \p session_id, if present.
  */
-int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session );
+int mbedtls_ssl_cache_get( void *data,
+                           unsigned char const *session_id,
+                           size_t session_id_len,
+                           mbedtls_ssl_session *session );
 
 /**
  * \brief          Cache set callback implementation
  *                 (Thread-safe if MBEDTLS_THREADING_C is enabled)
  *
- * \param data     SSL cache context
- * \param session  session to store entry for
+ * \param data            The SSL cache context to use.
+ * \param session_id      The pointer to the buffer holding the session ID
+ *                        associated to \p session.
+ * \param session_id_len  The length of \p session_id in bytes.
+ * \param session         The session to store.
  */
-int mbedtls_ssl_cache_set( void *data, const mbedtls_ssl_session *session );
+int mbedtls_ssl_cache_set( void *data,
+                           unsigned char const *session_id,
+                           size_t session_id_len,
+                           const mbedtls_ssl_session *session );
 
 #if defined(MBEDTLS_HAVE_TIME)
 /**
diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index ce85157d2f..e0e2177964 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -50,7 +50,10 @@ void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache )
 #endif
 }
 
-int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session )
+int mbedtls_ssl_cache_get( void *data,
+                           unsigned char const *session_id,
+                           size_t session_id_len,
+                           mbedtls_ssl_session *session )
 {
     int ret = 1;
 #if defined(MBEDTLS_HAVE_TIME)
@@ -78,8 +81,8 @@ int mbedtls_ssl_cache_get( void *data, mbedtls_ssl_session *session )
             continue;
 #endif
 
-        if( session->id_len != entry->session.id_len ||
-            memcmp( session->id, entry->session.id,
+        if( session_id_len != entry->session.id_len ||
+            memcmp( session_id, entry->session.id,
                     entry->session.id_len ) != 0 )
         {
             continue;
@@ -135,7 +138,10 @@ exit:
     return( ret );
 }
 
-int mbedtls_ssl_cache_set( void *data, const mbedtls_ssl_session *session )
+int mbedtls_ssl_cache_set( void *data,
+                           unsigned char const *session_id,
+                           size_t session_id_len,
+                           const mbedtls_ssl_session *session )
 {
     int ret = 1;
 #if defined(MBEDTLS_HAVE_TIME)
@@ -167,8 +173,11 @@ int mbedtls_ssl_cache_set( void *data, const mbedtls_ssl_session *session )
         }
 #endif
 
-        if( memcmp( session->id, cur->session.id, cur->session.id_len ) == 0 )
+        if( session_id_len == cur->session.id_len &&
+            memcmp( session_id, cur->session.id, cur->session.id_len ) == 0 )
+        {
             break; /* client reconnected, keep timestamp for session id */
+        }
 
 #if defined(MBEDTLS_HAVE_TIME)
         if( oldest == 0 || cur->timestamp < oldest )
diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 40ad490d8e..784ab2d516 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2784,10 +2784,9 @@ static void ssl_check_id_based_session_resumption( mbedtls_ssl_context *ssl )
         return;
 #endif
 
-    session_tmp.id_len = session->id_len;
-    memcpy( session_tmp.id, session->id, session->id_len );
-
     ret = ssl->conf->f_get_cache( ssl->conf->p_cache,
+                                  session->id,
+                                  session->id_len,
                                   &session_tmp );
     if( ret != 0 )
         goto exit;
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 67fcebfddc..c26f68bee2 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -3411,7 +3411,10 @@ void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl )
         ssl->session->id_len != 0 &&
         resume == 0 )
     {
-        if( ssl->conf->f_set_cache( ssl->conf->p_cache, ssl->session ) != 0 )
+        if( ssl->conf->f_set_cache( ssl->conf->p_cache,
+                                    ssl->session->id,
+                                    ssl->session->id_len,
+                                    ssl->session ) != 0 )
             MBEDTLS_SSL_DEBUG_MSG( 1, ( "cache did not store session" ) );
     }
 

From 02a68ebc0eea891482091f6fe2f83e90539d9d66 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 09:57:17 +0100
Subject: [PATCH 04/26] Add helper function to finding a fresh entry in the SSL
 cache

This commit improves the readability of the SSL session cache
reference implementation of mbedtls_ssl_cache_set() by moving
the logic for finding a suitable free slot for the session to
store into a static helper function.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 63 +++++++++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index e0e2177964..216b192739 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -138,24 +138,18 @@ exit:
     return( ret );
 }
 
-int mbedtls_ssl_cache_set( void *data,
-                           unsigned char const *session_id,
-                           size_t session_id_len,
-                           const mbedtls_ssl_session *session )
+static int ssl_cache_find_fresh_entry( mbedtls_ssl_cache_context *cache,
+                                       unsigned char const *session_id,
+                                       size_t session_id_len,
+                                       mbedtls_ssl_cache_entry **dst )
 {
     int ret = 1;
 #if defined(MBEDTLS_HAVE_TIME)
     mbedtls_time_t t = mbedtls_time( NULL ), oldest = 0;
     mbedtls_ssl_cache_entry *old = NULL;
 #endif
-    mbedtls_ssl_cache_context *cache = (mbedtls_ssl_cache_context *) data;
-    mbedtls_ssl_cache_entry *cur, *prv;
     int count = 0;
-
-#if defined(MBEDTLS_THREADING_C)
-    if( ( ret = mbedtls_mutex_lock( &cache->mutex ) ) != 0 )
-        return( ret );
-#endif
+    mbedtls_ssl_cache_entry *cur, *prv;
 
     cur = cache->chain;
     prv = NULL;
@@ -249,17 +243,46 @@ int mbedtls_ssl_cache_set( void *data,
 #endif
     }
 
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-    /*
-     * If we're reusing an entry, free its certificate first
-     */
-    if( cur->peer_cert.p != NULL )
+    if( cur != NULL )
     {
-        mbedtls_free( cur->peer_cert.p );
-        memset( &cur->peer_cert, 0, sizeof(mbedtls_x509_buf) );
+        *dst = cur;
+
+        /*
+         * If we're reusing an entry, free its certificate first
+         */
+        if( cur->peer_cert.p != NULL )
+        {
+            mbedtls_free( cur->peer_cert.p );
+            memset( &cur->peer_cert, 0, sizeof(mbedtls_x509_buf) );
+        }
+
+        ret = 0;
     }
-#endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
+
+exit:
+
+    return( ret );
+}
+
+int mbedtls_ssl_cache_set( void *data,
+                           unsigned char const *session_id,
+                           size_t session_id_len,
+                           const mbedtls_ssl_session *session )
+{
+    int ret = 1;
+    mbedtls_ssl_cache_context *cache = (mbedtls_ssl_cache_context *) data;
+    mbedtls_ssl_cache_entry *cur;
+
+#if defined(MBEDTLS_THREADING_C)
+    if( ( ret = mbedtls_mutex_lock( &cache->mutex ) ) != 0 )
+        return( ret );
+#endif
+
+    ret = ssl_cache_find_fresh_entry( cache,
+                                      session_id, session_id_len,
+                                      &cur );
+    if( ret != 0 )
+        goto exit;
 
     /* Copy the entire session; this temporarily makes a copy of the
      * X.509 CRT structure even though we only want to store the raw CRT.

From f938c436fb33e43b566402b9f0e024cb26256936 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 10:17:53 +0100
Subject: [PATCH 05/26] Add helper function to find entry in SSL session cache

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 120 +++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 53 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 216b192739..3fdab5b48e 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -50,84 +50,98 @@ void mbedtls_ssl_cache_init( mbedtls_ssl_cache_context *cache )
 #endif
 }
 
+static int ssl_cache_find_entry( mbedtls_ssl_cache_context *cache,
+                                 unsigned char const *session_id,
+                                 size_t session_id_len,
+                                 mbedtls_ssl_cache_entry **dst )
+{
+    int ret = 1;
+#if defined(MBEDTLS_HAVE_TIME)
+    mbedtls_time_t t = mbedtls_time( NULL );
+#endif
+    mbedtls_ssl_cache_entry *cur;
+
+    for( cur = cache->chain; cur != NULL; cur = cur->next )
+    {
+#if defined(MBEDTLS_HAVE_TIME)
+        if( cache->timeout != 0 &&
+            (int) ( t - cur->timestamp ) > cache->timeout )
+            continue;
+#endif
+
+        if( session_id_len != cur->session.id_len ||
+            memcmp( session_id, cur->session.id,
+                    cur->session.id_len ) != 0 )
+        {
+            continue;
+        }
+
+        break;
+    }
+
+    if( cur != NULL )
+    {
+        *dst = cur;
+        ret = 0;
+    }
+
+    return( ret );
+}
+
+
 int mbedtls_ssl_cache_get( void *data,
                            unsigned char const *session_id,
                            size_t session_id_len,
                            mbedtls_ssl_session *session )
 {
     int ret = 1;
-#if defined(MBEDTLS_HAVE_TIME)
-    mbedtls_time_t t = mbedtls_time( NULL );
-#endif
     mbedtls_ssl_cache_context *cache = (mbedtls_ssl_cache_context *) data;
-    mbedtls_ssl_cache_entry *cur, *entry;
+    mbedtls_ssl_cache_entry *entry;
 
 #if defined(MBEDTLS_THREADING_C)
     if( mbedtls_mutex_lock( &cache->mutex ) != 0 )
         return( 1 );
 #endif
 
-    cur = cache->chain;
-    entry = NULL;
+    ret = ssl_cache_find_entry( cache, session_id, session_id_len, &entry );
+    if( ret != 0 )
+        goto exit;
 
-    while( cur != NULL )
+    ret = mbedtls_ssl_session_copy( session, &entry->session );
+    if( ret != 0 )
+        goto exit;
+
+#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
+    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
+    /*
+     * Restore peer certificate (without rest of the original chain)
+     */
+    if( entry->peer_cert.p != NULL )
     {
-        entry = cur;
-        cur = cur->next;
+        /* `session->peer_cert` is NULL after the call to
+         * mbedtls_ssl_session_copy(), because cache entries
+         * have the `peer_cert` field set to NULL. */
 
-#if defined(MBEDTLS_HAVE_TIME)
-        if( cache->timeout != 0 &&
-            (int) ( t - entry->timestamp ) > cache->timeout )
-            continue;
-#endif
-
-        if( session_id_len != entry->session.id_len ||
-            memcmp( session_id, entry->session.id,
-                    entry->session.id_len ) != 0 )
-        {
-            continue;
-        }
-
-        ret = mbedtls_ssl_session_copy( session, &entry->session );
-        if( ret != 0 )
+        if( ( session->peer_cert = mbedtls_calloc( 1,
+                             sizeof(mbedtls_x509_crt) ) ) == NULL )
         {
             ret = 1;
             goto exit;
         }
 
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-        /*
-         * Restore peer certificate (without rest of the original chain)
-         */
-        if( entry->peer_cert.p != NULL )
+        mbedtls_x509_crt_init( session->peer_cert );
+        if( mbedtls_x509_crt_parse( session->peer_cert, entry->peer_cert.p,
+                            entry->peer_cert.len ) != 0 )
         {
-            /* `session->peer_cert` is NULL after the call to
-             * mbedtls_ssl_session_copy(), because cache entries
-             * have the `peer_cert` field set to NULL. */
-
-            if( ( session->peer_cert = mbedtls_calloc( 1,
-                                 sizeof(mbedtls_x509_crt) ) ) == NULL )
-            {
-                ret = 1;
-                goto exit;
-            }
-
-            mbedtls_x509_crt_init( session->peer_cert );
-            if( mbedtls_x509_crt_parse( session->peer_cert, entry->peer_cert.p,
-                                entry->peer_cert.len ) != 0 )
-            {
-                mbedtls_free( session->peer_cert );
-                session->peer_cert = NULL;
-                ret = 1;
-                goto exit;
-            }
+            mbedtls_free( session->peer_cert );
+            session->peer_cert = NULL;
+            ret = 1;
+            goto exit;
         }
+    }
 #endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
 
-        ret = 0;
-        goto exit;
-    }
+    ret = 0;
 
 exit:
 #if defined(MBEDTLS_THREADING_C)

From 7e6eb9fa27a67eb8ab9833c3e5fb1075073513f6 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 10:26:06 +0100
Subject: [PATCH 06/26] Simplify SSL session cache implementation via session
 serialization

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl_cache.h |  12 ++--
 library/ssl_cache.c         | 125 +++++++++++++++---------------------
 2 files changed, 57 insertions(+), 80 deletions(-)

diff --git a/include/mbedtls/ssl_cache.h b/include/mbedtls/ssl_cache.h
index cb55f7f96f..ac7b77cfbb 100644
--- a/include/mbedtls/ssl_cache.h
+++ b/include/mbedtls/ssl_cache.h
@@ -67,11 +67,13 @@ struct mbedtls_ssl_cache_entry
 #if defined(MBEDTLS_HAVE_TIME)
     mbedtls_time_t timestamp;           /*!< entry timestamp    */
 #endif
-    mbedtls_ssl_session session;        /*!< entry session      */
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-    mbedtls_x509_buf peer_cert;         /*!< entry peer_cert    */
-#endif
+
+    unsigned char session_id[32];       /*!< session ID         */
+    size_t session_id_len;
+
+    unsigned char *session;             /*!< serialized session */
+    size_t session_len;
+
     mbedtls_ssl_cache_entry *next;      /*!< chain pointer      */
 };
 
diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 3fdab5b48e..3677e51539 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -69,9 +69,9 @@ static int ssl_cache_find_entry( mbedtls_ssl_cache_context *cache,
             continue;
 #endif
 
-        if( session_id_len != cur->session.id_len ||
-            memcmp( session_id, cur->session.id,
-                    cur->session.id_len ) != 0 )
+        if( session_id_len != cur->session_id_len ||
+            memcmp( session_id, cur->session_id,
+                    cur->session_id_len ) != 0 )
         {
             continue;
         }
@@ -107,40 +107,12 @@ int mbedtls_ssl_cache_get( void *data,
     if( ret != 0 )
         goto exit;
 
-    ret = mbedtls_ssl_session_copy( session, &entry->session );
+    ret = mbedtls_ssl_session_load( session,
+                                    entry->session,
+                                    entry->session_len );
     if( ret != 0 )
         goto exit;
 
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-    /*
-     * Restore peer certificate (without rest of the original chain)
-     */
-    if( entry->peer_cert.p != NULL )
-    {
-        /* `session->peer_cert` is NULL after the call to
-         * mbedtls_ssl_session_copy(), because cache entries
-         * have the `peer_cert` field set to NULL. */
-
-        if( ( session->peer_cert = mbedtls_calloc( 1,
-                             sizeof(mbedtls_x509_crt) ) ) == NULL )
-        {
-            ret = 1;
-            goto exit;
-        }
-
-        mbedtls_x509_crt_init( session->peer_cert );
-        if( mbedtls_x509_crt_parse( session->peer_cert, entry->peer_cert.p,
-                            entry->peer_cert.len ) != 0 )
-        {
-            mbedtls_free( session->peer_cert );
-            session->peer_cert = NULL;
-            ret = 1;
-            goto exit;
-        }
-    }
-#endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
-
     ret = 0;
 
 exit:
@@ -181,8 +153,8 @@ static int ssl_cache_find_fresh_entry( mbedtls_ssl_cache_context *cache,
         }
 #endif
 
-        if( session_id_len == cur->session.id_len &&
-            memcmp( session_id, cur->session.id, cur->session.id_len ) == 0 )
+        if( session_id_len == cur->session_id_len &&
+            memcmp( session_id, cur->session_id, cur->session_id_len ) == 0 )
         {
             break; /* client reconnected, keep timestamp for session id */
         }
@@ -262,12 +234,15 @@ static int ssl_cache_find_fresh_entry( mbedtls_ssl_cache_context *cache,
         *dst = cur;
 
         /*
-         * If we're reusing an entry, free its certificate first
+         * If we're reusing an entry, free it first.
          */
-        if( cur->peer_cert.p != NULL )
+        if( cur->session != NULL )
         {
-            mbedtls_free( cur->peer_cert.p );
-            memset( &cur->peer_cert, 0, sizeof(mbedtls_x509_buf) );
+            mbedtls_free( cur->session );
+            cur->session = NULL;
+            cur->session_len = 0;
+            memset( cur->session_id, 0, sizeof( cur->session_id ) );
+            cur->session_id_len = 0;
         }
 
         ret = 0;
@@ -287,6 +262,9 @@ int mbedtls_ssl_cache_set( void *data,
     mbedtls_ssl_cache_context *cache = (mbedtls_ssl_cache_context *) data;
     mbedtls_ssl_cache_entry *cur;
 
+    size_t session_serialized_len;
+    unsigned char *session_serialized = NULL;
+
 #if defined(MBEDTLS_THREADING_C)
     if( ( ret = mbedtls_mutex_lock( &cache->mutex ) ) != 0 )
         return( ret );
@@ -298,41 +276,41 @@ int mbedtls_ssl_cache_set( void *data,
     if( ret != 0 )
         goto exit;
 
-    /* Copy the entire session; this temporarily makes a copy of the
-     * X.509 CRT structure even though we only want to store the raw CRT.
-     * This inefficiency will go away as soon as we implement on-demand
-     * parsing of CRTs, in which case there's no need for the `peer_cert`
-     * field anymore in the first place, and we're done after this call. */
-    ret = mbedtls_ssl_session_copy( &cur->session, session );
-    if( ret != 0 )
+    /* Check how much space we need to serialize the session
+     * and allocate a sufficiently large buffer. */
+    ret = mbedtls_ssl_session_save( session, NULL, 0, &session_serialized_len );
+    if( ret != MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL )
     {
         ret = 1;
         goto exit;
     }
 
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-    /* If present, free the X.509 structure and only store the raw CRT data. */
-    if( cur->session.peer_cert != NULL )
+    session_serialized = mbedtls_calloc( 1, session_serialized_len );
+    if( session_serialized == NULL )
     {
-        cur->peer_cert.p =
-            mbedtls_calloc( 1, cur->session.peer_cert->raw.len );
-        if( cur->peer_cert.p == NULL )
-        {
-            ret = 1;
-            goto exit;
-        }
-
-        memcpy( cur->peer_cert.p,
-                cur->session.peer_cert->raw.p,
-                cur->session.peer_cert->raw.len );
-        cur->peer_cert.len = session->peer_cert->raw.len;
-
-        mbedtls_x509_crt_free( cur->session.peer_cert );
-        mbedtls_free( cur->session.peer_cert );
-        cur->session.peer_cert = NULL;
+        ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
+        goto exit;
     }
-#endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
+
+    /* Now serialize the session into the allocated buffer. */
+    ret = mbedtls_ssl_session_save( session,
+                                    session_serialized,
+                                    session_serialized_len,
+                                    &session_serialized_len );
+    if( ret != 0 )
+        goto exit;
+
+    if( session_id_len > 32 )
+    {
+        ret = 1;
+        goto exit;
+    }
+    cur->session_id_len = session_id_len;
+    memcpy( cur->session_id, session_id, session_id_len );
+
+    cur->session = session_serialized;
+    cur->session_len = session_serialized_len;
+    session_serialized = NULL;
 
     ret = 0;
 
@@ -342,6 +320,9 @@ exit:
         ret = 1;
 #endif
 
+    if( session_serialized != NULL )
+        mbedtls_platform_zeroize( session_serialized, session_serialized_len );
+
     return( ret );
 }
 
@@ -372,13 +353,7 @@ void mbedtls_ssl_cache_free( mbedtls_ssl_cache_context *cache )
         prv = cur;
         cur = cur->next;
 
-        mbedtls_ssl_session_free( &prv->session );
-
-#if defined(MBEDTLS_X509_CRT_PARSE_C) && \
-    defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
-        mbedtls_free( prv->peer_cert.p );
-#endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
-
+        mbedtls_free( prv->session );
         mbedtls_free( prv );
     }
 

From a5b1a3945b8561f0247e7f7ab5269911b6265e40 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 16:48:01 +0100
Subject: [PATCH 07/26] Don't use 0-initializer for structs

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 784ab2d516..7bdad3d0de 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2768,7 +2768,7 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl )
 static void ssl_check_id_based_session_resumption( mbedtls_ssl_context *ssl )
 {
     int ret;
-    mbedtls_ssl_session session_tmp = { 0 };
+    mbedtls_ssl_session session_tmp;
     mbedtls_ssl_session * const session = ssl->session_negotiate;
 
     /* Resume is 0  by default, see ssl_handshake_init().
@@ -2784,6 +2784,8 @@ static void ssl_check_id_based_session_resumption( mbedtls_ssl_context *ssl )
         return;
 #endif
 
+    mbedtls_ssl_session_init( &session_tmp );
+
     ret = ssl->conf->f_get_cache( ssl->conf->p_cache,
                                   session->id,
                                   session->id_len,

From aee4cc4cbbb29b63c6a81c8bb4d470054b5978d5 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 15 Apr 2021 16:49:32 +0100
Subject: [PATCH 08/26] Use sizeof() instead of magic constant

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 3677e51539..6ef1a972c8 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -300,7 +300,7 @@ int mbedtls_ssl_cache_set( void *data,
     if( ret != 0 )
         goto exit;
 
-    if( session_id_len > 32 )
+    if( session_id_len > sizeof( cur->session_id ) )
     {
         ret = 1;
         goto exit;

From f6e09c6f831a7504dcf93266e1bb56ac5d3605df Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 06:12:38 +0100
Subject: [PATCH 09/26] Don't use ssl_check_xxx() for functions with void
 return

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 7bdad3d0de..bdfdebc9a4 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2765,7 +2765,7 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl )
 }
 #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY */
 
-static void ssl_check_id_based_session_resumption( mbedtls_ssl_context *ssl )
+static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl )
 {
     int ret;
     mbedtls_ssl_session session_tmp;
@@ -2883,7 +2883,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl )
 
     MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, random bytes", buf + 6, 32 );
 
-    ssl_check_id_based_session_resumption( ssl );
+    ssl_handle_id_based_session_resumption( ssl );
 
     if( ssl->handshake->resume == 0 )
     {

From 7ad77963d17fdbe84e92459d0567caff6929e8db Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 06:13:34 +0100
Subject: [PATCH 10/26] Use shorthand local variable for session under
 negotiation

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index bdfdebc9a4..196f69de7d 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2775,7 +2775,7 @@ static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl )
      * It may be already set to 1 by ssl_parse_session_ticket_ext(). */
     if( ssl->handshake->resume == 1 )
         return;
-    if( ssl->session_negotiate->id_len == 0 )
+    if( session->id_len == 0 )
         return;
     if( ssl->conf->f_get_cache == NULL )
         return;

From df5640262324ebe8c1bd0445e35ab7811caa6214 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 06:26:37 +0100
Subject: [PATCH 11/26] Fix memory leak upon ciphersuite mismatch during
 session resumption

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 196f69de7d..213cd3ce66 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2810,7 +2810,7 @@ static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl )
 
 exit:
 
-    mbedtls_platform_zeroize( &session_tmp, sizeof( session_tmp ) );
+    mbedtls_ssl_session_free( &session_tmp );
 }
 
 static int ssl_write_server_hello( mbedtls_ssl_context *ssl )

From 0248785081911f8881b84114aa94e1e3c27d6f9b Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 06:26:57 +0100
Subject: [PATCH 12/26] Document session cache callbacks

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl.h | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 89912c606f..972cac2c16 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -627,12 +627,46 @@ typedef struct mbedtls_ssl_key_cert mbedtls_ssl_key_cert;
 typedef struct mbedtls_ssl_flight_item mbedtls_ssl_flight_item;
 #endif
 
-/* TODO: Document */
+/**
+ * \brief          Callback type: server-side session cache getter
+ *
+ *                 The session cache is logically a key value store, with
+ *                 keys being session IDs and values being instances of
+ *                 mbedtls_ssl_session.
+ *
+ *                 This callback retrieves an entry in this key-value store.
+ *
+ * \param data            The address of the session cache structure to query.
+ * \param session_id      The buffer holding the session ID to query.
+ * \param session_id_len  The length of \p session_id in Bytes.
+ * \param session         The address at which to store the session found
+ *                        in the cache.
+ *
+ * \return                \c 0 on success
+ * \return                A non-zero return value on failure.
+ */
 typedef int mbedtls_ssl_cache_get_t( void *data,
                                      unsigned char const *session_id,
                                      size_t session_id_len,
                                      mbedtls_ssl_session *session );
-/* TODO: Document */
+/**
+ * \brief          Callback type: server-side session cache setter
+ *
+ *                 The session cache is logically a key value store, with
+ *                 keys being session IDs and values being instances of
+ *                 mbedtls_ssl_session.
+ *
+ *                 This callback sets an entry in this key-value store.
+ *
+ * \param data            The address of the session cache structure to modify.
+ * \param session_id      The buffer holding the session ID to query.
+ * \param session_id_len  The length of \p session_id in Bytes.
+ * \param session         The address of the session to be stored in the
+ *                        session cache.
+ *
+ * \return                \c 0 on success
+ * \return                A non-zero return value on failure.
+ */
 typedef int mbedtls_ssl_cache_set_t( void *data,
                                      unsigned char const *session_id,
                                      size_t session_id_len,

From f47199da26d6fdf22433dae4c5fbf5c9097e2403 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 06:29:13 +0100
Subject: [PATCH 13/26] Improve naming of helper function for reference session
 cache

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 6ef1a972c8..5790fc6bf6 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -124,10 +124,10 @@ exit:
     return( ret );
 }
 
-static int ssl_cache_find_fresh_entry( mbedtls_ssl_cache_context *cache,
-                                       unsigned char const *session_id,
-                                       size_t session_id_len,
-                                       mbedtls_ssl_cache_entry **dst )
+static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
+                                        unsigned char const *session_id,
+                                        size_t session_id_len,
+                                        mbedtls_ssl_cache_entry **dst )
 {
     int ret = 1;
 #if defined(MBEDTLS_HAVE_TIME)
@@ -270,9 +270,9 @@ int mbedtls_ssl_cache_set( void *data,
         return( ret );
 #endif
 
-    ret = ssl_cache_find_fresh_entry( cache,
-                                      session_id, session_id_len,
-                                      &cur );
+    ret = ssl_cache_pick_writing_slot( cache,
+                                       session_id, session_id_len,
+                                       &cur );
     if( ret != 0 )
         goto exit;
 

From 845ceb7cc84f983d681e3487293c1c0368cd327b Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Thu, 13 May 2021 07:05:07 +0100
Subject: [PATCH 14/26] Improve readability and efficiency of SSL cache
 reference impl

The reference session cache implementation may end up storing multiple
sessions associated to the same session ID if the set()-call for the
second session finds an outdated cache entry prior to noticing the entry
with the matching session ID. While this logically overwrites the existing
entry since we always search the cache in order, this is at least a waste
of resources.

This commit fixes this by always checking first whether the given ID is
already present in the cache.

It also restructures the code for easier readability.

Fixes #4509.

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 168 ++++++++++++++++++++++----------------------
 1 file changed, 83 insertions(+), 85 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 5790fc6bf6..1bbcc957b9 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -129,7 +129,6 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
                                         size_t session_id_len,
                                         mbedtls_ssl_cache_entry **dst )
 {
-    int ret = 1;
 #if defined(MBEDTLS_HAVE_TIME)
     mbedtls_time_t t = mbedtls_time( NULL ), oldest = 0;
     mbedtls_ssl_cache_entry *old = NULL;
@@ -140,117 +139,116 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
     cur = cache->chain;
     prv = NULL;
 
+    /* Check 1: Is there already an entry with the given session ID?
+     *
+     * If yes, overwrite it.
+     *
+     * If not, `count` will hold the size of the session cache
+     * at the end of this loop, and `prv` will point to the last
+     * entry, both of which will be used later. */
+
     while( cur != NULL )
     {
         count++;
 
-#if defined(MBEDTLS_HAVE_TIME)
-        if( cache->timeout != 0 &&
-            (int) ( t - cur->timestamp ) > cache->timeout )
-        {
-            cur->timestamp = t;
-            break; /* expired, reuse this slot, update timestamp */
-        }
-#endif
-
         if( session_id_len == cur->session_id_len &&
             memcmp( session_id, cur->session_id, cur->session_id_len ) == 0 )
         {
-            break; /* client reconnected, keep timestamp for session id */
+            goto found;
         }
 
-#if defined(MBEDTLS_HAVE_TIME)
-        if( oldest == 0 || cur->timestamp < oldest )
-        {
-            oldest = cur->timestamp;
-            old = cur;
-        }
-#endif
-
         prv = cur;
         cur = cur->next;
     }
 
-    if( cur == NULL )
-    {
+    /* Check 2: Is there an outdated entry in the cache?
+     *
+     * If so, overwrite it.
+     *
+     * If not, remember the oldest entry in `old` for later.
+     */
+
 #if defined(MBEDTLS_HAVE_TIME)
-        /*
-         * Reuse oldest entry if max_entries reached
-         */
-        if( count >= cache->max_entries )
+    while( cur != NULL )
+    {
+        if( cache->timeout != 0 &&
+            (int) ( t - cur->timestamp ) > cache->timeout )
         {
-            if( old == NULL )
-            {
-                ret = 1;
-                goto exit;
-            }
-
-            cur = old;
+            goto found;
         }
-#else /* MBEDTLS_HAVE_TIME */
-        /*
-         * Reuse first entry in chain if max_entries reached,
-         * but move to last place
-         */
-        if( count >= cache->max_entries )
+
+        if( oldest == 0 || cur->timestamp < oldest )
         {
-            if( cache->chain == NULL )
-            {
-                ret = 1;
-                goto exit;
-            }
-
-            cur = cache->chain;
-            cache->chain = cur->next;
-            cur->next = NULL;
-            prv->next = cur;
+            oldest = cur->timestamp;
+            old = cur;
         }
+
+        prv = cur;
+        cur = cur->next;
+    }
 #endif /* MBEDTLS_HAVE_TIME */
-        else
-        {
-            /*
-             * max_entries not reached, create new entry
-             */
-            cur = mbedtls_calloc( 1, sizeof(mbedtls_ssl_cache_entry) );
-            if( cur == NULL )
-            {
-                ret = 1;
-                goto exit;
-            }
 
-            if( prv == NULL )
-                cache->chain = cur;
-            else
-                prv->next = cur;
-        }
+    /* Check 3: Is there free space in the cache? */
+
+    if( count < cache->max_entries )
+    {
+        /* Create new entry */
+        cur = mbedtls_calloc( 1, sizeof(mbedtls_ssl_cache_entry) );
+        if( cur == NULL )
+            return( 1 );
+
+        /* Append to the end of the linked list. */
+        if( prv == NULL )
+            cache->chain = cur;
+        else
+            prv->next = cur;
+
+        goto found;
+    }
+
+    /* Last resort: The cache is full and doesn't contain any outdated
+     * elements. In this case, we evict the oldest one, judged by timestamp
+     * (if present) or cache-order. */
 
 #if defined(MBEDTLS_HAVE_TIME)
-        cur->timestamp = t;
-#endif
-    }
-
-    if( cur != NULL )
+    if( old == NULL )
     {
-        *dst = cur;
+        /* This should only happen on an ill-configured cache
+         * with max_entries == 0. */
+        return( 1 );
+    }
+#else /* MBEDTLS_HAVE_TIME */
+    /* Reuse first entry in chain, but move to last place. */
+    if( cache->chain == NULL )
+        return( 1 );
 
-        /*
-         * If we're reusing an entry, free it first.
-         */
-        if( cur->session != NULL )
-        {
-            mbedtls_free( cur->session );
-            cur->session = NULL;
-            cur->session_len = 0;
-            memset( cur->session_id, 0, sizeof( cur->session_id ) );
-            cur->session_id_len = 0;
-        }
+    old = cache->chain;
+    cache->chain = old->next;
+    cur->next = NULL;
+    prv->next = old;
+#endif /* MBEDTLS_HAVE_TIME */
 
-        ret = 0;
+    /* Now `old` points to the oldest entry to be overwritten. */
+    cur = old;
+
+found:
+
+#if defined(MBEDTLS_HAVE_TIME)
+    cur->timestamp = t;
+#endif
+
+    /* If we're reusing an entry, free it first. */
+    if( cur->session != NULL )
+    {
+        mbedtls_free( cur->session );
+        cur->session = NULL;
+        cur->session_len = 0;
+        memset( cur->session_id, 0, sizeof( cur->session_id ) );
+        cur->session_id_len = 0;
     }
 
-exit:
-
-    return( ret );
+    *dst = cur;
+    return( 0 );
 }
 
 int mbedtls_ssl_cache_set( void *data,

From b94fdae3c387628cbc5b934bb33c76a926880e22 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 04:42:43 +0100
Subject: [PATCH 15/26] Improve code structure for session cache query

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 213cd3ce66..5190367669 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2801,9 +2801,9 @@ static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl )
     }
 
     /* Move semantics */
-    /* Zeroization of session_tmp happens at the end of the function. */
     mbedtls_ssl_session_free( session );
     *session = session_tmp;
+    memset( &session_tmp, 0, sizeof( mbedtls_ssl_session ) );
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from cache" ) );
     ssl->handshake->resume = 1;

From 0d05f40222a513a9dcce937583f9de337aa0659f Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 04:54:01 +0100
Subject: [PATCH 16/26] Clarify that session cache query must return free-able
 session

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 972cac2c16..63417c3551 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -639,11 +639,16 @@ typedef struct mbedtls_ssl_flight_item mbedtls_ssl_flight_item;
  * \param data            The address of the session cache structure to query.
  * \param session_id      The buffer holding the session ID to query.
  * \param session_id_len  The length of \p session_id in Bytes.
- * \param session         The address at which to store the session found
- *                        in the cache.
+ * \param session         The address of the session structure to populate.
+ *                        It is initialized with mbdtls_ssl_session_init(),
+ *                        and the callback must always leave it in a state
+ *                        where it can savely be freed via
+ *                        mbedtls_ssl_session_free() independent of the
+ *                        return code of this function.
  *
  * \return                \c 0 on success
  * \return                A non-zero return value on failure.
+ *
  */
 typedef int mbedtls_ssl_cache_get_t( void *data,
                                      unsigned char const *session_id,

From 006f2cce2eb1b73fa966fe52e75089dd5ec34912 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 04:55:35 +0100
Subject: [PATCH 17/26] Fix compile-time guard in session cache implementation

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 1bbcc957b9..f33754798b 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -131,8 +131,9 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
 {
 #if defined(MBEDTLS_HAVE_TIME)
     mbedtls_time_t t = mbedtls_time( NULL ), oldest = 0;
+#endif /* MBEDTLS_HAVE_TIME */
+
     mbedtls_ssl_cache_entry *old = NULL;
-#endif
     int count = 0;
     mbedtls_ssl_cache_entry *cur, *prv;
 

From 5cf6f7eafe7a22af2481b973c2b69c317522d618 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:45:04 +0100
Subject: [PATCH 18/26] Fix swapping of first and last entry in SSL session
 cache

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index f33754798b..11afd86240 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -225,7 +225,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
 
     old = cache->chain;
     cache->chain = old->next;
-    cur->next = NULL;
+    old->next = NULL;
     prv->next = old;
 #endif /* MBEDTLS_HAVE_TIME */
 

From 466ed6fd08a62b6d65416b8a78fe0682720e0408 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:54:00 +0100
Subject: [PATCH 19/26] Improve local variable naming in SSL session cache
 implementation

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 11afd86240..cdefa01b31 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -135,17 +135,17 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
 
     mbedtls_ssl_cache_entry *old = NULL;
     int count = 0;
-    mbedtls_ssl_cache_entry *cur, *prv;
+    mbedtls_ssl_cache_entry *cur, *last;
 
     cur = cache->chain;
-    prv = NULL;
+    last = NULL;
 
     /* Check 1: Is there already an entry with the given session ID?
      *
      * If yes, overwrite it.
      *
      * If not, `count` will hold the size of the session cache
-     * at the end of this loop, and `prv` will point to the last
+     * at the end of this loop, and `last` will point to the last
      * entry, both of which will be used later. */
 
     while( cur != NULL )
@@ -158,7 +158,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
             goto found;
         }
 
-        prv = cur;
+        last = cur;
         cur = cur->next;
     }
 
@@ -184,7 +184,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
             old = cur;
         }
 
-        prv = cur;
+        last = cur;
         cur = cur->next;
     }
 #endif /* MBEDTLS_HAVE_TIME */
@@ -199,10 +199,10 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
             return( 1 );
 
         /* Append to the end of the linked list. */
-        if( prv == NULL )
+        if( last == NULL )
             cache->chain = cur;
         else
-            prv->next = cur;
+            last->next = cur;
 
         goto found;
     }
@@ -226,7 +226,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
     old = cache->chain;
     cache->chain = old->next;
     old->next = NULL;
-    prv->next = old;
+    last->next = old;
 #endif /* MBEDTLS_HAVE_TIME */
 
     /* Now `old` points to the oldest entry to be overwritten. */

From c3f4a97b8fd7113f22a23c56035021fe5f3015a3 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:54:24 +0100
Subject: [PATCH 20/26] Don't infer last element of SSL session cache twice

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index cdefa01b31..367edf51d6 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -151,14 +151,11 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
     while( cur != NULL )
     {
         count++;
-
         if( session_id_len == cur->session_id_len &&
             memcmp( session_id, cur->session_id, cur->session_id_len ) == 0 )
         {
             goto found;
         }
-
-        last = cur;
         cur = cur->next;
     }
 

From 78196e366faab94b9cd57320e54c3b2c9bcf157d Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:45:38 +0100
Subject: [PATCH 21/26] Fix search for outdated entries in SSL session cache

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_cache.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 367edf51d6..fe4f30cf8d 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -137,9 +137,6 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
     int count = 0;
     mbedtls_ssl_cache_entry *cur, *last;
 
-    cur = cache->chain;
-    last = NULL;
-
     /* Check 1: Is there already an entry with the given session ID?
      *
      * If yes, overwrite it.
@@ -148,7 +145,8 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
      * at the end of this loop, and `last` will point to the last
      * entry, both of which will be used later. */
 
-    while( cur != NULL )
+    last = NULL;
+    for( cur = cache->chain; cur != NULL; cur = cur->next )
     {
         count++;
         if( session_id_len == cur->session_id_len &&
@@ -156,7 +154,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
         {
             goto found;
         }
-        cur = cur->next;
+        last = cur;
     }
 
     /* Check 2: Is there an outdated entry in the cache?
@@ -167,7 +165,7 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
      */
 
 #if defined(MBEDTLS_HAVE_TIME)
-    while( cur != NULL )
+    for( cur = cache->chain; cur != NULL; cur = cur->next )
     {
         if( cache->timeout != 0 &&
             (int) ( t - cur->timestamp ) > cache->timeout )
@@ -180,9 +178,6 @@ static int ssl_cache_pick_writing_slot( mbedtls_ssl_cache_context *cache,
             oldest = cur->timestamp;
             old = cur;
         }
-
-        last = cur;
-        cur = cur->next;
     }
 #endif /* MBEDTLS_HAVE_TIME */
 

From 9caed14a218e0a5e1bf16bc48e2a427405870e42 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:57:13 +0100
Subject: [PATCH 22/26] Fix typo in ssl session cache documentation

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 include/mbedtls/ssl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 63417c3551..dd30161080 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -642,7 +642,7 @@ typedef struct mbedtls_ssl_flight_item mbedtls_ssl_flight_item;
  * \param session         The address of the session structure to populate.
  *                        It is initialized with mbdtls_ssl_session_init(),
  *                        and the callback must always leave it in a state
- *                        where it can savely be freed via
+ *                        where it can safely be freed via
  *                        mbedtls_ssl_session_free() independent of the
  *                        return code of this function.
  *

From fc1f4135c3ff35a15e930c96f234e20fe2f90e58 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Fri, 14 May 2021 14:57:54 +0100
Subject: [PATCH 23/26] Use `memset( x, 0, sizeof( x ) )` to clear local
 structure

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 library/ssl_srv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/library/ssl_srv.c b/library/ssl_srv.c
index 5190367669..8c6967885d 100644
--- a/library/ssl_srv.c
+++ b/library/ssl_srv.c
@@ -2803,7 +2803,7 @@ static void ssl_handle_id_based_session_resumption( mbedtls_ssl_context *ssl )
     /* Move semantics */
     mbedtls_ssl_session_free( session );
     *session = session_tmp;
-    memset( &session_tmp, 0, sizeof( mbedtls_ssl_session ) );
+    memset( &session_tmp, 0, sizeof( session_tmp ) );
 
     MBEDTLS_SSL_DEBUG_MSG( 3, ( "session successfully restored from cache" ) );
     ssl->handshake->resume = 1;

From 9039303cf58882d1fde68d2f6047795dfc750e9c Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Tue, 18 May 2021 05:27:18 +0100
Subject: [PATCH 24/26] Add migration guide

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 .../session-cache-api.md                      | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 docs/3.0-migration-guide.d/session-cache-api.md

diff --git a/docs/3.0-migration-guide.d/session-cache-api.md b/docs/3.0-migration-guide.d/session-cache-api.md
new file mode 100644
index 0000000000..b28ce1946a
--- /dev/null
+++ b/docs/3.0-migration-guide.d/session-cache-api.md
@@ -0,0 +1,28 @@
+Session Cache API Change
+-----------------------------------------------------------------
+
+This affects users who use `mbedtls_ssl_conf_session_cache()`
+to configure a custom session cache implementation different
+from the one Mbed TLS implements in `library/ssl_cache.c`.
+
+Those users will need to modify the API of their session cache
+implementation to that of a key-value store with keys being
+session IDs and values being instances of `mbedtls_ssl_session`:
+
+```
+typedef int mbedtls_ssl_cache_get_t( void *data,
+                                     unsigned char const *session_id,
+                                     size_t session_id_len,
+                                     mbedtls_ssl_session *session );
+typedef int mbedtls_ssl_cache_set_t( void *data,
+                                     unsigned char const *session_id,
+                                     size_t session_id_len,
+                                     const mbedtls_ssl_session *session );
+```
+
+Since the structure of `mbedtls_ssl_session` is no longer public from 3.0
+onwards, portable session cache implementations must not access fields of
+`mbedtls_ssl_session`. See the corresponding migration guide. Users that
+find themselves unable to migrate their session cache functionality without
+accessing fields of `mbedtls_ssl_session` should describe their usecase
+on the Mbed TLS mailing list.

From 217715d32bb64840306660b355336110b3b55215 Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Tue, 18 May 2021 05:28:53 +0100
Subject: [PATCH 25/26] Add ChangeLog entry

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 ChangeLog.d/session-cache-api.txt | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 ChangeLog.d/session-cache-api.txt

diff --git a/ChangeLog.d/session-cache-api.txt b/ChangeLog.d/session-cache-api.txt
new file mode 100644
index 0000000000..e3f25d6b86
--- /dev/null
+++ b/ChangeLog.d/session-cache-api.txt
@@ -0,0 +1,5 @@
+API Changes
+    * The getter and setter API of the SSL session cache (used for
+      session-ID based session resumption) has changed to that of
+      a key-value store with keys being session IDs and values
+      being opaque instances of `mbedtls_ssl_session`.

From ea620864ac82aeb8a5c219459a62eaeaf620377b Mon Sep 17 00:00:00 2001
From: Hanno Becker <hanno.becker@arm.com>
Date: Tue, 18 May 2021 08:36:36 +0100
Subject: [PATCH 26/26] Fix formatting of changelog entry

Signed-off-by: Hanno Becker <hanno.becker@arm.com>
---
 ChangeLog.d/session-cache-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ChangeLog.d/session-cache-api.txt b/ChangeLog.d/session-cache-api.txt
index e3f25d6b86..75cc9438f8 100644
--- a/ChangeLog.d/session-cache-api.txt
+++ b/ChangeLog.d/session-cache-api.txt
@@ -1,4 +1,4 @@
-API Changes
+API changes
     * The getter and setter API of the SSL session cache (used for
       session-ID based session resumption) has changed to that of
       a key-value store with keys being session IDs and values