Merge pull request #8280 from gilles-peskine-arm/ssl_cache-negative_e…

…rrors-2.28

Backport 2.28: ssl_cache: misc improvements

ChangeLog.d/MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND.txt

@@ -0,0 +1,4 @@
+Bugfix
+   * Functions in the ssl_cache module now return a negative MBEDTLS_ERR_xxx
+     error code on failure. Before, they returned 1 to indicate failure in
+     some cases involving a missing entry or a full cache.

include/mbedtls/ssl.h

@@ -193,6 +193,8 @@
 #define MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS                -0x7000
 /** Invalid value in SSL config */
 #define MBEDTLS_ERR_SSL_BAD_CONFIG                        -0x5E80
+/** Cache entry not found */
+#define MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND             -0x5E00
 
 /*
  * Various constants

include/mbedtls/ssl_cache.h

@@ -99,6 +99,11 @@ void mbedtls_ssl_cache_init(mbedtls_ssl_cache_context *cache);
  *
  * \param data     SSL cache context
  * \param session  session to retrieve entry for
+ *
+ * \return                \c 0 on success.
+ * \return                #MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND if there is
+ *                        no cache entry with specified session ID found, or
+ *                        any other negative error code for other failures.
  */
 int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session);
 
@@ -108,6 +113,9 @@ int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session);
  *
  * \param data     SSL cache context
  * \param session  session to store entry for
+ *
+ * \return                \c 0 on success.
+ * \return                A negative error code on failure.
  */
 int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session);
 

library/error.c

@@ -518,6 +518,8 @@ const char *mbedtls_high_level_strerr(int error_code)
             return( "SSL - A cryptographic operation is in progress. Try again later" );
         case -(MBEDTLS_ERR_SSL_BAD_CONFIG):
             return( "SSL - Invalid value in SSL config" );
+        case -(MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND):
+            return( "SSL - Cache entry not found" );
 #endif /* MBEDTLS_SSL_TLS_C */
 
 #if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C)

library/ssl_cache.c

@@ -26,6 +26,7 @@
 #if defined(MBEDTLS_SSL_CACHE_C)
 
 #include "mbedtls/platform.h"
+#include "mbedtls/error.h"
 
 #include "mbedtls/ssl_cache.h"
 #include "mbedtls/ssl_internal.h"
@@ -46,16 +47,16 @@ void mbedtls_ssl_cache_init(mbedtls_ssl_cache_context *cache)
 
 int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session)
 {
-    int ret = 1;
+    int ret = MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND;
 #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;
 
 #if defined(MBEDTLS_THREADING_C)
-    if (mbedtls_mutex_lock(&cache->mutex) != 0) {
-        return 1;
+    if ((ret = mbedtls_mutex_lock(&cache->mutex)) != 0) {
+        return ret;
     }
 #endif
 
@@ -81,7 +82,6 @@ int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session)
 
         ret = mbedtls_ssl_session_copy(session, &entry->session);
         if (ret != 0) {
-            ret = 1;
             goto exit;
         }
 
@@ -97,16 +97,15 @@ int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session)
 
             if ((session->peer_cert = mbedtls_calloc(1,
                                                      sizeof(mbedtls_x509_crt))) == NULL) {
-                ret = 1;
+                ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
                 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) {
+            if ((ret = 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;
             }
         }
@@ -119,7 +118,7 @@ int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session)
 exit:
 #if defined(MBEDTLS_THREADING_C)
     if (mbedtls_mutex_unlock(&cache->mutex) != 0) {
-        ret = 1;
+        ret = MBEDTLS_ERR_THREADING_MUTEX_ERROR;
     }
 #endif
 
@@ -128,7 +127,7 @@ int mbedtls_ssl_cache_get(void *data, mbedtls_ssl_session *session)
 
 int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
 {
-    int ret = 1;
+    int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
 #if defined(MBEDTLS_HAVE_TIME)
     mbedtls_time_t t = mbedtls_time(NULL), oldest = 0;
     mbedtls_ssl_cache_entry *old = NULL;
@@ -179,7 +178,9 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
          */
         if (count >= cache->max_entries) {
             if (old == NULL) {
-                ret = 1;
+                /* This should only happen on an ill-configured cache
+                 * with max_entries == 0. */
+                ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR;
                 goto exit;
             }
 
@@ -192,7 +193,7 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
          */
         if (count >= cache->max_entries) {
             if (cache->chain == NULL) {
-                ret = 1;
+                ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR;
                 goto exit;
             }
 
@@ -208,7 +209,7 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
              */
             cur = mbedtls_calloc(1, sizeof(mbedtls_ssl_cache_entry));
             if (cur == NULL) {
-                ret = 1;
+                ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
                 goto exit;
             }
 
@@ -242,7 +243,6 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
      * field anymore in the first place, and we're done after this call. */
     ret = mbedtls_ssl_session_copy(&cur->session, session);
     if (ret != 0) {
-        ret = 1;
         goto exit;
     }
 
@@ -253,7 +253,7 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
         cur->peer_cert.p =
             mbedtls_calloc(1, cur->session.peer_cert->raw.len);
         if (cur->peer_cert.p == NULL) {
-            ret = 1;
+            ret = MBEDTLS_ERR_SSL_ALLOC_FAILED;
             goto exit;
         }
 
@@ -273,7 +273,7 @@ int mbedtls_ssl_cache_set(void *data, const mbedtls_ssl_session *session)
 exit:
 #if defined(MBEDTLS_THREADING_C)
     if (mbedtls_mutex_unlock(&cache->mutex) != 0) {
-        ret = 1;
+        ret = MBEDTLS_ERR_THREADING_MUTEX_ERROR;
     }
 #endif