New configuration option MBEDTLS_CHECK_RETURN_WARNING

MBEDTLS_CHECK_RETURN_TYPICAL defaults off, but is enabled if
MBEDTLS_CHECK_RETURN_WARNING is enabled at compile time.
(MBEDTLS_CHECK_RETURN_CRITICAL is always enabled.)

The default is off so that a plausible program that builds with one version
of Mbed TLS in the default configuration will still build under the next
version.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

ChangeLog.d/check-return.txt

@@ -6,7 +6,12 @@ Bugfix
      where this function cannot fail, or full-module replacements with
      MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092.
 
-Changes
+Features
-   * Warn if errors from AES or DES functions are ignored. This is currently
+   * Warn if errors from certain functions are ignored. This is currently
-     supported on GCC-like compilers and on MSVC and can be configured by
+     supported on GCC-like compilers and on MSVC and can be configured through
-     setting MBEDTLS_CHECK_RETURN in config.h.
+     the macro MBEDTLS_CHECK_RETURN. The warnings are always enabled
+     (where supported) for critical functions where ignoring the return
+     value is almost always a bug. Enable the new configuration option
+     MBEDTLS_CHECK_RETURN_WARNING to get warnings for other functions. This
+     is currently implemented in the AES and DES modules, and will be extended
+     to other modules in the future.

include/mbedtls/config.h

@@ -616,6 +616,29 @@
  */
 //#define MBEDTLS_CAMELLIA_SMALL_MEMORY
 
+/**
+ * \def MBEDTLS_CHECK_RETURN_WARNING
+ *
+ * If this macro is defined, emit a compile-time warning if application code
+ * calls a function without checking its return value, but the return value
+ * should generally be checked in portable applications.
+ *
+ * This is only supported on platforms where #MBEDTLS_CHECK_RETURN is
+ * implemented. Otherwise this option has no effect.
+ *
+ * Uncomment to get warnings on using fallible functions without checking
+ * their return value.
+ *
+ * \note  This feature is a work in progress.
+ *        Warnings will be added to more functions in the future.
+ *
+ * \note  A few functions are considered critical, and ignoring the return
+ *        value of these functions will trigger a warning even if this
+ *        macro is not defined. To completely disable return value check
+ *        warnings, define #MBEDTLS_CHECK_RETURN with an empty expansion.
+ */
+//#define MBEDTLS_CHECK_RETURN_WARNING
+
 /**
  * \def MBEDTLS_CIPHER_MODE_CBC
  *

include/mbedtls/platform_util.h

@@ -173,7 +173,8 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t;
  * This macro appearing at the beginning of the declaration of a function
  * indicates that its return value should be generally be checked in portable
  * applications. Omitting the check will result in a compile-time warning if
- * #MBEDTLS_CHECK_RETURN is implemented for the compiler in use.
+ * #MBEDTLS_CHECK_RETURN is implemented for the compiler in use and
+ * #MBEDTLS_CHECK_RETURN_WARNING is enabled in the compile-time configuration.
  *
  * \note  The use of this macro is a work in progress.
  *        This macro will be added to more functions in the future.
@@ -181,7 +182,11 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t;
  *        an error code (as \c int in the \c mbedtls_xxx API or
  *        as ::psa_status_t in the \c psa_xxx API).
  */
+#if defined(MBEDTLS_CHECK_RETURN_WARNING)
 #define MBEDTLS_CHECK_RETURN_TYPICAL MBEDTLS_CHECK_RETURN
+#else
+#define MBEDTLS_CHECK_RETURN_TYPICAL
+#endif
 
 /** Benign-failure function
  *