Merge pull request #8280 from gilles-peskine-arm/ssl_cache-negative_errors-2.28
Backport 2.28: ssl_cache: misc improvements
diff --git a/ChangeLog.d/MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND.txt b/ChangeLog.d/MBEDTLS_ERR_SSL_CACHE_ENTRY_NOT_FOUND.txt
new file mode 100644
index 0000000..6f091bb
--- /dev/null
+++ b/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.
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index cc9a270..3ec558b 100644
--- a/include/mbedtls/ssl.h
+++ b/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
diff --git a/include/mbedtls/ssl_cache.h b/include/mbedtls/ssl_cache.h
index e358c6c..b1ea801 100644
--- a/include/mbedtls/ssl_cache.h
+++ b/include/mbedtls/ssl_cache.h
@@ -99,6 +99,11 @@
*
* \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 @@
*
* \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);
diff --git a/library/error.c b/library/error.c
index f4826c8..6aa4b92 100644
--- a/library/error.c
+++ b/library/error.c
@@ -518,6 +518,8 @@
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)
diff --git a/library/ssl_cache.c b/library/ssl_cache.c
index 0f0e610..6082074 100644
--- a/library/ssl_cache.c
+++ b/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,7 +47,7 @@
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
@@ -54,8 +55,8 @@
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 @@
ret = mbedtls_ssl_session_copy(session, &entry->session);
if (ret != 0) {
- ret = 1;
goto exit;
}
@@ -97,16 +97,15 @@
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 @@
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_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 @@
*/
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 @@
*/
if (count >= cache->max_entries) {
if (cache->chain == NULL) {
- ret = 1;
+ ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR;
goto exit;
}
@@ -208,7 +209,7 @@
*/
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 @@
* 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 @@
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 @@
exit:
#if defined(MBEDTLS_THREADING_C)
if (mbedtls_mutex_unlock(&cache->mutex) != 0) {
- ret = 1;
+ ret = MBEDTLS_ERR_THREADING_MUTEX_ERROR;
}
#endif