commit aee8717877f38731f15fb056c8046ea884ee24f1
author Hanno Becker <hanno.becker@arm.com> Wed Feb 06 14:53:19 2019 +0000
committer Hanno Becker <hanno.becker@arm.com> Tue Feb 26 14:38:09 2019 +0000
tree 461cf55110329f8a7bdd8c523743e5bab0f48c23
parent 52055ae91f0bfb93502f495cf80474ebca8cceae

Simplify session cache implementation via mbedtls_ssl_session_copy()

library/ssl_cache.c

diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 47867f1..f542594 100644
--- a/library/ssl_cache.c
+++ b/library/ssl_cache.c
@@ -40,6 +40,7 @@
 #endif
 
 #include "mbedtls/ssl_cache.h"
+#include "mbedtls/ssl_internal.h"
 
 #include <string.h>
 
@@ -92,9 +93,12 @@
                     entry->session.id_len ) != 0 )
             continue;
 
-        memcpy( session->master, entry->session.master, 48 );
-
-        session->verify_result = entry->session.verify_result;
+        ret = mbedtls_ssl_session_copy( session, &entry->session );
+        if( ret != 0 )
+        {
+            ret = 1;
+            goto exit;
+        }
 
 #if defined(MBEDTLS_X509_CRT_PARSE_C)
         /*
@@ -102,6 +106,10 @@
          */
         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 )
             {
@@ -239,8 +247,6 @@
 #endif
     }
 
-    memcpy( &cur->session, session, sizeof( mbedtls_ssl_session ) );
-
 #if defined(MBEDTLS_X509_CRT_PARSE_C)
     /*
      * If we're reusing an entry, free its certificate first
@@ -250,23 +256,39 @@
         mbedtls_free( cur->peer_cert.p );
         memset( &cur->peer_cert, 0, sizeof(mbedtls_x509_buf) );
     }
+#endif /* MBEDTLS_X509_CRT_PARSE_C */
 
-    /*
-     * Store peer certificate
-     */
-    if( session->peer_cert != NULL )
+    /* 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 )
     {
-        cur->peer_cert.p = mbedtls_calloc( 1, session->peer_cert->raw.len );
+        ret = 1;
+        goto exit;
+    }
+
+#if defined(MBEDTLS_X509_CRT_PARSE_C)
+    /* If present, free the X.509 structure and only store the raw CRT data. */
+    if( cur->session.peer_cert != 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, session->peer_cert->raw.p,
-                session->peer_cert->raw.len );
+        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;
     }
 #endif /* MBEDTLS_X509_CRT_PARSE_C */